From c457615e9c4e22b918e53ee4b555c67922c9f144 Mon Sep 17 00:00:00 2001 From: Steve Goodwin Date: Thu, 30 Jul 2026 15:53:51 -0400 Subject: [PATCH] OCPBUGS-99943: set API-server-defaulted fields on deployment specs to prevent pathological update events ApplyDeploymentWithForce replaces the entire deployment spec on every metadata-triggered update. Fields omitted from the operator's desired spec (progressDeadlineSeconds, revisionHistoryLimit, dnsPolicy, serviceAccount) were written as null, forcing the API server to re-default them on each write. This doubled the deployment update event count per legitimate trigger (e.g., plugin registration), exceeding the pathological event threshold in TechPreview CI jobs. Set these fields explicitly in both deployment YAML templates to match the API server defaults. Also set MaxSurge/MaxUnavailable on non-HA rolling update strategy for the same reason. Update test expectations to match. Assisted by: Claude Code (Opus 4.6) --- .../deployments/console-deployment.yaml | 4 ++ .../deployments/downloads-deployment.yaml | 4 ++ .../subresource/deployment/deployment.go | 5 +- .../subresource/deployment/deployment_test.go | 63 +++++++++++++------ 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/bindata/assets/deployments/console-deployment.yaml b/bindata/assets/deployments/console-deployment.yaml index 0189d3a0ea..79a941e3c9 100644 --- a/bindata/assets/deployments/console-deployment.yaml +++ b/bindata/assets/deployments/console-deployment.yaml @@ -7,6 +7,8 @@ metadata: app: console component: ui spec: + progressDeadlineSeconds: 600 + revisionHistoryLimit: 10 selector: matchLabels: app: console @@ -23,10 +25,12 @@ spec: target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' openshift.io/required-scc: restricted-v2 spec: + dnsPolicy: ClusterFirst nodeSelector: node-role.kubernetes.io/master: "" restartPolicy: Always serviceAccountName: console + serviceAccount: console schedulerName: default-scheduler securityContext: runAsNonRoot: true diff --git a/bindata/assets/deployments/downloads-deployment.yaml b/bindata/assets/deployments/downloads-deployment.yaml index e5a8dafa15..ce23f4fc1c 100644 --- a/bindata/assets/deployments/downloads-deployment.yaml +++ b/bindata/assets/deployments/downloads-deployment.yaml @@ -8,6 +8,8 @@ metadata: component: downloads annotations: {} spec: + progressDeadlineSeconds: 600 + revisionHistoryLimit: 10 selector: matchLabels: app: console @@ -24,7 +26,9 @@ spec: target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' openshift.io/required-scc: restricted-v2 spec: + dnsPolicy: ClusterFirst serviceAccountName: downloads + serviceAccount: downloads nodeSelector: kubernetes.io/os: linux node-role.kubernetes.io/master: "" diff --git a/pkg/console/subresource/deployment/deployment.go b/pkg/console/subresource/deployment/deployment.go index a9d6491e4e..4a015ea943 100644 --- a/pkg/console/subresource/deployment/deployment.go +++ b/pkg/console/subresource/deployment/deployment.go @@ -182,7 +182,10 @@ func withAffinity( } func withStrategy(deployment *appsv1.Deployment, infrastructureConfig *configv1.Infrastructure) { - rollingUpdateParams := &appsv1.RollingUpdateDeployment{} + rollingUpdateParams := &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + } if ShouldDeployHA(infrastructureConfig) { rollingUpdateParams = &appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index afd28d53fd..ea9592440b 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -238,7 +238,9 @@ func TestDefaultDeployment(t *testing.T) { Annotations: consoleDeploymentTemplateAnnotations, }, Spec: corev1.PodSpec{ - ServiceAccountName: "console", + DNSPolicy: corev1.DNSClusterFirst, + ServiceAccountName: "console", + DeprecatedServiceAccount: "console", // we want to deploy on master nodes NodeSelector: map[string]string{"node-role.kubernetes.io/master": ""}, Affinity: consoleDeploymentAffinity, @@ -272,9 +274,9 @@ func TestDefaultDeployment(t *testing.T) { }, }, MinReadySeconds: 0, - RevisionHistoryLimit: nil, + RevisionHistoryLimit: ptr.To(int32(10)), Paused: false, - ProgressDeadlineSeconds: nil, + ProgressDeadlineSeconds: ptr.To(int32(600)), }, Status: appsv1.DeploymentStatus{}, }, @@ -317,7 +319,9 @@ func TestDefaultDeployment(t *testing.T) { Annotations: consoleDeploymentTemplateAnnotations, }, Spec: corev1.PodSpec{ - ServiceAccountName: "console", + DNSPolicy: corev1.DNSClusterFirst, + ServiceAccountName: "console", + DeprecatedServiceAccount: "console", // we want to deploy on master nodes NodeSelector: map[string]string{"node-role.kubernetes.io/master": ""}, Affinity: consoleDeploymentAffinity, @@ -351,9 +355,9 @@ func TestDefaultDeployment(t *testing.T) { }, }, MinReadySeconds: 0, - RevisionHistoryLimit: nil, + RevisionHistoryLimit: ptr.To(int32(10)), Paused: false, - ProgressDeadlineSeconds: nil, + ProgressDeadlineSeconds: ptr.To(int32(600)), }, Status: appsv1.DeploymentStatus{}, }, @@ -396,7 +400,9 @@ func TestDefaultDeployment(t *testing.T) { Annotations: consoleDeploymentTemplateAnnotations, }, Spec: corev1.PodSpec{ - ServiceAccountName: "console", + DNSPolicy: corev1.DNSClusterFirst, + ServiceAccountName: "console", + DeprecatedServiceAccount: "console", // we want to deploy on master nodes NodeSelector: map[string]string{"node-role.kubernetes.io/master": ""}, Affinity: &corev1.Affinity{}, @@ -419,13 +425,16 @@ func TestDefaultDeployment(t *testing.T) { }, }, Strategy: appsv1.DeploymentStrategy{ - Type: appsv1.RollingUpdateDeploymentStrategyType, - RollingUpdate: &appsv1.RollingUpdateDeployment{}, + Type: appsv1.RollingUpdateDeploymentStrategyType, + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + }, }, MinReadySeconds: 0, - RevisionHistoryLimit: nil, + RevisionHistoryLimit: ptr.To(int32(10)), Paused: false, - ProgressDeadlineSeconds: nil, + ProgressDeadlineSeconds: ptr.To(int32(600)), }, Status: appsv1.DeploymentStatus{}, }, @@ -468,7 +477,9 @@ func TestDefaultDeployment(t *testing.T) { Annotations: consoleDeploymentTemplateAnnotations, }, Spec: corev1.PodSpec{ - ServiceAccountName: "console", + DNSPolicy: corev1.DNSClusterFirst, + ServiceAccountName: "console", + DeprecatedServiceAccount: "console", // we do not want to deploy on master nodes NodeSelector: map[string]string{}, Affinity: consoleDeploymentAffinity, @@ -502,9 +513,9 @@ func TestDefaultDeployment(t *testing.T) { }, }, MinReadySeconds: 0, - RevisionHistoryLimit: nil, + RevisionHistoryLimit: ptr.To(int32(10)), Paused: false, - ProgressDeadlineSeconds: nil, + ProgressDeadlineSeconds: ptr.To(int32(600)), }, Status: appsv1.DeploymentStatus{}, }, @@ -1519,7 +1530,10 @@ func TestWithStrategy(t *testing.T) { infrastructureConfigExternalTopologyHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.HighlyAvailableTopologyMode) infrastructureConfigExternalTopologySingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.SingleReplicaTopologyMode) - singleReplicaStrategy := appsv1.RollingUpdateDeployment{} + singleReplicaStrategy := appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + } highAvailStrategy := appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ IntVal: int32(3), @@ -1719,7 +1733,9 @@ func TestDefaultDownloadsDeployment(t *testing.T) { configv1.SingleReplicaTopologyMode) downloadsDeploymentPodSpecSingleReplica := corev1.PodSpec{ - ServiceAccountName: "downloads", + DNSPolicy: corev1.DNSClusterFirst, + ServiceAccountName: "downloads", + DeprecatedServiceAccount: "downloads", NodeSelector: map[string]string{ "kubernetes.io/os": "linux", "node-role.kubernetes.io/master": "", @@ -1850,10 +1866,15 @@ func TestDefaultDownloadsDeployment(t *testing.T) { }, ObjectMeta: downloadsDeploymentObjectMeta, Spec: appsv1.DeploymentSpec{ - Replicas: &singleNodeReplicaCount, + Replicas: &singleNodeReplicaCount, + RevisionHistoryLimit: ptr.To(int32(10)), + ProgressDeadlineSeconds: ptr.To(int32(600)), Strategy: appsv1.DeploymentStrategy{ - Type: appsv1.RollingUpdateDeploymentStrategyType, - RollingUpdate: &appsv1.RollingUpdateDeployment{}, + Type: appsv1.RollingUpdateDeploymentStrategyType, + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + }, }, Selector: &metav1.LabelSelector{ MatchLabels: labels, @@ -1886,7 +1907,9 @@ func TestDefaultDownloadsDeployment(t *testing.T) { }, ObjectMeta: downloadsDeploymentObjectMeta, Spec: appsv1.DeploymentSpec{ - Replicas: &defaultReplicaCount, + Replicas: &defaultReplicaCount, + RevisionHistoryLimit: ptr.To(int32(10)), + ProgressDeadlineSeconds: ptr.To(int32(600)), Strategy: appsv1.DeploymentStrategy{ Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{