Skip to content

Commit f851b77

Browse files
stuggistuggi
committed
Use deployment.IsReady() to validate status
Changes to use the common IsReady() func to validate a deployment is fully up as requested and e.g. no update rollout in progress. During a minor update this has seen to already report/mark the condition ready, even the deployment is still in progress, or the replacement pod failed. Jira: OSPRH-14472 Depends-On: openstack-k8s-operators/lib-common#616 Signed-off-by: Martin Schuppert <[email protected]>
1 parent 4e4066d commit f851b77

File tree

6 files changed

+107
-8
lines changed

6 files changed

+107
-8
lines changed

‎api/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require (
77
github.com/google/uuid v1.6.0
88
github.com/onsi/gomega v1.34.1
99
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d
10-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1
10+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b
1111
k8s.io/api v0.29.15
1212
k8s.io/apimachinery v0.29.15
1313
sigs.k8s.io/controller-runtime v0.17.6

‎api/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6
8080
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
8181
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d h1:/C+ysubV00VYrqGFhQlDeQ5tUtnhIWPwQUc8MOfCEBA=
8282
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d/go.mod h1:IY5zp1GRhacGMvjZMUZQ0LlxWDaogIRb/4H3by1Pivk=
83-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1 h1:hO90JhfinKysbdrWCJugTmJbkrs1d9tR7ypg3yOD12E=
84-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1/go.mod h1:A9Ohw5Q90YOGhcDF4ZHkMr5RArz3phoBu9+ibbYDKG4=
83+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b h1:T+N6xOT2NP+hVp2K1xl/NV3uheVHu38CcBuW+8uOBYw=
84+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b/go.mod h1:A9Ohw5Q90YOGhcDF4ZHkMr5RArz3phoBu9+ibbYDKG4=
8585
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
8686
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
8787
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

‎controllers/ovnnorthd_controller.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,12 +485,27 @@ func (r *OVNNorthdReconciler) reconcileNormal(ctx context.Context, instance *ovn
485485
return ctrlResult, nil
486486
}
487487

488-
instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
488+
deploy := depl.GetDeployment()
489+
if deploy.Generation == deploy.Status.ObservedGeneration {
490+
instance.Status.ReadyCount = deploy.Status.ReadyReplicas
491+
}
489492

490-
if instance.Status.ReadyCount > 0 {
493+
// Mark the Deployment as Ready only if the number of Replicas is equals
494+
// to the Deployed instances (ReadyCount), and the the Status.Replicas
495+
// match Status.ReadyReplicas. If a deployment update is in progress,
496+
// Replicas > ReadyReplicas.
497+
// In addition, make sure the controller sees the last Generation
498+
// by comparing it with the ObservedGeneration.
499+
if deployment.IsReady(deploy) {
491500
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
492501
} else if *instance.Spec.Replicas == 0 {
493502
instance.Status.Conditions.Remove(condition.DeploymentReadyCondition)
503+
} else {
504+
instance.Status.Conditions.Set(condition.FalseCondition(
505+
condition.DeploymentReadyCondition,
506+
condition.RequestedReason,
507+
condition.SeverityInfo,
508+
condition.DeploymentReadyRunningMessage))
494509
}
495510
// create Deployment - end
496511

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ require (
99
github.com/onsi/ginkgo/v2 v2.20.1
1010
github.com/onsi/gomega v1.34.1
1111
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d
12-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1
12+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b
1313
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20250402133843-5a4c5f4fb4f1
1414
github.com/openstack-k8s-operators/ovn-operator/api v0.0.0-20230418071801-b5843d9e05fb
1515
go.uber.org/zap v1.27.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6
7878
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
7979
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d h1:/C+ysubV00VYrqGFhQlDeQ5tUtnhIWPwQUc8MOfCEBA=
8080
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250403063905-eb287d52f38d/go.mod h1:IY5zp1GRhacGMvjZMUZQ0LlxWDaogIRb/4H3by1Pivk=
81-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1 h1:hO90JhfinKysbdrWCJugTmJbkrs1d9tR7ypg3yOD12E=
82-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250402133843-5a4c5f4fb4f1/go.mod h1:A9Ohw5Q90YOGhcDF4ZHkMr5RArz3phoBu9+ibbYDKG4=
81+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b h1:T+N6xOT2NP+hVp2K1xl/NV3uheVHu38CcBuW+8uOBYw=
82+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250408123225-0d9e9b82c41b/go.mod h1:A9Ohw5Q90YOGhcDF4ZHkMr5RArz3phoBu9+ibbYDKG4=
8383
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20250402133843-5a4c5f4fb4f1 h1:Tdq+6lI4yPMjEwTMyw4+EGMuyEA9Gql07kDBBVm50bI=
8484
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20250402133843-5a4c5f4fb4f1/go.mod h1:oKvVb28i6wwBR5uQO2B2KMzZnCFTPCUCj31c5Zvz2lo=
8585
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

‎tests/functional/ovnnorthd_controller_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,90 @@ var _ = Describe("OVNNorthd controller", func() {
132132
})
133133
})
134134

135+
When("A OVNNorthd Deployment rollout is progressing", func() {
136+
var ovnNorthdName types.NamespacedName
137+
var deploymentName types.NamespacedName
138+
BeforeEach(func() {
139+
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
140+
DeferCleanup(DeleteOVNDBClusters, dbs)
141+
spec := GetDefaultOVNNorthdSpec()
142+
ovnNorthdName = ovn.CreateOVNNorthd(nil, namespace, spec)
143+
DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName)
144+
deploymentName = types.NamespacedName{
145+
Namespace: namespace,
146+
Name: "ovn-northd",
147+
}
148+
th.SimulateDeploymentProgressing(deploymentName)
149+
})
150+
151+
It("shows the deployment progressing in DeploymentReadyCondition", func() {
152+
th.ExpectConditionWithDetails(
153+
ovnNorthdName,
154+
ConditionGetterFunc(OVNNorthdConditionGetter),
155+
condition.DeploymentReadyCondition,
156+
corev1.ConditionFalse,
157+
condition.RequestedReason,
158+
condition.DeploymentReadyRunningMessage,
159+
)
160+
th.ExpectCondition(
161+
ovnNorthdName,
162+
ConditionGetterFunc(OVNNorthdConditionGetter),
163+
condition.ReadyCondition,
164+
corev1.ConditionFalse,
165+
)
166+
})
167+
168+
It("still shows the deployment progressing in DeploymentReadyCondition when rollout hits ProgressDeadlineExceeded", func() {
169+
th.SimulateDeploymentProgressDeadlineExceeded(deploymentName)
170+
th.ExpectConditionWithDetails(
171+
ovnNorthdName,
172+
ConditionGetterFunc(OVNNorthdConditionGetter),
173+
condition.DeploymentReadyCondition,
174+
corev1.ConditionFalse,
175+
condition.RequestedReason,
176+
condition.DeploymentReadyRunningMessage,
177+
)
178+
th.ExpectCondition(
179+
ovnNorthdName,
180+
ConditionGetterFunc(OVNNorthdConditionGetter),
181+
condition.ReadyCondition,
182+
corev1.ConditionFalse,
183+
)
184+
})
185+
186+
It("reaches Ready when deployment rollout finished", func() {
187+
th.ExpectConditionWithDetails(
188+
ovnNorthdName,
189+
ConditionGetterFunc(OVNNorthdConditionGetter),
190+
condition.DeploymentReadyCondition,
191+
corev1.ConditionFalse,
192+
condition.RequestedReason,
193+
condition.DeploymentReadyRunningMessage,
194+
)
195+
th.ExpectCondition(
196+
ovnNorthdName,
197+
ConditionGetterFunc(OVNNorthdConditionGetter),
198+
condition.ReadyCondition,
199+
corev1.ConditionFalse,
200+
)
201+
202+
th.SimulateDeploymentReplicaReady(deploymentName)
203+
th.ExpectCondition(
204+
ovnNorthdName,
205+
ConditionGetterFunc(OVNNorthdConditionGetter),
206+
condition.DeploymentReadyCondition,
207+
corev1.ConditionTrue,
208+
)
209+
210+
th.ExpectCondition(
211+
ovnNorthdName,
212+
ConditionGetterFunc(OVNNorthdConditionGetter),
213+
condition.ReadyCondition,
214+
corev1.ConditionTrue,
215+
)
216+
})
217+
})
218+
135219
When("OVNNorthd is created with nodeSelector", func() {
136220
var ovnNorthdName types.NamespacedName
137221
var deploymentName types.NamespacedName

0 commit comments

Comments
 (0)