OSAC-3985: requeue instead of firing CaaS storage job when no tier definitions registered

[zszabo-rh]

Summary

• On fresh install, osac-create-tenant-cluster-storage fired before any StorageTiers were registered, producing a predictable AAP job failure (storage_tier_definitions must be non-empty). Self-healed on retry but generated noise.
• Add a 4-line guard in handleUpdate(): if TiersClient is configured but tier definitions are empty, requeue with StatusPollInterval instead of firing the job.
• New test case in Stage 2 context: verifies no job is fired and RequeueAfter = pollInterval when TiersClient returns empty tier list.

Fixes: OSAC-3985

Test plan

• make test — 633 passed, 0 failed
• make lint — 0 issues
• Observed on fresh install: no failed AAP job before StorageTiers register

Breaking changes

• This PR does not introduce breaking changes

Summary by CodeRabbit

• Bug Fixes
  • Improved storage provisioning when tier definitions are not yet available.
  • The system now retries after the configured polling interval instead of proceeding prematurely.
  • Prevents cluster-storage jobs from being created until tier information is ready.
  • Handles tier-resolution service failures more reliably.

[openshift-ci-robot]

@zszabo-rh: This pull request references OSAC-3985 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 task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• On fresh install, osac-create-tenant-cluster-storage fired before any StorageTiers were registered, producing a predictable AAP job failure (storage_tier_definitions must be non-empty). Self-healed on retry but generated noise.
• Add a 4-line guard in handleUpdate(): if TiersClient is configured but tier definitions are empty, requeue with StatusPollInterval instead of firing the job.
• New test case in Stage 2 context: verifies no job is fired and RequeueAfter = pollInterval when TiersClient returns empty tier list.

Fixes: OSAC-3985

Test plan

• make test — 633 passed, 0 failed
• make lint — 0 issues
• Observed on fresh install: no failed AAP job before StorageTiers register

Breaking changes

• This PR does not introduce breaking changes

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.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:03 PM UTC · Completed 1:18 PM UTC

Commit: 72dfc2d · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [error-handling] osac-operator/internal/controller/storage_controller.go:361 — The guard len(tierDefinitions) == 0 does not distinguish between "no tiers registered yet" (empty API response) and "tier resolution API failed" (error swallowed by resolveAndInjectTierContext, which sets tierDefinitions = nil). Both cases requeue, which is the correct safe default — but the log message ("no storage tier definitions registered yet, requeueing") is misleading when the actual cause is an API failure. The error is already logged at ERROR level upstream in resolveAndInjectTierContext, so operators monitoring the full log stream will see both entries, but those monitoring only the storage controller's info lines could be misled about the root cause.
  Remediation: Consider amending the log message or propagating a flag from resolveAndInjectTierContext to distinguish the two cases (e.g., a separate log line or condition reason when tier resolution failed vs. when no tiers are registered).

Previous run

Review

Findings

Low

• [edge-case] osac-operator/internal/controller/storage_controller.go:359 — The guard r.TiersClient != nil && len(tierDefinitions) == 0 cannot distinguish between "zero tier definitions registered" (the intended trigger) and "tier API call failed" (a transient error), since resolveAndInjectTierContext returns nil in both cases and len(nil) == 0. However, the practical impact is negligible: without tier data, the downstream AAP job would fail anyway, so requeuing cleanly is arguably better behavior in both cases. If the team wants to distinguish these paths in the future, have resolveTierDefinitions return []provisioning.TierDefinition{} (empty non-nil) on success with zero tiers.

• [logging-conventions] osac-operator/internal/controller/storage_controller.go:361 — The structured log key requeueAfter is not used by any other requeueing log message in the controller package. Other controllers log only identifying context (resource name, parent reference) without the requeue interval.
  Remediation: Consider dropping the requeueAfter key to match the established pattern: log.Info("no storage tier definitions registered yet, requeueing", "tenant", tenantName).

---

Labels: PR fixes a storage controller bug - adding component and type labels

[akshaynadkarni]

Left a couple of nits.
Overall lgtm.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni, zszabo-rh

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:

• osac-operator/OWNERS [akshaynadkarni,zszabo-rh]

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

[openshift-ci]

New changes are detected. LGTM label has been removed.

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 11:04 AM UTC · Ended 11:18 AM UTC

Commit: 009ec15 · View workflow run →

[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: CHILL

Plan: Pro Plus

Run ID: 5089b4a7-ab72-4ca2-ac03-ed668e3ff6c9

📥 Commits

Reviewing files that changed from the base of the PR and between 1bc8491 and 009ec15.

📒 Files selected for processing (2)

• osac-operator/internal/controller/storage_controller.go
• osac-operator/internal/controller/storage_controller_test.go

---

Walkthrough

The storage reconciler now requeues when configured fulfillment clients have no tier definitions. Tests cover empty tier definitions and tier-resolution failures without creating cluster-storage provisioning jobs.

Changes

Storage tier retry handling

Layer / File(s)	Summary
Tier resolution requeue and validation osac-operator/internal/controller/storage_controller.go, osac-operator/internal/controller/storage_controller_test.go	handleUpdate requeues at StatusPollInterval when tier definitions are unavailable. Tests verify that tier-resolution failures and empty definitions do not create cluster-storage jobs.

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

Mergeability Score: ⚪ Minimal · up to 009ec

The change only delays storage job creation until tier definitions are available and adds focused test coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: eranco74, larsks, danmanor

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: requeueing instead of starting the CaaS storage job when no tier definitions exist.
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.
No-Hardcoded-Secrets	✅ Passed	The PR adds no API keys, tokens, private keys, credential URLs, or secret-shaped blobs; test fixtures use non-secret values and are covered by the unit-test exception.
No-Weak-Crypto	✅ Passed	Pull request introduces no weak cryptography, custom crypto implementations, or insecure secret comparisons. Changes are purely operational (requeue logic) with no cryptographic code paths.
No-Injection-Vectors	✅ Passed	PR introduces a requeue guard for empty tier definitions with structured logging only; no SQL concatenation, shell=True, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInner...
Container-Privileges	✅ Passed	PR modifies only Go controller source code and tests. No container/K8s manifests with privilege settings (privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation) were...
No-Sensitive-Data-In-Logs	✅ Passed	The only added log field is the Tenant resource name, already logged by this controller; the PR adds no password, token, API key, PII, session ID, hostname, or customer payload.
Ai-Attribution	✅ Passed	Both commits in the OSAC-3985 PR properly attribute AI assistance using the correct Assisted-by: Claude Code <noreply@anthropic.com> trailer. No Co-Authored-By misuse detected.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:04 AM UTC · Completed 11:18 AM UTC

Commit: 009ec15 · View workflow run →

[zszabo-rh]

/retest

[github-actions]

Re-triggered failed runs:

• label-gate (#31693850814)
• Unit Tests (#31693848425)

[zszabo-rh]

/retest

[github-actions]

Re-triggered failed runs:

• label-gate (#31693850814)
• E2E CaaS Full Install (#31693848875)
• Unit Tests (#31693848425)
• E2E VMaaS Full Install (#31693848734)

[zszabo-rh]

/retest

[github-actions]

Re-triggered failed runs:

• label-gate (#31693850814)
• E2E CaaS Full Install (#31693848875)
• E2E VMaaS Full Install (#31693848734)