Skip to content

CORS-4536: Auto-upload RHCOS image for sovereign clouds#10686

Open
barbacbd wants to merge 1 commit into
openshift:mainfrom
barbacbd:CORS-4536
Open

CORS-4536: Auto-upload RHCOS image for sovereign clouds#10686
barbacbd wants to merge 1 commit into
openshift:mainfrom
barbacbd:CORS-4536

Conversation

@barbacbd

@barbacbd barbacbd commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

In GCP sovereign clouds (non-default universe domain), the pre-built
RHCOS images hosted in the public rhcos-cloud project are not
accessible. Previously, users were required to manually download,
upload, and configure a custom RHCOS image via the osImage field in
the install config.

This change automates the process by mirroring the existing AWS AMI
copy pattern:

1. Image resolution (pkg/asset/rhcos/image.go): When a non-default
   universe domain is detected via the GCP credentials, the RHCOS
   image asset returns the tar.gz download URL from the coreos stream
   metadata instead of a rhcos-cloud project reference.

2. Machine generation (pkg/asset/machines/clusterapi.go): When the
   resolved image is a URL rather than a projects/ reference, the
   GCPMachine manifests are generated with an empty image field,
   serving as a sentinel for PreProvision.

3. PreProvision upload (pkg/infrastructure/gcp/clusterapi/image.go):
   When empty GCPMachine images are detected, PreProvision downloads
   the RHCOS tar.gz, creates a temporary GCS staging bucket, uploads
   the image, creates a compute image via images.insert, cleans up
   the staging resources, and updates both CAPI GCPMachine manifests
   and MAPI worker MachineSet provider specs with the new image
   reference.

4. Validation relaxation (pkg/asset/installconfig/gcp/validation.go):
   The osImage field is no longer required for sovereign cloud
   installations since the installer handles image provisioning
   automatically. Validation for explicitly provided osImage fields
   (including project requirement) is preserved.

The GCS URL uses the universe domain from credentials for the storage
hostname. The compute image includes GuestOsFeatures (GVNIC,
UEFI_COMPATIBLE, VIRTIO_SCSI_MULTIQUEUE, SEV_CAPABLE, SEV_SNP_CAPABLE)
matching the standard RHCOS images. Worker MachineSet provider specs
use json.Marshal for RawExtension.Raw as required by the Kubernetes
API machinery.

Summary by CodeRabbit

  • New Features
    • Added support for sovereign GCP environments, including architecture- and project-aware defaults for instance and disk types.
    • Added automatic RHCOS image upload and provisioning for scenarios where referenced images are not directly accessible.
    • Improved sovereign-aware handling of RHCOS image locations and storage endpoints.
  • Bug Fixes
    • Prevented invalid or unnecessary image lookups when sovereign image project details are missing.
    • Ensured uploaded RHCOS images are consistently used for both control-plane and worker machine manifests.
  • Tests
    • Extended marketplace image validation coverage for sovereign-cloud scenarios.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 15, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@barbacbd: This pull request references CORS-4536 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:

In GCP sovereign clouds (non-default universe domain), the pre-built
RHCOS images hosted in the public rhcos-cloud project are not
accessible. Previously, users were required to manually download,
upload, and configure a custom RHCOS image via the osImage field in
the install config.

This change automates the process by mirroring the existing AWS AMI
copy pattern:

  1. Image resolution (pkg/asset/rhcos/image.go): When a non-default
    universe domain is detected via the GCP credentials, the RHCOS
    image asset returns the tar.gz download URL from the coreos stream
    metadata instead of a rhcos-cloud project reference.

  2. Machine generation (pkg/asset/machines/clusterapi.go): When the
    resolved image is a URL rather than a projects/ reference, the
    GCPMachine manifests are generated with an empty image field,
    serving as a sentinel for PreProvision.

  3. PreProvision upload (pkg/infrastructure/gcp/clusterapi/image.go):
    When empty GCPMachine images are detected, PreProvision downloads
    the RHCOS tar.gz, creates a temporary GCS staging bucket, uploads
    the image, creates a compute image via images.insert, cleans up
    the staging resources, and updates both CAPI GCPMachine manifests
    and MAPI worker MachineSet provider specs with the new image
    reference.

  4. Validation relaxation (pkg/asset/installconfig/gcp/validation.go):
    The osImage field is no longer required for sovereign cloud
    installations since the installer handles image provisioning
    automatically. Validation for explicitly provided osImage fields
    (including project requirement) is preserved.

