Merge pull request #1211 from olliewalsh/openstackclient-pod-template-hash

[openstackclient] Store hash to determine when pod spec changes

Author: openshift-merge-bot[bot]

apis/bases/client.openstack.org_openstackclients.yaml

@@ -82,6 +82,10 @@ spec:
                   - type
                   type: object
                 type: array
+              hash:
+                additionalProperties:
+                  type: string
+                type: object
               observedGeneration:
                 format: int64
                 type: integer

apis/client/v1beta1/openstackclient_types.go

@@ -68,6 +68,9 @@ type OpenStackClientStatus struct {
 
 	//ObservedGeneration - the most recent generation observed for this object.
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
+	// Map of hashes to track e.g. pod spec
+	Hash map[string]string `json:"hash,omitempty"`
 }
 
 // +kubebuilder:object:root=true

apis/client/v1beta1/zz_generated.deepcopy.go

@@ -82,6 +82,10 @@ spec:
                   - type
                   type: object
                 type: array
+              hash:
+                additionalProperties:
+                  type: string
+                type: object
               observedGeneration:
                 format: int64
                 type: integer

config/crd/bases/client.openstack.org_openstackclients.yaml

@@ -307,16 +307,19 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 	spec := openstackclient.ClientPodSpec(ctx, instance, helper, configVarsHash)
 
+	podSpecHash, err := util.ObjectHash(spec)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	podSpecHashName := "podSpec"
+
 	op, err := controllerutil.CreateOrPatch(ctx, r.Client, osclient, func() error {
 		isPodUpdate := !osclient.ObjectMeta.CreationTimestamp.IsZero()
-		if !isPodUpdate {
+		currentPodSpecHash := instance.Status.Hash[podSpecHashName]
+		if !isPodUpdate || currentPodSpecHash != podSpecHash {
 			osclient.Spec = spec
-		} else {
-			osclient.Spec.Containers[0].Env = spec.Containers[0].Env
-			osclient.Spec.NodeSelector = spec.NodeSelector
-			osclient.Spec.Containers[0].Image = instance.Spec.ContainerImage
 		}
-
 		osclient.Labels = util.MergeStringMaps(osclient.Labels, clientLabels)
 
 		err = controllerutil.SetControllerReference(instance, osclient, r.Scheme)
@@ -360,6 +363,8 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, err
 	}
 
+	instance.Status.Hash, _ = util.SetHash(instance.Status.Hash, podSpecHashName, podSpecHash)
+
 	if op != controllerutil.OperationResultNone {
 		util.LogForObject(
 			helper,

controllers/client/openstackclient_controller.go

@@ -3,6 +3,13 @@ set -euxo pipefail
 
 CHECKER=$INSTALL_DIR/crd-schema-checker
 
+DISABLED_VALIDATORS=NoMaps # TODO: https://issues.redhat.com/browse/OSPRH-12254
+
+CHECKER_ARGS=""
+if [[ ${DISABLED_VALIDATORS:+x} ]]; then
+    CHECKER_ARGS="$CHECKER_ARGS --disabled-validators $DISABLED_VALIDATORS"
+fi
+
 TMP_DIR=$(mktemp -d)
 
 function cleanup {
@@ -16,6 +23,7 @@ for crd in config/crd/bases/*.yaml; do
     mkdir -p "$(dirname "$TMP_DIR/$crd")"
     if git show "$BASE_REF:$crd" > "$TMP_DIR/$crd"; then
         $CHECKER check-manifests \
+            $CHECKER_ARGS \
             --existing-crd-filename="$TMP_DIR/$crd" \
             --new-crd-filename="$crd"
     fi

hack/crd-schema-checker.sh

@@ -1,5 +1,7 @@
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
+  - script: |
+      oc annotate namespace $NAMESPACE openshift.io/node-selector="beta.kubernetes.io/os=linux"
   - script: |
       oc kustomize ../../../../config/samples/nodeselectors/global | oc apply -n $NAMESPACE -f -

tests/kuttl/tests/ctlplane-nodeselectors/01-deploy-openstack.yaml

@@ -3,7 +3,7 @@ kind: TestAssert
 commands:
   - script: |
       echo "Checking all pods have expected nodeselector"
-      EXPECTED_NODE_SELECTOR="node-role.kubernetes.io/worker:"
+      EXPECTED_NODE_SELECTOR="beta.kubernetes.io/os:linux node-role.kubernetes.io/worker:"
       BAD_OR_MISSING_NODE_SELECTOR=$(oc get pods -n $NAMESPACE -l service!=dnsmasq -o=go-template --template='{{ range .items }}{{ .metadata.name}}: {{ .spec.nodeSelector }}{{"\n"}}{{ end }}' | grep -v 'ovn-controller-.*-config' | sed -e '\!map\['"$EXPECTED_NODE_SELECTOR"'\]$!d')
       BAD_OR_MISSING_NODE_SELECTOR_COUNT=$(echo -n "$BAD_OR_MISSING_NODE_SELECTOR" | wc -l)
       if [ $BAD_OR_MISSING_NODE_SELECTOR_COUNT -ne 0 ]; then

tests/kuttl/tests/ctlplane-nodeselectors/02-assert-nodeselector.yaml

@@ -2,12 +2,6 @@ apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 timeout: 60
 commands:
-  - script: |
-      oc patch dnsmasq -n $NAMESPACE dnsmasq --type='json' -p='[{
-        "op": "replace",
-        "path": "/spec/nodeSelector",
-        "value": {"kubernetes.io/os":"linux"}
-      }]'
   - script: |
       oc patch openstackcontrolplane -n $NAMESPACE openstack --type='json' -p='[{
         "op": "replace",

tests/kuttl/tests/ctlplane-nodeselectors/03-update-nodeselector.yaml

@@ -3,7 +3,7 @@ kind: TestAssert
 commands:
   - script: |
       echo "Checking all running pods have new nodeselector"
-      EXPECTED_NODE_SELECTOR="kubernetes.io/os:linux"
+      EXPECTED_NODE_SELECTOR="beta.kubernetes.io/os:linux kubernetes.io/os:linux"
       BAD_OR_MISSING_NODE_SELECTOR=$(oc get pods -n $NAMESPACE -l service!=dnsmasq --field-selector=status.phase=Running -o=go-template --template='{{ range .items }}{{ .metadata.name}}: {{ .spec.nodeSelector }}{{"\n"}}{{ end }}' | grep -v 'ovn-controller-.*-config' | sed -e '\!map\['"$EXPECTED_NODE_SELECTOR"'\]$!d')
       BAD_OR_MISSING_NODE_SELECTOR_COUNT=$(echo -n "$BAD_OR_MISSING_NODE_SELECTOR" | wc -l)
       if [ $BAD_OR_MISSING_NODE_SELECTOR_COUNT -ne 0 ]; then