From 1a41ac59f88ef7eb535ee1ea6f53d3592d6ae085 Mon Sep 17 00:00:00 2001 From: Luca Miccini Date: Tue, 13 Jan 2026 07:43:21 +0100 Subject: [PATCH] Add function to check the status of the owner of an object Function verifies: - Ready condition is True - observedGeneration matches generation --- modules/common/object/metadata.go | 111 +++++++ .../test/functional/object_generation_test.go | 297 ++++++++++++++++++ modules/common/test/functional/object_test.go | 197 ++++++++++++ 3 files changed, 605 insertions(+) create mode 100644 modules/common/test/functional/object_generation_test.go diff --git a/modules/common/object/metadata.go b/modules/common/object/metadata.go index 1660df5a..fd2fc812 100644 --- a/modules/common/object/metadata.go +++ b/modules/common/object/metadata.go @@ -31,6 +31,8 @@ import ( k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" ) // CheckOwnerRefExist - returns true if the owner is already in the owner ref list @@ -114,3 +116,112 @@ func EnsureOwnerRef( return nil } + +// IsOwnerServiceReady checks if the owner service that owns this object is ready. +// Returns true if the owner is ready, false if not ready, and error only for unexpected failures. +// If there's no owner with controller=true, it returns true (safe to proceed). +func IsOwnerServiceReady( + ctx context.Context, + h *helper.Helper, + obj client.Object, +) (bool, error) { + // Find the controller owner reference (e.g., Cinder, Nova, etc.) + var ownerRef *metav1.OwnerReference + for _, owner := range obj.GetOwnerReferences() { + if owner.Controller != nil && *owner.Controller { + ownerRef = &owner + break + } + } + + // If no controlling owner, safe to proceed + if ownerRef == nil { + h.GetLogger().Info("No controller owner found, owner is considered ready") + return true, nil + } + + // Parse the APIVersion to extract group and version + gv, err := schema.ParseGroupVersion(ownerRef.APIVersion) + if err != nil { + h.GetLogger().Error(err, "Failed to parse owner APIVersion", "apiVersion", ownerRef.APIVersion) + return false, err + } + + // Fetch the owner resource using unstructured client + owner := &unstructured.Unstructured{} + owner.SetGroupVersionKind(schema.GroupVersionKind{ + Group: gv.Group, + Version: gv.Version, + Kind: ownerRef.Kind, + }) + + err = h.GetClient().Get(ctx, types.NamespacedName{ + Name: ownerRef.Name, + Namespace: obj.GetNamespace(), + }, owner) + + if err != nil { + if k8s_errors.IsNotFound(err) { + // Owner deleted, safe to proceed + h.GetLogger().Info("Owner resource not found, owner is considered ready", "kind", ownerRef.Kind, "name", ownerRef.Name) + return true, nil + } + // Unexpected error, log and return error + h.GetLogger().Error(err, "Failed to fetch owner resource", "kind", ownerRef.Kind, "name", ownerRef.Name) + return false, err + } + + // Check status.conditions for Ready condition + conditions, found, err := unstructured.NestedSlice(owner.Object, "status", "conditions") + if err != nil || !found { + h.GetLogger().Info("No conditions found in owner status, waiting", "kind", ownerRef.Kind, "name", ownerRef.Name) + return false, nil + } + + // Look for Ready condition with status=True + isReady := false + for _, c := range conditions { + condition, ok := c.(map[string]any) + if !ok { + continue + } + + condType, _, _ := unstructured.NestedString(condition, "type") + status, _, _ := unstructured.NestedString(condition, "status") + + if condType == "Ready" && status == "True" { + isReady = true + break + } + } + + if !isReady { + h.GetLogger().Info("Owner service not ready, waiting", "kind", ownerRef.Kind, "name", ownerRef.Name) + return false, nil + } + + // Check if owner has reconciled (observedGeneration matches generation) + generation, foundGen, err := unstructured.NestedInt64(owner.Object, "metadata", "generation") + if err != nil || !foundGen { + h.GetLogger().Info("Could not get owner generation, waiting", "kind", ownerRef.Kind, "name", ownerRef.Name) + return false, nil + } + + observedGeneration, foundObsGen, err := unstructured.NestedInt64(owner.Object, "status", "observedGeneration") + if err != nil || !foundObsGen { + h.GetLogger().Info("Could not get owner observedGeneration, waiting", "kind", ownerRef.Kind, "name", ownerRef.Name) + return false, nil + } + + if observedGeneration != generation { + h.GetLogger().Info("Owner service has not reconciled yet (observedGeneration != generation), waiting", + "kind", ownerRef.Kind, + "name", ownerRef.Name, + "generation", generation, + "observedGeneration", observedGeneration) + return false, nil + } + + h.GetLogger().Info("Owner service is ready and has reconciled, safe to proceed", "kind", ownerRef.Kind, "name", ownerRef.Name) + return true, nil +} diff --git a/modules/common/test/functional/object_generation_test.go b/modules/common/test/functional/object_generation_test.go new file mode 100644 index 00000000..f7d06ded --- /dev/null +++ b/modules/common/test/functional/object_generation_test.go @@ -0,0 +1,297 @@ +/* +Copyright 2023 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package functional + +import ( + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" // nolint:revive + . "github.com/onsi/gomega" // nolint:revive + "github.com/openstack-k8s-operators/lib-common/modules/common/object" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +var _ = Describe("object package - observedGeneration tests", func() { + var namespace string + + BeforeEach(func() { + // NOTE(gibi): We need to create a unique namespace for each test run + // as namespaces cannot be deleted in a locally running envtest. See + // https://book.kubebuilder.io/reference/envtest.html#namespace-usage-limitation + namespace = uuid.New().String() + th.CreateNamespace(namespace) + // We still request the delete of the Namespace to properly cleanup if + // we run the test in an existing cluster. + DeferCleanup(th.DeleteNamespace, namespace) + }) + + When("checking if owner service is ready with observedGeneration", func() { + It("returns false when controller owner is ready but observedGeneration does not match generation", func() { + // Create a Deployment as the owner + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owner-deployment-not-reconciled", + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: func(i int32) *int32 { return &i }(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "test:latest", + }, + }, + }, + }, + }, + } + Expect(th.K8sClient.Create(th.Ctx, deployment)).Should(Succeed()) + + // Update deployment to increment generation + Eventually(func(g Gomega) { + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-not-reconciled", + }) + dep.Spec.Replicas = func(i int32) *int32 { return &i }(2) + g.Expect(th.K8sClient.Update(th.Ctx, dep)).Should(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Simulate deployment with Ready condition but observedGeneration < generation + Eventually(func(g Gomega) { + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-not-reconciled", + }) + + // Set status with Ready-like condition (Available) and observedGeneration behind + dep.Status.ObservedGeneration = dep.Generation - 1 + dep.Status.Conditions = []appsv1.DeploymentCondition{ + { + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }, + } + // Also add a Ready condition for completeness + dep.Status.Conditions = append(dep.Status.Conditions, appsv1.DeploymentCondition{ + Type: "Ready", + Status: corev1.ConditionTrue, + }) + + g.Expect(th.K8sClient.Status().Update(th.Ctx, dep)).Should(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Get the deployment to get its UID + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-not-reconciled", + }) + + // Create a child ConfigMap owned by the Deployment + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-not-reconciled", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "name": dep.Name, + "uid": string(dep.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeFalse()) + }) + + It("returns true when controller owner is ready and observedGeneration matches generation", func() { + // Create a Deployment as the owner + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owner-deployment-reconciled", + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: func(i int32) *int32 { return &i }(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "test:latest", + }, + }, + }, + }, + }, + } + Expect(th.K8sClient.Create(th.Ctx, deployment)).Should(Succeed()) + + // Simulate deployment with Ready condition and observedGeneration = generation + Eventually(func(g Gomega) { + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-reconciled", + }) + + // Set status with Ready condition and observedGeneration matching generation + dep.Status.ObservedGeneration = dep.Generation + dep.Status.Conditions = []appsv1.DeploymentCondition{ + { + Type: "Ready", + Status: corev1.ConditionTrue, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, dep)).Should(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Get the deployment to get its UID + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-reconciled", + }) + + // Create a child ConfigMap owned by the Deployment + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-reconciled", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "name": dep.Name, + "uid": string(dep.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeTrue()) + }) + + It("returns false when controller owner is ready but has no observedGeneration field", func() { + // Create a Deployment as the owner + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owner-deployment-no-observed", + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: func(i int32) *int32 { return &i }(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "test:latest", + }, + }, + }, + }, + }, + } + Expect(th.K8sClient.Create(th.Ctx, deployment)).Should(Succeed()) + + // Set status with Ready condition but explicitly set observedGeneration to 0 (unset) + Eventually(func(g Gomega) { + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-no-observed", + }) + + // Set status with Ready condition but no observedGeneration (leave it at 0) + dep.Status.ObservedGeneration = 0 + dep.Status.Conditions = []appsv1.DeploymentCondition{ + { + Type: "Ready", + Status: corev1.ConditionTrue, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, dep)).Should(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Get the deployment to get its UID + dep := th.GetDeployment(types.NamespacedName{ + Namespace: namespace, + Name: "owner-deployment-no-observed", + }) + + // Create a child ConfigMap owned by the Deployment + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-no-observed", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "name": dep.Name, + "uid": string(dep.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeFalse()) + }) + }) +}) diff --git a/modules/common/test/functional/object_test.go b/modules/common/test/functional/object_test.go index e3bbc504..465e02f0 100644 --- a/modules/common/test/functional/object_test.go +++ b/modules/common/test/functional/object_test.go @@ -21,6 +21,7 @@ import ( . "github.com/onsi/gomega" // nolint:revive "github.com/openstack-k8s-operators/lib-common/modules/common/object" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" ) @@ -72,4 +73,200 @@ var _ = Describe("object package", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(object.CheckOwnerRefExist(ownerCM.GetUID(), cm.GetOwnerReferences())).To(BeTrue()) }) + + When("checking if owner service is ready", func() { + It("returns true when object has no controller owner", func() { + cmName := types.NamespacedName{ + Namespace: namespace, + Name: "test-cm-no-owner", + } + cm := th.CreateConfigMap(cmName, map[string]interface{}{}) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeTrue()) + }) + + It("returns true when controller owner is deleted", func() { + // Create a ConfigMap with an owner reference to a non-existent ConfigMap + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm-deleted-owner", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "name": "non-existent-configmap", + "uid": "11111111-1111-1111-1111-111111111111", + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeTrue()) + }) + + It("returns false when controller owner exists but has no status", func() { + // Create owner ConfigMap (no status.conditions) + ownerCM := th.CreateConfigMap(types.NamespacedName{ + Namespace: namespace, + Name: "owner-cm-no-status", + }, map[string]interface{}{}) + + // Create child ConfigMap with owner reference + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-no-status", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "name": ownerCM.GetName(), + "uid": string(ownerCM.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + // ConfigMaps don't have status.conditions, so should return false + Expect(ready).To(BeFalse()) + }) + + It("returns false when controller owner has Ready condition with status True but no observedGeneration (Pod case)", func() { + // Create a Pod as the owner (Pods do not have observedGeneration, so the check will return false) + th.CreatePod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-ready", + }, map[string]string{}, map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "test", + "image": "test:latest", + }, + }, + }) + + // Update the Pod status to include a Ready condition + Eventually(func(g Gomega) { + pod := th.GetPod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-ready", + }) + // Manually set status with Ready condition + pod.Status.Conditions = []corev1.PodCondition{ + { + Type: "Ready", + Status: corev1.ConditionTrue, + }, + } + err := th.K8sClient.Status().Update(th.Ctx, pod) + g.Expect(err).ShouldNot(HaveOccurred()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Get the updated pod to get its UID + pod := th.GetPod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-ready", + }) + + // Create a ConfigMap owned by the Pod + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-ready-owner", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "name": pod.Name, + "uid": string(pod.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeFalse()) + }) + + It("returns false when controller owner has Ready condition with status False", func() { + // Create a Pod as the owner + th.CreatePod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-not-ready", + }, map[string]string{}, map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "test", + "image": "test:latest", + }, + }, + }) + + // Update the Pod status to include a Ready=False condition + Eventually(func(g Gomega) { + pod := th.GetPod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-not-ready", + }) + pod.Status.Conditions = []corev1.PodCondition{ + { + Type: "Ready", + Status: corev1.ConditionFalse, + }, + } + err := th.K8sClient.Status().Update(th.Ctx, pod) + g.Expect(err).ShouldNot(HaveOccurred()) + }, th.Timeout, th.Interval).Should(Succeed()) + + // Get the updated pod + pod := th.GetPod(types.NamespacedName{ + Namespace: namespace, + Name: "owner-pod-not-ready", + }) + + // Create a ConfigMap owned by the Pod + rawCM := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "child-cm-not-ready-owner", + "namespace": namespace, + "ownerReferences": []interface{}{ + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "name": pod.Name, + "uid": string(pod.GetUID()), + "controller": true, + }, + }, + }, + } + cm := th.CreateUnstructured(rawCM) + + ready, err := object.IsOwnerServiceReady(th.Ctx, h, cm) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ready).To(BeFalse()) + }) + }) })