Skip to content

Commit e19f49a

Browse files
oliashishclaude
oliashish
and
claude
committed
Fix: Trigger nodeSet reconciler with ansibleVarsFrom changes
The NodeSet reconciler was not being triggered when ConfigMaps and Secrets referenced in ansibleVarsFrom were modified. This caused the NodeSet status to not update when configuration data changed, preventing deployments from recognizing that nodes redeployment. Changes: - Add ProcessAnsibleVarsFrom function in internal/dataplane/hashes.go to compute hashes for ConfigMaps and Secrets referenced in AnsibleVarsFrom at both NodeTemplate and individual Node - Add checkAnsibleVarsFromChanged function in the NodeSet controller to compare current hashes with deployed hashes and detect configuration drift. - Update deployment controller with setNodeSetAnsibleVarsFromHashes to store ConfigMap and Secret hashes in deployment status for comparison - Add kuttl test cases to verify ansibleVarsFrom change detection triggers proper reconciliation (08-ansiblevars-deploy and 09-ansiblevars-update) The fix ensures that when a ConfigMap or Secret referenced by ansibleVarsFrom is modified, the NodeSet controller detects the change and updates its status to indicate it is no longer ready for deployment, allowing operators to trigger a new deployment with the updated configuration. Jira: OSPRH-17961 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent bc2323a commit e19f49a

7 files changed

Lines changed: 321 additions & 0 deletions

File tree

‎internal/controller/dataplane/openstackdataplanedeployment_controller.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,11 @@ func (r *OpenStackDataPlaneDeploymentReconciler) setHashes(
451451

452452
for _, nodeSet := range nodeSets.Items {
453453
instance.Status.NodeSetHashes[nodeSet.Name] = nodeSet.Status.ConfigHash
454+
455+
err = setNodeSetAnsibleVarsFromHashes(ctx, helper, &nodeSet, instance.Status.ConfigMapHashes, instance.Status.SecretHashes)
456+
if err != nil {
457+
return err
458+
}
454459
}
455460

456461
return nil
@@ -547,3 +552,27 @@ func (r *OpenStackDataPlaneDeploymentReconciler) listNodeSets(ctx context.Contex
547552
}
548553
return &nodeSets, err
549554
}
555+
556+
func setNodeSetAnsibleVarsFromHashes(
557+
ctx context.Context,
558+
helper *helper.Helper,
559+
nodeSetInstance *dataplanev1.OpenStackDataPlaneNodeSet,
560+
configMapHashes map[string]string, // configMapHashes from deployment.instance.configMapHashes
561+
secretHashes map[string]string, // secretHashes from deployment.instance.secretHashes
562+
) error {
563+
namespace := nodeSetInstance.Namespace
564+
565+
// Process NodeTemplate level AnsibleVarsFrom
566+
if err := deployment.ProcessAnsibleVarsFrom(ctx, helper, namespace, configMapHashes, secretHashes, nodeSetInstance.Spec.NodeTemplate.Ansible.AnsibleVarsFrom); err != nil {
567+
return err
568+
}
569+
570+
// Process individual Node level AnsibleVarsFrom
571+
for _, node := range nodeSetInstance.Spec.Nodes {
572+
if err := deployment.ProcessAnsibleVarsFrom(ctx, helper, namespace, configMapHashes, secretHashes, node.Ansible.AnsibleVarsFrom); err != nil {
573+
return err
574+
}
575+
}
576+
577+
return nil
578+
}

‎internal/controller/dataplane/openstackdataplanenodeset_controller.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,17 @@ func checkDeployment(ctx context.Context, helper *helper.Helper,
576576
deployment.Status.BmhRefHashes[instance.Name] != instance.Status.BmhRefHash) {
577577
continue
578578
}
579+
580+
hasAnsibleVarsFromChanged, err := checkAnsibleVarsFromChanged(ctx, helper, instance, deployment.Status.ConfigMapHashes, deployment.Status.SecretHashes)
581+
582+
if err != nil {
583+
return isNodeSetDeploymentReady, isNodeSetDeploymentRunning, isNodeSetDeploymentFailed, failedDeploymentName, err
584+
}
585+
586+
if hasAnsibleVarsFromChanged {
587+
continue
588+
}
589+
579590
isNodeSetDeploymentReady = true
580591
for k, v := range deployment.Status.ConfigMapHashes {
581592
instance.Status.ConfigMapHashes[k] = v
@@ -911,3 +922,49 @@ func (r *OpenStackDataPlaneNodeSetReconciler) GetSpecConfigHash(instance *datapl
911922
}
912923
return configHash, nil
913924
}
925+
926+
// checkAnsibleVarsFromChanged computes current hashes for ConfigMaps/Secrets
927+
// referenced in AnsibleVarsFrom and compares them with deployed hashes.
928+
// Returns true if any content has changed, false otherwise.
929+
func checkAnsibleVarsFromChanged(
930+
ctx context.Context,
931+
helper *helper.Helper,
932+
instance *dataplanev1.OpenStackDataPlaneNodeSet,
933+
deployedConfigMapHashes map[string]string,
934+
deployedSecretHashes map[string]string,
935+
) (bool, error) {
936+
currentConfigMapHashes := make(map[string]string)
937+
currentSecretHashes := make(map[string]string)
938+
939+
namespace := instance.Namespace
940+
941+
// Process NodeTemplate level AnsibleVarsFrom
942+
if err := deployment.ProcessAnsibleVarsFrom(ctx, helper, namespace, currentConfigMapHashes, currentSecretHashes, instance.Spec.NodeTemplate.Ansible.AnsibleVarsFrom); err != nil {
943+
return false, err
944+
}
945+
946+
// Process individual Node level AnsibleVarsFrom
947+
for _, node := range instance.Spec.Nodes {
948+
if err := deployment.ProcessAnsibleVarsFrom(ctx, helper, namespace, currentConfigMapHashes, currentSecretHashes, node.Ansible.AnsibleVarsFrom); err != nil {
949+
return false, err
950+
}
951+
}
952+
953+
// Compare current ConfigMap hashes with deployed hashes
954+
for name, currentHash := range currentConfigMapHashes {
955+
if deployedHash, exists := deployedConfigMapHashes[name]; !exists || deployedHash != currentHash {
956+
helper.GetLogger().Info("ConfigMap content changed", "configMap", name)
957+
return true, nil
958+
}
959+
}
960+
961+
// Compare current Secret hashes with deployed hashes
962+
for name, currentHash := range currentSecretHashes {
963+
if deployedHash, exists := deployedSecretHashes[name]; !exists || deployedHash != currentHash {
964+
helper.GetLogger().Info("Secret content changed", "secret", name)
965+
return true, nil
966+
}
967+
}
968+
969+
return false, nil
970+
}

‎internal/dataplane/hashes.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,42 @@ func GetDeploymentHashesForService(
100100

101101
return nil
102102
}
103+
104+
// ProcessAnsibleVarsFrom computes hashes for ConfigMaps and Secrets
105+
// referenced in the NodeSet's AnsibleVarsFrom field (both NodeTemplate and
106+
// individual Nodes)
107+
func ProcessAnsibleVarsFrom(
108+
ctx context.Context,
109+
helper *helper.Helper,
110+
namespace string,
111+
configMapHashes map[string]string,
112+
secretHashes map[string]string,
113+
varsFrom []dataplanev1.DataSource,
114+
) error {
115+
for _, dataSource := range varsFrom {
116+
cm, sec, err := dataplaneutil.GetDataSourceCmSecret(ctx, helper, namespace, dataSource)
117+
118+
if err != nil {
119+
return err
120+
}
121+
122+
if cm != nil {
123+
hash, err := configmap.Hash(cm)
124+
if err != nil {
125+
helper.GetLogger().Error(err, "Unable to hash ConfigMap", "configMap", cm.Name)
126+
return err
127+
}
128+
configMapHashes[cm.Name] = hash
129+
}
130+
131+
if sec != nil {
132+
hash, err := secret.Hash(sec)
133+
if err != nil {
134+
helper.GetLogger().Error(err, "Unable to hash Secret", "secret", sec.Name)
135+
return err
136+
}
137+
secretHashes[sec.Name] = hash
138+
}
139+
}
140+
return nil
141+
}

