From e050163adf4c14835454f19c207eff8c439cf554 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 17 Jul 2026 08:23:07 -0400 Subject: [PATCH 1/6] Add test utility functions for component rendering tests This commit adds helper functions to testutil_test.go that are used by component rendering tests to verify rendered Kubernetes objects: - mustFindRenderedObj: Finds and converts unstructured objects using generics - mustFindContainer: Finds a container by name in a container list - findExecCommand: Extracts exec command strings from container command args These utilities facilitate testing of rendered manifests and container configurations across multiple components. Signed-off-by: Tom Pantelis Co-Authored-By: Ori Braunshtein (cherry picked from commit c617131b92f33d65a9c0938cc111a6a7821ccea4) Cherry-pick not clean: - dropped definiton of mustFindContainer and findExecCommand (otherwise lint fails) (cherry picked from commit cedf0d7cacdcbdbfd0d06f2870d61415e45f2200) --- pkg/network/testutil_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/pkg/network/testutil_test.go b/pkg/network/testutil_test.go index 3a71c075ca..caf993b841 100644 --- a/pkg/network/testutil_test.go +++ b/pkg/network/testutil_test.go @@ -3,7 +3,10 @@ package network import ( "context" "fmt" + "slices" + "testing" + . "github.com/onsi/gomega" "github.com/onsi/gomega/types" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" @@ -11,6 +14,7 @@ import ( "github.com/openshift/cluster-network-operator/pkg/hypershift" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) // Fun matcher for testing the presence of Kubernetes objects @@ -108,3 +112,21 @@ func createProxy(client client.Client) error { } return client.Default().CRClient().Create(context.TODO(), proxy) } + +// mustFindRenderedObj finds and converts an unstructured object from a list of rendered objects. +// It uses Go generics to return the properly typed object. +func mustFindRenderedObj[T any](t *testing.T, objs []*uns.Unstructured, kind, name string) T { + t.Helper() + g := NewWithT(t) + + index := slices.IndexFunc(objs, func(obj *uns.Unstructured) bool { + return obj.GetKind() == kind && obj.GetName() == name + }) + g.Expect(index).NotTo(Equal(-1), "Could not find object with kind %q and name %q", kind, name) + + var result T + err := runtime.DefaultUnstructuredConverter.FromUnstructured(objs[index].Object, &result) + g.Expect(err).NotTo(HaveOccurred()) + + return result +} From d9b2fa7b9605e223c1a97590829998e836e3b592 Mon Sep 17 00:00:00 2001 From: Federico Paolinelli Date: Tue, 4 Aug 2026 10:43:30 -0400 Subject: [PATCH 2/6] frr-k8s: use Recreate strategy for statuscleaner deployment The statuscleaner deployment uses hostNetwork with a fixed port (9123). With the default RollingUpdate strategy, upgrades on SNO clusters get stuck because the new pod cannot bind the host port already held by the old pod. Switching to Recreate ensures the old pod is terminated before the new one starts. Signed-off-by: Federico Paolinelli Co-Authored-By: Ori Braunshtein Co-Authored-By: Claude Sonnet 4.6 (1M context) (cherry picked from commit 3d86b7522a7c313ba49d49be23b9f11b51a96d96) Cherry-pick not clean: - added bootstrap result wiring to additionalRoutingCapibilities (that was already there for 5.0), aligned existing unit test with fakeBootstrapResult() as in 5.0 (cherry picked from commit a5752d9f8a6c77c6c824a8ea3cd7d5c250a469af) Conflicts: pkg/network/render.go preserving Very small conflict due to the presence of isSupportedDualStackPlatform. Resolution: keep isSupportedDualStackPlatform + add bootstrapResult to renderAdditionalRoutingCapabilities. --- bindata/network/frr-k8s/webhook.yaml | 4 ++++ pkg/network/render.go | 5 ++-- pkg/network/render_test.go | 34 +++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/bindata/network/frr-k8s/webhook.yaml b/bindata/network/frr-k8s/webhook.yaml index ef699535f1..aa707a3237 100644 --- a/bindata/network/frr-k8s/webhook.yaml +++ b/bindata/network/frr-k8s/webhook.yaml @@ -52,6 +52,10 @@ metadata: annotations: release.openshift.io/version: "{{.ReleaseVersion}}" spec: +{{- if .IsSNO }} + strategy: + type: Recreate +{{- end }} selector: matchLabels: component: frr-k8s-webhook-server diff --git a/pkg/network/render.go b/pkg/network/render.go index 0cda29961d..eee3bcefac 100644 --- a/pkg/network/render.go +++ b/pkg/network/render.go @@ -147,7 +147,7 @@ func Render(operConf *operv1.NetworkSpec, clusterConf *configv1.NetworkSpec, man } objs = append(objs, o...) - o, err = renderAdditionalRoutingCapabilities(operConf, manifestDir) + o, err = renderAdditionalRoutingCapabilities(operConf, bootstrapResult, manifestDir) if err != nil { return nil, progressing, err } @@ -978,7 +978,7 @@ func isSupportedDualStackPlatform(platformType configv1.PlatformType) bool { return dualStackPlatforms.Has(string(platformType)) } -func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir string) ([]*uns.Unstructured, error) { +func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.BootstrapResult, manifestDir string) ([]*uns.Unstructured, error) { if conf == nil || conf.AdditionalRoutingCapabilities == nil { return nil, nil } @@ -990,6 +990,7 @@ func renderAdditionalRoutingCapabilities(conf *operv1.NetworkSpec, manifestDir s data.Data["FRRK8sImage"] = os.Getenv("FRR_K8S_IMAGE") data.Data["KubeRBACProxyImage"] = os.Getenv("KUBE_RBAC_PROXY_IMAGE") data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") + data.Data["IsSNO"] = bootstrapResult.OVN.ControlPlaneReplicaCount == 1 objs, err := render.RenderDir(filepath.Join(manifestDir, "network/frr-k8s"), &data) if err != nil { return nil, fmt.Errorf("failed to render frr-k8s manifests: %w", err) diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index 10f7bf41a6..dd06b16fec 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -9,6 +9,7 @@ import ( "github.com/openshift/cluster-network-operator/pkg/client/fake" "github.com/openshift/cluster-network-operator/pkg/hypershift" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" "k8s.io/client-go/kubernetes/scheme" "testing" @@ -644,7 +645,7 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, manifestDir) + got, err := renderAdditionalRoutingCapabilities(tt.args.operConf, fakeBootstrapResult(), manifestDir) if !reflect.DeepEqual(tt.expectedErr, err) { t.Errorf("renderAdditionalRoutingCapabilities() err = %v, want %v", err, tt.expectedErr) } @@ -652,3 +653,34 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { }) } } + +func Test_renderFRRStatusCleanerStrategy(t *testing.T) { + frrConf := &operv1.NetworkSpec{ + AdditionalRoutingCapabilities: &operv1.AdditionalRoutingCapabilities{ + Providers: []operv1.RoutingCapabilitiesProvider{ + operv1.RoutingCapabilitiesProviderFRR, + }, + }, + } + + render := func(replicaCount int) *appsv1.Deployment { + g := NewWithT(t) + br := fakeBootstrapResult() + br.OVN.ControlPlaneReplicaCount = replicaCount + objs, err := renderAdditionalRoutingCapabilities(frrConf, br, manifestDir) + g.Expect(err).NotTo(HaveOccurred()) + return mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "frr-k8s-statuscleaner") + } + + t.Run("SNO: strategy is Recreate", func(t *testing.T) { + g := NewWithT(t) + d := render(1) + g.Expect(d.Spec.Strategy.Type).To(Equal(appsv1.RecreateDeploymentStrategyType)) + }) + + t.Run("HA: no strategy override", func(t *testing.T) { + g := NewWithT(t) + d := render(3) + g.Expect(d.Spec.Strategy.Type).To(BeEmpty()) + }) +} From 79005b9db56f1e8c1cefb4cf59b0a80f9466496a Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Thu, 13 Aug 2026 20:46:20 +0200 Subject: [PATCH 3/6] frr-k8s: fix strategy switch to Recreate on SNO upgrades On upgrade, the frr-k8s-statuscleaner Deployment has rollingUpdate fields defaulted by the API server. SSA cannot remove fields it does not own, so switching strategy.type to Recreate fails with: spec.strategy.rollingUpdate: Forbidden: may not be specified when strategy `type` is 'Recreate' Add a generic pre-patch annotation: networkoperator.openshift.io/pre-patch that applies a strategic-merge-patch to the live object before SSA. This lets the template atomically set type=Recreate and remove rollingUpdate in a single patch, before SSA takes over. Also explicitly set strategy to RollingUpdate with rollingUpdate fields on non-SNO clusters so that CNO owns these fields going forward, preventing the issue from recurring in case we have to make the same switch for non-SNO clusters in the future. Signed-off-by: Andreas Karis Co-Authored-By: Claude Opus 4.6 (cherry picked from commit 8ebb78e658b4ab30e7cdd9ff59449398eaf41213) (cherry picked from commit d59aba6b2e23f5cd0003e9b15cc6ae4e70298194) --- bindata/network/frr-k8s/webhook.yaml | 9 +++++++++ pkg/apply/apply.go | 17 +++++++++++++++++ pkg/names/names.go | 6 ++++++ pkg/network/render_test.go | 12 ++++++++++-- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/bindata/network/frr-k8s/webhook.yaml b/bindata/network/frr-k8s/webhook.yaml index aa707a3237..c842892d14 100644 --- a/bindata/network/frr-k8s/webhook.yaml +++ b/bindata/network/frr-k8s/webhook.yaml @@ -51,10 +51,19 @@ metadata: component: frr-k8s-webhook-server annotations: release.openshift.io/version: "{{.ReleaseVersion}}" +{{- if .IsSNO }} + networkoperator.openshift.io/pre-patch: '{"spec":{"strategy":{"type":"Recreate","rollingUpdate":null}}}' +{{- end }} spec: {{- if .IsSNO }} strategy: type: Recreate +{{- else }} + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 25% + maxUnavailable: 25% {{- end }} selector: matchLabels: diff --git a/pkg/apply/apply.go b/pkg/apply/apply.go index a98a06fe61..e85818b0dd 100644 --- a/pkg/apply/apply.go +++ b/pkg/apply/apply.go @@ -110,6 +110,23 @@ func ApplyObject(ctx context.Context, client cnoclient.Client, obj Object, subco fieldManager = fmt.Sprintf("%s/%s", fieldManager, subcontroller) } + // If pre-patch is specified, apply it as a strategic-merge-patch to the live + // object before SSA. This handles cases where SSA cannot remove a field it + // does not own (e.g. defaulted rollingUpdate when switching to Recreate). + // The pre-patch is silently skipped if the object does not exist yet, making + // this mainly relevant on upgrades where defaulted fields must be removed. + if prePatch, ok := obj.GetAnnotations()[names.PrePatchAnnotation]; ok { + log.Printf("Object %s has pre-patch annotation, attempting strategic-merge-patch before SSA", objDesc) + patchOptions := metav1.PatchOptions{FieldManager: fieldManager} + _, err := clusterClient.Dynamic().Resource(rm.Resource).Namespace(namespace).Patch( + ctx, name, types.StrategicMergePatchType, []byte(prePatch), patchOptions) + if apierrors.IsNotFound(err) { + log.Printf("Object %s not found, skipping pre-patch", objDesc) + } else if err != nil { + return fmt.Errorf("failed to pre-patch %s: %w", objDesc, err) + } + } + // Use server-side apply to merge the desired object with the object on disk patchOptions := metav1.PatchOptions{ // It is considered best-practice for controllers to force diff --git a/pkg/names/names.go b/pkg/names/names.go index db0d7bf93f..f9a4a8018b 100644 --- a/pkg/names/names.go +++ b/pkg/names/names.go @@ -48,6 +48,12 @@ const CreateOnlyAnnotation = "networkoperator.openshift.io/create-only" // tells the CNO reconciliation engine to ignore creating this object until conditions are met. const CreateWaitAnnotation = "networkoperator.openshift.io/create-wait" +// PrePatchAnnotation is an annotation whose value is a JSON object applied as a +// strategic-merge-patch to the live object before the SSA apply. This is needed +// when SSA cannot remove a field it does not own (e.g. removing defaulted +// rollingUpdate when switching a Deployment strategy to Recreate). +const PrePatchAnnotation = "networkoperator.openshift.io/pre-patch" + // NonCriticalAnnotation is an annotation on Deployments/DaemonSets to indicate // that they are not critical to the functioning of the pod network const NonCriticalAnnotation = "networkoperator.openshift.io/non-critical" diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index dd06b16fec..c37c9b78d3 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -17,6 +17,7 @@ import ( configv1 "github.com/openshift/api/config/v1" operv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" + "github.com/openshift/cluster-network-operator/pkg/names" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -676,11 +677,18 @@ func Test_renderFRRStatusCleanerStrategy(t *testing.T) { g := NewWithT(t) d := render(1) g.Expect(d.Spec.Strategy.Type).To(Equal(appsv1.RecreateDeploymentStrategyType)) + g.Expect(d.Annotations).To( + HaveKeyWithValue( + names.PrePatchAnnotation, + `{"spec":{"strategy":{"type":"Recreate","rollingUpdate":null}}}`, + ), + ) }) - t.Run("HA: no strategy override", func(t *testing.T) { + t.Run("HA: strategy is RollingUpdate", func(t *testing.T) { g := NewWithT(t) d := render(3) - g.Expect(d.Spec.Strategy.Type).To(BeEmpty()) + g.Expect(d.Spec.Strategy.Type).To(Equal(appsv1.RollingUpdateDeploymentStrategyType)) + g.Expect(d.Annotations).NotTo(HaveKey(names.PrePatchAnnotation)) }) } From 0a9ed9a8304edd716276cc1f1615d53c064247c7 Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Fri, 14 Aug 2026 21:02:16 +0200 Subject: [PATCH 4/6] apply: add unit tests for PrePatchAnnotation and replace fakeRESTMapper Add tests for ApplyObject's pre-patch behavior: strategic-merge-patch runs before SSA, NotFound is tolerated, other errors stop reconciliation, and objects without the annotation skip pre-patch entirely. Remove fakeRESTMapper and replace it with testrestmapper.TetsOnlyStaticRESTMapper to return a proper mapper, and return scheme.Scheme from Scheme(). Both changes needed by ApplyObject under test. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Andreas Karis (cherry picked from commit 6ca59b1b1abf4b909ceed78cbd71aaec42516507) (cherry picked from commit b1775c96f273665131c630d7c76b08c2e6642a40) Not clean cherry-pick: Make the linter happy by applying the changes to pkg/apply from https://github.com/openshift/cluster-network-operator/pull/3138. See that PR for further details. --- pkg/apply/apply_test.go | 130 +++++++++++++++++++++++++++++++++ pkg/client/fake/fake_client.go | 42 +---------- 2 files changed, 133 insertions(+), 39 deletions(-) create mode 100644 pkg/apply/apply_test.go diff --git a/pkg/apply/apply_test.go b/pkg/apply/apply_test.go new file mode 100644 index 0000000000..566895ae16 --- /dev/null +++ b/pkg/apply/apply_test.go @@ -0,0 +1,130 @@ +package apply + +import ( + "fmt" + "testing" + + "github.com/openshift/cluster-network-operator/pkg/client/fake" + "github.com/openshift/cluster-network-operator/pkg/names" + + . "github.com/onsi/gomega" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + fakedynamic "k8s.io/client-go/dynamic/fake" + clienttesting "k8s.io/client-go/testing" +) + +const prePatchValue = `{"spec":{"strategy":{"type":"Recreate","rollingUpdate":null}}}` + +func newTestDeployment(annotations map[string]string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}) + obj.SetName("test-deployment") + obj.SetNamespace("test-ns") + obj.SetAnnotations(annotations) + return obj +} + +func patchRecorder(patchTypes *[]types.PatchType, prePatchBody *string, prePatchErr error) func(action clienttesting.Action) (bool, runtime.Object, error) { + return func(action clienttesting.Action) (bool, runtime.Object, error) { + patchAction := action.(clienttesting.PatchAction) + *patchTypes = append(*patchTypes, patchAction.GetPatchType()) + if patchAction.GetPatchType() == types.StrategicMergePatchType { + *prePatchBody = string(patchAction.GetPatch()) + if prePatchErr != nil { + return true, nil, prePatchErr + } + } + return true, newTestDeployment(nil), nil + } +} + +func TestApplyObjectPrePatchRunsBeforeSSA(t *testing.T) { + g := NewWithT(t) + + client := fake.NewFakeClient() + + var patchTypes []types.PatchType + var prePatchBody string + pr := patchRecorder(&patchTypes, &prePatchBody, nil) + client.Default().Dynamic().(*fakedynamic.FakeDynamicClient).PrependReactor("patch", "deployments", pr) + + obj := newTestDeployment(map[string]string{ + names.PrePatchAnnotation: prePatchValue, + }) + + err := ApplyObject(t.Context(), client, obj, "test-controller") + g.Expect(err).To(Succeed()) + g.Expect(patchTypes).To(HaveLen(2)) + g.Expect(patchTypes[0]).To(Equal(types.StrategicMergePatchType), "pre-patch should be strategic-merge-patch") + g.Expect(patchTypes[1]).To(Equal(types.ApplyPatchType), "second patch should be SSA apply") + g.Expect(prePatchBody).To(Equal(prePatchValue), "pre-patch body should match annotation value") +} + +func TestApplyObjectPrePatchNotFoundAllowsSSA(t *testing.T) { + g := NewWithT(t) + + client := fake.NewFakeClient() + + var patchTypes []types.PatchType + var prePatchBody string + pr := patchRecorder(&patchTypes, &prePatchBody, apierrors.NewNotFound( + schema.GroupResource{Group: "apps", Resource: "deployments"}, "test-deployment")) + client.Default().Dynamic().(*fakedynamic.FakeDynamicClient).PrependReactor("patch", "deployments", pr) + + obj := newTestDeployment(map[string]string{ + names.PrePatchAnnotation: prePatchValue, + }) + + err := ApplyObject(t.Context(), client, obj, "test-controller") + g.Expect(err).To(Succeed()) + g.Expect(patchTypes).To(HaveLen(2)) + g.Expect(patchTypes[0]).To(Equal(types.StrategicMergePatchType), "pre-patch was attempted") + g.Expect(patchTypes[1]).To(Equal(types.ApplyPatchType), "SSA apply still proceeded") + g.Expect(prePatchBody).To(Equal(prePatchValue), "pre-patch body should match annotation value") +} + +func TestApplyObjectPrePatchErrorStopsReconciliation(t *testing.T) { + g := NewWithT(t) + + client := fake.NewFakeClient() + + var patchTypes []types.PatchType + var prePatchBody string + pr := patchRecorder(&patchTypes, &prePatchBody, fmt.Errorf("API server error")) + client.Default().Dynamic().(*fakedynamic.FakeDynamicClient).PrependReactor("patch", "deployments", pr) + + obj := newTestDeployment(map[string]string{ + names.PrePatchAnnotation: prePatchValue, + }) + + err := ApplyObject(t.Context(), client, obj, "test-controller") + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("failed to pre-patch")) + g.Expect(patchTypes).To(HaveLen(1), "SSA apply should not have been reached") + g.Expect(patchTypes[0]).To(Equal(types.StrategicMergePatchType), "pre-patch should be strategic-merge-patch") + g.Expect(prePatchBody).To(Equal(prePatchValue), "pre-patch body should match annotation value") +} + +func TestApplyObjectNoPrePatchAnnotationSkipsPrePatch(t *testing.T) { + g := NewWithT(t) + + client := fake.NewFakeClient() + + var patchTypes []types.PatchType + var prePatchBody string + pr := patchRecorder(&patchTypes, &prePatchBody, nil) + client.Default().Dynamic().(*fakedynamic.FakeDynamicClient).PrependReactor("patch", "deployments", pr) + + obj := newTestDeployment(nil) + + err := ApplyObject(t.Context(), client, obj, "test-controller") + g.Expect(err).To(Succeed()) + g.Expect(patchTypes).To(HaveLen(1)) + g.Expect(patchTypes[0]).To(Equal(types.ApplyPatchType), "only SSA apply should have run") + g.Expect(prePatchBody).To(BeEmpty(), "no pre-patch should have been sent") +} diff --git a/pkg/client/fake/fake_client.go b/pkg/client/fake/fake_client.go index be061d27ed..f9786f946d 100644 --- a/pkg/client/fake/fake_client.go +++ b/pkg/client/fake/fake_client.go @@ -5,9 +5,9 @@ import ( "strings" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta/testrestmapper" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" fakedynamic "k8s.io/client-go/dynamic/fake" "k8s.io/client-go/kubernetes" @@ -113,42 +113,6 @@ func NewFakeClient(objs ...crclient.Object) cnoclient.Client { } } -type fakeRESTMapper struct { - kindForInput schema.GroupVersionResource -} - -func (f *fakeRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { - f.kindForInput = resource - return schema.GroupVersionKind{ - Group: "test", - Version: "test", - Kind: "test"}, nil -} - -func (f *fakeRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { - return nil, nil -} - -func (f *fakeRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { - return schema.GroupVersionResource{}, nil -} - -func (f *fakeRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { - return nil, nil -} - -func (f *fakeRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - return nil, nil -} - -func (f *fakeRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { - return nil, nil -} - -func (f *fakeRESTMapper) ResourceSingularizer(resource string) (singular string, err error) { - return "", nil -} - func (fc *FakeClusterClient) Kubernetes() kubernetes.Interface { return fc.kClient } @@ -170,11 +134,11 @@ func (fc *FakeClusterClient) CRClient() crclient.Client { } func (fc *FakeClusterClient) RESTMapper() meta.RESTMapper { - return &fakeRESTMapper{} + return testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme) } func (fc *FakeClusterClient) Scheme() *runtime.Scheme { - panic("not implemented!") + return scheme.Scheme } func (fc *FakeClusterClient) OperatorHelperClient() operatorv1helpers.OperatorClient { panic("not implemented!") From e564d5d483fc4690d918a8e926f486201a70f3ef Mon Sep 17 00:00:00 2001 From: Jean Chen Date: Tue, 25 Aug 2026 15:44:34 -0400 Subject: [PATCH 5/6] Fix 4.20 backport test for frr-k8s webhook server strategy On release-4.20 the hostNetwork deployment is frr-k8s-webhook-server, not frr-k8s-statuscleaner (which was split out in 4.21). Update the strategy unit test to target the correct deployment name. Signed-off-by: Jean Chen Co-authored-by: Cursor --- pkg/network/render_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index c37c9b78d3..64d95967bf 100644 --- a/pkg/network/render_test.go +++ b/pkg/network/render_test.go @@ -655,7 +655,8 @@ func Test_renderAdditionalRoutingCapabilities(t *testing.T) { } } -func Test_renderFRRStatusCleanerStrategy(t *testing.T) { +func Test_renderFRRWebhookServerStrategy(t *testing.T) { + // On 4.20 the hostNetwork workload is frr-k8s-webhook-server; statuscleaner was split out in 4.21. frrConf := &operv1.NetworkSpec{ AdditionalRoutingCapabilities: &operv1.AdditionalRoutingCapabilities{ Providers: []operv1.RoutingCapabilitiesProvider{ @@ -670,7 +671,7 @@ func Test_renderFRRStatusCleanerStrategy(t *testing.T) { br.OVN.ControlPlaneReplicaCount = replicaCount objs, err := renderAdditionalRoutingCapabilities(frrConf, br, manifestDir) g.Expect(err).NotTo(HaveOccurred()) - return mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "frr-k8s-statuscleaner") + return mustFindRenderedObj[*appsv1.Deployment](t, objs, "Deployment", "frr-k8s-webhook-server") } t.Run("SNO: strategy is Recreate", func(t *testing.T) { From 8b97f6f279e82e51ebc14cce29dbcf747a3affbd Mon Sep 17 00:00:00 2001 From: Jean Chen Date: Tue, 25 Aug 2026 16:16:20 -0400 Subject: [PATCH 6/6] Fix statusmanager test after fake REST mapper replacement The apply backport replaced fakeRESTMapper with testrestmapper. TestStatusManager_set used a fake test/test GVK that the real mapper cannot resolve. Use ConfigMap instead so related-object deletion is exercised with a scheme-known type. Signed-off-by: Jean Chen Co-authored-by: Cursor --- .../statusmanager/status_manager_test.go | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/controller/statusmanager/status_manager_test.go b/pkg/controller/statusmanager/status_manager_test.go index 9ae9ea9b44..bb25d49eae 100644 --- a/pkg/controller/statusmanager/status_manager_test.go +++ b/pkg/controller/statusmanager/status_manager_test.go @@ -250,30 +250,33 @@ func TestStatusManager_set(t *testing.T) { obj := &uns.Unstructured{} gvk := schema.GroupVersionKind{ - Group: "test", - Version: "test", - Kind: "test", + Group: "", + Version: "v1", + Kind: "ConfigMap", } obj.SetGroupVersionKind(gvk) obj.SetName("current") + obj.SetNamespace("default") set(t, client, obj) co.Status.RelatedObjects = []configv1.ObjectReference{ { - Group: "test", - Resource: "test", - Name: "current", + Group: "", + Resource: "configmaps", + Name: "current", + Namespace: "default", }, } status.relatedObjects = []configv1.ObjectReference{ { - Group: "test", - Resource: "test", - Name: "related", + Group: "", + Resource: "configmaps", + Name: "related", + Namespace: "default", }, } status.deleteRelatedObjectsNotRendered(co) - err = status.client.ClientFor("").CRClient().Get(context.TODO(), types.NamespacedName{Name: "current"}, obj) + err = status.client.ClientFor("").CRClient().Get(context.TODO(), types.NamespacedName{Namespace: "default", Name: "current"}, obj) if err == nil { t.Fatalf("unexpected related object in ClusterOperator object was not deleted") }