OCPBUGS-64582: frr-k8s: use Recreate strategy for statuscleaner deployment by fedepaol · Pull Request #3104 · openshift/cluster-network-operator

fedepaol · 2026-08-04T14:44:06Z

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.

openshift-merge-bot · 2026-08-04T14:44:08Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci-robot · 2026-08-04T14:44:12Z

@fedepaol: This pull request references Jira Issue OCPBUGS-64582, 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:

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.

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.

coderabbitai · 2026-08-04T14:44:25Z

Summary by CodeRabbit

Bug Fixes
- Improved status-cleaner deployment updates for single-node control planes by using a recreate strategy, helping prevent conflicts during rollout.
- Preserved the default deployment behavior for highly available control planes.
Tests
- Added coverage to verify the appropriate deployment strategy is selected for both single-node and highly available control planes.

Walkthrough

The renderer now provides an IsSNO flag based on the OVN control-plane replica count. The frr-k8s-statuscleaner Deployment uses Recreate for single-replica control planes. Tests cover single-replica and highly available configurations.

Changes

FRR-K8s status-cleaner

Layer / File(s)	Summary
Single-node strategy selection `pkg/network/render.go`, `bindata/network/frr-k8s/node-status-cleaner.yaml`, `pkg/network/render_test.go`	The renderer sets `IsSNO` when the control plane has one replica. The Deployment uses `Recreate` when `IsSNO` is enabled. Tests verify the single-replica and three-replica cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: arghosh93, marty-power, tpantelis

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

Ignore

❌ Failed checks (4 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Pr Quality	❌ Error	The functional bug-fix PR has no Testing/CI section, no bug link in its title or description, and no explicit Why/What sections; its 4-line code change is appropriately small.	Add Why, What, and Testing sections with automated CNO CI lanes and platforms, and include the OCPBUGS-64582 link in the title or description.
E2e Tests For Feature Changes	❌ Error	pkg/network/render.go changes FRR status-cleaner behavior for SNO, but no test/e2e files changed and the PR description has no Testing or How to verify it section.	Add or modify test/e2e coverage and document CI lanes, platforms, and results in a Testing or How to verify it section. If infeasible, document why there and request the specified override.
Container-Privileges	❌ Error	The changed Deployment contains `hostNetwork: true` at line 98, which the custom check explicitly flags; the field was present before this PR but remains in the manifest.	Remove `hostNetwork` or document and approve an explicit exception before merging this manifest under the container-privileges check.
Title check	❌ Error	The title describes the change and uses an imperative verb, but it is 74 characters and exceeds the 72-character limit.	Shorten the title to 72 characters or fewer while retaining the affected component and imperative wording.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	The added test has three Gomega assertions without diagnostic messages, including the rendering error check; it otherwise uses in-memory objects, so cleanup and timeouts are not needed.	Add meaningful failure messages to the error and strategy assertions, such as identifying the SNO or HA rendering case.

✅ Passed checks (18 passed)

Check name	Status	Explanation
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.
Commit Message Quality	✅ Passed	The PR contains one self-contained commit. Its scoped subject is descriptive, its body explains the hostNetwork port conflict, and it has no merge or generic review commits.
Unit Tests For Go Changes	✅ Passed	The PR modifies bindata/network/frr-k8s/node-status-cleaner.yaml and includes the corresponding pkg/network/render_test.go changes with SNO and HA strategy tests.
Rbac Least Privilege	✅ Passed	The pull request changes only a Deployment strategy in bindata and adds Go render data/tests; it adds or modifies no Role or ClusterRole rules.
Docs For Feature And Behavior Changes	✅ Passed	The PR fixes a documented SNO upgrade bug: hostNetwork status-cleaner uses fixed port 9123, so Recreate prevents port conflicts; no docs change is required for this bug fix.
Stale Project Docs And Config	✅ Passed	The PR changes only FRR-K8s rendering and SNO rollout strategy; scoped docs and .coderabbit.yaml contain no affected statuscleaner or strategy claims, and their generic guidance remains accurate.
Go And Test Code Quality	✅ Passed	The Go diff adds only the IsSNO assignment and a test; it introduces no banned logging, bare returns, duration literals, err shadowing, IPv4-only logic, unsafe concurrency, bare Fatal calls, sleeps...
Ai-Generated Code Smell	✅ Passed	The diff adds no slop or AI references, changes one render flag and a small two-case test, and uses the existing IsSNO convention without unnecessary logic.
Stable And Deterministic Test Names	✅ Passed	The added subtests use static titles, `SNO: strategy is Recreate` and `HA: no strategy override`; no generated names, timestamps, nodes, namespaces, IPs, or random values appear.
Microshift Test Compatibility	✅ Passed	The only added test is a Go `testing` unit test, not a Ginkgo e2e test; it renders local objects and references no MicroShift-hostile API or runtime feature.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds no Ginkgo e2e tests; it adds one standard Go Test_renderFRRStatusCleanerStrategy unit test that checks replica counts only and makes no multi-node assumption.
Topology-Aware Scheduling Compatibility	✅ Passed	The change adds only a SNO-gated Recreate rollout strategy; it adds no anti-affinity, spread, node selector, replica, or PDB constraint.
Ote Binary Stdout Contract	✅ Passed	The patch changes only YAML rendering and a Gomega unit test; it adds no OTE binary, main/suite setup, logging, or stdout write, and no OTE/Ginkgo suite exists.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds a Go unit test, not a Ginkgo e2e test. It creates no IPv4-only networking assumptions and requires no external connectivity.
No-Weak-Crypto	✅ Passed	The PR adds only a conditional Recreate strategy, an IsSNO render flag, and tests; it adds no weak crypto, custom crypto, or secret/token comparison.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds no logging calls or log data. It only adds an IsSNO render flag, a conditional Recreate strategy, and tests; no sensitive values are emitted.
Description check	✅ Passed	The description clearly explains the SNO upgrade failure and the Recreate strategy change in the statuscleaner deployment.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@bindata/network/frr-k8s/node-status-cleaner.yaml`:
- Around line 12-13: Add a test case to pkg/network/render_test.go that
validates the rendered frr-k8s-statuscleaner Deployment uses the appsv1.Recreate
strategy type. This ensures the deployment strategy configuration in the bindata
template is preserved through rendering and prevents future regressions that
could reintroduce port conflicts on SNO clusters.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 0835789f-0968-4416-b2a9-f8edc6fdbde6

📥 Commits

Reviewing files that changed from the base of the PR and between 713e229 and 60ac532.

📒 Files selected for processing (1)

bindata/network/frr-k8s/node-status-cleaner.yaml

fedepaol · 2026-08-04T15:51:42Z

/jira refresh

openshift-ci-robot · 2026-08-04T15:51:49Z

@fedepaol: This pull request references Jira Issue OCPBUGS-64582, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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.

fedepaol · 2026-08-05T07:14:05Z

/retest

fedepaol · 2026-08-05T09:53:41Z

/test e2e-metal-ipi-ovn-dualstack-bgp
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
/test frrk8s-e2e

fedepaol · 2026-08-05T09:56:43Z

/test frrk8s-e2e

fedepaol · 2026-08-05T09:56:52Z

/test e2e-metal-ipi-ovn-dualstack-bgp

fedepaol · 2026-08-05T09:57:03Z

/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw

fedepaol · 2026-08-05T10:38:45Z

/test ?

fedepaol · 2026-08-05T10:39:43Z

/test e2e-metal-ipi-ovn-dualstack-bgp

fedepaol · 2026-08-05T10:39:46Z

/test frrk8s-e2e

fedepaol · 2026-08-05T10:40:23Z

/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw

fedepaol · 2026-08-05T10:54:18Z

/test frrk8s-e2e

fedepaol · 2026-08-05T10:54:28Z

/test e2e-metal-ipi-ovn-dualstack-bgp

fedepaol · 2026-08-05T12:39:23Z

/test frrk8s-e2e

fedepaol · 2026-08-05T14:37:30Z

/test frrk8s-e2e

fedepaol · 2026-08-06T07:26:25Z

/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw

fedepaol · 2026-08-06T07:26:47Z

/test e2e-metal-ipi-ovn-dualstack-bgp

fedepaol · 2026-08-06T10:50:59Z

/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw

fedepaol · 2026-08-06T13:02:37Z

/jira refresh

openshift-ci-robot · 2026-08-06T13:02:42Z

@fedepaol: This pull request references Jira Issue OCPBUGS-64582, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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.

jcaamano · 2026-08-07T08:04:36Z

  annotations:
    release.openshift.io/version: "{{.ReleaseVersion}}"
 spec:
+  strategy:


Wouldn't it be best to guard this by the isSNO variable?

ah thanks! Didn't know about its existance. I'll do that, although the effect of this change is a small window (during upgrades) where changes to CRDs are rejected, so I don't think it would hurt much in non - sno scenarios.

Anyways, adding it

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 <fpaoline@redhat.com> Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-08-07T08:46:06Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

jcaamano · 2026-08-07T09:13:54Z

/lgtm

openshift-merge-bot · 2026-08-07T09:14:09Z

Scheduling required tests:
/test e2e-aws-ovn-upgrade-ipsec

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test frrk8s-e2e
/test e2e-aws-ovn-fdp-qe
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-upgrade
/test e2e-aws-ovn-windows
/test e2e-azure-ovn-upgrade
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-dualstack-bgp
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-ipv6-ipsec
/test e2e-ovn-ipsec-step-registry
/test hypershift-e2e-aks

openshift-ci · 2026-08-07T09:14:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fedepaol, jcaamano

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jcaamano]

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

fedepaol · 2026-08-07T09:40:01Z

/verified later

openshift-ci-robot · 2026-08-07T09:40:04Z

@fedepaol: /verified later <@username> requires at least one GitHub @username to be specified (it can be a comma delimited list). It indicates the engineer(s) that will be performing the verification. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified later

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.

fedepaol · 2026-08-07T10:32:45Z

/cherry-pick release-4.22

openshift-cherrypick-robot · 2026-08-07T10:32:48Z

@fedepaol: once the present PR merges, I will cherry-pick it on top of release-4.22 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.22

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.

fedepaol · 2026-08-07T11:18:42Z

/verified by ci

openshift-ci-robot · 2026-08-07T11:18:54Z

@fedepaol: This PR has been marked as verified by ci.

Details

In response to this:

/verified by ci

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-merge-bot · 2026-08-07T12:17:26Z

/retest-required

Remaining retests: 0 against base HEAD a0ebeb0 and 2 for PR HEAD 3d86b75 in total

oribon · 2026-08-09T06:09:32Z

/retest-required

oribon · 2026-08-09T11:11:07Z

/retest-required

openshift-ci · 2026-08-09T18:11:34Z

@fedepaol: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/5.0-upgrade-from-stable-4.22-e2e-gcp-ovn-upgrade	`3d86b75`	link	false	`/test 5.0-upgrade-from-stable-4.22-e2e-gcp-ovn-upgrade`
ci/prow/e2e-metal-ipi-ovn-ipv6	`3d86b75`	link	true	`/test e2e-metal-ipi-ovn-ipv6`
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw	`3d86b75`	link	true	`/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Aug 4, 2026

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Aug 4, 2026

openshift-ci Bot requested review from arghosh93 and marty-power August 4, 2026 14:45

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2026

coderabbitai Bot requested changes Aug 4, 2026

View reviewed changes

Comment thread bindata/network/frr-k8s/node-status-cleaner.yaml

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Aug 4, 2026

openshift-ci-robot removed the jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. label Aug 6, 2026

openshift-ci-robot added the jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. label Aug 6, 2026

jcaamano reviewed Aug 7, 2026

View reviewed changes

fedepaol force-pushed the frrk8swebhooksno branch from 60ac532 to 3d86b75 Compare August 7, 2026 08:45

openshift-ci Bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2026

openshift-ci Bot assigned jcaamano Aug 7, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Aug 7, 2026

Uh oh!

Conversation

fedepaol commented Aug 4, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 4, 2026

Uh oh!

openshift-ci-robot commented Aug 4, 2026

Uh oh!

coderabbitai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Pre-merge checks failed

❌ Failed checks (4 errors, 2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fedepaol commented Aug 4, 2026

Uh oh!

openshift-ci-robot commented Aug 4, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 5, 2026

Uh oh!

fedepaol commented Aug 6, 2026

Uh oh!

fedepaol commented Aug 6, 2026

Uh oh!

fedepaol commented Aug 6, 2026

Uh oh!

fedepaol commented Aug 6, 2026

Uh oh!

openshift-ci-robot commented Aug 6, 2026

Uh oh!

jcaamano Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

fedepaol Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 7, 2026

Uh oh!

jcaamano commented Aug 7, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 7, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

fedepaol commented Aug 7, 2026

Uh oh!

openshift-ci-robot commented Aug 7, 2026

Uh oh!

fedepaol commented Aug 7, 2026

Uh oh!

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading