Skip to content

Commit 104a36e

Browse files
committed
If multiple install plans exist for the same OSSM version, approve the most recently created one.
This is part of the fix for OCPBUGS-53424
1 parent 288f96c commit 104a36e

File tree

2 files changed

+121
-6
lines changed

2 files changed

+121
-6
lines changed

pkg/operator/controller/gatewayclass/subscription.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,32 @@ func (r *reconciler) currentInstallPlan(ctx context.Context) (bool, *operatorsv1
158158
if installPlans == nil || len(installPlans.Items) == 0 {
159159
return false, nil, nil
160160
}
161+
var currentInstallPlan *operatorsv1alpha1.InstallPlan
161162
for _, installPlan := range installPlans.Items {
162163
if len(installPlan.OwnerReferences) == 0 || len(installPlan.Spec.ClusterServiceVersionNames) == 0 {
163164
continue
164165
}
166+
ownerRefMatches := false
165167
for _, ownerRef := range installPlan.OwnerReferences {
166168
if ownerRef.UID == subscription.UID {
167-
for _, csvName := range installPlan.Spec.ClusterServiceVersionNames {
168-
if csvName == r.config.GatewayAPIOperatorVersion {
169-
return true, &installPlan, nil
170-
}
169+
ownerRefMatches = true
170+
break
171+
}
172+
}
173+
if !ownerRefMatches {
174+
continue
175+
}
176+
for _, csvName := range installPlan.Spec.ClusterServiceVersionNames {
177+
if csvName == r.config.GatewayAPIOperatorVersion {
178+
// Keep the newest InstallPlan to return at the end of the loop.
179+
if currentInstallPlan == nil || currentInstallPlan.ObjectMeta.CreationTimestamp.Before(&installPlan.ObjectMeta.CreationTimestamp) {
180+
currentInstallPlan = &installPlan
181+
break
171182
}
172183
}
173184
}
174185
}
175-
// No valid InstallPlan found.
176-
return false, nil, nil
186+
return (currentInstallPlan != nil), currentInstallPlan, nil
177187
}
178188

179189
// desiredInstallPlan returns a version of the expected InstallPlan that is approved.

pkg/operator/controller/gatewayclass/subscription_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package gatewayclass
33
import (
44
"context"
55
"testing"
6+
"time"
67

78
configv1 "github.com/openshift/api/config/v1"
89
operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
@@ -312,6 +313,110 @@ func Test_ensureServiceMeshOperatorInstallPlan(t *testing.T) {
312313
expectUpdate: []client.Object{},
313314
expectDelete: []client.Object{},
314315
},
316+
{
317+
// Multiple InstallPlans exist, all with the correct cluster service version and owner reference. Expect
318+
// InstallPlan.Spec.Approved = true for only the most recent InstallPlan (by creation timestamp).
319+
name: "Multiple valid InstallPlans, should approve all",
320+
channel: "stable",
321+
version: "servicemeshoperator.v1.0.0",
322+
existingObjects: []runtime.Object{
323+
&operatorsv1alpha1.Subscription{
324+
ObjectMeta: metav1.ObjectMeta{
325+
Namespace: operatorcontroller.ServiceMeshOperatorSubscriptionName().Namespace,
326+
Name: operatorcontroller.ServiceMeshOperatorSubscriptionName().Name,
327+
UID: "foobar",
328+
CreationTimestamp: metav1.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC),
329+
},
330+
Spec: &operatorsv1alpha1.SubscriptionSpec{
331+
Channel: "stable",
332+
InstallPlanApproval: operatorsv1alpha1.ApprovalManual,
333+
Package: "servicemeshoperator",
334+
CatalogSource: "redhat-operators",
335+
CatalogSourceNamespace: "openshift-marketplace",
336+
StartingCSV: "servicemeshoperator.v1.0.0",
337+
},
338+
},
339+
&operatorsv1alpha1.InstallPlan{
340+
ObjectMeta: metav1.ObjectMeta{
341+
Name: "Install-foo",
342+
Namespace: operatorcontroller.OpenshiftOperatorNamespace,
343+
OwnerReferences: []metav1.OwnerReference{
344+
{
345+
UID: "foobar",
346+
},
347+
},
348+
},
349+
Spec: operatorsv1alpha1.InstallPlanSpec{
350+
ClusterServiceVersionNames: []string{
351+
"servicemeshoperator.v1.0.0",
352+
},
353+
Approval: operatorsv1alpha1.ApprovalManual,
354+
Approved: false,
355+
},
356+
},
357+
&operatorsv1alpha1.InstallPlan{
358+
ObjectMeta: metav1.ObjectMeta{
359+
Name: "Install-bar",
360+
Namespace: operatorcontroller.OpenshiftOperatorNamespace,
361+
OwnerReferences: []metav1.OwnerReference{
362+
{
363+
UID: "foobar",
364+
},
365+
},
366+
CreationTimestamp: metav1.Date(2025, 1, 1, 2, 30, 0, 0, time.UTC),
367+
},
368+
Spec: operatorsv1alpha1.InstallPlanSpec{
369+
ClusterServiceVersionNames: []string{
370+
"servicemeshoperator.v1.0.0",
371+
},
372+
Approval: operatorsv1alpha1.ApprovalManual,
373+
Approved: false,
374+
},
375+
},
376+
&operatorsv1alpha1.InstallPlan{
377+
ObjectMeta: metav1.ObjectMeta{
378+
Name: "Install-baz",
379+
Namespace: operatorcontroller.OpenshiftOperatorNamespace,
380+
OwnerReferences: []metav1.OwnerReference{
381+
{
382+
UID: "foobar",
383+
},
384+
},
385+
CreationTimestamp: metav1.Date(2024, 12, 31, 23, 59, 0, 0, time.UTC),
386+
},
387+
Spec: operatorsv1alpha1.InstallPlanSpec{
388+
ClusterServiceVersionNames: []string{
389+
"servicemeshoperator.v1.0.0",
390+
},
391+
Approval: operatorsv1alpha1.ApprovalManual,
392+
Approved: false,
393+
},
394+
},
395+
},
396+
expectCreate: []client.Object{},
397+
expectUpdate: []client.Object{
398+
&operatorsv1alpha1.InstallPlan{
399+
ObjectMeta: metav1.ObjectMeta{
400+
Name: "Install-bar",
401+
Namespace: operatorcontroller.OpenshiftOperatorNamespace,
402+
OwnerReferences: []metav1.OwnerReference{
403+
{
404+
UID: "foobar",
405+
},
406+
},
407+
CreationTimestamp: metav1.Date(2025, 1, 1, 2, 30, 0, 0, time.UTC),
408+
},
409+
Spec: operatorsv1alpha1.InstallPlanSpec{
410+
ClusterServiceVersionNames: []string{
411+
"servicemeshoperator.v1.0.0",
412+
},
413+
Approval: operatorsv1alpha1.ApprovalManual,
414+
Approved: true,
415+
},
416+
},
417+
},
418+
expectDelete: []client.Object{},
419+
},
315420
}
316421

317422
scheme := runtime.NewScheme()

0 commit comments

Comments
 (0)