OSAC-2116: migrate E2E tests from cores/memory_gib to instance_type

[ygalblum]

Replace direct cores/memory_gib parameters with instance_type across all E2E test infrastructure, preparing for the breaking API removal in Phase 5.

• Add default_instance_type to OsacCLI with explicit None-check fallback
• Remove cores/memory_gib params from create_compute_instance (clean break)
• Add session-scoped default_instance_type fixture in vmaas/conftest.py
• Migrate CLI and API field tests to use instance_type path
• Replace cores/memoryGiB immutability tests with instanceType immutability
• Migrate CatalogItem field_definitions from cpu_cores/memory_gb to instance_type

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

• Bug Fixes
  • Compute instance creation now consistently relies on an instance type for sizing, applying a default when none is specified and rejecting unsupported sizing inputs.
  • Updated validation to reflect instance-type–driven defaults, including resource immutability behavior.
  • Added/adjusted checks to ensure instance-type metadata labels are present and non-empty on created instances.
• Tests
  • Refreshed compute instance CLI/API field tests to use shared default sizing values and cleaner assertion flows.

[openshift-ci-robot]

@ygalblum: This pull request references OSAC-2116 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:

Replace direct cores/memory_gib parameters with instance_type across all E2E test infrastructure, preparing for the breaking API removal in Phase 5.

• Add default_instance_type to OsacCLI with explicit None-check fallback
• Remove cores/memory_gib params from create_compute_instance (clean break)
• Add session-scoped default_instance_type fixture in vmaas/conftest.py
• Migrate CLI and API field tests to use instance_type path
• Replace cores/memoryGiB immutability tests with instanceType immutability
• Migrate CatalogItem field_definitions from cpu_cores/memory_gb to instance_type

Assisted-by: Claude Code noreply@anthropic.com

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

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 4dabdaff-d558-4ab3-b6fe-e674cb70535f

📥 Commits

Reviewing files that changed from the base of the PR and between 3c974e0 and fcc76b2.

📒 Files selected for processing (5)

• tests/catalog/test_compute_instance_catalog_item_lifecycle.py
• tests/core/osac_cli.py
• tests/vmaas/conftest.py
• tests/vmaas/test_compute_instance_api_fields.py
• tests/vmaas/test_compute_instance_cli_fields.py

---

Walkthrough

Tests now resolve compute-instance sizing through instance type defaults instead of explicit cores/memoryGiB arguments. A session fixture provisions the default instance type, OsacCLI uses it when creating instances, and the catalog and API field tests were updated to match the instance-type shape.

Changes

Instance-type based sizing migration

Layer / File(s)	Summary
OsacCLI default_instance_type support tests/core/osac_cli.py	Constructor stores an optional default instance type, and create_compute_instance resolves --instance-type from the explicit argument or the stored default, otherwise raising ValueError.
Session fixtures for default instance type tests/vmaas/conftest.py	Adds shared sizing defaults, creates and tears down a session-scoped instance type via private_grpc, and wires the name into cli.default_instance_type for the session.
API fields test uses instance type manifest tests/vmaas/test_compute_instance_api_fields.py	Reorders sizing constants and reformats PATCH calls used in immutability checks without changing the asserted behavior.
CLI fields test validates resolved defaults tests/vmaas/test_compute_instance_cli_fields.py	Uses shared default sizing values, removes explicit sizing arguments, and checks the created instance includes the instance-type-name label.
Catalog field-definitions test simplified to instance_type tests/catalog/test_compute_instance_catalog_item_lifecycle.py	Reduces the field-definition test to a single spec.instance_type field and updates the edit assertions for that field.

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

Possibly related PRs

• osac-project/osac-test-infra#104: Adds the underlying instance-type CLI and gRPC support that this PR’s default-instance-type wiring depends on.

Suggested labels: lgtm

Suggested reviewers: danmanor, trewest

🚥 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 accurately summarizes the main change: E2E tests are being migrated from cores/memory_gib to instance_type.
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	No hardcoded secrets were added; scans found no api_key/secret/token/password literals or embedded-credential URLs in changed files.
No-Weak-Crypto	✅ Passed	No weak crypto, custom crypto, or non-constant-time secret comparisons were added; the PR only changes test fixtures and instance-type plumbing.
No-Injection-Vectors	✅ Passed	No new injection sinks: changed code only builds argv lists for subprocess.run(shell=False) or JSON/gRPC payloads; no eval/pickle/yaml.load/os.system/dangerouslySetInnerHTML.
Container-Privileges	✅ Passed	PASS: The PR only touches Python test/fixture code; no container/K8s manifests or privilege settings (privileged/hostPID/hostNetwork/etc.) are present.
No-Sensitive-Data-In-Logs	✅ Passed	No new logs/prints expose secrets; the only added code catches delete errors without printing stdout/stderr, and existing prints show resource IDs only.
Ai-Attribution	✅ Passed	Latest commit/PR text includes 'Assisted-by: Claude Code'; no Co-Authored-By trailers were found in recent commit messages.

✨ Finishing Touches

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

[coderabbitai]

Actionable comments posted: 3

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

Inline comments:
In `@tests/catalog/test_compute_instance_catalog_item_lifecycle.py`:
- Around line 124-137: The `reduced_fds` step in
`test_compute_instance_catalog_item_lifecycle` is redundant because it matches
the earlier `updated_fds` payload, so the
`grpc.update_compute_instance_catalog_item` call and the following
`get_compute_instance_catalog_item` assertions do not exercise any new behavior.
Either remove this block entirely and keep the existing `updated_fds` checks, or
change `reduced_fds` to a meaningfully different payload in this test by
updating the `fieldDefinitions` data through
`grpc.update_compute_instance_catalog_item` and asserting the distinct result
from `grpc.get_compute_instance_catalog_item`.

In `@tests/vmaas/test_compute_instance_api_fields.py`:
- Around line 89-94: The immutability check in the compute instance API test
uses a compound assertion that makes failures hard to diagnose. In the test
around the k8s_hub_client.patch call for instanceType, split the combined
substring check into separate assertions so it is clear whether the output is
missing "instanceType" or "immutable". Keep the existing rc check and use the
same output variable to preserve the current test flow while improving failure
diagnostics.

In `@tests/vmaas/test_compute_instance_cli_fields.py`:
- Around line 44-49: Split the combined assertion in the test around ci_spec and
it_label so absence of the "osac.openshift.io/instance-type-name" label and an
empty label value fail separately with distinct messages. Update the check near
labels/get("osac.openshift.io/instance-type-name") to use two assertions instead
of one compound condition, preserving the existing test intent while improving
diagnostics.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0a1ea8fe-98e6-42f9-b70f-25b7fef5a890

📥 Commits

Reviewing files that changed from the base of the PR and between 64023b5 and 3c974e0.

📒 Files selected for processing (5)

• tests/catalog/test_compute_instance_catalog_item_lifecycle.py
• tests/core/osac_cli.py
• tests/vmaas/conftest.py
• tests/vmaas/test_compute_instance_api_fields.py
• tests/vmaas/test_compute_instance_cli_fields.py

[openshift-ci]

@ygalblum: The following test 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/e2e-vmaas	3c974e0	link	true	/test e2e-vmaas

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DakCrowder, ygalblum

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

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