The GCS URL uses the universe domain from credentials for the storage
hostname. The compute image includes GuestOsFeatures (GVNIC,
UEFI_COMPATIBLE, VIRTIO_SCSI_MULTIQUEUE, SEV_CAPABLE, SEV_SNP_CAPABLE)
matching the standard RHCOS images. Worker MachineSet provider specs
use json.Marshal for RawExtension.Raw as required by the Kubernetes
API machinery.

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

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Changes

The change adds sovereign-GCP-aware instance, disk, and marketplace-image validation, resolves RHCOS artifacts for non-default universes, uploads cluster-specific RHCOS images when required, and wires those references into generated machine manifests.

Sovereign GCP provisioning

Layer / File(s) Summary
Sovereign GCP defaults and marketplace validation
pkg/asset/installconfig/gcp/validation.go, pkg/asset/installconfig/gcp/validation_test.go
Instance and disk defaults use the project ID; sovereign marketplace images require osImage.project, with coverage for default machine, control-plane, and compute configurations.
RHCOS image selection and upload contract
pkg/asset/rhcos/image.go, pkg/types/gcp/platform.go
Non-default universe domains resolve the GCP RHCOS artifact, while helpers identify sovereign machine pools that need uploaded images and format their image references.
Cluster-scoped RHCOS image upload
pkg/infrastructure/gcp/clusterapi/clusterapi.go, pkg/infrastructure/gcp/clusterapi/image.go
Pre-provisioning downloads and optionally verifies the RHCOS archive, stages it in GCS, creates a global compute image, and removes staging resources.
Machine manifest image wiring
pkg/asset/machines/clusterapi.go, pkg/asset/machines/master.go, pkg/asset/machines/worker.go
Control-plane, bootstrap, master, and worker generation use the uploaded cluster-specific image reference when sovereign provisioning requires it.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 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: automatic RHCOS image upload for GCP sovereign clouds.
Docstring Coverage ✅ Passed Docstring coverage is 85.00% which is sufficient. The required threshold is 80.00%.
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.
Stable And Deterministic Test Names ✅ Passed The new subtests use static table names like 'Sovereign cloud with osImage but no project in controlPlane'; no dynamic values or generated identifiers appear in titles.
Test Structure And Quality ✅ Passed PASS: The PR only adds standard table-driven unit tests; no Ginkgo, cluster waits, or resource cleanup issues were introduced, and the cases are isolated.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e specs were added; the changed tests are plain unit tests and don’t reference MicroShift-unsupported APIs.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The PR adds only unit tests and code changes; no new Ginkgo e2e tests with multi-node assumptions were introduced.
Topology-Aware Scheduling Compatibility ✅ Passed Touched code only changes GCP image resolution/provisioning and validation; no nodeSelector, affinity, topology spread, replica, or PDB logic was added.
Ote Binary Stdout Contract ✅ Passed No touched file has main/TestMain/init/suite setup or stdout prints; the PR only changes library code and a plain unit test file.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the touched test file is a standard unit test using testing.T/mocks, with no IPv4 or external connectivity assumptions.
No-Weak-Crypto ✅ Passed No weak algorithms, custom crypto, or secret comparisons found; only SHA-256 is used for RHCOS checksum validation and deterministic fencing-secret naming.
Container-Privileges ✅ Passed PASS: Touched files only alter GCP image provisioning; searches found no privileged, hostPID/hostNetwork/hostIPC, or allowPrivilegeEscalation fields in changed manifests.
No-Sensitive-Data-In-Logs ✅ Passed New logs only print cluster/image/bucket identifiers and public CoreOS URLs; no passwords, tokens, PII, or internal hostnames are logged.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci

