OCPBUGS-86719: Use zero-downtime rollout strategy for console pods

[asadawar]

Summary

• Change the console deployment rollout strategy from maxSurge=3, maxUnavailable=1 to maxSurge=1, maxUnavailable=0 on 3+ node topologies (HighlyAvailable, External+HA), ensuring no old pod is terminated until its replacement passes readiness checks
• On 2-node topologies (DualReplica, HighlyAvailableArbiter), keep maxUnavailable=1 with maxSurge reduced from 3 to 1 to avoid rollout deadlock caused by required pod anti-affinity
• Add test coverage for DualReplica and HighlyAvailableArbiter topology strategies

Why this approach

Three approaches were considered:

1. maxUnavailable=0 for all HA topologies (rejected)
On DualReplica (2 masters, 2 replicas) and HighlyAvailableArbiter (2 full masters + 1 arbiter) clusters, the console deployment uses RequiredDuringSchedulingIgnoredDuringExecution pod anti-affinity on kubernetes.io/hostname. When every eligible node already runs a console pod, the scheduler cannot place a surge pod. With maxUnavailable=0, no old pod can be terminated either, causing a rollout deadlock that stalls until ProgressDeadlineExceeded (10 minutes). This approach was rejected because it would break recently added DualReplica support (PR #1151, merged 2026-05-07).

2. Keep maxUnavailable=1 for all topologies, only reduce maxSurge (rejected)
Reducing maxSurge from 3 to 1 aligns with other operators (CMO monitoring-plugin uses maxUnavailable=1 with default maxSurge) but does not fix the reported bug. With maxUnavailable=1, Kubernetes is still allowed to terminate one old pod before its replacement is ready, causing the console flap. This approach was rejected because it does not address the root cause.

3. Topology-aware strategy (chosen)
Use maxUnavailable=0 on topologies where a free node is available for the surge pod (HighlyAvailable with 3+ masters, External+HA with multiple workers), and maxUnavailable=1 on constrained topologies (DualReplica, HighlyAvailableArbiter) where rollout deadlock is possible. This fixes the bug for the most common topology while preserving correct behavior on constrained clusters.

For the HighlyAvailableArbiter case, the conservative choice (maxUnavailable=1) was made because arbiter nodes may have taints or resource constraints that prevent scheduling console pods, effectively making it a 2-node topology for console scheduling. Maintainers familiar with arbiter node scheduling can adjust this if arbiter nodes are known to be eligible.

Root cause

The withStrategy function in pkg/console/subresource/deployment/deployment.go:184 set maxSurge=3, maxUnavailable=1 for all HA topologies. These values were introduced in PR #1107 (OCPBUGS-74872) as part of a refactor that moved deployment construction from bindata to Go code, without specific rationale for the strategy values.

With maxUnavailable=1 and 2 replicas, the Kubernetes deployment controller is allowed to terminate one old pod immediately when a rollout starts, even before any new pod is ready. This creates a window (approximately 10-15 seconds based on observed pod startup times) where only one pod serves traffic. During this window:

• The terminating pod is removed from Service endpoints (even though the preStop hook keeps the process alive for 25 seconds)
• New connections are routed only to the single remaining pod
• WebSocket connections to the terminated pod are dropped, causing visible console "flapping"

Cluster verification

Verified on a live OCP 4.22.0-rc.4 vSphere IPI cluster:

Cluster topology:

$ oc get infrastructure cluster -o jsonpath='{.status.controlPlaneTopology}'
HighlyAvailable

$ oc get nodes -l node-role.kubernetes.io/master= -o name
node/master-0
node/master-1
node/master-2

Current strategy (before fix):

$ oc get deployment console -n openshift-console -o jsonpath='{.spec.strategy}'
{"rollingUpdate":{"maxSurge":3,"maxUnavailable":1},"type":"RollingUpdate"}

Pod distribution (2 pods on 2 of 3 masters, 3rd master free for surge):

$ oc get pods -n openshift-console -o wide
NAME                        READY   STATUS    NODE
console-7dfb9f987d-4rbcd    1/1     Running   master-1
console-7dfb9f987d-zc7n4    1/1     Running   master-0

With the fix applied (maxSurge=1, maxUnavailable=0), the rollout behavior would be:

1. New pod created on master-2 (the free node). Total: 3 pods, 2 available.
2. New pod passes readiness check. Total: 3 pods, 3 available.
3. One old pod terminated. Total: 2 pods, 2 available.
4. Second new pod created on the freed node. Total: 3 pods, 2 available.
5. Second new pod passes readiness check. Old pod terminated. Total: 2 pods, 2 available.

At no point does availability drop below 2 (full capacity).

Test plan

• Unit tests pass (make test-unit): all deployment strategy tests updated and passing
• Added new test cases for DualReplica and HighlyAvailableArbiter topologies
• gofmt and govet clean (make check)
• Verified current cluster topology and strategy on live OCP 4.22 cluster

OWNERS

/cc @spadgett @jhadvig @TheRealJon

Bug: https://issues.redhat.com/browse/OCPBUGS-86719

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Console Deployment now uses topology-aware rolling update strategy with optimized parameters (maxSurge=1, maxUnavailable=0-1) based on control plane topology configuration.

• Tests

  • Updated test expectations for console Deployment rolling update strategy across all topology modes.

[openshift-ci-robot]

@asadawar: This pull request references Jira Issue OCPBUGS-86719, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

• Change the console deployment rollout strategy from maxSurge=3, maxUnavailable=1 to maxSurge=1, maxUnavailable=0 on 3+ node topologies (HighlyAvailable, External+HA), ensuring no old pod is terminated until its replacement passes readiness checks
• On 2-node topologies (DualReplica, HighlyAvailableArbiter), keep maxUnavailable=1 with maxSurge reduced from 3 to 1 to avoid rollout deadlock caused by required pod anti-affinity
• Add test coverage for DualReplica and HighlyAvailableArbiter topology strategies

Why this approach

Three approaches were considered:

1. maxUnavailable=0 for all HA topologies (rejected)
On DualReplica (2 masters, 2 replicas) and HighlyAvailableArbiter (2 full masters + 1 arbiter) clusters, the console deployment uses RequiredDuringSchedulingIgnoredDuringExecution pod anti-affinity on kubernetes.io/hostname. When every eligible node already runs a console pod, the scheduler cannot place a surge pod. With maxUnavailable=0, no old pod can be terminated either, causing a rollout deadlock that stalls until ProgressDeadlineExceeded (10 minutes). This approach was rejected because it would break recently added DualReplica support (PR #1151, merged 2026-05-07).

2. Keep maxUnavailable=1 for all topologies, only reduce maxSurge (rejected)
Reducing maxSurge from 3 to 1 aligns with other operators (CMO monitoring-plugin uses maxUnavailable=1 with default maxSurge) but does not fix the reported bug. With maxUnavailable=1, Kubernetes is still allowed to terminate one old pod before its replacement is ready, causing the console flap. This approach was rejected because it does not address the root cause.

3. Topology-aware strategy (chosen)
Use maxUnavailable=0 on topologies where a free node is available for the surge pod (HighlyAvailable with 3+ masters, External+HA with multiple workers), and maxUnavailable=1 on constrained topologies (DualReplica, HighlyAvailableArbiter) where rollout deadlock is possible. This fixes the bug for the most common topology while preserving correct behavior on constrained clusters.

For the HighlyAvailableArbiter case, the conservative choice (maxUnavailable=1) was made because arbiter nodes may have taints or resource constraints that prevent scheduling console pods, effectively making it a 2-node topology for console scheduling. Maintainers familiar with arbiter node scheduling can adjust this if arbiter nodes are known to be eligible.

Root cause

The withStrategy function in pkg/console/subresource/deployment/deployment.go:184 set maxSurge=3, maxUnavailable=1 for all HA topologies. These values were introduced in PR #1107 (OCPBUGS-74872) as part of a refactor that moved deployment construction from bindata to Go code, without specific rationale for the strategy values.

With maxUnavailable=1 and 2 replicas, the Kubernetes deployment controller is allowed to terminate one old pod immediately when a rollout starts, even before any new pod is ready. This creates a window (approximately 10-15 seconds based on observed pod startup times) where only one pod serves traffic. During this window:

• The terminating pod is removed from Service endpoints (even though the preStop hook keeps the process alive for 25 seconds)
• New connections are routed only to the single remaining pod
• WebSocket connections to the terminated pod are dropped, causing visible console "flapping"

Cluster verification

Verified on a live OCP 4.22.0-rc.4 vSphere IPI cluster:

Cluster topology:

$ oc get infrastructure cluster -o jsonpath='{.status.controlPlaneTopology}'
HighlyAvailable

$ oc get nodes -l node-role.kubernetes.io/master= -o name
node/master-0
node/master-1
node/master-2

Current strategy (before fix):

$ oc get deployment console -n openshift-console -o jsonpath='{.spec.strategy}'
{"rollingUpdate":{"maxSurge":3,"maxUnavailable":1},"type":"RollingUpdate"}

Pod distribution (2 pods on 2 of 3 masters, 3rd master free for surge):

$ oc get pods -n openshift-console -o wide
NAME                        READY   STATUS    NODE
console-7dfb9f987d-4rbcd    1/1     Running   master-1
console-7dfb9f987d-zc7n4    1/1     Running   master-0

With the fix applied (maxSurge=1, maxUnavailable=0), the rollout behavior would be:

1. New pod created on master-2 (the free node). Total: 3 pods, 2 available.
2. New pod passes readiness check. Total: 3 pods, 3 available.
3. One old pod terminated. Total: 2 pods, 2 available.
4. Second new pod created on the freed node. Total: 3 pods, 2 available.
5. Second new pod passes readiness check. Old pod terminated. Total: 2 pods, 2 available.

At no point does availability drop below 2 (full capacity).

Test plan

• Unit tests pass (make test-unit): all deployment strategy tests updated and passing
• Added new test cases for DualReplica and HighlyAvailableArbiter topologies
• gofmt and govet clean (make check)
• Verified current cluster topology and strategy on live OCP 4.22 cluster

OWNERS

/cc @spadgett @jhadvig @TheRealJon

Bug: https://issues.redhat.com/browse/OCPBUGS-86719

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: asadawar
Once this PR has been reviewed and has the lgtm label, please assign jhadvig for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5b464819-6cf1-474b-a457-90cd71969835

📥 Commits

Reviewing files that changed from the base of the PR and between cba2bb3 and 77df00f.

📒 Files selected for processing (2)

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (5)

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Follow Go coding standards and patterns documented in CONVENTIONS.md
Organize imports according to conventions documented in CONVENTIONS.md
Use gofmt to format Go code with standard formatting
Run go vet checks on all Go packages

Follow Go coding standards and patterns as documented in CONVENTIONS.md, including proper import organization

Organize Go code following the repository structure: main entry point in cmd/console/main.go, API constants in pkg/api/, operator command setup in pkg/cmd/operator/, and version command in pkg/cmd/version/

**/*.go: Use gofmt for formatting Go code
Follow standard Go naming conventions
Group imports in order: standard lib, 3rd party, kube/openshift, internal (marked with comments)
Use meaningful error messages with context in Go code
Set status conditions using status.Handle* functions with type prefixes (*Degraded, *Progressing, *Available, *Upgradeable)
Use typed errors and wrap errors to preserve stack context

Files:

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

---

⚙️ CodeRabbit configuration file

**/*.go: Review Go code following OpenShift operator patterns.
See CONVENTIONS.md for coding standards and patterns.

Refer to the following skills based on CODE PATTERNS, not just file paths:

Refer to /controller-review when code contains:

• Controller struct types (e.g., type *Controller struct)
• func New*Controller( factory functions
• factory.New().WithFilteredEventsInformers( pattern
• .ToController( method calls
• Sync(ctx context.Context, controllerContext factory.SyncContext) methods
• operatorConfig.Spec.ManagementState checks
• status.NewStatusHandler or status.Handle* functions

Refer to /sync-handler-review when code contains:

• Main operator sync functions (e.g., sync_v400.go content)
• Sequential resource syncing with early returns
• Incremental reconciliation loops
• Multiple resourceapply.Apply*() calls in sequence
• Dependency ordering of ConfigMaps → Secrets → Service Accounts → RBAC → Services → Deployments → Routes
• Feature gate conditional logic

Refer to /go-quality-review for all Go code to check:

• Deprecated imports: ioutil.ReadFile, ioutil.WriteFile, ioutil.ReadAll
• Deprecated patterns: Dial without DialContext
• Error handling: missing %w in fmt.Errorf
• Code smells: deep nesting (4+ levels), functions >100 lines
• Magic values: unexplained numbers/strings
• Context propagation: context.Background() instead of passed ctx
• Missing godoc on exported functions

Files:

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

{pkg,cmd}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Use gofmt for code formatting on pkg and cmd directories

{pkg,cmd}/**/*.go: Format code using gofmt -w ./pkg ./cmd
Run go vet checks on all Go packages in ./pkg and ./cmd

Files:

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

pkg/console/subresource/**/*.go

📄 CodeRabbit inference engine (ARCHITECTURE.md)

Use pkg/console/subresource/ packages for resource builders, with separate packages for each resource type (authentication, configmap, deployment, oauthclient, route, secret, etc.)

Files:

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}

⚙️ CodeRabbit configuration file

**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):

• SQL: parameterized queries only; no string concatenation
• Command: no shell=True, os.system, or backtick exec with user input
• LDAP/XPath: escape special characters in filters
• Path traversal: canonicalize paths, reject ../
• Deserialization: no pickle/yaml.load()/eval on untrusted data
• Prototype pollution: no recursive merge of untrusted objects
• Validate at trust boundaries with allow-lists, not deny-lists
• Normalize Unicode and anchor regexes (^$); watch for ReDoS

Files:

• pkg/console/subresource/deployment/deployment.go
• pkg/console/subresource/deployment/deployment_test.go

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

Follow testing patterns and commands documented in TESTING.md

Follow testing patterns and commands as documented in TESTING.md, including running unit tests with 'make test-unit' and checks with 'make check'

**/*_test.go: Use table-driven tests for comprehensive coverage
Use httptest for HTTP handler testing in Go
Include proper cleanup functions in tests
Test both success and failure paths

Files:

• pkg/console/subresource/deployment/deployment_test.go

---

⚙️ CodeRabbit configuration file

**/*_test.go: Review test code for quality and patterns.

Refer to /unit-test-review when test is in pkg//*_test.go:**

• Table-driven test structure with test cases
• Use of go-test/deep for struct comparisons
• Test naming conventions (TestFunctionName)
• Error handling with wantErr pattern
• Edge case coverage (nil, empty, boundary values)
• Proper assertions with helpful error messages
• Test isolation (no shared mutable state)

Refer to /e2e-test-review when test contains:

• framework.MustNewClientset(t, nil) or similar e2e framework usage
• wait.Poll or wait.PollImmediate patterns
• retry.RetryOnConflict for updates
• Cleanup via defer functions
• Console/operator CR manipulations
• Test assertions on cluster state

Suggest to use /e2e-test-review when:

• PR adds new feature requiring e2e coverage
• Test file is empty or skeleton
• Comments indicate "TODO: add test"

Review for common issues:

• Missing cleanup (defer statements)
• Using time.Sleep instead of wait.Poll
• Missing context timeouts
• Vague error messages in assertions
• Tests without table-driven structure when testing multiple cases
• Ignoring errors with _
• Tests without assertions

Files:

• pkg/console/subresource/deployment/deployment_test.go

🔇 Additional comments (5)

pkg/console/subresource/deployment/deployment.go (1)

187-213: LGTM!

pkg/console/subresource/deployment/deployment_test.go (4)

267-271: LGTM!

Also applies to: 346-350, 497-501

---

1521-1540: LGTM!

---

1570-1630: LGTM!

---

1932-1936: LGTM!

---

Walkthrough

The PR updates console deployment rolling update strategy from a fixed configuration (maxSurge=3, maxUnavailable=1) to a topology-aware approach. For DualReplica and HighlyAvailableArbiter topologies, it uses maxSurge=1/maxUnavailable=1; for other HA topologies, it uses maxSurge=1/maxUnavailable=0 to enable zero-downtime rollouts. Tests are expanded to cover topology-specific cases and default expectations are updated.

Changes

Topology-aware rolling update strategy

Layer / File(s)	Summary
Topology-aware rolling update strategy implementation pkg/console/subresource/deployment/deployment.go	The withStrategy function is updated to set rolling update parameters (MaxSurge and MaxUnavailable) based on control-plane topology: MaxSurge=1/MaxUnavailable=1 for DualReplica and HighlyAvailableArbiter topologies to accommodate anti-affinity constraints, and MaxSurge=1/MaxUnavailable=0 for other HA configurations to aim for zero-downtime rollouts.
Strategy test fixtures and topology-specific test cases pkg/console/subresource/deployment/deployment_test.go	Test infrastructure is expanded with topology mode fixtures for DualReplicaTopologyMode and HighlyAvailableArbiterMode, rolling update strategy variables are introduced for zero-downtime and constrained high-availability configurations, the highly available case expectation is updated to use the new zero-downtime strategy, and new test cases are added to assert DualReplica and Arbiter topology-specific rolling update configurations.
Default deployment and downloads test expectations pkg/console/subresource/deployment/deployment_test.go	Test expectations across TestDefaultDeployment (default config-map, trusted-CA, and external topology scenarios) and TestDefaultDownloadsDeployment are updated to reflect the new default rolling update strategy of maxSurge=1 and maxUnavailable=0.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Test Structure And Quality	❓ Inconclusive	Custom check specifies Ginkgo test code, but PR modifies standard Go tests using t.Run(), not Ginkgo Describe/Context/It blocks. Check is not applicable to this codebase.	Clarify if check applies to standard Go tests or only Ginkgo. If all tests, remove Ginkgo-specific language from check instructions.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: updating the console deployment rollout strategy to use zero-downtime parameters for appropriate topologies.
Description check	✅ Passed	The PR description is comprehensive and well-structured with all critical sections filled: summary, rationale with three considered approaches, root cause analysis, cluster verification, test plan, and OWNERS cc'd.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	This repository uses standard Go testing.T framework, not Ginkgo. No Ginkgo test patterns found. Test case names are all static, descriptive strings without dynamic content.
Microshift Test Compatibility	✅ Passed	No Ginkgo e2e tests are added in this PR. Changes are only to production code and standard Go unit tests in deployment_test.go, which are not subject to MicroShift compatibility requirements.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PR contains only Go unit tests (TestWithStrategy, TestDefaultDeployment, etc.) in the testing package, not Ginkgo e2e tests. The custom check only applies to new Ginkgo e2e tests.
Topology-Aware Scheduling Compatibility	✅ Passed	PR properly handles topology-aware rolling updates: maxUnavailable=1 for 2-node topologies to avoid anti-affinity deadlock, maxUnavailable=0 for 3+ nodes for zero-downtime. All topologies tested.
Ote Binary Stdout Contract	✅ Passed	PR modifies only a library package with standard Go unit tests. No main(), init(), OTE binary infrastructure, or stdout-writing code is present. Changes are purely Kubernetes deployment configuration.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR does not add Ginkgo e2e tests; it only modifies unit tests in pkg/console/subresource/deployment/deployment_test.go using standard Go testing framework, so the check is not applicable.
No-Weak-Crypto	✅ Passed	PR contains no weak cryptography usage. Changes are limited to Kubernetes Deployment strategy parameters; no crypto imports, operations, or custom implementations detected.
Container-Privileges	✅ Passed	PR only modifies rolling update strategy parameters (maxSurge/maxUnavailable); no privileged settings, host access, dangerous capabilities, or allowPrivilegeEscalation: true are introduced.
No-Sensitive-Data-In-Logs	✅ Passed	The withStrategy function changes contain no logging. Existing logs only record resource version metadata and deployment status metrics, not passwords, tokens, keys, or PII.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

Hi @asadawar. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.