Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

OSAC-899: add --windows flag and OS column to CLI compute instances - #756

Merged
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
ygalblum:feat/is-windows-cli
Jun 24, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
ygalblum:feat/is-windows-cli

Conversation

@ygalblum

@ygalblum ygalblum commented Jun 23, 2026 •

Copy link
Copy Markdown
Contributor

Add Windows VM support to the CLI by introducing a --windows flag for compute instance creation and an OS column in table output.

Users need to provision Windows VMs through the same OSAC CLI workflow used for Linux VMs, with the guest OS type visible in listings.

  • Add --windows boolean flag to the create compute-instance command
  • Set IsWindows on the compute instance spec when the flag is provided
  • Apply the flag in both template-based and catalog-item-based creation paths
  • Add an OS column to public and private compute instance table definitions, displaying "windows" or "linux" based on the is_windows spec field

Unit tests added for the --windows flag: flag registration, spec building with the flag enabled and disabled, covering both template and catalog-item creation paths.

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

  • New Features
    • Added --windows flag to create Windows-based compute instances (Linux is the default)
    • Display tables now include an OS column showing the operating system (Windows or Linux) for each compute instance

Add Windows VM support to the CLI by introducing a `--windows` flag for
compute instance creation and an OS column in table output.

Users need to provision Windows VMs through the same OSAC CLI workflow
used for Linux VMs, with the guest OS type visible in listings.

- Add `--windows` boolean flag to the create compute-instance command
- Set `IsWindows` on the compute instance spec when the flag is provided
- Apply the flag in both template-based and catalog-item-based creation paths
- Add an OS column to public and private compute instance table definitions,
  displaying "windows" or "linux" based on the `is_windows` spec field

Unit tests added for the `--windows` flag: flag registration, spec building
with the flag enabled and disabled, covering both template and catalog-item
creation paths.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
@openshift-ci-robot

openshift-ci-robot commented Jun 23, 2026 •

Copy link
Copy Markdown

@ygalblum: This pull request references OSAC-899 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 story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Add Windows VM support to the CLI by introducing a --windows flag for compute instance creation and an OS column in table output.

Users need to provision Windows VMs through the same OSAC CLI workflow used for Linux VMs, with the guest OS type visible in listings.

  • Add --windows boolean flag to the create compute-instance command
  • Set IsWindows on the compute instance spec when the flag is provided
  • Apply the flag in both template-based and catalog-item-based creation paths
  • Add an OS column to public and private compute instance table definitions, displaying "windows" or "linux" based on the is_windows spec field

Unit tests added for the --windows flag: flag registration, spec building with the flag enabled and disabled, covering both template and catalog-item creation paths.

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.

@openshift-ci
openshift-ci Bot requested review from larsks and rgolangh June 23, 2026 20:36
@coderabbitai

coderabbitai Bot commented Jun 23, 2026 •

Copy link
Copy Markdown

Review Change Stack

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: 6dc5b784-8ab2-4f41-bdc5-475d0100d2cb

📥 Commits

Reviewing files that changed from the base of the PR and between dcb9c72 and e6cead2.

📒 Files selected for processing (4)
  • internal/cmd/cli/create/computeinstance/create_compute_instance_cmd.go
  • internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go
  • internal/rendering/tables/osac.private.v1.ComputeInstance.yaml
  • internal/rendering/tables/osac.public.v1.ComputeInstance.yaml

Walkthrough

Adds a --windows boolean CLI flag to the compute instance create command. When set, spec.IsWindows is set to true in both template-based and catalog-item-based spec builders. Tests verify the flag registration and pointer semantics. Both private and public ComputeInstance table renderers gain an OS column derived from is_windows.

Changes

Windows VM support via --windows flag