‎test/kuttl/tests/dataplane-deploy-no-nodes-test/08-ansiblevars-deploy.yaml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Test ansibleVarsFrom ConfigMap/Secret change detection
2+
# First, delete deployments from previous steps to ensure NodeSet is in a clean state
3+
apiVersion: kuttl.dev/v1beta1
4+
kind: TestStep
5+
commands:
6+
- script: oc delete openstackdataplanedeployment edpm-compute-no-nodes-non-existent-service -n openstack-kuttl-tests --ignore-not-found=true
7+
- script: oc delete openstackdataplanedeployment edpm-compute-node-selection -n openstack-kuttl-tests --ignore-not-found=true
8+
---
9+
# Create ConfigMap and Secret for ansibleVarsFrom
10+
apiVersion: v1
11+
kind: ConfigMap
12+
metadata:
13+
name: ansiblevars-test-cm
14+
data:
15+
test_var: original-value
16+
---
17+
apiVersion: v1
18+
kind: Secret
19+
metadata:
20+
name: ansiblevars-test-secret
21+
stringData:
22+
secret_var: original-secret-value
23+
---
24+
# Patch existing NodeSet to add ansibleVarsFrom
25+
apiVersion: dataplane.openstack.org/v1beta1
26+
kind: OpenStackDataPlaneNodeSet
27+
metadata:
28+
name: edpm-compute-no-nodes
29+
spec:
30+
nodeTemplate:
31+
ansible:
32+
ansibleVarsFrom:
33+
- configMapRef:
34+
name: ansiblevars-test-cm
35+
- secretRef:
36+
name: ansiblevars-test-secret
37+
---
38+
# Create deployment to capture the hashes
39+
apiVersion: dataplane.openstack.org/v1beta1
40+
kind: OpenStackDataPlaneDeployment
41+
metadata:
42+
name: edpm-ansiblevars-deploy
43+
spec:
44+
nodeSets:
45+
- edpm-compute-no-nodes
46+
servicesOverride:
47+
- configure-os

‎test/kuttl/tests/dataplane-deploy-no-nodes-test/08-assert.yaml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
apiVersion: kuttl.dev/v1beta1
2+
kind: TestAssert
3+
timeout: 600
4+
collectors:
5+
- type: command
6+
command: oc logs -n openstack-operators -l openstack.org/operator-name=openstack
7+
name: operator-logs
8+
---
9+
# Assert NodeSet is Ready with configMapHashes populated
10+
apiVersion: dataplane.openstack.org/v1beta1
11+
kind: OpenStackDataPlaneNodeSet
12+
metadata:
13+
name: edpm-compute-no-nodes
14+
namespace: openstack-kuttl-tests
15+
status:
16+
conditions:
17+
- message: NodeSet Ready
18+
reason: Ready
19+
status: "True"
20+
type: Ready
21+
- message: Deployment completed
22+
reason: Ready
23+
status: "True"
24+
type: DeploymentReady
25+
- message: Input data complete
26+
reason: Ready
27+
status: "True"
28+
type: InputReady
29+
- message: NodeSetDNSDataReady ready
30+
reason: Ready
31+
status: "True"
32+
type: NodeSetDNSDataReady
33+
- message: NodeSetIPReservationReady ready
34+
reason: Ready
35+
status: "True"
36+
type: NodeSetIPReservationReady
37+
- message: ServiceAccount created
38+
reason: Ready
39+
status: "True"
40+
type: ServiceAccountReady
41+
- message: Setup complete
42+
reason: Ready
43+
status: "True"
44+
type: SetupReady
45+
configMapHashes:
46+
ansiblevars-test-cm: n5bbhc6h5fdhf8h57bh665h5b9h576h598h7dh5fdh667hch5b5h89h9dh668hc4hc6h669h74h575hf4h596h66dhd5h5f7hb7h689hf9hddh64q
47+
secretHashes:
48+
ansiblevars-test-secret: n5d8h67h5b8hc9h5cfh54bh76h544h66chf7h8bh5fbh59chdch76h96h54bh74h9bh5bh5cdh94h566h699h99hdch65fh5d6h5fbh5bfh586hfcq
49+
---
50+
# Assert deployment completed with hashes
51+
apiVersion: dataplane.openstack.org/v1beta1
52+
kind: OpenStackDataPlaneDeployment
53+
metadata:
54+
name: edpm-ansiblevars-deploy
55+
namespace: openstack-kuttl-tests
56+
status:
57+
deployed: true
58+
configMapHashes:
59+
ansiblevars-test-cm: n5bbhc6h5fdhf8h57bh665h5b9h576h598h7dh5fdh667hch5b5h89h9dh668hc4hc6h669h74h575hf4h596h66dhd5h5f7hb7h689hf9hddh64q
60+
secretHashes:
61+
ansiblevars-test-secret: n5d8h67h5b8hc9h5cfh54bh76h544h66chf7h8bh5fbh59chdch76h96h54bh74h9bh5bh5cdh94h566h699h99hdch65fh5d6h5fbh5bfh586hfcq

‎test/kuttl/tests/dataplane-deploy-no-nodes-test/09-ansiblevars-update.yaml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Update ConfigMap and Secret content to trigger change detection
2+
apiVersion: v1
3+
kind: ConfigMap
4+
metadata:
5+
name: ansiblevars-test-cm
6+
data:
7+
test_var: updated-value
8+
---
9+
apiVersion: v1
10+
kind: Secret
11+
metadata:
12+
name: ansiblevars-test-secret
13+
stringData:
14+
secret_var: updated-secret-value
15+
---
16+
# Create deployment to capture the new hashes after content update
17+
apiVersion: dataplane.openstack.org/v1beta1
18+
kind: OpenStackDataPlaneDeployment
19+
metadata:
20+
name: edpm-ansiblevars-deploy-update
21+
spec:
22+
nodeSets:
23+
- edpm-compute-no-nodes
24+
servicesOverride:
25+
- configure-os

‎test/kuttl/tests/dataplane-deploy-no-nodes-test/09-assert.yaml‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
apiVersion: kuttl.dev/v1beta1
2+
kind: TestAssert
3+
timeout: 600
4+
collectors:
5+
- type: command
6+
command: oc logs -n openstack-operators -l openstack.org/operator-name=openstack
7+
name: operator-logs
8+
---
9+
# Assert NodeSet is Ready after deployment with updated ConfigMap/Secret completes
10+
# The hash values will differ from step 08 due to content change
11+
apiVersion: dataplane.openstack.org/v1beta1
12+
kind: OpenStackDataPlaneNodeSet
13+
metadata:
14+
name: edpm-compute-no-nodes
15+
namespace: openstack-kuttl-tests
16+
status:
17+
conditions:
18+
- message: NodeSet Ready
19+
reason: Ready
20+
status: "True"
21+
type: Ready
22+
- message: Deployment completed
23+
reason: Ready
24+
status: "True"
25+
type: DeploymentReady
26+
- message: Input data complete
27+
reason: Ready
28+
status: "True"
29+
type: InputReady
30+
- message: NodeSetDNSDataReady ready
31+
reason: Ready
32+
status: "True"
33+
type: NodeSetDNSDataReady
34+
- message: NodeSetIPReservationReady ready
35+
reason: Ready
36+
status: "True"
37+
type: NodeSetIPReservationReady
38+
- message: ServiceAccount created
39+
reason: Ready
40+
status: "True"
41+
type: ServiceAccountReady
42+
- message: Setup complete
43+
reason: Ready
44+
status: "True"
45+
type: SetupReady
46+
configMapHashes:
47+
ansiblevars-test-cm: nd7h5fdh54bh8chb7h5hc8h654hf8h5d5h66bh54dh56fh7bhf5h98h66fh59ch87h9ch5d6h664h668hb7h66fh7fh598h9fh57dhfh5fch655q
48+
secretHashes:
49+
ansiblevars-test-secret: n56hffh656h98hc9h4h79h86h5bfh695hd7h5f9h5d4h87h85h546h7dhc5h98h676h5fh66bh66bhb5h679h77hf8hdbh569h66dhb5h59dq
50+
---
51+
# Assert deployment completed with new hashes
52+
apiVersion: dataplane.openstack.org/v1beta1
53+
kind: OpenStackDataPlaneDeployment
54+
metadata:
55+
name: edpm-ansiblevars-deploy-update
56+
namespace: openstack-kuttl-tests
57+
status:
58+
deployed: true
59+
configMapHashes:
60+
ansiblevars-test-cm: nd7h5fdh54bh8chb7h5hc8h654hf8h5d5h66bh54dh56fh7bhf5h98h66fh59ch87h9ch5d6h664h668hb7h66fh7fh598h9fh57dhfh5fch655q
61+
secretHashes:
62+
ansiblevars-test-secret: n56hffh656h98hc9h4h79h86h5bfh695hd7h5f9h5d4h87h85h546h7dhc5h98h676h5fh66bh66bhb5h679h77hf8hdbh569h66dhb5h59dq
63+

0 commit comments

Comments
 (0)