Skip to content

Commit 4547016

Browse files
committed
git_sync::v1alpha2::GitSyncResources usage inline with nifi
1 parent 604d6ca commit 4547016

5 files changed

Lines changed: 98 additions & 51 deletions

File tree

rust/operator-binary/src/airflow_controller.rs

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use stackable_operator::{
1010
cli::OperatorEnvironmentOptions,
1111
cluster_resources::{ClusterResourceApplyStrategy, ClusterResources},
1212
commons::{random_secret_creation, rbac::build_rbac_resources},
13-
crd::git_sync,
1413
k8s_openapi::api::core::v1::EnvVar,
1514
kube::{
1615
ResourceExt,
@@ -34,16 +33,13 @@ use strum::{EnumDiscriminants, IntoStaticStr};
3433
use crate::{
3534
controller::{
3635
ValidatedCluster, ValidatedExecutorTemplate,
37-
build::{
38-
resource::{
39-
config_map,
40-
executor::build_executor_template_config_map,
41-
listener::build_group_listener,
42-
pdb::build_pdb,
43-
service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
44-
statefulset::build_server_rolegroup_statefulset,
45-
},
46-
volumes::LOG_VOLUME_NAME,
36+
build::resource::{
37+
config_map,
38+
executor::build_executor_template_config_map,
39+
listener::build_group_listener,
40+
pdb::build_pdb,
41+
service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
42+
statefulset::build_server_rolegroup_statefulset,
4743
},
4844
controller_name, operator_name, product_name,
4945
},
@@ -135,9 +131,6 @@ pub enum Error {
135131
source: crate::controller::build::resource::config_map::Error,
136132
},
137133

138-
#[snafu(display("invalid git-sync specification"))]
139-
InvalidGitSyncSpec { source: git_sync::v1alpha2::Error },
140-
141134
#[snafu(display("failed to delete orphaned resources"))]
142135
DeleteOrphanedResources {
143136
source: stackable_operator::cluster_resources::Error,
@@ -340,16 +333,6 @@ pub async fn reconcile_airflow(
340333
for (rolegroup_name, validated_rg_config) in role_group_configs {
341334
let logging = &validated_rg_config.config.logging;
342335

343-
let git_sync_resources = git_sync::v1alpha2::GitSyncResources::new(
344-
&validated_cluster.cluster_config.dags_git_sync,
345-
&validated_cluster.image,
346-
&Vec::<EnvVar>::from(validated_rg_config.env_overrides.clone()),
347-
&validated_cluster.volume_mounts(),
348-
LOG_VOLUME_NAME.as_ref(),
349-
&logging.git_sync_container,
350-
)
351-
.context(InvalidGitSyncSpecSnafu)?;
352-
353336
let rg_headless_service =
354337
build_rolegroup_headless_service(&validated_cluster, airflow_role, rolegroup_name);
355338

@@ -392,7 +375,6 @@ pub async fn reconcile_airflow(
392375
validated_rg_config,
393376
logging,
394377
&rbac_sa,
395-
&git_sync_resources,
396378
)
397379
.context(BuildStatefulSetSnafu)?;
398380

@@ -452,23 +434,12 @@ async fn build_executor_template(
452434
role_group: executor_role_group_name(),
453435
})?;
454436

455-
let git_sync_resources = git_sync::v1alpha2::GitSyncResources::new(
456-
&validated_cluster.cluster_config.dags_git_sync,
457-
&validated_cluster.image,
458-
&env_vars_from_overrides(&executor_template.env_overrides),
459-
&validated_cluster.volume_mounts(),
460-
LOG_VOLUME_NAME.as_ref(),
461-
&executor_config.logging.git_sync_container,
462-
)
463-
.context(InvalidGitSyncSpecSnafu)?;
464-
465437
let worker_pod_template_config_map = build_executor_template_config_map(
466438
validated_cluster,
467439
&rbac_sa.name_unchecked(),
468440
executor_config,
469441
&executor_template.env_overrides,
470442
&executor_template.pod_overrides,
471-
&git_sync_resources,
472443
)
473444
.context(BuildExecutorTemplateSnafu)?;
474445
cluster_resources
@@ -492,7 +463,7 @@ pub fn error_policy(
492463
}
493464

494465
/// Convert user-supplied `envOverrides` into a list of [`EnvVar`]s.
495-
fn env_vars_from_overrides(env_overrides: &HashMap<String, String>) -> Vec<EnvVar> {
466+
pub(crate) fn env_vars_from_overrides(env_overrides: &HashMap<String, String>) -> Vec<EnvVar> {
496467
// Collect into a `BTreeMap` first so the env vars come out in a deterministic (sorted) order;
497468
// `HashMap` iteration order is randomised per instance and would otherwise churn the containers
498469
// this feeds between reconciles. Mirrors the override handling in `env_vars.rs`.

rust/operator-binary/src/controller/build/resource/executor.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use stackable_operator::{
1010
meta::ObjectMetaBuilder,
1111
pod::{PodBuilder, security::PodSecurityContextBuilder},
1212
},
13-
crd::git_sync,
1413
k8s_openapi::{
1514
DeepMerge,
1615
api::core::v1::{ConfigMap, PodTemplateSpec},
@@ -83,10 +82,12 @@ pub fn build_executor_template_config_map(
8382
executor_config: &ValidatedAirflowConfig,
8483
env_overrides: &HashMap<String, String>,
8584
pod_overrides: &PodTemplateSpec,
86-
git_sync_resources: &git_sync::v1alpha2::GitSyncResources,
8785
) -> Result<ConfigMap> {
8886
let resolved_product_image = &cluster.image;
8987
let authentication_config = &cluster.cluster_config.authentication_config;
88+
// The git-sync resources were resolved up-front during validation; read them off the validated
89+
// executor config rather than reconstructing them here.
90+
let git_sync_resources = &executor_config.git_sync_resources;
9091

9192
let mut pb = PodBuilder::new();
9293
let pb_metadata =

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use stackable_operator::{
77
security::PodSecurityContextBuilder, volume::VolumeBuilder,
88
},
99
},
10-
crd::git_sync,
1110
k8s_openapi::{
1211
DeepMerge,
1312
api::{
@@ -105,10 +104,12 @@ pub fn build_server_rolegroup_statefulset(
105104
validated_rg_config: &AirflowRoleGroupConfig,
106105
logging: &ValidatedLogging,
107106
service_account: &ServiceAccount,
108-
git_sync_resources: &git_sync::v1alpha2::GitSyncResources,
109107
) -> Result<StatefulSet> {
110108
let merged_airflow_config = &validated_rg_config.config;
111109
let env_overrides = &validated_rg_config.env_overrides;
110+
// The git-sync resources were resolved up-front during validation; read them off the validated
111+
// config rather than reconstructing them here.
112+
let git_sync_resources = &merged_airflow_config.git_sync_resources;
112113

113114
let resolved_product_image = &validated_cluster.image;
114115
let authentication_config = &validated_cluster.cluster_config.authentication_config;

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,35 +82,51 @@ pub type AirflowRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupC
8282
/// A validated, merged Airflow role-group config: the merged [`AirflowConfig`] with its raw
8383
/// `logging` replaced by the up-front-validated [`ValidatedLogging`] (so an invalid custom log
8484
/// ConfigMap name or a missing Vector aggregator name fails reconciliation during validation).
85-
#[derive(Clone, Debug, PartialEq)]
85+
// Not `Clone`/`Debug`/`PartialEq`: `git_sync_resources` (a `GitSyncResources`) implements none of
86+
// them, mirroring nifi's `ValidatedNifiConfig`.
8687
pub struct ValidatedAirflowConfig {
8788
pub resources: Resources<AirflowStorageConfig, NoRuntimeLimits>,
8889
pub logging: ValidatedLogging,
8990
pub affinity: StackableAffinity,
9091
pub graceful_shutdown_timeout: Option<Duration>,
92+
/// The git-sync resources (containers, volumes, mounts) for the DAGs, resolved up-front in the
93+
/// [`validate`] step (the env vars and logging differ per role group / executor, so they are
94+
/// computed there). Consumed by the StatefulSet, executor-template and env-var builders, which
95+
/// read it off here rather than reconstructing it.
96+
pub git_sync_resources: git_sync::v1alpha2::GitSyncResources,
9197
}
9298

9399
impl ValidatedAirflowConfig {
94100
/// Builds the validated config from the merged [`AirflowConfig`], swapping in the
95101
/// already-validated logging.
96-
pub(crate) fn from_merged(merged: AirflowConfig, logging: ValidatedLogging) -> Self {
102+
pub(crate) fn from_merged(
103+
merged: AirflowConfig,
104+
logging: ValidatedLogging,
105+
git_sync_resources: git_sync::v1alpha2::GitSyncResources,
106+
) -> Self {
97107
Self {
98108
resources: merged.resources,
99109
logging,
100110
affinity: merged.affinity,
101111
graceful_shutdown_timeout: merged.graceful_shutdown_timeout,
112+
git_sync_resources,
102113
}
103114
}
104115

105116
/// Builds the validated config from the merged [`ExecutorConfig`] (Kubernetes-executor pod
106117
/// template), swapping in the already-validated logging. [`ExecutorConfig`] is field-identical
107118
/// to [`AirflowConfig`].
108-
pub(crate) fn from_merged_executor(merged: ExecutorConfig, logging: ValidatedLogging) -> Self {
119+
pub(crate) fn from_merged_executor(
120+
merged: ExecutorConfig,
121+
logging: ValidatedLogging,
122+
git_sync_resources: git_sync::v1alpha2::GitSyncResources,
123+
) -> Self {
109124
Self {
110125
resources: merged.resources,
111126
logging,
112127
affinity: merged.affinity,
113128
graceful_shutdown_timeout: merged.graceful_shutdown_timeout,
129+
git_sync_resources,
114130
}
115131
}
116132
}
@@ -150,8 +166,6 @@ pub struct ValidatedClusterConfig {
150166
pub executor_template: Option<ValidatedExecutorTemplate>,
151167
pub authentication_config: AirflowClientAuthenticationDetailsResolved,
152168
pub authorization_config: AirflowAuthorizationResolved,
153-
/// The Git-sync definitions for the DAGs (`spec.clusterConfig.dagsGitSync`).
154-
pub dags_git_sync: Vec<git_sync::v1alpha2::GitSync>,
155169
pub credentials_secret_name: SecretName,
156170
pub load_examples: bool,
157171
pub expose_config: bool,

rust/operator-binary/src/controller/validate.rs

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use snafu::{OptionExt, ResultExt, Snafu};
44
use stackable_operator::{
55
commons::product_image_selection,
66
config::fragment,
7+
crd::git_sync,
8+
k8s_openapi::api::core::v1::{EnvVar, VolumeMount},
79
kube::ResourceExt,
810
product_logging::spec::Logging,
911
role_utils::{GenericRoleConfig, RoleGroup},
@@ -25,10 +27,10 @@ use strum::IntoEnumIterator;
2527
use super::{
2628
AirflowRoleGroupConfig, ValidatedAirflowConfig, ValidatedCluster, ValidatedClusterConfig,
2729
ValidatedExecutorTemplate, ValidatedLogging, ValidatedRoleConfig,
28-
dereference::DereferencedObjects,
30+
build::volumes::LOG_VOLUME_NAME, dereference::DereferencedObjects,
2931
};
3032
use crate::{
31-
airflow_controller::CONTAINER_IMAGE_BASE_NAME,
33+
airflow_controller::{CONTAINER_IMAGE_BASE_NAME, env_vars_from_overrides},
3234
crd::{
3335
AirflowConfig, AirflowConfigFragment, AirflowConfigOverrides, AirflowExecutor, AirflowRole,
3436
AirflowRoleType, Container, v1alpha2,
@@ -87,6 +89,9 @@ pub enum Error {
8789
"the Vector aggregator discovery ConfigMap name must be set when the Vector agent is enabled"
8890
))]
8991
MissingVectorAggregatorConfigMapName,
92+
93+
#[snafu(display("invalid git-sync specification"))]
94+
InvalidGitSyncSpec { source: git_sync::v1alpha2::Error },
9095
}
9196

9297
pub fn validate_cluster(
@@ -155,6 +160,9 @@ pub fn validate_cluster(
155160
rolegroup,
156161
&default_config,
157162
&vector_aggregator_config_map_name,
163+
&resolved_product_image,
164+
&airflow.spec.cluster_config.dags_git_sync,
165+
&airflow.spec.cluster_config.volume_mounts,
158166
)?;
159167

160168
group_configs.insert(role_group_name, config);
@@ -199,8 +207,22 @@ pub fn validate_cluster(
199207
&Container::Base,
200208
&vector_aggregator_config_map_name,
201209
)?;
210+
// Resolve the executor's git-sync resources up-front too, mirroring the role groups.
211+
let git_sync_resources = git_sync::v1alpha2::GitSyncResources::new(
212+
&airflow.spec.cluster_config.dags_git_sync,
213+
&resolved_product_image,
214+
&env_vars_from_overrides(&common_configuration.env_overrides),
215+
&airflow.spec.cluster_config.volume_mounts,
216+
LOG_VOLUME_NAME.as_ref(),
217+
&logging.git_sync_container,
218+
)
219+
.context(InvalidGitSyncSpecSnafu)?;
202220
Some(ValidatedExecutorTemplate {
203-
config: ValidatedAirflowConfig::from_merged_executor(merged, logging),
221+
config: ValidatedAirflowConfig::from_merged_executor(
222+
merged,
223+
logging,
224+
git_sync_resources,
225+
),
204226
env_overrides: common_configuration.env_overrides.clone(),
205227
pod_overrides: common_configuration.pod_overrides.clone(),
206228
})
@@ -218,7 +240,6 @@ pub fn validate_cluster(
218240
executor_template,
219241
authentication_config,
220242
authorization_config,
221-
dags_git_sync: airflow.spec.cluster_config.dags_git_sync.clone(),
222243
credentials_secret_name: airflow.spec.cluster_config.credentials_secret_name.clone(),
223244
metadata_database: airflow.spec.cluster_config.metadata_database.clone(),
224245
celery_results_backend: airflow.spec.cluster_config.celery_results_backend.clone(),
@@ -249,12 +270,16 @@ pub fn validate_cluster(
249270
///
250271
/// Note the override `Merge` semantics: a role-group `null` inherits the role-level value rather
251272
/// than unsetting it (config overrides), and env overrides layer role-group on top of role.
273+
#[allow(clippy::too_many_arguments)]
252274
fn validate_role_group(
253275
role: &AirflowRoleType,
254276
role_group_name: &RoleGroupName,
255277
rolegroup: &RoleGroup<AirflowConfigFragment, GenericCommonConfig, AirflowConfigOverrides>,
256278
default_config: &AirflowConfigFragment,
257279
vector_aggregator_config_map_name: &Option<ConfigMapName>,
280+
image: &product_image_selection::ResolvedProductImage,
281+
dags_git_sync: &[git_sync::v1alpha2::GitSync],
282+
volume_mounts: &[VolumeMount],
258283
) -> Result<AirflowRoleGroupConfig, Error> {
259284
let validated = with_validated_config::<
260285
AirflowConfig,
@@ -282,9 +307,21 @@ fn validate_role_group(
282307
vector_aggregator_config_map_name,
283308
)?;
284309

310+
// The git-sync resources depend on this role group's env-var overrides and (git-sync) logging
311+
// config, so they are resolved (and validated) here, up-front, rather than at build time.
312+
let git_sync_resources = git_sync::v1alpha2::GitSyncResources::new(
313+
dags_git_sync,
314+
image,
315+
&Vec::<EnvVar>::from(env_overrides.clone()),
316+
volume_mounts,
317+
LOG_VOLUME_NAME.as_ref(),
318+
&logging.git_sync_container,
319+
)
320+
.context(InvalidGitSyncSpecSnafu)?;
321+
285322
Ok(RoleGroupConfig {
286323
replicas: validated.replicas,
287-
config: ValidatedAirflowConfig::from_merged(merged_config, logging),
324+
config: ValidatedAirflowConfig::from_merged(merged_config, logging, git_sync_resources),
288325
config_overrides: validated.config.config_overrides,
289326
env_overrides,
290327
cli_overrides: validated.config.cli_overrides,
@@ -342,6 +379,20 @@ mod tests {
342379
use super::validate_role_group;
343380
use crate::crd::{AirflowConfig, AirflowRole, v1alpha2};
344381

382+
/// A minimal resolved product image for tests that exercise `validate_role_group` (which needs
383+
/// one to resolve git-sync resources; the test role groups configure no git-sync, so the value
384+
/// is otherwise unused).
385+
fn test_resolved_product_image()
386+
-> stackable_operator::commons::product_image_selection::ResolvedProductImage {
387+
stackable_operator::commons::product_image_selection::ResolvedProductImage {
388+
product_version: "3.0.6".to_string(),
389+
app_version_label_value: "3.0.6".parse().expect("valid label value"),
390+
image: "oci.example.org/sdp/airflow:3.0.6-stackable0.0.0-dev".to_string(),
391+
image_pull_policy: "IfNotPresent".to_string(),
392+
pull_secrets: None,
393+
}
394+
}
395+
345396
fn test_cluster() -> v1alpha2::AirflowCluster {
346397
let cluster_yaml = r#"
347398
apiVersion: airflow.stackable.tech/v1alpha2
@@ -404,6 +455,9 @@ mod tests {
404455
rolegroup,
405456
&default_config,
406457
&None,
458+
&test_resolved_product_image(),
459+
&[],
460+
&[],
407461
)
408462
.expect("validated role group");
409463
let config_overrides = validated.config_overrides;
@@ -502,6 +556,9 @@ mod tests {
502556
rolegroup,
503557
&default_config,
504558
&None,
559+
&test_resolved_product_image(),
560+
&[],
561+
&[],
505562
)
506563
.expect("validated role group");
507564

@@ -576,6 +633,9 @@ mod tests {
576633
rolegroup,
577634
&default_config,
578635
&None,
636+
&test_resolved_product_image(),
637+
&[],
638+
&[],
579639
)
580640
.expect("validated role group");
581641

0 commit comments

Comments
 (0)