From 900cd6c58a3d6b6ddd3882bae119a150ce24be6f Mon Sep 17 00:00:00 2001 From: Martin Gencur Date: Mon, 15 Jun 2026 09:02:16 +0200 Subject: [PATCH 1/2] test(controlplane-component): add RestartDateAnnotation propagation tests Verify that setDefaultOptions propagates RestartDateAnnotation from HCP to the pod template when set, and omits it when absent. This covers the mechanism that triggers rolling restarts via pod template annotation changes. Rolling out the new ReplicaSet is then Kubernetes' concern. The annotation propagation from HostedCluster to HostedControlPlane is already tested in TestReconcileHostedControlPlaneAnnotations. Co-Authored-By: Claude Opus 4.6 --- .../controlplane-component/defaults_test.go | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/support/controlplane-component/defaults_test.go b/support/controlplane-component/defaults_test.go index e02e29e730f1..5b58c2705d5c 100644 --- a/support/controlplane-component/defaults_test.go +++ b/support/controlplane-component/defaults_test.go @@ -649,4 +649,62 @@ func TestSetDefaultOptions(t *testing.T) { g.Expect(deployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(test.expectedResources)) }) } + + annotationTests := []struct { + name string + hcpAnnotations map[string]string + expectSet bool + }{ + { + name: "When HCP has RestartDateAnnotation it should propagate it to the pod template", + hcpAnnotations: map[string]string{ + hyperv1.RestartDateAnnotation: "2024-01-15T12:00:00Z", + }, + expectSet: true, + }, + { + name: "When HCP has no RestartDateAnnotation it should not set it on the pod template", + expectSet: false, + }, + } + + for _, test := range annotationTests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + workload := &controlPlaneWorkload[*appsv1.Deployment]{ + name: "kube-apiserver", + workloadProvider: &deploymentProvider{}, + ComponentOptions: &testComponent{}, + } + deployment := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "kube-apiserver", Image: "hyperkube"}, + }, + }, + }, + }, + } + hcp := &hyperv1.HostedControlPlane{} + hcp.Annotations = test.hcpAnnotations + + err := workload.setDefaultOptions(ControlPlaneContext{ + HCP: hcp, + Client: fake.NewClientBuilder().WithScheme(scheme).Build(), + ReleaseImageProvider: releaseProvider, + }, deployment, nil) + g.Expect(err).NotTo(HaveOccurred()) + + if test.expectSet { + g.Expect(deployment.Spec.Template.Annotations).To(HaveKeyWithValue( + hyperv1.RestartDateAnnotation, test.hcpAnnotations[hyperv1.RestartDateAnnotation])) + } else { + g.Expect(deployment.Spec.Template.Annotations).NotTo(HaveKey(hyperv1.RestartDateAnnotation)) + } + }) + } } From 928c70f81e4ac067d107bf8d1352b5181c48daf3 Mon Sep 17 00:00:00 2001 From: Martin Gencur Date: Mon, 15 Jun 2026 09:51:47 +0200 Subject: [PATCH 2/2] fix(cno): fix nil-map bug in SetRestartAnnotationAndPatch The intermediate variable `podMeta := patch.Spec.Template.ObjectMeta` copied ObjectMeta by value, so when Annotations was nil, the new map was created on the copy and never written back to the patch object. Access annotations directly on the patch to fix. Co-Authored-By: Claude Opus 4.6 --- .../hostedcontrolplane/v2/cno/component.go | 7 +- .../v2/cno/component_test.go | 100 ++++++++++++++++++ .../controlplane-component/defaults_test.go | 12 ++- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go b/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go index 139e1163995f..5d3e2200f833 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go @@ -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) diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go index 355b439d0f64..2b5c19eabffa 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go @@ -1,11 +1,20 @@ 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" + + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestPlatformHasCloudNetworkConfigController(t *testing.T) { @@ -69,3 +78,94 @@ func TestPlatformHasCloudNetworkConfigController(t *testing.T) { }) } } + +func TestSetRestartAnnotationAndPatch(t *testing.T) { + scheme := runtime.NewScheme() + if err := appsv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add appsv1 to scheme: %v", err) + } + + tests := []struct { + name string + existingDeployment *appsv1.Deployment + restartAnnotation string + expectAnnotationPresent bool + }{ + { + name: "When deployment exists it should set the restart annotation on the pod template", + existingDeployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multus-admission-controller", + Namespace: "test-namespace", + }, + }, + restartAnnotation: "2024-01-15T12:00:00Z", + expectAnnotationPresent: true, + }, + { + name: "When deployment does not exist it should return nil", + existingDeployment: nil, + restartAnnotation: "2024-01-15T12:00:00Z", + expectAnnotationPresent: false, + }, + { + name: "When deployment already has pod template annotations it should preserve them and add the restart annotation", + existingDeployment: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multus-admission-controller", + Namespace: "test-namespace", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "existing-key": "existing-value", + }, + }, + }, + }, + }, + restartAnnotation: "CertHash:abc123", + expectAnnotationPresent: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if tt.existingDeployment != nil { + builder = builder.WithObjects(tt.existingDeployment) + } + fakeClient := builder.Build() + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multus-admission-controller", + Namespace: "test-namespace", + }, + } + + err := SetRestartAnnotationAndPatch(context.Background(), fakeClient, dep, tt.restartAnnotation) + g.Expect(err).NotTo(HaveOccurred()) + + if !tt.expectAnnotationPresent { + return + } + + updated := &appsv1.Deployment{} + err = fakeClient.Get(context.Background(), crclient.ObjectKeyFromObject(dep), updated) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(updated.Spec.Template.Annotations).To(HaveKeyWithValue( + hyperv1.RestartDateAnnotation, tt.restartAnnotation)) + + if tt.existingDeployment != nil && tt.existingDeployment.Spec.Template.Annotations != nil { + for k, v := range tt.existingDeployment.Spec.Template.Annotations { + g.Expect(updated.Spec.Template.Annotations).To(HaveKeyWithValue(k, v)) + } + } + }) + } +} diff --git a/support/controlplane-component/defaults_test.go b/support/controlplane-component/defaults_test.go index 5b58c2705d5c..4be8663b1b31 100644 --- a/support/controlplane-component/defaults_test.go +++ b/support/controlplane-component/defaults_test.go @@ -523,9 +523,15 @@ func TestApplyNonOvercommitableResourceLimits(t *testing.T) { func TestSetDefaultOptions(t *testing.T) { scheme := runtime.NewScheme() - _ = hyperv1.AddToScheme(scheme) - _ = corev1.AddToScheme(scheme) - _ = appsv1.AddToScheme(scheme) + if err := hyperv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add hyperv1 to scheme: %v", err) + } + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add corev1 to scheme: %v", err) + } + if err := appsv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add appsv1 to scheme: %v", err) + } t.Run("When SetDefaultSecurityContext is true it should set RunAsUser and FSGroup", func(t *testing.T) { t.Parallel()