OSAC-1585: Fix ClusterOrder controller writing observed NodeRequest state to .spec instead of .status

[eliorerz]

Rebase of #343 onto current main.

#343's branch (from the osac-dev-bot fork, osac-jira-ai-issue-solver/OSAC-1585) was 447 commits behind main — old enough to predate charts/operator-crds entirely. That's why its osac-installer E2E VMaaS run failed with Error: directory base/osac-operator/charts/operator-crds not found: the E2E workflow copies that chart directory from the component branch's checkout, and it simply didn't exist yet at that branch's base commit.

I don't have push access to osac-dev-bot's fork (and "allow edits from maintainers" is off on #343), so I couldn't update that PR's branch in place. This PR carries the identical 2-file diff, rebased cleanly with no conflicts.

Closes #343.

Summary by CodeRabbit

• Bug Fixes
  • Corrected synchronization of node request replica counts during node pool reconciliation.
  • Ensured updates are stored in status without modifying the requested configuration.
  • Added handling for missing status entries and cases where values are already up to date.

[openshift-ci-robot]

@eliorerz: This pull request references OSAC-1585 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Rebase of #343 onto current main.

#343's branch (from the osac-dev-bot fork, osac-jira-ai-issue-solver/OSAC-1585) was 447 commits behind main — old enough to predate charts/operator-crds entirely. That's why its osac-installer E2E VMaaS run failed with Error: directory base/osac-operator/charts/operator-crds not found: the E2E workflow copies that chart directory from the component branch's checkout, and it simply didn't exist yet at that branch's base commit.

I don't have push access to osac-dev-bot's fork (and "allow edits from maintainers" is off on #343), so I couldn't update that PR's branch in place. This PR carries the identical 2-file diff, rebased cleanly with no conflicts.

Closes #343.

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 APPROVED

This pull-request has been approved by: eliorerz

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 [eliorerz]

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: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d7f0f9f2-51a2-49d0-9133-db54c31a6e2f

📥 Commits

Reviewing files that changed from the base of the PR and between 7a053d7 and d65353a.

📒 Files selected for processing (2)

• internal/controller/clusterorder_controller.go
• internal/controller/clusterorder_controller_test.go

---

Walkthrough

The ClusterOrder controller now writes observed NodePool replica counts to status.nodeRequests instead of spec.nodeRequests. Tests cover creation, updates, no-op cases, and preservation of the desired specification.

Changes

NodeRequest status reconciliation

Layer / File(s)	Summary
Persist observed NodeRequest state internal/controller/clusterorder_controller.go, internal/controller/clusterorder_controller_test.go	handleNodePool creates or updates status entries from NodePool replica counts, while tests verify edge cases and ensure spec.NodeRequests remains unchanged.

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

Suggested reviewers: akshaynadkarni, clobrano, zszabo-rh

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Ai-Attribution	⚠️ Warning	The PR’s sole commit is from osac-jira-ai-issue-solver[bot] and uses Co-authored-by, while AGENTS.md պահանջs Assisted-by/Generated-by for AI use.	Replace the AI-related Co-Authored-By trailer with an Assisted-by or Generated-by trailer per AGENTS.md on the PR commit.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: moving observed NodeRequest state writes from spec to status.
Linked Issues check	✅ Passed	The diff matches the linked issue by updating handleNodePool to write to status and adding five targeted unit tests.
Out of Scope Changes check	✅ Passed	The changes stay within scope, limited to the controller fix and its corresponding tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
No-Hardcoded-Secrets	✅ Passed	No hardcoded secrets or credential-shaped literals appear in the changed controller/test files; scans found only ordinary test data and messages.
No-Weak-Crypto	✅ Passed	The diff only moves NodeRequest updates from spec to status and adds tests; no weak crypto, custom crypto, or insecure secret comparisons found.
No-Injection-Vectors	✅ Passed	Changed code only moves NodeRequest state to status; no SQL/shell/eval/yaml/exec/dangerous DOM patterns appear in the touched files.
Container-Privileges	✅ Passed	Changed files only adjust controller status logic and tests; no privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation settings appear.
No-Sensitive-Data-In-Logs	✅ Passed	No new logs expose secrets/PII; only resource names, namespaces, resource classes, and replica counts are logged.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rebase-pr-343-OSAC-1585

---

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

[eliorerz]

/test?

[openshift-ci]

@eliorerz: No presubmit jobs available for osac-project/osac-operator@main

Details

In response to this:

/test?

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.

[github-actions]

Slash commands

Commands match workflow file names by prefix.

Command	Workflow
/test pre-commit	pre-commit
/test build-image	Build container image
/test check-pull-request	check-generated-code
/test e2e-vmaas-full-install	E2E VMaaS Full Install
/test integration-tests	Integration Tests
/test e2e-bmaas-full-install	E2E BMaaS Full Install
/test all	Re-run all PR workflows
/test ? or /test help	Show this help
/retest	Re-run all failed checks
/cancel [name]	Cancel in-progress runs
/ok-to-test	Approve fork PR for E2E testing

Tip: /test e2e runs all e2e-* workflows.