NO-ISSUE: fix Volume reconciler test referencing removed VolumeSpec.PVCRef by slintes · Pull Request #367 · osac-project/osac

slintes · 2026-08-18T15:27:28Z

What

Removes an orphaned test assertion that broke the build on main:

fulfillment-service/internal/controllers/volume/volume_reconciler_function_test.go:103:15:
  spec.PVCRef undefined (type v1alpha1.VolumeSpec has no field or method PVCRef)

Why — cross-package semantic merge conflict

Two PRs merged back-to-back with a conflict that neither CI could catch:

OSAC-3274: remove pvcRef and pvRef from Volume #341 (OSAC-3274) removed pvcRef/pvRef from VolumeSpec in osac-operator/api/v1alpha1.
OSAC-3276: add Volume reconciler to fulfillment-service #339 (OSAC-3276) added a Volume reconciler test in fulfillment-service asserting spec.PVCRef.

PR 339 was branched before PR 341 merged, so it compiled against a VolumeSpec that still had PVCRef and passed its own CI. After both landed, the combined tree no longer compiles. git merges both textually clean because they touch different files — the breakage only surfaces when the test package is built.

Fix

Drop the stale Expect(spec.PVCRef).To(BeNil()) assertion. buildSpec() never populated PVCRef and nothing else in the repo references it, so removal is the correct resolution (not restoring the field).

Verification

go vet ./internal/controllers/volume/ — clean
go test ./internal/controllers/volume/ — pass

FYI

Original author of both PRs: @akshaynadkarni
LGTM on OSAC-3274: remove pvcRef and pvRef from Volume #341: @zszabo-rh · LGTM on OSAC-3276: add Volume reconciler to fulfillment-service #339: @rgolangh

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Updated volume reconciliation test expectations by removing an outdated nil assertion.

…VCRef Cross-package semantic merge conflict between two PRs that merged back-to-back: - osac-project#341 (OSAC-3274) removed pvcRef/pvRef from VolumeSpec in osac-operator/api/v1alpha1. - osac-project#339 (OSAC-3276) added a Volume reconciler test in fulfillment-service asserting spec.PVCRef. PR osac-project#339 was branched before osac-project#341 merged, so it compiled against a VolumeSpec that still had PVCRef and passed its own CI. After both landed on main, the combined tree fails to compile: volume_reconciler_function_test.go:103:15: spec.PVCRef undefined (type v1alpha1.VolumeSpec has no field or method PVCRef) git merges both textually clean because neither touches the same lines; the breakage is only visible when the test package is built. Fix: drop the orphaned `Expect(spec.PVCRef).To(BeNil())` assertion. buildSpec() never populated PVCRef and no other code references it, so removal is the correct resolution (not restoring the field). Verified: go vet + go test ./internal/controllers/volume/ pass. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Marc Sluiter <msluiter@redhat.com>

openshift-ci-robot · 2026-08-18T15:27:32Z

@slintes: This pull request explicitly references no jira issue.

Details

In response to this:

What

Removes an orphaned test assertion that broke the build on main:
fulfillment-service/internal/controllers/volume/volume_reconciler_function_test.go:103:15:
 spec.PVCRef undefined (type v1alpha1.VolumeSpec has no field or method PVCRef)
Why — cross-package semantic merge conflict

Two PRs merged back-to-back with a conflict that neither CI could catch:

OSAC-3274: remove pvcRef and pvRef from Volume #341 (OSAC-3274) removed pvcRef/pvRef from VolumeSpec in osac-operator/api/v1alpha1.

OSAC-3276: add Volume reconciler to fulfillment-service #339 (OSAC-3276) added a Volume reconciler test in fulfillment-service asserting spec.PVCRef.

PR 339 was branched before PR 341 merged, so it compiled against a VolumeSpec that still had PVCRef and passed its own CI. After both landed, the combined tree no longer compiles. git merges both textually clean because they touch different files — the breakage only surfaces when the test package is built.

Fix

Drop the stale Expect(spec.PVCRef).To(BeNil()) assertion. buildSpec() never populated PVCRef and nothing else in the repo references it, so removal is the correct resolution (not restoring the field).

Verification

go vet ./internal/controllers/volume/ — clean

go test ./internal/controllers/volume/ — pass

FYI

Original author of both PRs: @akshaynadkarni

LGTM on OSAC-3274: remove pvcRef and pvRef from Volume #341: @zszabo-rh · LGTM on OSAC-3276: add Volume reconciler to fulfillment-service #339: @rgolangh

🤖 Generated with Claude Code

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-18T15:28:24Z

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: 7b6ee99f-4409-47e1-9c54-2d245d8ed10b

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce6896 and 49c2574.

📒 Files selected for processing (1)

fulfillment-service/internal/controllers/volume/volume_reconciler_function_test.go

💤 Files with no reviewable changes (1)

fulfillment-service/internal/controllers/volume/volume_reconciler_function_test.go

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

Walkthrough

The change removes the PVCRef nil assertion from the volume reconciler buildSpec test.

Changes

Cohort / File(s)	Summary
Test cleanup `fulfillment-service/internal/controllers/volume/volume_reconciler_function_test.go`	Removes the `PVCRef` nil assertion from the `buildSpec` test.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 49c25

This localized test-only change removes an assertion for a field that no longer exists, with the affected package passing verification. No actionable merge-blocking risk remains.

🚥 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 removed stale PVCRef assertion in the Volume reconciler test.
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 commit adds no lines and only removes one test assertion; it introduces no hardcoded secret, credential, token, password, or key.
No-Weak-Crypto	✅ Passed	The diff only removes a stale PVCRef test assertion; it adds no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret-comparison code.
No-Injection-Vectors	✅ Passed	The commit changes one test file by deleting only the stale PVCRef assertion; no listed injection vector or user-input handling is introduced.
Container-Privileges	✅ Passed	The diff changes only one Go test assertion and adds no container or Kubernetes manifest privilege settings.
No-Sensitive-Data-In-Logs	✅ Passed	The PR changes only one test assertion and adds no logging or sensitive-data output; the diff introduces no passwords, tokens, PII, hostnames, or customer data.
Ai-Attribution	✅ Passed	The PR and commit mention Claude Code; the changed commit includes an Assisted-by trailer and no Co-Authored-By trailer for AI.

✨ 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

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

fullsend-ai-review · 2026-08-18T15:28:43Z

🤖 Finished Review · ✅ Success · Started 3:28 PM UTC · Completed 3:34 PM UTC

Commit: 49c2574 · View workflow run →

akshaynadkarni

lgtm

openshift-ci · 2026-08-18T15:29:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni, slintes

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:

~~fulfillment-service/OWNERS~~ [akshaynadkarni]

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

fullsend-ai-review · 2026-08-18T15:34:46Z

Looks good to me

Labels: PR fixes a volume reconciler test — storage domain

osac-project#410, osac-project#417 (osac-project#645) At time of publishing this PR ( Aug 31 2026 ) Whats present in the osac-test-infra but missing from the osac mono repo are - PR#410: [OSAC-4272](https://redhat.atlassian.net/browse/OSAC-4272): register k8s_only NetworkClass manager for E2E - osac-project#410 - PR#417 : Storage tier protocol refactor (grpc_client.py) - PR#367: Per-disk storage tier tests (8 files) --------- Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com> Signed-off-by: Ori Amizur <oamizur@redhat.com> Co-authored-by: Carlo Lobrano <c.lobrano@gmail.com> Co-authored-by: Ori Amizur <60868946+ori-amizur@users.noreply.github.com> Co-authored-by: Will Gordon <wgordon@redhat.com>

…osac-project#670) Ported from osac-project/osac-test-infra#423 After OSAC-3632, AAP will resolve a StorageClass per disk from the disk's storageTier field. When a disk has no tier, provisioning will fail. The [OSAC-3633](https://redhat.atlassian.net/browse/OSAC-3633) PR (osac-project#367) added the storage tier pattern but only in its new tests; pre-existing VM-provisioning tests still create tier-less instances. Changes: - Add boot_disk_storage_tier parameter to GRPCClient.create_compute_instance and GRPCClient.create_compute_instance_with_disk_image helpers - Move default_storage_tier fixture from tests/vmaas/conftest.py to root tests/conftest.py so all suites (catalog, references) can use it - Update all gRPC-based ComputeInstance creation calls across vmaas, catalog, and references suites to set boot_disk_storage_tier - Update _ci_create_data helper and inline gRPC data in references tests to include boot_disk with storage_tier Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com>

openshift-ci-robot added the jira/valid-reference label Aug 18, 2026

openshift-ci Bot requested review from eranco74 and larsks August 18, 2026 15:27

slintes temporarily deployed to e2e-test August 18, 2026 15:27 — with GitHub Actions Inactive

omer-vishlitzky enabled auto-merge August 18, 2026 15:27

coderabbitai Bot approved these changes Aug 18, 2026

View reviewed changes

akshaynadkarni approved these changes Aug 18, 2026

View reviewed changes

openshift-ci Bot assigned akshaynadkarni Aug 18, 2026

openshift-ci Bot added the lgtm label Aug 18, 2026

openshift-ci Bot added the approved label Aug 18, 2026

fullsend-ai-review Bot approved these changes Aug 18, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge storage labels Aug 18, 2026

omer-vishlitzky added this pull request to the merge queue Aug 18, 2026

slintes mentioned this pull request Aug 18, 2026

[AI] OSAC-4127: List DAO tenancy filter bypassed by top-level OR in user filter (cross-tenant resolution by name) #368

Merged

6 tasks

Merged via the queue into osac-project:main with commit 84db065 Aug 18, 2026
143 of 146 checks passed

slintes deleted the fix/main branch August 18, 2026 17:55

fullsend-ai-review Bot mentioned this pull request Aug 31, 2026

OSAC-3593: Sync test changes from osac-test-infra PRs #367, #410, #417 #645

Merged

amej mentioned this pull request Aug 31, 2026

OSAC-3593: Specifies the path of the component unique_name post migration of e2e tests #659

Merged

clobrano mentioned this pull request Sep 1, 2026

OSAC-4575: set explicit storageTier on every ComputeInstance in tests #670

Merged

amej mentioned this pull request Sep 2, 2026

[AI] OSAC-3593: Move e2e test suite (tests/) from osac-test-infra into the osac mono-repo #409

Closed

6 tasks

Navigation Menu

Uh oh!

NO-ISSUE: fix Volume reconciler test referencing removed VolumeSpec.PVCRef - #367

NO-ISSUE: fix Volume reconciler test referencing removed VolumeSpec.PVCRef#367
omer-vishlitzky merged 1 commit into
osac-project:mainfrom
slintes:fix/main

slintes commented Aug 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

openshift-ci-robot commented Aug 18, 2026 •

edited by openshift-ci Bot

Loading

What

Why — cross-package semantic merge conflict

Fix

Verification

FYI

Uh oh!

coderabbitai Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

akshaynadkarni left a comment

Uh oh!

openshift-ci Bot commented Aug 18, 2026

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

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

Uh oh!

What

Why — cross-package semantic merge conflict

Fix

Verification

FYI

Summary by CodeRabbit

Uh oh!

openshift-ci-robot commented Aug 18, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why — cross-package semantic merge conflict

Fix

Verification

FYI

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akshaynadkarni left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Aug 18, 2026

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

slintes commented Aug 18, 2026 •

edited by coderabbitai Bot

Loading

openshift-ci-robot commented Aug 18, 2026 •

edited by openshift-ci Bot

Loading

coderabbitai Bot commented Aug 18, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 18, 2026 •

edited

Loading