From f193a71eb57b4db38c278303dda11d906858ceaa Mon Sep 17 00:00:00 2001 From: Erick Bourgeois Date: Sat, 3 Oct 2026 17:28:35 -0400 Subject: [PATCH] fix(bind9instance): roll BIND pods when their rendered config changes Config changes never reached running BIND pods. The shared cluster ConfigMap was re-rendered only when the Bind9Cluster generation changed and that reconcile won a race, and BIND reads named.conf only at start. An operator upgrade that renders the same spec differently (the key-directory fix in #518) therefore never reached an existing cluster. - desired_configmap_for_instance renders the ConfigMap an instance mounts (the cluster's shared one, exactly as the cluster reconciler renders it, or a standalone instance's own); it is written only when its data differs. - The pod template carries bindy.firestoned.io/config-hash, the sha256 of that data; a changed hash marks the Deployment for update and is patched onto the template, which rolls the pods. - config_drifted makes stale config content, or pods with another hash, a reason to run the instance's resource step, which was otherwise skipped unless the generation, labels, parent generation or key rotation changed. Known, not changed here: deployment_needs_update ignores the bind9 container image, and instances sharing a cluster ConfigMap roll together. Signed-off-by: Erick Bourgeois --- .claude/CHANGELOG.md | 54 +++++ src/bind9_resources.rs | 54 +++++ src/reconcilers/bind9instance/mod.rs | 41 +++- src/reconcilers/bind9instance/resources.rs | 208 ++++++++++++----- .../bind9instance/resources_tests.rs | 221 ++++++++++++++++++ 5 files changed, 519 insertions(+), 59 deletions(-) diff --git a/.claude/CHANGELOG.md b/.claude/CHANGELOG.md index 2d154dde..022eca6a 100644 --- a/.claude/CHANGELOG.md +++ b/.claude/CHANGELOG.md @@ -1,3 +1,57 @@ +## [2026-10-03 16:30] - Config changes reach running BIND pods (operator upgrades included) + +**Author:** Erick Bourgeois + +### Fixed +- `src/reconcilers/bind9instance/resources.rs`: the instance reconciler now + keeps the ConfigMap its pods mount current by **content**, not by the + `Bind9Cluster` generation. `desired_configmap_for_instance` renders the + cluster's shared `-config` (exactly as the cluster reconciler does) + or a standalone instance's own `-config`, and it is written only when + its data differs. Before, the shared ConfigMap was re-rendered only when the + cluster's spec generation changed and that reconcile won a race, so an + operator upgrade that renders the same spec differently (e.g. the + `key-directory` fix) never reached a running cluster; a generation bump and + deleting the ConfigMap did not reliably repair it either. +- `src/bind9_resources.rs` (`CONFIG_HASH_ANNOTATION`, + `configmap_data_hash`, `stamp_config_hash`) and the Deployment update path: + the pod template carries `bindy.firestoned.io/config-hash`, the sha256 of + the mounted ConfigMap's data. A changed hash makes `deployment_needs_update` + true and is patched onto the template, which rolls the pods. BIND reads + `named.conf` only at start, so before, even a rewritten ConfigMap ran only + after an unrelated restart. + +- `src/reconcilers/bind9instance/mod.rs`, `resources.rs` + (`config_drifted`): config drift is a reason to run the instance's + resource step. That step was skipped unless the generation changed, + something was missing, labels or the parent generation changed, or a key + rotation was due, so the content-based write above never ran after an + upgrade. Drift is: the mounted ConfigMap's data differs from what this + operator renders, or the Deployment's pods carry another config hash. A + render error counts as drift, so the reconcile runs and reports it. + +### Tests +- `reconcilers/bind9instance/resources_tests.rs` (`config_rollout`): drift + from stale content, from a missing or older hash, and none when nothing is + generated; hash + determinism and sensitivity (a byte moving between files counts), + managed instance wants the cluster's shared ConfigMap with the cluster + reconciler's exact data, standalone wants its own, custom `configMapRefs` + leave nothing to write, and a changed hash rolls the Deployment while an + unchanged one does not. + +### Known, not changed here +- `deployment_needs_update` still ignores the **bind9** container's image, so + a BIND version change alone does not roll the pods. +- Instances sharing one cluster ConfigMap roll together when it changes. + +### Impact +- [ ] Breaking change +- [x] Requires cluster rollout (every BIND pod rolls once, as the annotation + is added) +- [ ] Config change only +- [ ] Documentation only + ## [2026-10-03 14:30] - DNSSEC signing actually signs: reserved policy names, inheritance, existing zones, key directory **Author:** Erick Bourgeois diff --git a/src/bind9_resources.rs b/src/bind9_resources.rs index 4de4af06..11eb8521 100644 --- a/src/bind9_resources.rs +++ b/src/bind9_resources.rs @@ -517,6 +517,60 @@ pub(crate) fn build_dnssec_key_volumes( (volumes, volume_mounts) } +/// Pod-template annotation holding [`configmap_data_hash`] of the BIND +/// configuration the pod mounts. BIND reads `named.conf` only at start, so a +/// changed ConfigMap reaches it only through a new pod; changing this +/// annotation is what rolls the Deployment. It covers changes from a spec +/// edit and from an operator upgrade that renders the same spec differently. +pub const CONFIG_HASH_ANNOTATION: &str = "bindy.firestoned.io/config-hash"; + +/// Lowercase hex sha256 over a ConfigMap's `data`, key and value pairs in key +/// order, each NUL-terminated so a byte moving from one file to the next is a +/// different config. +/// +/// # Arguments +/// +/// * `configmap` - The ConfigMap whose content is hashed +/// +/// # Returns +/// +/// The digest, 64 hex characters. +#[must_use] +pub fn configmap_data_hash(configmap: &ConfigMap) -> String { + use sha2::{Digest, Sha256}; + + let mut hasher = Sha256::new(); + for (key, value) in configmap.data.iter().flatten() { + hasher.update(key.as_bytes()); + hasher.update([0u8]); + hasher.update(value.as_bytes()); + hasher.update([0u8]); + } + hasher + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() +} + +/// Set [`CONFIG_HASH_ANNOTATION`] on a Deployment's pod template. +/// +/// # Arguments +/// +/// * `deployment` - The Deployment to stamp +/// * `hash` - The hash of the ConfigMap its pods mount +pub fn stamp_config_hash(deployment: &mut Deployment, hash: &str) { + let Some(spec) = deployment.spec.as_mut() else { + return; + }; + spec.template + .metadata + .get_or_insert_with(Default::default) + .annotations + .get_or_insert_with(Default::default) + .insert(CONFIG_HASH_ANNOTATION.to_string(), hash.to_string()); +} + /// Builds standardized Kubernetes labels for BIND9 instance resources. /// /// Creates labels for resources managed by `Bind9Instance` controller. diff --git a/src/reconcilers/bind9instance/mod.rs b/src/reconcilers/bind9instance/mod.rs index 0c59bdac..c85b0e53 100644 --- a/src/reconcilers/bind9instance/mod.rs +++ b/src/reconcilers/bind9instance/mod.rs @@ -392,19 +392,20 @@ pub async fn reconcile_bind9instance(ctx: Arc, instance: Bind9Instance) } // Check if ALL required resources actually exist AND match desired state (drift detection) - let (all_resources_exist, deployment_labels_match, rotation_needed) = { + let (all_resources_exist, deployment_labels_match, rotation_needed, config_drifted) = { let deployment_api: Api = Api::namespaced(client.clone(), &namespace); let service_api: Api = Api::namespaced(client.clone(), &namespace); let configmap_api: Api = Api::namespaced(client.clone(), &namespace); let secret_api: Api = Api::namespaced(client.clone(), &namespace); // Fetch deployment to check if it exists AND if OUR labels match - let (deployment_exists, labels_match) = match deployment_api.get(&name).await { - Ok(deployment) => ( + let current_deployment = deployment_api.get(&name).await.ok(); + let (deployment_exists, labels_match) = match current_deployment.as_ref() { + Some(deployment) => ( true, - deployment_labels_are_current(&deployment, &name, &instance), + deployment_labels_are_current(deployment, &name, &instance), ), - Err(_) => (false, false), + None => (false, false), }; let service_exists = service_api.get(&name).await.is_ok(); @@ -415,7 +416,25 @@ pub async fn reconcile_bind9instance(ctx: Arc, instance: Bind9Instance) } else { format!("{name}-config") }; - let configmap_exists = configmap_api.get(&configmap_name).await.is_ok(); + let current_configmap = configmap_api.get(&configmap_name).await.ok(); + let configmap_exists = current_configmap.is_some(); + + // Config drift: the mounted config is behind what this operator renders + // (e.g. after an upgrade), or the pods were never rolled onto it. A + // render error counts as drift so the reconcile runs and reports it. + let config_drifted = match resources::desired_configmap_for_instance( + &name, + &namespace, + &instance, + cluster.as_ref(), + ) { + Ok(desired) => resources::config_drifted( + current_configmap.as_ref(), + current_deployment.as_ref(), + desired.as_ref(), + ), + Err(_) => true, + }; // Check Secret existence AND rotation status let secret_name = format!("{name}-rndc-key"); @@ -445,7 +464,7 @@ pub async fn reconcile_bind9instance(ctx: Arc, instance: Bind9Instance) }; let all_exist = deployment_exists && service_exists && configmap_exists && secret_exists; - (all_exist, labels_match, needs_rotation) + (all_exist, labels_match, needs_rotation, config_drifted) }; let cluster_ref = build_cluster_reference(cluster.as_ref(), cluster_provider.as_ref()); @@ -479,6 +498,7 @@ pub async fn reconcile_bind9instance(ctx: Arc, instance: Bind9Instance) && deployment_labels_match && !rotation_needed && !parent_config_changed + && !config_drifted { debug!( "Spec unchanged (generation={:?}), all resources exist, deployment labels match, no rotation needed, and parent config unchanged - skipping resource reconciliation", @@ -509,6 +529,13 @@ pub async fn reconcile_bind9instance(ctx: Arc, instance: Bind9Instance) // - Deployment labels don't match desired state (drift), OR // - RNDC Secret rotation is due, OR // - Parent cluster configuration has changed + if config_drifted && all_resources_exist { + info!( + "BIND config for {}/{} is behind what this operator renders, updating it and rolling the pods", + namespace, name + ); + } + if !deployment_labels_match && all_resources_exist { info!( "Deployment labels don't match desired state for {}/{}, triggering reconciliation to update labels", diff --git a/src/reconcilers/bind9instance/resources.rs b/src/reconcilers/bind9instance/resources.rs index 4e8142d3..10160cd7 100644 --- a/src/reconcilers/bind9instance/resources.rs +++ b/src/reconcilers/bind9instance/resources.rs @@ -11,7 +11,8 @@ use super::types::*; use crate::bind9::Bind9Manager; use crate::bind9_resources::{ - build_configmap, build_deployment, build_service, build_service_account, + build_cluster_configmap, build_configmap, build_deployment, build_service, + build_service_account, }; use crate::constants::{API_GROUP_VERSION, KIND_BIND9_INSTANCE}; use crate::reconcilers::resources::create_or_apply; @@ -188,17 +189,11 @@ pub(super) async fn create_or_update_resources( // 3. Create/update ConfigMap debug!("Step 3: Creating/updating ConfigMap"); - create_or_update_configmap( - client, - namespace, - name, - instance, - cluster.as_ref(), - cluster_provider.as_ref(), - ) - .await?; + let config_hash = + create_or_update_configmap(client, namespace, name, instance, cluster.as_ref()).await?; - // 4. Create/update Deployment (mounts the resolved RNDC Secret) + // 4. Create/update Deployment (mounts the resolved RNDC Secret; its pod + // template carries the config hash, so a config change rolls the pods) debug!("Step 4: Creating/updating Deployment"); create_or_update_deployment( client, @@ -208,6 +203,7 @@ pub(super) async fn create_or_update_resources( cluster.as_ref(), cluster_provider.as_ref(), &secret_name, + config_hash.as_deref(), ) .await?; @@ -862,33 +858,44 @@ async fn trigger_deployment_rollout( /// **Note:** If the instance belongs to a cluster (has `spec.clusterRef`), this function /// does NOT create an instance-specific `ConfigMap`. Instead, the instance will use the /// cluster-level shared `ConfigMap` created by the `Bind9Cluster` reconciler. -async fn create_or_update_configmap( - client: &Client, - namespace: &str, +/// The ConfigMap `instance`'s pods mount, rendered as bindy wants it. +/// +/// * A cluster-managed instance (`clusterRef` naming a `Bind9Cluster`) +/// mounts the cluster's shared `-config`, rendered exactly as the +/// cluster reconciler renders it ([`build_cluster_configmap`]), so either +/// reconciler writing it writes the same bytes. +/// * A standalone instance mounts its own `-config` ([`build_configmap`]). +/// * Custom `configMapRefs` on the cluster, or a `clusterRef` that names no +/// `Bind9Cluster` here (a `ClusterBind9Provider`), leave nothing for this +/// reconciler to keep current: `None`. +/// +/// # Errors +/// +/// Returns an error if an ACL, forwarder or listen entry fails validation. +pub(super) fn desired_configmap_for_instance( name: &str, + namespace: &str, instance: &Bind9Instance, cluster: Option<&Bind9Cluster>, - _cluster_provider: Option<&crate::crd::ClusterBind9Provider>, -) -> Result<()> { - // If instance belongs to a cluster, skip ConfigMap creation - // The cluster creates a shared ConfigMap that all instances use +) -> anyhow::Result> { if !instance.spec.cluster_ref.is_empty() { - debug!( - "Instance {}/{} belongs to cluster '{}', using cluster ConfigMap", - namespace, name, instance.spec.cluster_ref - ); - return Ok(()); + let Some(cluster) = cluster else { + return Ok(None); + }; + let custom = cluster + .spec + .common + .config_map_refs + .as_ref() + .is_some_and(|refs| refs.named_conf.is_some() || refs.named_conf_options.is_some()); + if custom { + return Ok(None); + } + return build_cluster_configmap(&instance.spec.cluster_ref, namespace, cluster).map(Some); } - // Instance is standalone (no clusterRef), create instance-specific ConfigMap - info!( - "Instance {}/{} is standalone, creating instance-specific ConfigMap", - namespace, name - ); - - // Get role-specific allow-transfer override from cluster config - // Note: We only reach this code for standalone instances (no clusterRef), - // so we should only have a namespace-scoped cluster here, not a global cluster + // Standalone: role-specific allow-transfer from a namespace-scoped + // cluster, if one was passed in. let role_allow_transfer = cluster.and_then(|c| match instance.spec.role { crate::crd::ServerRole::Primary => c .spec @@ -903,30 +910,107 @@ async fn create_or_update_configmap( .as_ref() .and_then(|s| s.allow_transfer.as_ref()), }); + build_configmap(name, namespace, instance, cluster, role_allow_transfer).map(Some) +} + +/// Whether the BIND config `instance` runs on is behind what bindy renders: +/// the mounted ConfigMap's data differs from `desired`, or the Deployment's +/// pods carry another [`crate::bind9_resources::CONFIG_HASH_ANNOTATION`]. +/// +/// The instance reconciler skips its resource step when the spec, the parent +/// and every resource look unchanged; this is what makes an operator upgrade +/// that renders differently count as a change. +/// +/// # Arguments +/// +/// * `actual_configmap` - The ConfigMap the pods mount, if it exists +/// * `deployment` - The instance's Deployment, if it exists +/// * `desired` - [`desired_configmap_for_instance`]; `None` means nothing is +/// generated to keep current, which is never drift +#[must_use] +pub(super) fn config_drifted( + actual_configmap: Option<&ConfigMap>, + deployment: Option<&Deployment>, + desired: Option<&ConfigMap>, +) -> bool { + let Some(desired) = desired else { + return false; + }; + if actual_configmap.map(|cm| &cm.data) != Some(&desired.data) { + return true; + } + let desired_hash = crate::bind9_resources::configmap_data_hash(desired); + deployment.and_then(config_hash_of) != Some(desired_hash.as_str()) +} - // build_configmap always returns a ConfigMap (it always carries at least - // rndc.conf, plus any file not overridden by custom configMapRefs); it - // returns Err if any ACL/forwarder/listen entry fails validation. The - // generated ConfigMap must always be created because the Deployment's - // `config` volume references it unconditionally. - let configmap = build_configmap(name, namespace, instance, cluster, role_allow_transfer)?; +/// Write the ConfigMap `instance`'s pods mount when its content differs from +/// what bindy renders, and return the hash of that content. +/// +/// Comparing content rather than the cluster's generation is what lets an +/// operator upgrade that renders the same spec differently reach a running +/// cluster: every instance is reconciled when the operator starts. +/// +/// # Returns +/// +/// The [`crate::bind9_resources::configmap_data_hash`] of the desired +/// ConfigMap, or `None` when there is none to keep current. +/// +/// # Errors +/// +/// Returns an error if rendering fails or the API rejects the write. +async fn create_or_update_configmap( + client: &Client, + namespace: &str, + name: &str, + instance: &Bind9Instance, + cluster: Option<&Bind9Cluster>, +) -> Result> { + let Some(configmap) = desired_configmap_for_instance(name, namespace, instance, cluster)? + else { + debug!( + "Instance {}/{} has no generated ConfigMap to keep current", + namespace, name + ); + return Ok(None); + }; + let hash = crate::bind9_resources::configmap_data_hash(&configmap); + let cm_name = configmap.metadata.name.clone().unwrap_or_default(); let cm_api: Api = Api::namespaced(client.clone(), namespace); - let cm_name = format!("{name}-config"); - if (cm_api.get(&cm_name).await).is_ok() { - // ConfigMap exists, update it - info!("Updating ConfigMap {}/{}", namespace, cm_name); - cm_api - .replace(&cm_name, &PostParams::default(), &configmap) - .await?; - return Ok(()); + match cm_api.get_opt(&cm_name).await? { + Some(current) if current.data == configmap.data => { + debug!("ConfigMap {}/{} is current", namespace, cm_name); + } + Some(_) => { + info!( + "Updating ConfigMap {}/{} (rendered config changed)", + namespace, cm_name + ); + cm_api + .replace(&cm_name, &PostParams::default(), &configmap) + .await?; + } + None => { + info!("Creating ConfigMap {}/{}", namespace, cm_name); + cm_api.create(&PostParams::default(), &configmap).await?; + } } + Ok(Some(hash)) +} - // ConfigMap doesn't exist, create it - info!("Creating ConfigMap {}/{}", namespace, cm_name); - cm_api.create(&PostParams::default(), &configmap).await?; - - Ok(()) +/// The [`crate::bind9_resources::CONFIG_HASH_ANNOTATION`] on a Deployment's +/// pod template, if set. +fn config_hash_of(deployment: &Deployment) -> Option<&str> { + deployment + .spec + .as_ref()? + .template + .metadata + .as_ref()? + .annotations + .as_ref()? + .get(crate::bind9_resources::CONFIG_HASH_ANNOTATION) + .map(String::as_str) } /// Check if a deployment needs updating by comparing current and desired state. @@ -937,7 +1021,17 @@ async fn create_or_update_configmap( /// - API container environment variables /// - API container imagePullPolicy /// - API container resources +/// - The BIND config hash on the pod template +/// ([`crate::bind9_resources::CONFIG_HASH_ANNOTATION`]) fn deployment_needs_update(current: &Deployment, desired: &Deployment) -> bool { + // The BIND config the pods mount changed: roll them. + if let Some(desired_hash) = config_hash_of(desired) { + if config_hash_of(current) != Some(desired_hash) { + debug!("BIND config hash changed: rolling the pods"); + return true; + } + } + // Compare desired replicas with current replicas let desired_replicas = desired.spec.as_ref().and_then(|s| s.replicas); let current_replicas = current.spec.as_ref().and_then(|s| s.replicas); @@ -1083,8 +1177,9 @@ async fn create_or_update_deployment( cluster: Option<&Bind9Cluster>, cluster_provider: Option<&crate::crd::ClusterBind9Provider>, rndc_secret_name: &str, + config_hash: Option<&str>, ) -> Result<()> { - let deployment = build_deployment( + let mut deployment = build_deployment( name, namespace, instance, @@ -1092,6 +1187,9 @@ async fn create_or_update_deployment( cluster_provider, rndc_secret_name, ); + if let Some(hash) = config_hash { + crate::bind9_resources::stamp_config_hash(&mut deployment, hash); + } let api: Api = Api::namespaced(client.clone(), namespace); // Check if deployment exists - if not, create it and return early @@ -1217,6 +1315,12 @@ async fn create_or_update_deployment( patch["spec"]["template"]["metadata"] = json!({"labels": pod_labels}); } + // The config hash rolls the pods when the BIND config they mount changed. + if let Some(hash) = config_hash_of(&deployment) { + patch["spec"]["template"]["metadata"]["annotations"] = + json!({ crate::bind9_resources::CONFIG_HASH_ANNOTATION: hash }); + } + api.patch(name, &PatchParams::default(), &Patch::Strategic(&patch)) .await?; diff --git a/src/reconcilers/bind9instance/resources_tests.rs b/src/reconcilers/bind9instance/resources_tests.rs index 8503338f..c2d2dab4 100644 --- a/src/reconcilers/bind9instance/resources_tests.rs +++ b/src/reconcilers/bind9instance/resources_tests.rs @@ -1066,4 +1066,225 @@ mod tests { )); } } + + // ------------------------------------------------------------------ + // Config rollout: a change in the rendered BIND config must reach the + // running pods, whether it comes from a spec edit or an operator + // upgrade that renders differently. The ConfigMap is written when its + // content differs, and its hash on the pod template rolls the pods. + // ------------------------------------------------------------------ + mod config_rollout { + use crate::bind9_resources::{ + build_cluster_configmap, build_deployment, configmap_data_hash, stamp_config_hash, + CONFIG_HASH_ANNOTATION, + }; + use crate::crd::{ + Bind9Cluster, Bind9ClusterCommonSpec, Bind9ClusterSpec, Bind9Instance, + Bind9InstanceSpec, ConfigMapRefs, ServerRole, + }; + use crate::reconcilers::bind9instance::resources::{ + config_drifted, deployment_needs_update_for_test as deployment_needs_update, + desired_configmap_for_instance, + }; + use k8s_openapi::api::core::v1::ConfigMap; + use kube::api::ObjectMeta; + use std::collections::BTreeMap; + + fn instance(name: &str, cluster_ref: &str) -> Bind9Instance { + #[allow(deprecated)] + Bind9Instance { + metadata: ObjectMeta { + name: Some(name.into()), + namespace: Some("dns".into()), + ..Default::default() + }, + spec: Bind9InstanceSpec { + cluster_ref: cluster_ref.to_string(), + role: ServerRole::Primary, + replicas: Some(1), + version: Some("9.18".into()), + image: None, + config_map_refs: None, + config: None, + primary_servers: None, + volumes: None, + volume_mounts: None, + rndc_secret_ref: None, + rndc_key: None, + storage: None, + placement: None, + bindcar_config: None, + }, + status: None, + } + } + + fn cluster() -> Bind9Cluster { + let mut c = Bind9Cluster::new( + "my-dns", + Bind9ClusterSpec { + common: Bind9ClusterCommonSpec { + version: Some("9.18".into()), + primary: None, + secondary: None, + image: None, + config_map_refs: None, + global: None, + rndc_secret_refs: None, + acls: None, + volumes: None, + volume_mounts: None, + }, + }, + ); + c.metadata.namespace = Some("dns".into()); + c + } + + fn cm(data: &[(&str, &str)]) -> ConfigMap { + ConfigMap { + data: Some( + data.iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect::>(), + ), + ..Default::default() + } + } + + #[test] + fn hash_is_deterministic_and_follows_content() { + let a = cm(&[("named.conf", "a"), ("named.conf.options", "b")]); + let same = cm(&[("named.conf.options", "b"), ("named.conf", "a")]); + let other = cm(&[("named.conf", "a"), ("named.conf.options", "c")]); + let moved = cm(&[("named.conf", "ab"), ("named.conf.options", "")]); + assert_eq!(configmap_data_hash(&a), configmap_data_hash(&same)); + assert_ne!(configmap_data_hash(&a), configmap_data_hash(&other)); + assert_ne!( + configmap_data_hash(&a), + configmap_data_hash(&moved), + "a byte moving between files is a different config" + ); + } + + /// A cluster-managed instance mounts the cluster's shared ConfigMap, + /// so that is the one it must keep current, rendered exactly as the + /// cluster reconciler renders it. + #[test] + fn a_managed_instance_wants_the_clusters_shared_configmap() { + let c = cluster(); + let desired = desired_configmap_for_instance( + "my-dns-primary-0", + "dns", + &instance("my-dns-primary-0", "my-dns"), + Some(&c), + ) + .expect("renders") + .expect("a managed instance has a ConfigMap to keep current"); + let expected = build_cluster_configmap("my-dns", "dns", &c).expect("renders"); + assert_eq!(desired.metadata.name.as_deref(), Some("my-dns-config")); + assert_eq!(desired.data, expected.data); + } + + #[test] + fn a_standalone_instance_wants_its_own_configmap() { + let desired = + desired_configmap_for_instance("solo", "dns", &instance("solo", ""), None) + .expect("renders") + .expect("a standalone instance has its own ConfigMap"); + assert_eq!(desired.metadata.name.as_deref(), Some("solo-config")); + } + + /// Custom configMapRefs replace the generated config: nothing to + /// write, nothing to hash. + #[test] + fn custom_configmaps_are_left_alone() { + let mut c = cluster(); + c.spec.common.config_map_refs = Some(ConfigMapRefs { + named_conf: Some("mine".into()), + named_conf_options: None, + named_conf_zones: None, + }); + let desired = desired_configmap_for_instance( + "my-dns-primary-0", + "dns", + &instance("my-dns-primary-0", "my-dns"), + Some(&c), + ) + .expect("renders"); + assert!(desired.is_none()); + } + + /// The instance reconciler skips its resource step when nothing + /// looks changed. A ConfigMap whose content differs from what bindy + /// renders (an operator upgrade), or pods stamped with another hash, + /// must count as a change, or the fix never runs. + #[test] + fn config_drift_is_detected_from_content_and_hash() { + let inst = instance("my-dns-primary-0", "my-dns"); + let c = cluster(); + let desired = + desired_configmap_for_instance("my-dns-primary-0", "dns", &inst, Some(&c)) + .expect("renders") + .expect("managed"); + let hash = configmap_data_hash(&desired); + let mut deployment = + build_deployment("my-dns-primary-0", "dns", &inst, Some(&c), None, "k"); + stamp_config_hash(&mut deployment, &hash); + + // Everything current: no drift. + assert!(!config_drifted( + Some(&desired), + Some(&deployment), + Some(&desired) + )); + + // The ConfigMap's content is stale (rendered by an older operator). + let stale = cm(&[("named.conf.options", "options { };")]); + assert!(config_drifted( + Some(&stale), + Some(&deployment), + Some(&desired) + )); + + // Pods carry no hash yet, or an older one. + let unstamped = build_deployment("my-dns-primary-0", "dns", &inst, Some(&c), None, "k"); + assert!(config_drifted( + Some(&desired), + Some(&unstamped), + Some(&desired) + )); + + // Nothing generated to keep current: never drift. + assert!(!config_drifted(Some(&stale), Some(&unstamped), None)); + } + + #[test] + fn a_changed_config_hash_rolls_the_deployment() { + let inst = instance("my-dns-primary-0", "my-dns"); + let c = cluster(); + let mut current = + build_deployment("my-dns-primary-0", "dns", &inst, Some(&c), None, "k"); + let mut desired = current.clone(); + stamp_config_hash(&mut current, "old"); + stamp_config_hash(&mut desired, "new"); + assert_eq!( + desired + .spec + .as_ref() + .and_then(|s| s.template.metadata.as_ref()) + .and_then(|m| m.annotations.as_ref()) + .and_then(|a| a.get(CONFIG_HASH_ANNOTATION)) + .map(String::as_str), + Some("new") + ); + assert!(deployment_needs_update(¤t, &desired)); + + stamp_config_hash(&mut current, "new"); + assert!( + !deployment_needs_update(¤t, &desired), + "same hash, nothing to roll" + ); + } + } }