openshift-ci Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zaneb for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
pkg/clusterapi/system.go (1)

310-335: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Leftover duplicate block — old universe-domain check wasn't removed.

Lines 326-335 duplicate the exact same universe-domain detection/env-var-setting logic just added above (310-321), using the old direct != "googleapis.com" comparison this PR was meant to replace. Please delete the old block.

🧹 Suggested cleanup
 		if v, ok := capgEnvVars[gAppCredEnvVar]; ok {
 			logrus.Infof("setting %q to %s for capg infrastructure controller", gAppCredEnvVar, v)
 		}
 
-		// Google Cloud Dedicated support: detect universe domain from
-		// credentials and pass it to the CAPG controller via env var.
-		ud, err := session.Credentials.GetUniverseDomain()
-		if err != nil {
-			return fmt.Errorf("failed to get universe domain from gcp credentials: %w", err)
-		}
-		if ud != "googleapis.com" {
-			capgEnvVars["GOOGLE_CLOUD_UNIVERSE_DOMAIN"] = ud
-			logrus.Infof("setting GOOGLE_CLOUD_UNIVERSE_DOMAIN to %q for capg infrastructure controller", ud)
-		}
-
 		controllers = append(controllers,
🤖 Prompt for 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.

In `@pkg/clusterapi/system.go` around lines 310 - 335, Remove the duplicate
universe-domain handling block beginning with the second
session.Credentials.GetUniverseDomain() call and ending with its logging
statement. Keep the earlier block using
gcp.IsNonDefaultUniverseDomain(universeDomain), including its error handling and
GOOGLE_CLOUD_UNIVERSE_DOMAIN assignment.
pkg/infrastructure/gcp/clusterapi/image.go (1)

1-345: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

New file lacks unit tests.

This file introduces substantial new logic (download, GCS staging/upload, compute image creation, manifest patching) with no accompanying _test.go. As per coding guidelines, all code under ./cmd/..., ./data/..., and ./pkg/... must have unit tests. Given the correctness issues already flagged in this file (disk-iteration overwrite, single-arch image reuse), test coverage here would materially reduce risk.

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

In `@pkg/infrastructure/gcp/clusterapi/image.go` around lines 1 - 345, Add a
unit-test file covering the new logic in uploadRHCOSImage, downloadRHCOSImage,
createStagingBucket, uploadToGCS, createComputeImage, cleanupStaging,
needsImageUpload, updateMachineManifestImages, and updateWorkerMachineSetImages.
Include cases for checksum and HTTP failures, manifest image updates, preserving
existing disk images while updating eligible worker disks, and relevant GCP
operation or cleanup errors; use mocks or test doubles for external services.

Source: Coding guidelines

🧹 Nitpick comments (2)
pkg/infrastructure/gcp/clusterapi/image.go (1)

26-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Inconsistent alias for github.com/openshift/api/machine/v1beta1.

This file aliases it as machineapi, while the rest of the codebase (e.g. pkg/asset/machines/master.go) uses machinev1beta1 for the same import. As per coding guidelines, new imports that could conflict or overlap with existing package usage should follow established alias conventions already used in the project.

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

In `@pkg/infrastructure/gcp/clusterapi/image.go` around lines 26 - 32, Rename the
import alias for github.com/openshift/api/machine/v1beta1 from machineapi to the
project-standard machinev1beta1, and update all references to that package
within the file accordingly.

Source: Coding guidelines

pkg/asset/machines/clusterapi.go (1)

341-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fragile string-prefix contract for "needs upload" signal.

Detecting sovereign-cloud/download-URL images via strings.HasPrefix(controlPlaneImage, "http") works today only because pre-built GCP image references always look like projects/.../global/images/..., but it's an implicit, undocumented contract shared across three files (this one, pkg/asset/rhcos/image.go, and pkg/infrastructure/gcp/clusterapi/image.go's disk-image check). A safer approach would be an explicit boolean (e.g. rhcos.Image.NeedsUpload or a typed URL wrapper) set once in pkg/asset/rhcos/image.go and threaded through, rather than re-deriving it from string shape in multiple places.

