Skip to content

refactor: Flatten role configs #791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
adwk67 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: Flatten role configs #791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 67 additions & 17 deletions rust/operator-binary/src/controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use stackable_operator::{
cluster_resources::ClusterResourceApplyStrategy,
commons::{
affinity::StackableAffinity,
pdb::PdbConfig,
product_image_selection::ResolvedProductImage,
resources::{NoRuntimeLimits, Resources},
},
Expand Down Expand Up @@ -107,12 +108,27 @@ pub struct KubernetesResources<T> {
pub status: PhantomData<T>,
}

/// Per-role configuration extracted during validation.
/// The Node role's validated role-level configuration.
///
/// Separate from the other roles' because only the Node role serves the web UI, so only it has a
/// listener class and a group listener. One shared type would have to make both fields `Option` —
/// mandatory for the Node role, meaningless for Worker and Beat — leaving every reader to work out
/// which role it is holding.
#[derive(Clone, Debug)]
pub struct ValidatedNodeRoleConfig {
pub pdb: PdbConfig,
pub listener_class: ListenerClassName,
pub group_listener_name: ListenerName,
}

/// The Worker and Beat roles' validated role-level configuration.
///
/// Neither serves the web UI, so there is no listener class to carry and the Pod disruption budget
/// is all that is left. Structurally identical to `GenericRoleConfig`; kept as its own type so the
/// validated cluster holds controller-owned types throughout.
#[derive(Clone, Debug)]
pub struct ValidatedRoleConfig {
pub pdb: Option<stackable_operator::commons::pdb::PdbConfig>,
pub listener_class: Option<ListenerClassName>,
pub group_listener_name: Option<ListenerName>,
pub pdb: PdbConfig,
}

/// A validated, merged Superset role-group config.
Expand Down Expand Up @@ -201,22 +217,52 @@ pub struct ValidatedCluster {
pub product_version: ProductVersion,
pub image: ResolvedProductImage,
pub cluster_config: ValidatedClusterConfig,
pub role_groups: BTreeMap<SupersetRole, BTreeMap<RoleGroupName, SupersetRoleGroupConfig>>,
pub role_configs: BTreeMap<SupersetRole, ValidatedRoleConfig>,
pub node_config: ValidatedNodeRoleConfig,
pub node_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
pub worker_config: Option<ValidatedRoleConfig>,
pub worker_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
pub beat_config: Option<ValidatedRoleConfig>,
pub beat_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
}

/// The non-derived inputs to [`ValidatedCluster::new`].
///
/// Named fields, so the three same-typed role-group maps — and the two
/// `Option<ValidatedRoleConfig>` — cannot be swapped silently.
#[derive(Debug)]
pub struct ValidatedClusterParams {
pub name: ClusterName,
pub namespace: NamespaceName,
pub uid: Uid,
pub image: ResolvedProductImage,
pub cluster_config: ValidatedClusterConfig,
pub node_config: ValidatedNodeRoleConfig,
pub node_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
pub worker_config: Option<ValidatedRoleConfig>,
pub worker_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
pub beat_config: Option<ValidatedRoleConfig>,
pub beat_role_group_configs: BTreeMap<RoleGroupName, SupersetRoleGroupConfig>,
}

impl ValidatedCluster {
pub fn new(
name: ClusterName,
namespace: NamespaceName,
uid: Uid,
image: ResolvedProductImage,
cluster_config: ValidatedClusterConfig,
role_groups: BTreeMap<SupersetRole, BTreeMap<RoleGroupName, SupersetRoleGroupConfig>>,
role_configs: BTreeMap<SupersetRole, ValidatedRoleConfig>,
) -> Self {
pub fn new(params: ValidatedClusterParams) -> Self {
let ValidatedClusterParams {
name,
namespace,
uid,
image,
cluster_config,
node_config,
node_role_group_configs,
worker_config,
worker_role_group_configs,
beat_config,
beat_role_group_configs,
} = params;

let product_version = ProductVersion::from_str(&image.app_version_label_value)
.expect("the app version label value is a valid product version");

Self {
// Capture only the identity fields needed to own child objects, derived from the
// typed cluster identity rather than the raw CRD.
Expand All @@ -228,8 +274,12 @@ impl ValidatedCluster {
},
image,
cluster_config,
role_groups,
role_configs,
node_config,
node_role_group_configs,
worker_config,
worker_role_group_configs,
beat_config,
beat_role_group_configs,
name,
namespace,
uid,
Expand Down
167 changes: 89 additions & 78 deletions rust/operator-binary/src/controller/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::marker::PhantomData;
use snafu::{ResultExt, Snafu};
use stackable_operator::{
builder::meta::ObjectMetaBuilder,
k8s_openapi::api::core::v1::{ConfigMap, Service},
kvp::Labels,
v2::{
builder::meta::ownerreference_from_resource,
Expand All @@ -16,7 +17,7 @@ use stackable_operator::{
use crate::{
controller::{
CONTROLLER_NAME, KubernetesResources, OPERATOR_NAME, PRODUCT_NAME, Prepared,
ValidatedCluster,
SupersetRoleGroupConfig, ValidatedCluster,
build::resource::{
config_map::build_rolegroup_config_map,
deployment::build_rolegroup_deployment,
Expand Down Expand Up @@ -64,89 +65,74 @@ pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources<Prepared>
let mut config_maps = vec![];
let mut pod_disruption_budgets = vec![];

for (superset_role, role_group_configs) in &cluster.role_groups {
for (role_group_name, rolegroup_config) in role_group_configs {
let config = &rolegroup_config.config;

config_maps.push(
build_rolegroup_config_map(
cluster,
superset_role,
role_group_name,
config,
&rolegroup_config.config_overrides,
)
.context(ConfigMapSnafu {
for (role_group_name, rolegroup_config) in &cluster.node_role_group_configs {
let (config_map, metrics_service) = build_common_role_group_resources(
cluster,
&SupersetRole::Node,
role_group_name,
rolegroup_config,
)?;
config_maps.push(config_map);
services.push(metrics_service);

// Only the Node role's StatefulSet references a headless Service (as its `serviceName`);
// the Worker/Beat Deployments have no `serviceName` and do not serve the HTTP port.
services.push(build_rolegroup_headless_service(
cluster,
&SupersetRole::Node,
role_group_name,
));

stateful_sets.push(
build_node_rolegroup_statefulset(cluster, role_group_name, rolegroup_config).context(
StatefulSetSnafu {
role_group: role_group_name.clone(),
})?,
);
},
)?,
);
}

// Every role exposes metrics via the statsd-exporter sidecar, so each rolegroup gets a
// metrics Service.
services.push(build_rolegroup_metrics_service(
// The Celery roles differ from each other only in name: both produce a ConfigMap, a metrics
// Service and a Deployment.
for (role, role_group_configs) in [
(&SupersetRole::Worker, &cluster.worker_role_group_configs),
(&SupersetRole::Beat, &cluster.beat_role_group_configs),
] {
for (role_group_name, rolegroup_config) in role_group_configs {
let (config_map, metrics_service) = build_common_role_group_resources(
cluster,
superset_role,
role,
role_group_name,
));

match superset_role {
SupersetRole::Node => {
// Only the `Node` role's StatefulSet references a headless Service (as its
// `serviceName`); the `Worker`/`Beat` Deployments have no `serviceName` and do
// not serve the HTTP port, so they get no headless Service.
services.push(build_rolegroup_headless_service(
cluster,
superset_role,
role_group_name,
));

stateful_sets.push(
build_node_rolegroup_statefulset(
cluster,
superset_role,
role_group_name,
rolegroup_config,
)
.context(StatefulSetSnafu {
role_group: role_group_name.clone(),
})?,
);
}
SupersetRole::Worker | SupersetRole::Beat => {
deployments.push(
build_rolegroup_deployment(
cluster,
superset_role,
role_group_name,
rolegroup_config,
)
.context(DeploymentSnafu {
role_group: role_group_name.clone(),
})?,
);
}
}
rolegroup_config,
)?;
config_maps.push(config_map);
services.push(metrics_service);

deployments.push(
build_rolegroup_deployment(cluster, role, role_group_name, rolegroup_config)
.context(DeploymentSnafu {
role_group: role_group_name.clone(),
})?,
);
}
}

// Role-level resources (group listener, PDB) are built once per role, after its role
// groups — not once per role group.
if let Some(role_config) = cluster.role_configs.get(superset_role) {
if let (Some(listener_class), Some(listener_group_name)) = (
&role_config.listener_class,
&role_config.group_listener_name,
) {
listeners.push(build_group_listener(
cluster,
superset_role,
listener_class,
listener_group_name.to_string(),
));
}

if let Some(pdb_config) = &role_config.pdb {
pod_disruption_budgets.extend(build_pdb(pdb_config, cluster, superset_role));
}
}
listeners.push(build_group_listener(
cluster,
&SupersetRole::Node,
&cluster.node_config.listener_class,
cluster.node_config.group_listener_name.to_string(),
));
pod_disruption_budgets.extend(build_pdb(
&cluster.node_config.pdb,
cluster,
&SupersetRole::Node,
));
if let Some(worker) = &cluster.worker_config {
pod_disruption_budgets.extend(build_pdb(&worker.pdb, cluster, &SupersetRole::Worker));
}
if let Some(beat) = &cluster.beat_config {
pod_disruption_budgets.extend(build_pdb(&beat.pdb, cluster, &SupersetRole::Beat));
}

Ok(KubernetesResources {
Expand All @@ -162,6 +148,31 @@ pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources<Prepared>
})
}

/// The resources every role group gets regardless of its role: a ConfigMap and a metrics Service
/// (every role exposes metrics via the statsd-exporter sidecar).
fn build_common_role_group_resources(
cluster: &ValidatedCluster,
role: &SupersetRole,
role_group_name: &RoleGroupName,
rolegroup_config: &SupersetRoleGroupConfig,
) -> Result<(ConfigMap, Service), Error> {
let config_map = build_rolegroup_config_map(
cluster,
role,
role_group_name,
&rolegroup_config.config,
&rolegroup_config.config_overrides,
)
.context(ConfigMapSnafu {
role_group: role_group_name.clone(),
})?;

Ok((
config_map,
build_rolegroup_metrics_service(cluster, role, role_group_name),
))
}

/// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to
/// the cluster, and the recommended labels for a resource named `name` in `role`/
/// `role_group_name`.
Expand Down
5 changes: 2 additions & 3 deletions rust/operator-binary/src/controller/build/resource/config_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,8 @@ mod tests {

let role_group_name: RoleGroupName = "default".parse().expect("valid role group name");
let rolegroup_config = validated
.role_groups
.get(&SupersetRole::Node)
.and_then(|groups| groups.get(&role_group_name))
.node_role_group_configs
.get(&role_group_name)
.expect("node default rolegroup");

let config_map = build_rolegroup_config_map(
Expand Down
Loading
Loading