Skip to content

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

Merged
osac-ci-bot merged 1 commit into
osac-project:mainfrom
clobrano:feat/OSAC-4575-set-storage-tier
Sep 1, 2026
Merged

OSAC-4575: set explicit storageTier on every ComputeInstance in tests#670
osac-ci-bot merged 1 commit into
osac-project:mainfrom
clobrano:feat/OSAC-4575-set-storage-tier

Conversation

@clobrano

@clobrano clobrano commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

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 PR (#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

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

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Carlo Lobrano <c.lobrano@gmail.com>
@openshift-ci-robot

openshift-ci-robot commented Sep 1, 2026

Copy link
Copy Markdown

@clobrano: This pull request references OSAC-4575 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.1.0" version, but no target version was set.

Details

In response to this:

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 PR (#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

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

openshift-ci Bot commented Sep 1, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clobrano

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:

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

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

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: Enterprise

Run ID: 46bcfb1e-a6f3-4b10-8670-386281f8c387

📥 Commits

Reviewing files that changed from the base of the PR and between afdf1b1 and 0dff1ed.

📒 Files selected for processing (7)
  • tests/conftest.py
  • tests/core/grpc_client.py
  • tests/e2e/catalog/test_compute_instance_catalog_item_disk_image.py
  • tests/e2e/catalog/test_compute_instance_catalog_item_lifecycle.py
  • tests/e2e/references/test_compute_references.py
  • tests/e2e/vmaas/conftest.py
  • tests/e2e/vmaas/test_compute_instance_disk_image.py
💤 Files with no reviewable changes (1)
  • tests/e2e/vmaas/conftest.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

Changes

The test suite now reads a default storage tier from OSAC_STORAGE_TIER and passes it to compute instance creation requests across catalog, reference, and VMAAS disk-image scenarios.

Compute storage tier coverage

Layer / File(s) Summary
Storage-tier fixture and request helpers
tests/conftest.py, tests/core/grpc_client.py
Adds the shared storage-tier fixture. Updates both compute instance creation helpers to include boot_disk.storage_tier when provided.
Catalog and reference creation coverage
tests/e2e/catalog/*, tests/e2e/references/test_compute_references.py
Passes the default storage tier through catalog-item, raw request, and compute-reference creation tests. The catalog disk-image test also allows boot_disk.storage_tier in its field definitions.
VMAAS disk-image coverage
tests/e2e/vmaas/conftest.py, tests/e2e/vmaas/test_compute_instance_disk_image.py
Uses the shared fixture for disk-image creation, obsolete-image rejection, deprecated-image creation, and deletion-protection tests. Removes the duplicate VMAAS fixture.

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

Merge Risk: ⚪ Minimal · up to 0dff1

This change updates test setup so ComputeInstance fixtures provide the required storage tier; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: danielerez, wgordon17, amej

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (10 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.
No-Hardcoded-Secrets ✅ Passed PASS. The pull-request diff adds storage-tier plumbing and the OSAC_STORAGE_TIER fallback value "local". It adds no API key, token, password, private-key material, credential-bearing URL, or encod…
No-Weak-Crypto ✅ Passed PASS: The pull request changes only pytest fixtures, gRPC request construction, and ComputeInstance test payloads. No added line uses MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or …
No-Injection-Vectors ✅ Passed PASS: The pull request adds storage-tier values to JSON gRPC payloads and to argument lists passed through tests.core.runner.run. run uses subprocess.run(args, ...) without shell=True. The cha…
Container-Privileges ✅ Passed PASS — The pull request changes only Python test helpers, fixtures, and e2e tests. The diff adds no container/Kubernetes manifests and no added lines containing privileged, hostPID, hostNetwork,…
No-Sensitive-Data-In-Logs ✅ Passed PASS: The patch adds no logging, printing, exception output, or log configuration. Added code only reads OSAC_STORAGE_TIER and adds boot_disk.storage_tier to test request data. Existing bearer-token h…
Ai-Attribution ✅ Passed The pull-request commit 0dff1ed08398a248eaa15eff7aceae3ccf8743fa includes Assisted-by: Claude Code <noreply@anthropic.com>. It does not include a Co-Authored-By trailer for an AI tool.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: tests now set an explicit storage tier on every ComputeInstance.
Full details: No-Hardcoded-Secrets

Explanation

PASS. The pull-request diff adds storage-tier plumbing and the OSAC_STORAGE_TIER fallback value "local". It adds no API key, token, password, private-key material, credential-bearing URL, or encoded secret blob. Credential-related code already present in tests/conftest.py is unchanged and is not caused by this pull request.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request changes only pytest fixtures, gRPC request construction, and ComputeInstance test payloads. No added line uses MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparisons. Repository-wide weak-crypto matches are pre-existing and outside the changed paths. The existing grpcurl -insecure argument is unrelated to the explicitly listed algorithm and comparison conditions.

Full details: No-Injection-Vectors

Explanation

PASS: The pull request adds storage-tier values to JSON gRPC payloads and to argument lists passed through tests.core.runner.run. run uses subprocess.run(args, ...) without shell=True. The changed files contain no SQL concatenation, eval, exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInnerHTML. The environment-derived OSAC_STORAGE_TIER value is serialized with json.dumps or passed as one subprocess argument, so it does not introduce a listed injection vector.

Full details: Container-Privileges

Explanation

PASS — The pull request changes only Python test helpers, fixtures, and e2e tests. The diff adds no container/Kubernetes manifests and no added lines containing privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, root-user settings, or security capabilities. The changes only add storage_tier request fields and move a pytest fixture.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The patch adds no logging, printing, exception output, or log configuration. Added code only reads OSAC_STORAGE_TIER and adds boot_disk.storage_tier to test request data. Existing bearer-token handling and cleanup warnings are unchanged, and the new storage-tier value is not sensitive data under this check.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

E2E on approval (fork)

coderabbitai[bot] approved 0dff1ed on fork PR #670 — starting expensive e2e (PR run replay).

  • Started: 0/3
  • Already active/green (skipped rerun): 3

@udis

udis commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 1, 2026
@osac-ci-bot
osac-ci-bot added this pull request to the merge queue Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

E2E on lgtm

Label lgtm applied — starting expensive e2e (PR run replay).

  • Started: 0/3
  • Already active/green (skipped rerun): 3

Merged via the queue into osac-project:main with commit e745fa6 Sep 1, 2026
96 of 100 checks passed
@clobrano
clobrano deleted the feat/OSAC-4575-set-storage-tier branch September 1, 2026 12:30
obochan-rh pushed a commit to obochan-rh/osac that referenced this pull request Sep 9, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants