OCPBUGS-98617: Fixes daemonset Progressing=True logic

[theobarberbany]

Updates our daemonset status watching logic to not flip Progressing=True when scaling up nodes.

Summary by CodeRabbit

• Bug Fixes
  • Improved DaemonSet rollout completion detection by using the currently scheduled pod count rather than the desired count.
  • Rollouts are now considered complete when all currently scheduled DaemonSet pods are updated, even during node scale-up.
  • Prevented DaemonSet rollouts from being marked complete on initial runs before any pods are scheduled.
  • Expanded and refined rollout-completion test coverage for updated/current scheduling scenarios.

[openshift-ci-robot]

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

Updates our daemonset status watching logic to not flip Progressing=True when scaling up nodes.

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 653ce25b-ff7c-409a-a23d-e566f8f22f1e

📥 Commits

Reviewing files that changed from the base of the PR and between 781b669 and 08a614d.

📒 Files selected for processing (2)

• pkg/controllers/clusteroperator_controller.go
• pkg/controllers/clusteroperator_controller_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• pkg/controllers/clusteroperator_controller.go
• pkg/controllers/clusteroperator_controller_test.go

---

Walkthrough

The DaemonSet rollout completion predicate now uses observed generation and currently scheduled pods. Tests cover incomplete updates, generation lag, node reboot, misscheduled pods, node scale-up, and initial deployment with no scheduled pods.

Changes

DaemonSet rollout tracking

Layer / File(s)	Summary
Update rollout completion predicate pkg/controllers/clusteroperator_controller.go	Rollout completion now requires an observed generation, at least one scheduled pod, and updated pods covering all currently scheduled pods.
Expand rollout status scenarios pkg/controllers/clusteroperator_controller_test.go	Tests cover incomplete updates, generation lag, node reboot, misscheduled pods, node scale-up, and initial deployment with no scheduled pods.

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

Suggested reviewers: nrb, racheljpg

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing DaemonSet Progressing=True logic during rollout and scale-up scenarios.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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	All changed Ginkgo titles are static strings; no generated names, timestamps, IDs, or formatted values were found.
Test Structure And Quality	✅ Passed	The new DaemonSet rollout tests are self-contained unit cases: one behavior per It, no cluster resources or waits, and cleanup/setup isn’t needed.
Microshift Test Compatibility	✅ Passed	PASS: The added Ginkgo specs are unit tests under pkg/controllers, not e2e, and they only use appsv1 DaemonSet/core APIs with no MicroShift-unsupported resources.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The added Ginkgo specs are unit tests against synthetic DaemonSet status with fake clients, not e2e cluster tests; they make no multi-node/SNO assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	Only DaemonSet rollout-status logic changed; no node selectors, affinity, spread constraints, replica math, or topology assumptions were introduced.
Ote Binary Stdout Contract	✅ Passed	PASS: The PR only changes DaemonSet rollout logic/tests; no stdout-writing calls were added in process-level code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The added tests are unit-level DaemonSet status checks in pkg/controllers and only use in-memory status fields; no IPv4 literals or external connectivity.
No-Weak-Crypto	✅ Passed	No weak crypto APIs, custom crypto, or secret comparisons were added; changes are limited to DaemonSet rollout logic and tests.
Container-Privileges	✅ Passed	PR only changes Go controller/test logic; no container or Kubernetes manifests were modified, and no privileged settings were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	PASS: The PR adds no sensitive logging; the changed controller code only logs generic DaemonSet/Deployment names and errors, and the tests add no logs.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[theobarberbany]

/pipeline auto

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign theobarberbany 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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/controllers/clusteroperator_controller_test.go (1)

1313-1409: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Struct literals not gofmt-aligned.

golangci-lint reports a goimports formatting violation starting at Line 1314. Checking the field-value alignment in these new DaemonSetStatus{} literals (e.g. ObservedGeneration: 1, vs DesiredNumberScheduled: 3,), the value columns are inconsistent with what gofmt/goimports would produce. Run gofmt -w/goimports -w on this file.

🤖 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 `@pkg/controllers/clusteroperator_controller_test.go` around lines 1313 - 1409,
Run gofmt or goimports on the test file to align the field-value columns in the
DaemonSetStatus struct literals within the isDaemonSetRolloutComplete tests.
Preserve all test cases and logic while applying only the formatter’s required
alignment changes.

Source: Linters/SAST tools

🤖 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.

Outside diff comments:
In `@pkg/controllers/clusteroperator_controller_test.go`:
- Around line 1313-1409: Run gofmt or goimports on the test file to align the
field-value columns in the DaemonSetStatus struct literals within the
isDaemonSetRolloutComplete tests. Preserve all test cases and logic while
applying only the formatter’s required alignment changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 25a825a6-ba87-41a6-b778-cd36f4c90164

📥 Commits

Reviewing files that changed from the base of the PR and between a1fd447 and 781b669.

📒 Files selected for processing (2)

• pkg/controllers/clusteroperator_controller.go
• pkg/controllers/clusteroperator_controller_test.go

[openshift-ci]

@theobarberbany: 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/lint	781b669	link	true	/test lint
ci/prow/fmt	781b669	link	true	/test fmt

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]

@theobarberbany: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[openshift-ci]

@theobarberbany: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[theobarberbany]

/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial

[openshift-ci]

@theobarberbany: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

• periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-1of2
• periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-2of2

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/785b2780-8043-11f1-813f-8c33d30b0fd4-0

[theobarberbany]

/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial

[openshift-ci]

@theobarberbany: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

• periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-1of2
• periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-2of2

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/277f22b0-806d-11f1-9d9b-a9422b81550c-0

[theobarberbany]

/retest