Layer / File(s) Summary
CLI flag, args struct, and spec construction
internal/cmd/cli/create/computeinstance/create_compute_instance_cmd.go
Registers --windows boolean flag, adds windows bool to runnerContext.args, conditionally sets spec.IsWindows = true in both buildSpec and buildSpecFromCatalogItem, and defines windowsFlagHelp constant.
Unit tests for flag and spec
internal/cmd/cli/create/computeinstance/create_compute_instance_cmd_test.go
Adds buildSpec and buildSpecFromCatalogItem test cases asserting IsWindows is a non-nil true pointer when enabled and nil when disabled; adds a flag registration test confirming --windows defaults to "false".
OS column in table renderers
internal/rendering/tables/osac.private.v1.ComputeInstance.yaml, internal/rendering/tables/osac.public.v1.ComputeInstance.yaml
Adds an OS column to both private and public ComputeInstance table definitions, mapping this.spec.is_windows to 'windows' or 'linux'.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • osac-project/fulfillment-service#734: Adds the is_windows API field and reconciler mapping to GuestOSFamily — this PR's --windows flag and spec.IsWindows propagation builds directly on that foundation.

Suggested reviewers

  • adriengentil
  • rgolangh

Poem

A flag is born, --windows it's called,
No longer must Linux be all that's installed.
IsWindows set true, the pointer's non-nil,
A column says windows — the table's a thrill.
🪟 Gates open wide for VMs, now fulfilled!

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 directly and specifically describes the main changes: adding a --windows flag and OS column to CLI compute instances, matching the core functionality in the changeset.
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, API keys, tokens, passwords, or credentials found in the PR. Test data uses generic identifiers only.
No-Weak-Crypto ✅ Passed No weak cryptography patterns detected. PR adds Windows VM flag and table UI enhancements with no crypto operations, weak algorithms, or insecure comparisons.
No-Injection-Vectors ✅ Passed No injection vectors detected. Windows flag is a boolean parsed safely by cobra; filter uses fmt.Sprintf with %q (quoted) escaping; YAML expressions use hardcoded literal strings ('windows'/'linux'...
Container-Privileges ✅ Passed PR does not modify any K8s manifests or container security configurations; changes are CLI command logic and table rendering definitions only. No privileged container settings introduced.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data logging introduced. The --windows flag is never logged, and the OS column only displays 'windows' or 'linux' literals.
Ai-Attribution ✅ Passed AI tool (Claude Code by Anthropic) use is properly attributed with "Assisted-by: Claude Code noreply@anthropic.com" trailer in commit. No incorrect Co-Authored-By usage for AI tool detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

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

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/retest

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

💀 CI Triage: broken_main | Category: BOOT

Root cause: The main branch is broken due to a database migration number collision (both PR 706 and PR 740 merged with migration number 60).

Explanation: The e2e-vmaas job failed during the boot step's refresh phase because the fulfillment-console-proxy deployment timed out. The proxy depends on fulfillment-grpc-server, which was in a CrashLoopBackOff. Checking the fulfillment-grpc-server pod logs reveals it crashed at startup with 'failed to init driver with path migrations: duplicate migration file: 60_create_external_ip_tables.up.sql'. This happened because two recent PRs (PR 706 and PR 740) were both merged to main on 2026-06-23 with migration files starting with '60_'. The golang-migrate tool cannot handle duplicate migration numbers and crashes. PR 756 is a victim of this broken main branch.

Evidence:

pod-fulfillment-grpc-server-587ff64795-545tw-grpc-server-previous.log:

failed to init driver with path migrations: duplicate migration file: 60_create_external_ip_tables.up.sql

build-log.txt:

ERROR: command failed (exit 1): oc rollout status deploy/fulfillment-console-proxy -n osac-e2e-ci --timeout=360s

Suggestion: Rename one of the migration files on the main branch to '61_...' to resolve the collision. Retesting PR 756 will not help until main is fixed.


Prow job | Build 2069585887552868352 | 🤖 triagent

For deeper investigation, use the /osac-debug-e2e skill with this build ID.

@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand, 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:

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

@jhernand

Copy link
Copy Markdown
Contributor

/retest

@openshift-merge-bot
openshift-merge-bot Bot merged commit a0298ac into osac-project:main Jun 24, 2026
14 checks passed
@ygalblum
ygalblum deleted the feat/is-windows-cli branch June 24, 2026 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants