From ff6985897e29337d1385152c0808cfe1e9193ebd Mon Sep 17 00:00:00 2001
From: Malte Sander <contact@maltesander.com>
Date: Sat, 19 Nov 2022 15:01:14 +0000
Subject: [PATCH] Fix znode bug (#592)

# Description

When testing with Kafka, the ZooKeeper operator does no longer create respective ZNodes due to:
```
ERROR stackable_operator::logging::controller: Failed to reconcile object controller.name="znode.zookeeper.stackable.tech" error=reconciler for object ZookeeperZnode.v1alpha1.zookeeper.stackable.tech/simp
le-znode.default failed error.sources=[failed to save discovery information to ConfigMap.v1./simple-znode.default, Label contains unexpected content. Expected: app.kubernetes.io/instance=simple-znode, actual: app.kubernetes.io/insta
nce=simple-zk]
```

This was introduced in https://github.com/stackabletech/zookeeper-operator/pull/591

Added testcase to check if a znode ConfigMap is created successfully.

Test: https://ci.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/zookeeper-operator-it-custom/21/



Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
---
 CHANGELOG.md                                  |  2 ++
 rust/operator-binary/src/discovery.rs         | 18 ++++++++++--------
 rust/operator-binary/src/zk_controller.rs     | 13 ++++++++++---
 rust/operator-binary/src/znode_controller.rs  | 14 ++++++++++----
 tests/templates/kuttl/znode/00-assert.yaml    | 17 +++++++++++++++++
 .../kuttl/znode/00-install-zookeeper.yaml.j2  | 19 +++++++++++++++++++
 tests/test-definition.yaml                    |  6 ++++++
 7 files changed, 74 insertions(+), 15 deletions(-)
 create mode 100644 tests/templates/kuttl/znode/00-assert.yaml
 create mode 100644 tests/templates/kuttl/znode/00-install-zookeeper.yaml.j2

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ba24b220..0db81c8b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,9 +8,11 @@ All notable changes to this project will be documented in this file.
 
 - Updated stackable image versions ([#586]).
 - Operator-rs: 0.25.3 -> 0.27.1 ([#591]).
+- Fixed bug where ZNode ConfigMaps were not created due to labeling issues ([#592]).
 
 [#586]: https://github.com/stackabletech/zookeeper-operator/pull/586
 [#591]: https://github.com/stackabletech/zookeeper-operator/pull/591
+[#592]: https://github.com/stackabletech/zookeeper-operator/pull/592
 
 ## [0.12.0] - 2022-11-07
 
diff --git a/rust/operator-binary/src/discovery.rs b/rust/operator-binary/src/discovery.rs
index 75cd7aca..8b2cf6ce 100644
--- a/rust/operator-binary/src/discovery.rs
+++ b/rust/operator-binary/src/discovery.rs
@@ -1,5 +1,4 @@
 use crate::utils::build_recommended_labels;
-use crate::ZK_CONTROLLER_NAME;
 
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
@@ -53,16 +52,18 @@ pub async fn build_discovery_configmaps(
     zk: &ZookeeperCluster,
     owner: &impl Resource<DynamicType = ()>,
     client: &stackable_operator::client::Client,
+    controller_name: &str,
     svc: &Service,
     chroot: Option<&str>,
 ) -> Result<Vec<ConfigMap>, Error> {
     let name = owner.meta().name.as_deref().context(NoNameSnafu)?;
     Ok(vec![
-        build_discovery_configmap(name, owner, zk, chroot, pod_hosts(zk)?)?,
+        build_discovery_configmap(zk, owner, name, controller_name, chroot, pod_hosts(zk)?)?,
         build_discovery_configmap(
-            &format!("{}-nodeport", name),
-            owner,
             zk,
+            owner,
+            &format!("{}-nodeport", name),
+            controller_name,
             chroot,
             nodeport_hosts(client, svc, "zk").await?,
         )?,
@@ -73,9 +74,10 @@ pub async fn build_discovery_configmaps(
 ///
 /// `hosts` will usually come from either [`pod_hosts`] or [`nodeport_hosts`].
 fn build_discovery_configmap(
-    name: &str,
-    owner: &impl Resource<DynamicType = ()>,
     zk: &ZookeeperCluster,
+    owner: &impl Resource<DynamicType = ()>,
+    name: &str,
+    controller_name: &str,
     chroot: Option<&str>,
     hosts: impl IntoIterator<Item = (impl Into<String>, u16)>,
 ) -> Result<ConfigMap, Error> {
@@ -104,8 +106,8 @@ fn build_discovery_configmap(
                     zk: ObjectRef::from_obj(zk),
                 })?
                 .with_recommended_labels(build_recommended_labels(
-                    zk,
-                    ZK_CONTROLLER_NAME,
+                    owner,
+                    controller_name,
                     zk.image_version().context(NoVersionSnafu)?,
                     &ZookeeperRole::Server.to_string(),
                     "discovery",
diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs
index 7ea721fd..0f5207ed 100644
--- a/rust/operator-binary/src/zk_controller.rs
+++ b/rust/operator-binary/src/zk_controller.rs
@@ -311,9 +311,16 @@ pub async fn reconcile_zk(zk: Arc<ZookeeperCluster>, ctx: Arc<Ctx>) -> Result<co
     // std's SipHasher is deprecated, and DefaultHasher is unstable across Rust releases.
     // We don't /need/ stability, but it's still nice to avoid spurious changes where possible.
     let mut discovery_hash = FnvHasher::with_key(0);
-    for discovery_cm in build_discovery_configmaps(&zk, &*zk, client, &server_role_service, None)
-        .await
-        .context(BuildDiscoveryConfigSnafu)?
+    for discovery_cm in build_discovery_configmaps(
+        &zk,
+        &*zk,
+        client,
+        ZK_CONTROLLER_NAME,
+        &server_role_service,
+        None,
+    )
+    .await
+    .context(BuildDiscoveryConfigSnafu)?
     {
         let discovery_cm = cluster_resources
             .add(client, &discovery_cm)
diff --git a/rust/operator-binary/src/znode_controller.rs b/rust/operator-binary/src/znode_controller.rs
index cbbc666b..79eb0e7c 100644
--- a/rust/operator-binary/src/znode_controller.rs
+++ b/rust/operator-binary/src/znode_controller.rs
@@ -214,10 +214,16 @@ async fn reconcile_apply(
         .context(FindZkSvcSnafu {
             zk: ObjectRef::from_obj(&zk),
         })?;
-    for discovery_cm in
-        build_discovery_configmaps(&zk, znode, client, &server_role_service, Some(znode_path))
-            .await
-            .context(BuildDiscoveryConfigMapSnafu)?
+    for discovery_cm in build_discovery_configmaps(
+        &zk,
+        znode,
+        client,
+        ZNODE_CONTROLLER_NAME,
+        &server_role_service,
+        Some(znode_path),
+    )
+    .await
+    .context(BuildDiscoveryConfigMapSnafu)?
     {
         cluster_resources
             .add(client, &discovery_cm)
diff --git a/tests/templates/kuttl/znode/00-assert.yaml b/tests/templates/kuttl/znode/00-assert.yaml
new file mode 100644
index 00000000..35738200
--- /dev/null
+++ b/tests/templates/kuttl/znode/00-assert.yaml
@@ -0,0 +1,17 @@
+---
+apiVersion: kuttl.dev/v1beta1
+kind: TestAssert
+timeout: 600
+---
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: test-zk-server-default
+status:
+  readyReplicas: 1
+  replicas: 1
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-znode
diff --git a/tests/templates/kuttl/znode/00-install-zookeeper.yaml.j2 b/tests/templates/kuttl/znode/00-install-zookeeper.yaml.j2
new file mode 100644
index 00000000..4bb126c9
--- /dev/null
+++ b/tests/templates/kuttl/znode/00-install-zookeeper.yaml.j2
@@ -0,0 +1,19 @@
+---
+apiVersion: zookeeper.stackable.tech/v1alpha1
+kind: ZookeeperCluster
+metadata:
+  name: test-zk
+spec:
+  version: {{ test_scenario['values']['zookeeper-latest'] }}
+  servers:
+    roleGroups:
+      default:
+        replicas: 1
+---
+apiVersion: zookeeper.stackable.tech/v1alpha1
+kind: ZookeeperZnode
+metadata:
+  name: test-znode
+spec:
+  clusterRef:
+    name: test-zk
diff --git a/tests/test-definition.yaml b/tests/test-definition.yaml
index 1176f237..1a4885ef 100644
--- a/tests/test-definition.yaml
+++ b/tests/test-definition.yaml
@@ -6,6 +6,9 @@ dimensions:
       - 3.6.3-stackable0.8.0
       - 3.7.0-stackable0.8.0
       - 3.8.0-stackable0.8.0
+  - name: zookeeper-latest
+    values:
+      - 3.8.0-stackable0.8.0
   - name: use-client-tls
     values:
       - "true"
@@ -23,3 +26,6 @@ tests:
   - name: delete-rolegroup
     dimensions:
       - zookeeper
+  - name: znode
+    dimensions:
+      - zookeeper-latest