Skip to content

Commit 280b7fb

Browse files
authored
refactor: Flatten role configs (#867)
* refactor: Flatten role configs * changelog * use infallible webserver_role_group_listener_name in place of group_listener_name
1 parent 37a3eb4 commit 280b7fb

8 files changed

Lines changed: 486 additions & 291 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@
4646
overridden, whereas previously the operator's value always took precedence ([#838]).
4747
- Make operations infallible where appropriate ([#852], [#860]).
4848
- Deprecated airflow `3.2.2` ([#865]).
49+
- Internal operator refactoring: the validated cluster carries each role's configuration in its own
50+
typed fields instead of maps keyed by role, and only the webserver carries a listener class, a
51+
group listener and trusted proxies, so those are no longer optional fields that four of the five
52+
roles leave unset ([#867]).
4953
- Bump stackable-operator to 0.119.0 ([#868]).
5054
- Docs: remove Spark submit/monitor classes and refer to demo usage instead ([#870]).
5155

@@ -80,6 +84,7 @@
8084
[#860]: https://github.com/stackabletech/airflow-operator/pull/860
8185
[#862]: https://github.com/stackabletech/airflow-operator/pull/862
8286
[#865]: https://github.com/stackabletech/airflow-operator/pull/865
87+
[#867]: https://github.com/stackabletech/airflow-operator/pull/867
8388
[#868]: https://github.com/stackabletech/airflow-operator/pull/868
8489
[#870]: https://github.com/stackabletech/airflow-operator/pull/870
8590
[#872]: https://github.com/stackabletech/airflow-operator/pull/872

‎rust/operator-binary/src/controller/build/mod.rs‎

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::{
2727
statefulset::build_server_rolegroup_statefulset,
2828
},
2929
},
30-
crd::{AirflowConfigOverrides, Container},
30+
crd::{AirflowConfigOverrides, AirflowRole, Container},
3131
};
3232

3333
pub mod graceful_shutdown;
@@ -93,23 +93,27 @@ pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources<Prepared>
9393
config_maps.push(executor_template_config_map);
9494
}
9595

96-
for (role, role_group_configs) in &cluster.role_groups {
97-
if let Some(role_config) = cluster.role_configs.get(role) {
98-
if let Some(pdb_config) = &role_config.pdb {
99-
pod_disruption_budgets.extend(build_pdb(pdb_config, cluster, role));
100-
}
101-
if let Some(listener_class) = &role_config.listener_class
102-
&& let Some(group_listener_name) = &role_config.group_listener_name
103-
{
104-
listeners.push(build_group_listener(
105-
cluster,
106-
role,
107-
listener_class.clone(),
108-
group_listener_name.clone(),
109-
));
110-
}
111-
}
112-
96+
// One entry per role, in `AirflowRole` declaration order. Each role's groups come from its own
97+
// field, so the role and its groups cannot be paired up wrongly here.
98+
for (role, role_group_configs) in [
99+
(
100+
&AirflowRole::Webserver,
101+
&cluster.webserver_role_group_configs,
102+
),
103+
(
104+
&AirflowRole::Scheduler,
105+
&cluster.scheduler_role_group_configs,
106+
),
107+
(&AirflowRole::Worker, &cluster.worker_role_group_configs),
108+
(
109+
&AirflowRole::DagProcessor,
110+
&cluster.dagprocessor_role_group_configs,
111+
),
112+
(
113+
&AirflowRole::Triggerer,
114+
&cluster.triggerer_role_group_configs,
115+
),
116+
] {
113117
for (role_group_name, rg_config) in role_group_configs {
114118
let logging = &rg_config.config.logging;
115119

@@ -149,6 +153,21 @@ pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources<Prepared>
149153
})?,
150154
);
151155
}
156+
157+
if let Some(pdb) = cluster.pdb(role) {
158+
pod_disruption_budgets.extend(build_pdb(pdb, cluster, role));
159+
}
160+
}
161+
162+
// Only the webserver serves the web UI, so it is the only role with a group listener; it is
163+
// built once here rather than inside the role loop.
164+
if let Some(webserver) = &cluster.webserver_config {
165+
listeners.push(build_group_listener(
166+
cluster,
167+
&AirflowRole::Webserver,
168+
webserver.listener_class.clone(),
169+
webserver.group_listener_name.clone(),
170+
));
152171
}
153172

154173
Ok(KubernetesResources {
@@ -335,18 +354,22 @@ pub(crate) mod test_support {
335354
serde_yaml::with::singleton_map_recursive::deserialize(cluster_value)
336355
.expect("the test CR deserialises");
337356

338-
let dereferenced = DereferencedObjects {
357+
validate_cluster(&cluster, "oci.stackable.tech/sdp", dereferenced_objects())
358+
.expect("test cluster validates")
359+
}
360+
361+
/// The resolved external objects a test cluster is validated against: no AuthenticationClasses
362+
/// and no OPA authorization, so validation depends on the CR alone.
363+
pub fn dereferenced_objects() -> DereferencedObjects {
364+
DereferencedObjects {
339365
authentication_config: AirflowClientAuthenticationDetailsResolved {
340366
authentication_classes_resolved: vec![],
341367
user_registration: true,
342368
user_registration_role: "Public".to_string(),
343369
sync_roles_at: FlaskRolesSyncMoment::default(),
344370
},
345371
authorization_config: AirflowAuthorizationResolved { opa: None },
346-
};
347-
348-
validate_cluster(&cluster, "oci.stackable.tech/sdp", dereferenced)
349-
.expect("test cluster validates")
372+
}
350373
}
351374

352375
/// A Celery-executor cluster whose webserver trusts the given reverse proxies.

‎rust/operator-binary/src/controller/build/properties/env_vars.rs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,7 @@ fn add_version_specific_env_vars(
320320
// behind a reverse proxy.
321321
// This covers the uvicorn backend, the only one the SDP image can run.
322322
let trusted_proxies = cluster
323-
.role_configs
324-
.get(airflow_role)
325-
.map(|role_config| role_config.trusted_proxies.as_slice())
326-
.unwrap_or_default()
323+
.trusted_proxies(airflow_role)
327324
.iter()
328325
.map(TrustedProxy::to_string)
329326
.collect::<Vec<_>>()
@@ -342,11 +339,7 @@ fn add_version_specific_env_vars(
342339
// The 2.x uses Werkzeug's `ProxyFix` to allow forwarded-headers and it does so regardless
343340
// of the peer source. The only valid value for `spec.webservers.roleConfig.trustedProxies` is `["*"]`.
344341
if airflow_role == &AirflowRole::Webserver {
345-
let trusted_proxies = cluster
346-
.role_configs
347-
.get(airflow_role)
348-
.map(|role_config| role_config.trusted_proxies.as_slice())
349-
.unwrap_or_default();
342+
let trusted_proxies = cluster.trusted_proxies(airflow_role);
350343

351344
if !trusted_proxies.is_empty() {
352345
env_vars = env_vars

‎rust/operator-binary/src/controller/build/resource/statefulset.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,13 @@ pub fn build_server_rolegroup_statefulset(
221221

222222
let mut pvcs: Option<Vec<PersistentVolumeClaim>> = None;
223223

224-
if let Some(listener_group_name) = validated_cluster
225-
.role_configs
226-
.get(airflow_role)
227-
.and_then(|role_config| role_config.group_listener_name.clone())
228-
{
224+
if let Some(listener_group_name) = validated_cluster.group_listener_name(airflow_role) {
229225
// Listener endpoints for the Webserver role will use persistent volumes
230226
// so that load balancers can hard-code the target addresses. This will
231227
// be the case even when no class is set (and the value defaults to
232228
// cluster-internal) as the address should still be consistent.
233229
let pvc = listener_operator_volume_source_builder_build_pvc(
234-
&ListenerReference::Listener(listener_group_name),
230+
&ListenerReference::Listener(listener_group_name.clone()),
235231
&unversioned_recommended_labels,
236232
&LISTENER_PVC_NAME,
237233
);

0 commit comments

Comments
 (0)