Skip to content

Commit b7e366d

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2197 from tchap/workload-controller-cleanup
controller/workload: Clean up status reporting helpers and test infrastructure
2 parents da43577 + 1f03ec2 commit b7e366d

2 files changed

Lines changed: 23 additions & 30 deletions

File tree

‎pkg/operator/apiserver/controller/workload/workload.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
corev1listers "k8s.io/client-go/listers/core/v1"
1717
"k8s.io/client-go/tools/cache"
1818
"k8s.io/client-go/util/workqueue"
19+
"k8s.io/utils/ptr"
1920

2021
operatorv1 "github.com/openshift/api/operator/v1"
2122
applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
@@ -218,8 +219,8 @@ func (c *Controller) updateOperatorStatus(ctx context.Context, previousStatus *o
218219

219220
if !preconditionsReady {
220221
var message string
221-
for _, err := range errs {
222-
message = message + err.Error() + "\n"
222+
if err := errors.Join(errs...); err != nil {
223+
message = err.Error()
223224
}
224225
if len(message) == 0 {
225226
message = "the operator didn't specify what preconditions are missing"
@@ -249,9 +250,9 @@ func (c *Controller) updateOperatorStatus(ctx context.Context, previousStatus *o
249250
}
250251

251252
if len(errs) > 0 {
252-
message := ""
253-
for _, err := range errs {
254-
message = message + err.Error() + "\n"
253+
var message string
254+
if err := errors.Join(errs...); err != nil {
255+
message = err.Error()
255256
}
256257
workloadDegradedCondition = workloadDegradedCondition.
257258
WithStatus(operatorv1.ConditionTrue).
@@ -298,10 +299,7 @@ func (c *Controller) updateOperatorStatus(ctx context.Context, previousStatus *o
298299
WithReason("AsExpected")
299300
}
300301

301-
desiredReplicas := int32(1)
302-
if workload.Spec.Replicas != nil {
303-
desiredReplicas = *(workload.Spec.Replicas)
304-
}
302+
desiredReplicas := ptr.Deref(workload.Spec.Replicas, 1)
305303

306304
// If the workload is up to date, then we are no longer progressing
307305
workloadAtHighestGeneration := workload.ObjectMeta.Generation == workload.Status.ObservedGeneration

‎pkg/operator/apiserver/controller/workload/workload_test.go‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
9595
Message: "deployment/: could not be retrieved",
9696
},
9797
}
98-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
98+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
9999
},
100100
},
101101
{
@@ -112,7 +112,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
112112
{
113113
Type: fmt.Sprintf("%sWorkloadDegraded", defaultControllerName),
114114
Status: operatorv1.ConditionTrue,
115-
Message: "nasty error\n",
115+
Message: "nasty error",
116116
Reason: "SyncError",
117117
},
118118
{
@@ -129,7 +129,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
129129
Message: "deployment/: could not be retrieved",
130130
},
131131
}
132-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
132+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
133133
},
134134
},
135135
{
@@ -140,7 +140,6 @@ func TestUpdateOperatorStatus(t *testing.T) {
140140
Namespace: "openshift-apiserver",
141141
},
142142
Spec: appsv1.DeploymentSpec{
143-
Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}},
144143
Replicas: ptr.To[int32](3),
145144
},
146145
Status: appsv1.DeploymentStatus{
@@ -152,7 +151,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
152151
},
153152
pods: []*corev1.Pod{
154153
{
155-
ObjectMeta: metav1.ObjectMeta{Name: "apiserver", Namespace: "openshift-apiserver", Labels: map[string]string{"foo": "bar"}},
154+
ObjectMeta: metav1.ObjectMeta{Name: "apiserver", Namespace: "openshift-apiserver"},
156155
Status: corev1.PodStatus{
157156
Phase: corev1.PodPending,
158157
ContainerStatuses: []corev1.ContainerStatus{
@@ -203,7 +202,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
203202
Message: "deployment/apiserver.openshift-apiserver: 0/3 pods have been updated to the latest generation and 0/3 pods are available",
204203
},
205204
}
206-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
205+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
207206
},
208207
},
209208
{
@@ -214,7 +213,6 @@ func TestUpdateOperatorStatus(t *testing.T) {
214213
Namespace: "openshift-apiserver",
215214
},
216215
Spec: appsv1.DeploymentSpec{
217-
Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}},
218216
Replicas: ptr.To[int32](3),
219217
},
220218
Status: appsv1.DeploymentStatus{
@@ -226,7 +224,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
226224
},
227225
pods: []*corev1.Pod{
228226
{
229-
ObjectMeta: metav1.ObjectMeta{Name: "apiserver", Namespace: "openshift-apiserver", Labels: map[string]string{"foo": "bar"}},
227+
ObjectMeta: metav1.ObjectMeta{Name: "apiserver", Namespace: "openshift-apiserver"},
230228
Status: corev1.PodStatus{
231229
Phase: corev1.PodPending,
232230
ContainerStatuses: []corev1.ContainerStatus{
@@ -276,7 +274,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
276274
Message: "deployment/apiserver.openshift-apiserver: 0/3 pods have been updated to the latest generation and 0/3 pods are available",
277275
},
278276
}
279-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
277+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
280278
},
281279
},
282280
{
@@ -287,7 +285,6 @@ func TestUpdateOperatorStatus(t *testing.T) {
287285
Namespace: "openshift-apiserver",
288286
},
289287
Spec: appsv1.DeploymentSpec{
290-
Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}},
291288
Replicas: ptr.To[int32](3),
292289
},
293290
Status: appsv1.DeploymentStatus{
@@ -343,7 +340,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
343340
Message: "",
344341
},
345342
}
346-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
343+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
347344
},
348345
},
349346
{
@@ -389,7 +386,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
389386
Message: "",
390387
},
391388
}
392-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
389+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
393390
},
394391
},
395392
{
@@ -439,7 +436,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
439436
Message: "deployment/apiserver.openshift-apiserver: observed generation is 99, desired generation is 100.",
440437
},
441438
}
442-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
439+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
443440
},
444441
},
445442

@@ -490,7 +487,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
490487
Message: "deployment/apiserver.openshift-apiserver: observed generation is 99, desired generation is 100.",
491488
},
492489
}
493-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
490+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
494491
},
495492
},
496493
{
@@ -523,7 +520,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
523520
Message: "the operator didn't specify what preconditions are missing",
524521
},
525522
}
526-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
523+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
527524
},
528525
},
529526
{
@@ -573,7 +570,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
573570
Message: "deployment/apiserver.openshift-apiserver: 1/3 pods have been updated to the latest generation and 2/3 pods are available",
574571
},
575572
}
576-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
573+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
577574
},
578575
},
579576
{
@@ -624,7 +621,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
624621
Reason: "AsExpected",
625622
},
626623
}
627-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
624+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
628625
},
629626
},
630627
{
@@ -635,8 +632,6 @@ func TestUpdateOperatorStatus(t *testing.T) {
635632
Namespace: "openshift-apiserver",
636633
},
637634
Spec: appsv1.DeploymentSpec{
638-
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
639-
Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}},
640635
Replicas: ptr.To[int32](3),
641636
},
642637
Status: appsv1.DeploymentStatus{
@@ -673,7 +668,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
673668
Message: "deployment/apiserver.openshift-apiserver: 3/3 pods have been updated to the latest generation and 2/3 pods are available",
674669
},
675670
}
676-
return areCondidtionsEqual(expectedConditions, actualStatus.Conditions)
671+
return areConditionsEqual(expectedConditions, actualStatus.Conditions)
677672
},
678673
},
679674
}
@@ -756,7 +751,7 @@ func (f *fakePodLister) Pods(namespace string) corev1listers.PodNamespaceLister
756751
}
757752
}
758753

759-
func areCondidtionsEqual(expectedConditions []operatorv1.OperatorCondition, actualConditions []operatorv1.OperatorCondition) error {
754+
func areConditionsEqual(expectedConditions []operatorv1.OperatorCondition, actualConditions []operatorv1.OperatorCondition) error {
760755
if len(expectedConditions) != len(actualConditions) {
761756
return fmt.Errorf("expected %d conditions but got %d", len(expectedConditions), len(actualConditions))
762757
}

0 commit comments

Comments
 (0)