Skip to content

Commit 68fe562

Browse files
committed
move db connection resolution out of validate step
1 parent 4f689ae commit 68fe562

5 files changed

Lines changed: 88 additions & 89 deletions

File tree

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,8 @@ pub fn build_airflow_statefulset_envs(
8080
let auth_config = &cluster.cluster_config.authentication_config;
8181
let authorization_config = &cluster.cluster_config.authorization_config;
8282
let resolved_product_image = &cluster.image;
83-
let metadata_database_connection_details =
84-
&cluster.cluster_config.metadata_database_connection_details;
85-
let celery_database_connection_details =
86-
&cluster.cluster_config.celery_database_connection_details;
83+
let metadata_database_connection_details = cluster.metadata_database_connection_details();
84+
let celery_database_connection_details = cluster.celery_database_connection_details();
8785

8886
let mut env: BTreeMap<String, EnvVar> = BTreeMap::new();
8987
let internal_secret_name = cluster.internal_secret_name();
@@ -370,8 +368,7 @@ pub fn build_airflow_template_envs(
370368
name: AIRFLOW_DATABASE_SQL_ALCHEMY_CONN.into(),
371369
value: Some(
372370
cluster
373-
.cluster_config
374-
.metadata_database_connection_details
371+
.metadata_database_connection_details()
375372
.url_template
376373
.clone(),
377374
),

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ pub fn build_executor_template_config_map(
145145
.context(PodSnafu)?;
146146

147147
cluster
148-
.cluster_config
149-
.metadata_database_connection_details
148+
.metadata_database_connection_details()
150149
.add_to_container(&mut airflow_container);
151150

152151
pb.add_container(airflow_container.build());

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,10 @@ pub fn build_server_rolegroup_statefulset(
256256
.context(PodSnafu)?;
257257

258258
validated_cluster
259-
.cluster_config
260-
.metadata_database_connection_details
259+
.metadata_database_connection_details()
261260
.add_to_container(&mut airflow_container);
262-
if let Some((celery_result_backend, celery_broker)) = &validated_cluster
263-
.cluster_config
264-
.celery_database_connection_details
261+
if let Some((celery_result_backend, celery_broker)) =
262+
validated_cluster.celery_database_connection_details()
265263
{
266264
celery_result_backend.add_to_container(&mut airflow_container);
267265
celery_broker.add_to_container(&mut airflow_container);

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

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@ use stackable_operator::{
1111
resources::{NoRuntimeLimits, Resources},
1212
},
1313
crd::git_sync,
14-
database_connections::drivers::{
15-
celery::CeleryDatabaseConnectionDetails, sqlalchemy::SqlAlchemyDatabaseConnectionDetails,
14+
database_connections::{
15+
TemplatingMechanism,
16+
drivers::{
17+
celery::CeleryDatabaseConnectionDetails,
18+
sqlalchemy::SqlAlchemyDatabaseConnectionDetails,
19+
},
1620
},
1721
k8s_openapi::api::core::v1::{PodTemplateSpec, Volume, VolumeMount},
1822
kube::{Resource, ResourceExt, api::ObjectMeta},
@@ -43,7 +47,11 @@ use crate::{
4347
APP_NAME, AirflowConfig, AirflowConfigOverrides, AirflowExecutor, AirflowRole,
4448
AirflowStorageConfig, ExecutorConfig, OPERATOR_NAME,
4549
authentication::AirflowClientAuthenticationDetailsResolved,
46-
authorization::AirflowAuthorizationResolved, v1alpha2,
50+
authorization::AirflowAuthorizationResolved,
51+
databases::{
52+
CeleryBrokerConnection, CeleryResultBackendConnection, MetadataDatabaseConnection,
53+
},
54+
v1alpha2,
4755
},
4856
};
4957

@@ -148,13 +156,16 @@ pub struct ValidatedClusterConfig {
148156
pub load_examples: bool,
149157
pub expose_config: bool,
150158
pub database_initialization_enabled: bool,
151-
/// The templated SQLAlchemy connection details for the metadata database.
152-
pub metadata_database_connection_details: SqlAlchemyDatabaseConnectionDetails,
153-
/// The templated Celery result-backend and broker connection details, when configured.
154-
pub celery_database_connection_details: Option<(
155-
CeleryDatabaseConnectionDetails,
156-
CeleryDatabaseConnectionDetails,
157-
)>,
159+
/// The metadata database connection (`spec.clusterConfig.metadataDatabase`), as taken from the
160+
/// CRD. The templated connection details are derived on demand, see
161+
/// [`ValidatedCluster::metadata_database_connection_details`].
162+
pub metadata_database: MetadataDatabaseConnection,
163+
/// The Celery result-backend connection (`spec.clusterConfig.celeryResultsBackend`), when
164+
/// configured. See [`ValidatedCluster::celery_database_connection_details`].
165+
pub celery_results_backend: Option<CeleryResultBackendConnection>,
166+
/// The Celery broker connection (`spec.clusterConfig.celeryBroker`), when configured. See
167+
/// [`ValidatedCluster::celery_database_connection_details`].
168+
pub celery_broker: Option<CeleryBrokerConnection>,
158169
/// User-supplied extra Volumes (`spec.clusterConfig.volumes`).
159170
pub volumes: Vec<Volume>,
160171
/// User-supplied extra VolumeMounts (`spec.clusterConfig.volumeMounts`).
@@ -255,6 +266,48 @@ impl ValidatedCluster {
255266
self.cluster_config.volume_mounts.clone()
256267
}
257268

269+
/// The templated SQLAlchemy connection details for the metadata database, derived from the
270+
/// CRD connection ([`ValidatedClusterConfig::metadata_database`]).
271+
pub fn metadata_database_connection_details(&self) -> SqlAlchemyDatabaseConnectionDetails {
272+
self.cluster_config
273+
.metadata_database
274+
.sqlalchemy_connection_details_with_templating(
275+
"METADATA",
276+
&TemplatingMechanism::BashEnvSubstitution,
277+
)
278+
}
279+
280+
/// The templated Celery result-backend and broker connection details, derived from the CRD
281+
/// connections ([`ValidatedClusterConfig::celery_results_backend`] /
282+
/// [`ValidatedClusterConfig::celery_broker`]). `Some` only when both are configured, as the
283+
/// Celery executor needs both.
284+
pub fn celery_database_connection_details(
285+
&self,
286+
) -> Option<(
287+
CeleryDatabaseConnectionDetails,
288+
CeleryDatabaseConnectionDetails,
289+
)> {
290+
let templating_mechanism = TemplatingMechanism::BashEnvSubstitution;
291+
match (
292+
&self.cluster_config.celery_results_backend,
293+
&self.cluster_config.celery_broker,
294+
) {
295+
(Some(celery_results_backend), Some(celery_broker)) => {
296+
let celery_results_backend = celery_results_backend
297+
.celery_connection_details_with_templating(
298+
"CELERY_RESULT_BACKEND",
299+
&templating_mechanism,
300+
);
301+
let celery_broker = celery_broker.celery_connection_details_with_templating(
302+
"CELERY_BROKER",
303+
&templating_mechanism,
304+
);
305+
Some((celery_results_backend, celery_broker))
306+
}
307+
_ => None,
308+
}
309+
}
310+
258311
/// Type-safe names for the resources of a role group.
259312
///
260313
/// Infallible: the combined name length was validated during cluster validation

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

Lines changed: 18 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,6 @@ use snafu::{OptionExt, ResultExt, Snafu};
44
use stackable_operator::{
55
commons::product_image_selection,
66
config::fragment,
7-
database_connections::{
8-
TemplatingMechanism,
9-
drivers::{
10-
celery::CeleryDatabaseConnectionDetails,
11-
sqlalchemy::SqlAlchemyDatabaseConnectionDetails,
12-
},
13-
},
147
kube::ResourceExt,
158
product_logging::spec::Logging,
169
role_utils::{GenericRoleConfig, RoleGroup},
@@ -175,8 +168,21 @@ pub fn validate_cluster(
175168
authorization_config,
176169
} = dereferenced;
177170

178-
let (metadata_database_connection_details, celery_database_connection_details) =
179-
database_connection_details(airflow);
171+
// The Celery result-backend and broker only work with configured celeryExecutors. Emit a
172+
// warning if they are provided without a Celery executor. The connection details themselves are
173+
// derived from the CRD later, in the build step (see
174+
// `ValidatedCluster::celery_database_connection_details`).
175+
if airflow.spec.cluster_config.celery_results_backend.is_some()
176+
&& airflow.spec.cluster_config.celery_broker.is_some()
177+
&& !matches!(
178+
&airflow.spec.executor,
179+
AirflowExecutor::CeleryExecutors { .. }
180+
)
181+
{
182+
tracing::warn!(
183+
"No `spec.celeryExecutors` configured, but `spec.clusterConfig.celeryResultsBackend` and `spec.clusterConfig.celeryBroker` are provided. This only works in combination with a celery executor!"
184+
)
185+
}
180186

181187
// The Kubernetes-executor pod template is not an `AirflowRole` with role groups, so its config
182188
// is merged and its logging validated here, up-front, rather than in the build step.
@@ -214,8 +220,9 @@ pub fn validate_cluster(
214220
authorization_config,
215221
dags_git_sync: airflow.spec.cluster_config.dags_git_sync.clone(),
216222
credentials_secret_name: airflow.spec.cluster_config.credentials_secret_name.clone(),
217-
metadata_database_connection_details,
218-
celery_database_connection_details,
223+
metadata_database: airflow.spec.cluster_config.metadata_database.clone(),
224+
celery_results_backend: airflow.spec.cluster_config.celery_results_backend.clone(),
225+
celery_broker: airflow.spec.cluster_config.celery_broker.clone(),
219226
load_examples: airflow.spec.cluster_config.load_examples,
220227
expose_config: airflow.spec.cluster_config.expose_config,
221228
database_initialization_enabled: airflow
@@ -231,61 +238,6 @@ pub fn validate_cluster(
231238
))
232239
}
233240

234-
/// Builds the templated metadata-database (and optional Celery result-backend/broker) connection
235-
/// details from the cluster spec.
236-
fn database_connection_details(
237-
airflow: &v1alpha2::AirflowCluster,
238-
) -> (
239-
SqlAlchemyDatabaseConnectionDetails,
240-
Option<(
241-
CeleryDatabaseConnectionDetails,
242-
CeleryDatabaseConnectionDetails,
243-
)>,
244-
) {
245-
let templating_mechanism = TemplatingMechanism::BashEnvSubstitution;
246-
247-
let metadata_database_connection_details = airflow
248-
.spec
249-
.cluster_config
250-
.metadata_database
251-
.sqlalchemy_connection_details_with_templating("METADATA", &templating_mechanism);
252-
253-
let celery_database_connection_details = if let (
254-
Some(celery_results_backend),
255-
Some(celery_broker),
256-
) = (
257-
&airflow.spec.cluster_config.celery_results_backend,
258-
&airflow.spec.cluster_config.celery_broker,
259-
) {
260-
// The celery results backend and celery broker only work with configured celeryExecutors.
261-
// Emit a warning if celery executors were not configured properly.
262-
if !matches!(
263-
&airflow.spec.executor,
264-
AirflowExecutor::CeleryExecutors { .. }
265-
) {
266-
tracing::warn!(
267-
"No `spec.celeryExecutors` configured, but `spec.clusterConfig.celeryResultsBackend` and `spec.clusterConfig.celeryBroker` are provided. This only works in combination with a celery executor!"
268-
)
269-
}
270-
271-
let celery_results_backend = celery_results_backend
272-
.celery_connection_details_with_templating(
273-
"CELERY_RESULT_BACKEND",
274-
&templating_mechanism,
275-
);
276-
let celery_broker = celery_broker
277-
.celery_connection_details_with_templating("CELERY_BROKER", &templating_mechanism);
278-
Some((celery_results_backend, celery_broker))
279-
} else {
280-
None
281-
};
282-
283-
(
284-
metadata_database_connection_details,
285-
celery_database_connection_details,
286-
)
287-
}
288-
289241
/// Validate and merge one role group against its role, via the shared
290242
/// [`with_validated_config`] from `operator-rs`, returning the generic
291243
/// [`stackable_operator::v2::role_utils::RoleGroupConfig`].

0 commit comments

Comments
 (0)