From 672ee6741a8d7cac97ef54f0213614d71261bec1 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Fri, 5 Apr 2024 16:20:57 +0200 Subject: [PATCH] Move ObservedGeneration earlier in reconciliation We need to updated ObservedGeneration just after we reinit conditions to avoid race condtitions. --- api/go.mod | 2 +- api/go.sum | 4 ++-- controllers/placementapi_controller.go | 12 ++++++++---- go.mod | 6 +++--- go.sum | 12 ++++++------ tests/functional/placementapi_controller_test.go | 2 ++ 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/api/go.mod b/api/go.mod index b5570733..a994b717 100644 --- a/api/go.mod +++ b/api/go.mod @@ -3,7 +3,7 @@ module github.com/openstack-k8s-operators/placement-operator/api go 1.20 require ( - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484 + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034 k8s.io/api v0.28.8 k8s.io/apimachinery v0.28.8 sigs.k8s.io/controller-runtime v0.16.5 diff --git a/api/go.sum b/api/go.sum index 4ee7e6a2..44cebc55 100644 --- a/api/go.sum +++ b/api/go.sum @@ -64,8 +64,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8= github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484 h1:P3I3QBjZql8M5XXB/XBUdeM//e3XHtLv4yu7e+QlYQ8= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484/go.mod h1:gqByVGUdKQB/NkhKV4eD+8NWYkHq961nC96rTCB3ywE= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034 h1:HLI/aUA7KnBK5oLPIWXMasz7zqjal4GhNn1P/fdLI20= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034/go.mod h1:gqByVGUdKQB/NkhKV4eD+8NWYkHq961nC96rTCB3ywE= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index a929c8d3..a3d1ddc6 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -264,6 +264,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request if err = r.initStatus(ctx, h, instance); err != nil { return ctrl.Result{}, err } + instance.Status.ObservedGeneration = instance.Generation // Always patch the instance status when exiting this function so we can persist any changes. defer func() { @@ -503,7 +504,6 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } - instance.Status.ObservedGeneration = instance.Generation return ctrl.Result{}, nil } @@ -1158,7 +1158,11 @@ func (r *PlacementAPIReconciler) ensureDeployment( condition.DeploymentReadyRunningMessage)) return ctrl.Result{}, nil } - instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas + + deployment := depl.GetDeployment() + if depl.GetDeployment().Generation == depl.GetDeployment().Status.ObservedGeneration { + instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas + } // verify if network attachment matches expectations networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, h, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount) @@ -1181,7 +1185,7 @@ func (r *PlacementAPIReconciler) ensureDeployment( return ctrl.Result{}, err } - if instance.Status.ReadyCount > 0 || *instance.Spec.Replicas == 0 { + if instance.Status.ReadyCount == *instance.Spec.Replicas && deployment.Generation == deployment.Status.ObservedGeneration { instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) } else { Log.Info("Deployment is not ready") @@ -1193,11 +1197,11 @@ func (r *PlacementAPIReconciler) ensureDeployment( // It is OK to return success as we are watching for StatefulSet changes return ctrl.Result{}, nil } - // create Deployment - end Log.Info("Reconciled Service successfully") return ctrl.Result{}, nil + } // generateServiceConfigMaps - create create configmaps which hold scripts and service configuration diff --git a/go.mod b/go.mod index 77874dd6..71354287 100644 --- a/go.mod +++ b/go.mod @@ -9,9 +9,9 @@ require ( github.com/onsi/ginkgo/v2 v2.17.1 github.com/onsi/gomega v1.32.0 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240402154848-e5f862707f49 - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484 - github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240404123425-54f145c97484 - github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240403152257-75b048d878bf + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034 + github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240216173409-86913e6d5885 + github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240418060416-9de2d1f1915e github.com/openstack-k8s-operators/placement-operator/api v0.3.1-0.20240216174613-3d349f26e681 go.uber.org/zap v1.27.0 k8s.io/api v0.28.8 diff --git a/go.sum b/go.sum index 587db55e..63ef26f1 100644 --- a/go.sum +++ b/go.sum @@ -80,14 +80,14 @@ github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxC github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240402154848-e5f862707f49 h1:LSbLg+6iwX2jkVKe0ba6GqSO2mpoJlUZyWIWZA6jv6M= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240402154848-e5f862707f49/go.mod h1:opUQY0YZNCyA11FKLToVhaVZTTEMfbnf0ozOLmkKfGs= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484 h1:P3I3QBjZql8M5XXB/XBUdeM//e3XHtLv4yu7e+QlYQ8= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240404123425-54f145c97484/go.mod h1:gqByVGUdKQB/NkhKV4eD+8NWYkHq961nC96rTCB3ywE= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034 h1:HLI/aUA7KnBK5oLPIWXMasz7zqjal4GhNn1P/fdLI20= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240408095526-357d8fffa034/go.mod h1:gqByVGUdKQB/NkhKV4eD+8NWYkHq961nC96rTCB3ywE= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240404123425-54f145c97484 h1:0Wmtd+xyvPvU1O6xd2G27XwZfYY2ewhTrbd9ELHlHwg= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240404123425-54f145c97484/go.mod h1:U32T7Jz/vUBbMJVYuGSmhcPT2+WiOr+8zt2G9RPJGMI= -github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240404123425-54f145c97484 h1:PleqKcc38K3NxJEF8lgZEphEfBkAqRFwtspvv5yq5SM= -github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240404123425-54f145c97484/go.mod h1:ejPUK2xqh49WTnrLBPJyBAkMn3ZizO9pai39Aufg6C0= -github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240403152257-75b048d878bf h1:N0HGTE+WXkaVeCpsJO5QG79uyLhRuNa3/gCTGgfZfGo= -github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240403152257-75b048d878bf/go.mod h1:f9IIyWeoskWoeWaDFF3qmAJ2Kqyovfi0Ar/QUfk3qag= +github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240216173409-86913e6d5885 h1:ioJ2MO3vAcBkLM+0UBu5IuKW/DPXcyiNSOLq0Xvn+Nw= +github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240216173409-86913e6d5885/go.mod h1:82nzS+DbBe1tzaMvNHH8FctmZzQ14ZAJysFGsMJiivo= +github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240418060416-9de2d1f1915e h1:DnSo0dGQyS0BGPR+/1behQaiuO1trPh9g5G1CiHfOTk= +github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240418060416-9de2d1f1915e/go.mod h1:2wiOEd5wTbKQ00Js5pZx1ePwMM6xBkuZE+G4J38aYi0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index e963ccd5..3a5b0958 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -150,6 +150,7 @@ var _ = Describe("PlacementAPI controller", func() { DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPIName) }) + It("and deployment is Ready", func() { serviceSpec := corev1.ServiceSpec{Ports: []corev1.ServicePort{{Port: 3306}}} DeferCleanup( @@ -159,6 +160,7 @@ var _ = Describe("PlacementAPI controller", func() { mariadb.SimulateMariaDBDatabaseCompleted(names.MariaDBDatabaseName) mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount) th.SimulateJobSuccess(names.DBSyncJobName) + th.SimulateDeploymentAnyNumberReplicaReady(names.DeploymentName, 0) placement := GetPlacementAPI(names.PlacementAPIName) Expect(*(placement.Spec.Replicas)).Should(Equal(int32(0))) th.ExpectCondition(