Also applies to: 365-365

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

In `@pkg/asset/machines/clusterapi.go` around lines 341 - 352, The needs-upload
state is being inferred from the fragile “http” prefix in the cluster
machine-generation path. Add an explicit boolean or typed image state in the
RHCOS image model and set it once in the image-loading logic, then thread that
state through the relevant GCP generation and disk-image checks; update the
controlPlaneImage handling near gcp.GenerateMachines and the corresponding
symbols in rhcos image and GCP cluster API code to use the explicit state
instead of string-prefix detection.
🤖 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 `@pkg/asset/installconfig/gcp/session.go`:
- Around line 198-202: Add unit coverage for cliLoader.Load, verifying it passes
compute.CloudPlatformScope to googleoauth.FindDefaultCredentialsWithParams and
propagates finder errors unchanged. Introduce a small injectable finder seam
around FindDefaultCredentialsWithParams if needed to test these behaviors
without external credentials.

In `@pkg/asset/rhcos/image.go`:
- Around line 265-280: Update needsGCPImageUpload to propagate failures from
icgcp.GetSession and ssn.Credentials.GetUniverseDomain instead of returning
false; adjust its callers, including osImage, to handle the returned error and
fail the installation with actionable context before selecting the public
rhcos-cloud image. Preserve the existing universe-domain decision for successful
lookups.

In `@pkg/infrastructure/gcp/clusterapi/image.go`:
- Around line 180-197: Make staging bucket and compute image creation retry-safe
in createStagingBucket and createComputeImage: detect the cloud API’s
already-exists response for the deterministic resource names and treat it as
success, while still returning other errors. Preserve normal creation behavior
so retries after partial provisioning can continue past completed steps.
- Around line 123-177: Update downloadRHCOSImage to perform the image request
with a bounded timeout instead of calling http.Get directly. Use the file’s
established context.WithTimeout pattern or an HTTP client configured with an
appropriate timeout, and ensure the timeout is applied to each retry attempt
while preserving the existing retry and cleanup behavior.
- Around line 41-119: Update uploadRHCOSImage to select the RHCOS source based
on the target machine role instead of always using in.RhcosImage.ControlPlane.
Use in.RhcosImage.Compute when preparing the worker MachineSet image, while
preserving the control-plane image selection for control-plane provisioning and
the existing upload flow.

In `@pkg/types/gcp/platform.go`:
- Around line 215-217: Update GetCloudEnvironment so sovereign ARM64
installations never select the x86 c3-standard-4 machine type or its
incompatible ARM image; instead choose a supported sovereign ARM64 type, or
reject the configuration during validation while preserving existing behavior
for non-ARM64 environments.

---

Outside diff comments:
In `@pkg/clusterapi/system.go`:
- Around line 310-335: Remove the duplicate universe-domain handling block
beginning with the second session.Credentials.GetUniverseDomain() call and
ending with its logging statement. Keep the earlier block using
gcp.IsNonDefaultUniverseDomain(universeDomain), including its error handling and
GOOGLE_CLOUD_UNIVERSE_DOMAIN assignment.

In `@pkg/infrastructure/gcp/clusterapi/image.go`:
- Around line 1-345: Add a unit-test file covering the new logic in
uploadRHCOSImage, downloadRHCOSImage, createStagingBucket, uploadToGCS,
createComputeImage, cleanupStaging, needsImageUpload,
updateMachineManifestImages, and updateWorkerMachineSetImages. Include cases for
checksum and HTTP failures, manifest image updates, preserving existing disk
images while updating eligible worker disks, and relevant GCP operation or
cleanup errors; use mocks or test doubles for external services.

---

Nitpick comments:
In `@pkg/asset/machines/clusterapi.go`:
- Around line 341-352: The needs-upload state is being inferred from the fragile
“http” prefix in the cluster machine-generation path. Add an explicit boolean or
typed image state in the RHCOS image model and set it once in the image-loading
logic, then thread that state through the relevant GCP generation and disk-image
checks; update the controlPlaneImage handling near gcp.GenerateMachines and the
corresponding symbols in rhcos image and GCP cluster API code to use the
explicit state instead of string-prefix detection.

In `@pkg/infrastructure/gcp/clusterapi/image.go`:
- Around line 26-32: Rename the import alias for
github.com/openshift/api/machine/v1beta1 from machineapi to the project-standard
machinev1beta1, and update all references to that package within the file
accordingly.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b169c791-fb71-47d7-84bb-0e2b3a459bd7

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb9219 and 73294b8.

📒 Files selected for processing (13)
  • pkg/asset/installconfig/gcp/session.go
  • pkg/asset/installconfig/gcp/validation.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/asset/machines/clusterapi.go
  • pkg/asset/machines/master.go
  • pkg/asset/machines/worker.go
  • pkg/asset/rhcos/image.go
  • pkg/clusterapi/system.go
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/infrastructure/gcp/clusterapi/image.go
  • pkg/types/gcp/defaults/machinepool.go
  • pkg/types/gcp/machinepools.go
  • pkg/types/gcp/platform.go

Comment thread pkg/asset/installconfig/gcp/session.go Outdated
Comment thread pkg/asset/rhcos/image.go Outdated
Comment thread pkg/infrastructure/gcp/clusterapi/image.go
Comment thread pkg/infrastructure/gcp/clusterapi/image.go
Comment thread pkg/infrastructure/gcp/clusterapi/image.go
Comment thread pkg/types/gcp/platform.go
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
Connect Timeout Error (attempted address: api.github.com:443, timeout: 10000ms)

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2026
@barbacbd

Copy link
Copy Markdown
Contributor Author

Adding a note here to revisit or remove the changes we are adding in #10630. The original change is to require a user to add an image or compute and controlPlane when sovereign cloud is detected. This branch will allow the installer to create an image for the purposes of the install rather than requiring the user to bring their own.

@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/asset/installconfig/gcp/validation.go (1)

200-200: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Propagate a cancellable context to the disk lookup.

GetDiskTypeWithZones uses this context for GetZones before its internal timeout is created, so this new network path cannot inherit caller cancellation or a request deadline. Thread a context.Context through validation instead of introducing another context.TODO() call. As per path instructions, Go code must use context.Context for cancellation and timeouts.

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

In `@pkg/asset/installconfig/gcp/validation.go` at line 200, Update the validation
flow around GetDiskTypeWithZones to accept and propagate a caller-provided
context.Context, replacing context.TODO() with that context for the disk lookup.
Thread the context through any intervening validation functions and preserve
existing timeout behavior inside GetDiskTypeWithZones.

Source: Path instructions

🤖 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 `@pkg/asset/installconfig/gcp/validation.go`:
- Around line 250-253: The sovereign defaulting branch in the instance-type
selection logic must preserve architecture: update the cloudEnv ==
gcp.CloudEnvironmentSovereign handling to return an ARM-compatible default for
ARM64, or explicitly reject unsupported ARM64 before returning the x86
c3-standard-4 default. Keep c3-standard-4 for supported non-ARM architectures.

---

Outside diff comments:
In `@pkg/asset/installconfig/gcp/validation.go`:
- Line 200: Update the validation flow around GetDiskTypeWithZones to accept and
propagate a caller-provided context.Context, replacing context.TODO() with that
context for the disk lookup. Thread the context through any intervening
validation functions and preserve existing timeout behavior inside
GetDiskTypeWithZones.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b06e1bcb-6644-4f25-81ee-7272ce2b44ac

📥 Commits

Reviewing files that changed from the base of the PR and between a92c1a4 and 3f02975.

📒 Files selected for processing (11)
  • pkg/asset/installconfig/gcp/validation.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/asset/machines/clusterapi.go
  • pkg/asset/machines/master.go
  • pkg/asset/machines/worker.go
  • pkg/asset/rhcos/image.go
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/infrastructure/gcp/clusterapi/image.go
  • pkg/types/gcp/defaults/machinepool.go
  • pkg/types/gcp/machinepools.go
  • pkg/types/gcp/platform.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • pkg/asset/machines/master.go
  • pkg/asset/rhcos/image.go
  • pkg/types/gcp/defaults/machinepool.go
  • pkg/asset/machines/worker.go
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/types/gcp/platform.go
  • pkg/asset/machines/clusterapi.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/infrastructure/gcp/clusterapi/image.go

Comment thread pkg/asset/installconfig/gcp/validation.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/asset/machines/clusterapi.go (1)

329-329: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add unit coverage for the new GCP image-selection paths.

Please test that the project ID reaches GCP default generation, download URLs become empty image references for both control-plane and bootstrap machines, and normal GCP image references remain unchanged. These branches control whether the later cluster-specific upload flow is used.

As per coding guidelines, all Go code under pkg/ must have unit tests.

Also applies to: 341-352, 365-365

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

In `@pkg/asset/machines/clusterapi.go` at line 329, Extend unit tests for the GCP
image-selection logic around defaultGCPMachinePoolPlatform and the
control-plane/bootstrap machine paths. Verify the GCP project ID is passed
through, download URLs produce empty image references for both machine types,
and standard GCP image references remain unchanged.

Source: Coding guidelines

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

Outside diff comments:
In `@pkg/asset/machines/clusterapi.go`:
- Line 329: Extend unit tests for the GCP image-selection logic around
defaultGCPMachinePoolPlatform and the control-plane/bootstrap machine paths.
Verify the GCP project ID is passed through, download URLs produce empty image
references for both machine types, and standard GCP image references remain
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: a8146b2d-b7a2-4c4d-bc72-38430cd7ea9e

📥 Commits

Reviewing files that changed from the base of the PR and between 3f02975 and 907ea25.

📒 Files selected for processing (6)
  • pkg/asset/installconfig/gcp/validation.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/asset/machines/clusterapi.go
  • pkg/asset/rhcos/image.go
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/infrastructure/gcp/clusterapi/image.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/asset/rhcos/image.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/infrastructure/gcp/clusterapi/image.go
  • pkg/asset/installconfig/gcp/validation.go

Comment on lines +113 to +117
updateMachineManifestImages(in.MachineManifests, imageRef)
if err := updateWorkerMachineSetImages(in.WorkersAsset, imageRef); err != nil {
return fmt.Errorf("failed to update worker machineset images: %w", err)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to create these images with a deterministic name, in which case we should populate them in the machine manifests--for both control plane and worker.

That is, you create the manifest with the name the image will have, and then we upload it later in pkg/infrastructure

Require osImage configuration for sovereign cloud installations and
validate that the image project is specified. Sovereign clouds require
RHCOS images to be pre-loaded in specific projects, unlike public GCP
which uses shared image projects.

Add validation in validateMarketplaceImages to detect sovereign cloud
environments via project ID prefix (e.g., eu0:) and enforce:
- osImage must be specified in defaultMachinePlatform, controlPlane,
  or compute machine pools
- osImage.project must be non-empty when osImage is configured

Guard GetImage API calls to skip when project is empty, preventing
redundant error messages. Add comprehensive test coverage including
edge cases for nil DefaultMachinePlatform.

The validation is backwards compatible and only applies to sovereign
cloud project IDs. Existing public GCP installations are unaffected.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@pkg/infrastructure/gcp/clusterapi/clusterapi.go`:
- Around line 105-113: Update the RHCOS upload decision in the cluster creation
flow around NeedsRHCOSUpload and uploadRHCOSImage to evaluate all resolved
compute machine pools, not only controlPlaneMpool. Trigger the upload when any
control-plane or compute pool requires the cluster-specific image, while
preserving the existing upload error propagation.

In `@pkg/infrastructure/gcp/clusterapi/image.go`:
- Around line 33-110: Add unit tests covering the sovereign RHCOS flow in
pkg/infrastructure/gcp/clusterapi/image.go:33-110, including download
verification, staging, image creation, and cleanup failures with mocked clients;
cover missing and project-qualified images for default, control-plane, and
compute pools in pkg/asset/installconfig/gcp/validation.go:839-896; predicate
combinations and generated references in pkg/types/gcp/platform.go:246-254; the
worker-only upload path in clusterapi.go:105-113; and CAPI, MAPI, and worker
image references in pkg/asset/machines/clusterapi.go:341-363,
pkg/asset/machines/master.go:292-296, and pkg/asset/machines/worker.go:698-702.
- Around line 136-140: Update the file-writing flow around os.Create and io.Copy
to capture and return any error from f.Close(), including close failures after a
successful copy. Ensure the close error is propagated before the temporary
archive can be uploaded, while preserving existing create and copy error
handling.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5db78c1f-1826-4c92-9618-6eb9a5cbf5f4

📥 Commits

Reviewing files that changed from the base of the PR and between 907ea25 and 0198ee4.

📒 Files selected for processing (9)
  • pkg/asset/installconfig/gcp/validation.go
  • pkg/asset/installconfig/gcp/validation_test.go
  • pkg/asset/machines/clusterapi.go
  • pkg/asset/machines/master.go
  • pkg/asset/machines/worker.go
  • pkg/asset/rhcos/image.go
  • pkg/infrastructure/gcp/clusterapi/clusterapi.go
  • pkg/infrastructure/gcp/clusterapi/image.go
  • pkg/types/gcp/platform.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/asset/rhcos/image.go
  • pkg/asset/installconfig/gcp/validation_test.go

Comment on lines +105 to +113
// Upload a cluster-specific RHCOS image for sovereign clouds where the
// pre-built images in the rhcos-cloud project are not accessible.
// Machine manifests are already populated with the deterministic image
// reference during generation.
if gcptypes.NeedsRHCOSUpload(projectID, controlPlaneMpool) {
if _, err := uploadRHCOSImage(ctx, in); err != nil {
return fmt.Errorf("failed to upload RHCOS image: %w", err)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Upload when any resolved machine pool needs it.

This only checks controlPlaneMpool. With a custom control-plane osImage and a compute pool using the default image, worker manifests reference <infraID>-rhcos but this path skips creating it. Evaluate the resolved compute pools too before deciding to upload.

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

In `@pkg/infrastructure/gcp/clusterapi/clusterapi.go` around lines 105 - 113,
Update the RHCOS upload decision in the cluster creation flow around
NeedsRHCOSUpload and uploadRHCOSImage to evaluate all resolved compute machine
pools, not only controlPlaneMpool. Trigger the upload when any control-plane or
compute pool requires the cluster-specific image, while preserving the existing
upload error propagation.

Comment thread pkg/infrastructure/gcp/clusterapi/image.go
Comment on lines +136 to +140
f, err := os.Create(filePath)
if err != nil {
return err
}
defer f.Close()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="pkg/infrastructure/gcp/clusterapi/image.go"

echo "== File outline =="
ast-grep outline "$file" --view expanded || true

echo
echo "== Relevant lines =="
sed -n '110,170p' "$file" | cat -n

echo
echo "== Search for similar close handling in this file =="
rg -n "Close\(\)|os\.Create|io\.Copy|defer f\.Close" "$file"

Repository: openshift/installer

Length of output: 3478


Handle the os.File.Close() error here. A successful io.Copy can still be followed by a close failure; return that error so a corrupt temp archive doesn’t get uploaded.

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

In `@pkg/infrastructure/gcp/clusterapi/image.go` around lines 136 - 140, Update
the file-writing flow around os.Create and io.Copy to capture and return any
error from f.Close(), including close failures after a successful copy. Ensure
the close error is propagated before the temporary archive can be uploaded,
while preserving existing create and copy error handling.

Source: Path instructions

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@barbacbd: The following tests 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/gofmt de5c5f7 link true /test gofmt
ci/prow/golint de5c5f7 link true /test golint
ci/prow/gcp-custom-endpoints-proxy-wif de5c5f7 link false /test gcp-custom-endpoints-proxy-wif

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants