Skip to content

OCPBUGS-88531: Restart cloud-network-config-controller on restart-date annotation #8738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bryan-cox wants to merge 2 commits into from
+119 −4
Closed

OCPBUGS-88531: Restart cloud-network-config-controller on restart-date annotation #8738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdcabaf
fix(cpo): restart cloud-network-config-controller on restart-date ann…
bryan-cox Jun 15, 2026
1ac48ef
docs: add missing CNO-managed components to restart list
bryan-cox Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,12 @@ func (r *HostedControlPlaneReconciler) cleanupClusterNetworkOperatorResources(ct
if err := cnov2.SetRestartAnnotationAndPatch(ctx, r.Client, ovnKubeControlPlaneDeployment, restartAnnotation); err != nil {
return fmt.Errorf("failed to restart ovnkube-control-plane: %w", err)
}

// CNO manages overall cloud-network-config-controller deployment on cloud platforms (AWS/Azure/GCP/OpenStack). CPO manages restarts.
cloudNetworkConfigControllerDeployment := manifests.CloudNetworkConfigControllerDeployment(hcp.Namespace)
if err := cnov2.SetRestartAnnotationAndPatch(ctx, r.Client, cloudNetworkConfigControllerDeployment, restartAnnotation); err != nil {
return fmt.Errorf("failed to restart cloud-network-config-controller: %w", err)
}
}

// Clean up ovnkube-sbdb Route if exists
Expand Down
10 changes: 10 additions & 0 deletions control-plane-operator/controllers/hostedcontrolplane/manifests/cno.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const clusterNetworkOperator = "cluster-network-operator"
const multusAdmissionController = "multus-admission-controller"
const networkNodeIdentity = "network-node-identity"
const ovnKubeControlPlane = "ovnkube-control-plane"
const cloudNetworkConfigController = "cloud-network-config-controller"

func ClusterNetworkOperatorDeployment(ns string) *appsv1.Deployment {
return &appsv1.Deployment{
Expand Down Expand Up @@ -77,6 +78,15 @@ func OVNKubeControlPlaneDeployment(namespace string) *appsv1.Deployment {
}
}

func CloudNetworkConfigControllerDeployment(namespace string) *appsv1.Deployment {
return &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: cloudNetworkConfigController,
},
}
}

func OVNKubeSBDBRoute(namespace string) *routev1.Route {
return &routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Expand Down
7 changes: 3 additions & 4 deletions control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,10 @@ func SetRestartAnnotationAndPatch(ctx context.Context, crclient client.Client, d
}

patch := dep.DeepCopy()
podMeta := patch.Spec.Template.ObjectMeta
if podMeta.Annotations == nil {
podMeta.Annotations = map[string]string{}
if patch.Spec.Template.ObjectMeta.Annotations == nil {
patch.Spec.Template.ObjectMeta.Annotations = map[string]string{}
}
podMeta.Annotations[hyperv1.RestartDateAnnotation] = restartAnnotation
patch.Spec.Template.ObjectMeta.Annotations[hyperv1.RestartDateAnnotation] = restartAnnotation

if err := crclient.Patch(ctx, patch, client.MergeFrom(dep)); err != nil {
return fmt.Errorf("failed to set restart annotation: %w", err)
Expand Down
92 changes: 92 additions & 0 deletions control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,105 @@
package cno

import (
"context"
"testing"

. "github.com/onsi/gomega"

hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestSetRestartAnnotationAndPatch(t *testing.T) {
scheme := runtime.NewScheme()
_ = appsv1.AddToScheme(scheme)

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find and examine the target test file
fd -a 'component_test.go' control-plane-operator/controllers/hostedcontrolplane/v2/cno/

Repository: openshift/hypershift

Length of output: 162

🏁 Script executed:

# Read the specific file to see the context around line 21
wc -l control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go && \
head -30 control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go

Repository: openshift/hypershift

Length of output: 816

🏁 Script executed:

# Check if there are other AddToScheme calls in similar test files and how they're handled
rg -A 2 -B 2 "AddToScheme" control-plane-operator/controllers/hostedcontrolplane/v2/ --type go

Repository: openshift/hypershift

Length of output: 17118

Handle AddToScheme error in test setup instead of discarding it.

Line 21 discards the error return from appsv1.AddToScheme(scheme). If scheme registration fails, subsequent test assertions may fail for the wrong reason, making debugging harder.

Suggested fix 
 	scheme := runtime.NewScheme()
-	_ = appsv1.AddToScheme(scheme)
+	if err := appsv1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add apps/v1 to scheme: %v", err)
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_ = appsv1.AddToScheme(scheme)
scheme := runtime.NewScheme()
if err := appsv1.AddToScheme(scheme); err != nil {
t.Fatalf("failed to add apps/v1 to scheme: %v", err)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go`
at line 21, The error returned by appsv1.AddToScheme(scheme) on line 21 is being
discarded with a blank identifier, which can mask failures during test setup.
Instead of discarding the error, check if AddToScheme returns a non-nil error
and fail the test immediately using the appropriate test failure method (such as
t.Fatalf or Expect/Require from your testing library) to provide clear
diagnostics if scheme registration fails during test initialization.

Source: Coding guidelines


tests := []struct {
name string
existingDeploy *appsv1.Deployment
restartAnnotation string
expectError bool
expectAnnotation bool
}{
{
name: "When deployment exists with nil annotations it should set the restart annotation",
existingDeploy: &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
Name: "test-deploy",
},
},
restartAnnotation: "2026-06-15T12:00:00Z",
expectAnnotation: true,
},
{
name: "When deployment exists with existing annotations it should set the restart annotation",
existingDeploy: &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
Name: "test-deploy",
},
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"existing-key": "existing-value",
},
},
},
},
},
restartAnnotation: "2026-06-15T12:00:00Z",
expectAnnotation: true,
},
{
name: "When deployment does not exist it should return nil",
existingDeploy: nil,
restartAnnotation: "2026-06-15T12:00:00Z",
expectAnnotation: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

builder := fake.NewClientBuilder().WithScheme(scheme)
if tt.existingDeploy != nil {
builder = builder.WithObjects(tt.existingDeploy)
}
cl := builder.Build()

dep := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
Name: "test-deploy",
},
}

err := SetRestartAnnotationAndPatch(context.Background(), cl, dep, tt.restartAnnotation)
if tt.expectError {
g.Expect(err).To(HaveOccurred())
return
}
g.Expect(err).ToNot(HaveOccurred())

if tt.expectAnnotation {
updated := &appsv1.Deployment{}
err = cl.Get(context.Background(), client.ObjectKeyFromObject(dep), updated)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(updated.Spec.Template.ObjectMeta.Annotations).To(HaveKeyWithValue(hyperv1.RestartDateAnnotation, tt.restartAnnotation))
}
})
}
}

func TestPlatformHasCloudNetworkConfigController(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 4 additions & 0 deletions docs/content/how-to/restart-control-plane-components.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The list of components restarted are listed below:

* catalog-operator
* certified-operators-catalog
* cloud-network-config-controller (cloud platforms only: AWS, Azure, GCP, OpenStack)
* cluster-api
* cluster-autoscaler
* cluster-policy-controller
Expand All @@ -29,11 +30,14 @@ The list of components restarted are listed below:
* kube-controller-manager
* kube-scheduler
* machine-approver
* multus-admission-controller (if multi-network is enabled)
* network-node-identity
* oauth-openshift
* olm-operator
* openshift-apiserver
* openshift-controller-manager
* openshift-oauth-apiserver
* ovnkube-control-plane
* packageserver
* redhat-marketplace-catalog
* redhat-operators-catalog
4 changes: 4 additions & 0 deletions docs/content/reference/aggregated-docs.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading