Add e2e test for vSphere multi-vCenter Day 2 using openshift-tests-ex… - #66
Open
jcpowermac wants to merge 3 commits into
Open
Add e2e test for vSphere multi-vCenter Day 2 using openshift-tests-ex…#66jcpowermac wants to merge 3 commits into
jcpowermac wants to merge 3 commits into
Conversation
WalkthroughAdds an OpenShift tests extension binary, a vSphere Multi-VCenter Day 2 Ginkgo suite, helper utilities, build and lint wiring, and a test plan. ChangesvSphere Multi-VCenter Day 2 E2E
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Test as VSphereMultiVCenterDay2
participant Migration as VmwareCloudFoundationMigration
participant Infrastructure
participant ConfigMaps
participant Pods
Test->>Migration: Set state to Running
Test->>Migration: Poll MultiSiteConfigured=True
Test->>Infrastructure: Verify vCenters and failure domains
Test->>ConfigMaps: Verify propagated vCenter configuration
Test->>Pods: Verify CCCMO and CCM readiness
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcpowermac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
jcpowermac
force-pushed
the
e2e-multi-vcenter-day2
branch
from
June 24, 2026 15:40
fdba91f to
40a6bd7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/plans/e2e-multi-vcenter-day2.md (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to fenced code blocks.
The fenced blocks at Line 20 and Line 148 lack a language specifier (markdownlint MD040). Use
```text(or```go/```sh) as appropriate.🤖 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 `@docs/plans/e2e-multi-vcenter-day2.md` around lines 20 - 22, The fenced code blocks in this markdown plan are missing a language specifier and trigger markdownlint MD040. Update the affected fenced blocks in the document, including the one containing the dependency replacement near the top and the other matching fence later in the file, by adding an appropriate label such as text, go, or sh so the code fences are explicitly typed.Source: Linters/SAST tools
🤖 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 `@test/e2e/vsphere/multi_vcenter_day2.go`:
- Around line 64-66: The platform guard in the vSphere e2e test is using the
non-authoritative infra.Spec.PlatformSpec.Type, which can be empty on installed
clusters and cause incorrect skips. Update the platform check in the test logic
that gates multi_vcenter_day2.go to use infra.Status.PlatformStatus.Type
instead, keeping the existing vSphere skip behavior but basing it on the
authoritative status field.
---
Nitpick comments:
In `@docs/plans/e2e-multi-vcenter-day2.md`:
- Around line 20-22: The fenced code blocks in this markdown plan are missing a
language specifier and trigger markdownlint MD040. Update the affected fenced
blocks in the document, including the one containing the dependency replacement
near the top and the other matching fence later in the file, by adding an
appropriate label such as text, go, or sh so the code fences are explicitly
typed.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9914b0c9-dbda-4c6d-a531-b0134c263ff7
⛔ Files ignored due to path filters (52)
go.sumis excluded by!**/*.sumvendor/github.com/onsi/ginkgo/v2/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdimages/cmdimages.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdinfo/info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdlist/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdupdate/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/dbtime/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extension.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/scheduler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/task.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/viewer.htmlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/component.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/concurrency.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/names.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/logging.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/junit/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/byte.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/empty.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int32.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (7)
.golangci.ymlMakefilecmd/openshift-tests-extension/main.godocs/plans/e2e-multi-vcenter-day2.mdgo.modtest/e2e/vsphere/helpers.gotest/e2e/vsphere/multi_vcenter_day2.go
✅ Files skipped from review due to trivial changes (1)
- .golangci.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/openshift-tests-extension/main.go
- test/e2e/vsphere/helpers.go
- Makefile
- go.mod
jcpowermac
force-pushed
the
e2e-multi-vcenter-day2
branch
from
June 24, 2026 16:22
40a6bd7 to
7cf35b2
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tension Adds an OTE-integrated e2e test that exercises adding a second vCenter to a running cluster via the VmwareCloudFoundationMigration CR. The test verifies the full cloud config pipeline: Infrastructure CR, cloud-provider-config in openshift-config, kube-cloud-config in openshift-config-managed (CCCMO), cloud-conf in openshift-cloud-controller-manager (CCM), and pod health. The test is gated behind [OCPFeatureGate:VSphereMultiVCenterDay2] and [platform:vsphere] so it only runs on vSphere clusters with the feature gate enabled and a pre-created Migration CR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Spec.PlatformSpec.Type can be empty on installed clusters. Use the authoritative Status.PlatformStatus.Type field instead with a nil guard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jcpowermac
force-pushed
the
e2e-multi-vcenter-day2
branch
from
August 10, 2026 19:28
7cf35b2 to
9dedcae
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/plans/e2e-multi-vcenter-day2.md`:
- Around line 148-150: Update the command fence around the
openshift-tests-extension example to specify the shell language identifier,
changing the unlabeled fence to a shell-labeled fence while preserving the
command and surrounding documentation.
In `@test/e2e/vsphere/multi_vcenter_day2.go`:
- Around line 75-82: Update the VmwareCloudFoundationMigration selection after
crClient.List to avoid using unordered migrationList.Items[0]. Continue skipping
when no CR exists, then require exactly one migration CR or select the intended
resource by its configured name and namespace before changing its state.
- Around line 51-57: Regenerate and commit the API deepcopy output so
VmwareCloudFoundationMigration and VmwareCloudFoundationMigrationList implement
runtime.Object with DeepCopyObject methods. Ensure the generated methods are
available before migrationv1alpha1.AddToScheme registers the scheme in the test.
- Around line 100-103: Sanitize test logging in
test/e2e/vsphere/multi_vcenter_day2.go: at lines 100-103, 145, 166, 181, and
206, log only the counts of the parsed vCenter server collections or maps
instead of their contents; at lines 120-122, omit cond.Message or redact it
before logging. Preserve the existing assertions and status information while
ensuring no internal hostnames or unfiltered operator output is emitted.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 50b05506-c968-411a-960f-80489cc17bae
⛔ Files ignored due to path filters (52)
go.sumis excluded by!**/*.sumvendor/github.com/onsi/ginkgo/v2/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdimages/cmdimages.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdinfo/info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdlist/list.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runsuite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdrun/runtest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdupdate/update.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/dbtime/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extension.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/result_writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/scheduler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/task.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/viewer.htmlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/component.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/concurrency.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/environment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/names.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/output.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/flags/suite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/logging.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/parallel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/junit/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/byte.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/empty.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int32.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/int64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/util/sets/string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift-eng/openshift-tests-extension/pkg/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (7)
.golangci.ymlMakefilecmd/openshift-tests-extension/main.godocs/plans/e2e-multi-vcenter-day2.mdgo.modtest/e2e/vsphere/helpers.gotest/e2e/vsphere/multi_vcenter_day2.go
🚧 Files skipped from review as they are similar to previous changes (5)
- .golangci.yml
- test/e2e/vsphere/helpers.go
- go.mod
- cmd/openshift-tests-extension/main.go
- Makefile
Comment on lines
+148
to
+150
| ``` | ||
| ./bin/openshift-tests-extension run --suite example/tests | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the command fence.
Line 148 has no language identifier. This fails markdownlint rule MD040. Use shell.
Proposed fix
- ```
+ ```shell📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| ./bin/openshift-tests-extension run --suite example/tests | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/plans/e2e-multi-vcenter-day2.md` around lines 148 - 150, Update the
command fence around the openshift-tests-extension example to specify the shell
language identifier, changing the unlabeled fence to a shell-labeled fence while
preserving the command and surrounding documentation.
Source: Linters/SAST tools
Comment on lines
+51
to
+57
| scheme := runtime.NewScheme() | ||
| err = migrationv1alpha1.AddToScheme(scheme) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to add migration scheme") | ||
| err = configv1.Install(scheme) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to install configv1 scheme") | ||
|
|
||
| crClient, err = client.New(cfg, client.Options{Scheme: scheme}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Regenerate the API deepcopy methods before registering this scheme.
The current package cannot type-check. Static analysis shows that VmwareCloudFoundationMigration and VmwareCloudFoundationMigrationList do not implement runtime.Object because they lack DeepCopyObject. Regenerate and commit the API deepcopy output before this test imports and registers the migration scheme.
🤖 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 `@test/e2e/vsphere/multi_vcenter_day2.go` around lines 51 - 57, Regenerate and
commit the API deepcopy output so VmwareCloudFoundationMigration and
VmwareCloudFoundationMigrationList implement runtime.Object with DeepCopyObject
methods. Ensure the generated methods are available before
migrationv1alpha1.AddToScheme registers the scheme in the test.
Source: Linters/SAST tools
Comment on lines
+75
to
+82
| By("Finding VmwareCloudFoundationMigration CR") | ||
| migrationList := &migrationv1alpha1.VmwareCloudFoundationMigrationList{} | ||
| err = crClient.List(ctx, migrationList) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to list VmwareCloudFoundationMigration CRs") | ||
| if len(migrationList.Items) == 0 { | ||
| Skip("skipping: no VmwareCloudFoundationMigration CR found in cluster") | ||
| } | ||
| migration = &migrationList.Items[0] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Select one explicit migration CR before changing its state.
Line 82 selects the first item from an unordered list. If multiple migration CRs exist, this test can set spec.state to Running on the wrong resource. Skip when no CR exists, then require exactly one CR or select the intended CR by a configured name and namespace.
🤖 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 `@test/e2e/vsphere/multi_vcenter_day2.go` around lines 75 - 82, Update the
VmwareCloudFoundationMigration selection after crClient.List to avoid using
unordered migrationList.Items[0]. Continue skipping when no CR exists, then
require exactly one migration CR or select the intended resource by its
configured name and namespace before changing its state.
Comment on lines
+100
to
+103
| baselineServers, err := parseVCenterServersFromConfig(baselineCM, "config") | ||
| Expect(err).NotTo(HaveOccurred(), "failed to parse baseline cloud-provider-config") | ||
| GinkgoWriter.Printf("Baseline vCenters in cloud-provider-config: %v\n", baselineServers) | ||
| GinkgoWriter.Printf("Baseline vCenters in Infrastructure CR: %d\n", initialVCenterCount) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Do not write vCenter server names or unfiltered condition messages to test logs.
GinkgoWriter output can retain internal vCenter hostnames. cond.Message is also unfiltered operator output. Log counts and safe identifiers instead, or redact sensitive values.
test/e2e/vsphere/multi_vcenter_day2.go#L100-L103: replace the baseline server list with a count.test/e2e/vsphere/multi_vcenter_day2.go#L120-L122: omit or redactcond.Message.test/e2e/vsphere/multi_vcenter_day2.go#L145-L145: replace the Infrastructure server map with a count.test/e2e/vsphere/multi_vcenter_day2.go#L166-L166: replace the cloud-provider-config server list with a count.test/e2e/vsphere/multi_vcenter_day2.go#L181-L181: replace the kube-cloud-config server list with a count.test/e2e/vsphere/multi_vcenter_day2.go#L206-L206: replace the cloud-conf server list with a count.
As per coding guidelines, flag logging that may expose passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data.
📍 Affects 1 file
test/e2e/vsphere/multi_vcenter_day2.go#L100-L103(this comment)test/e2e/vsphere/multi_vcenter_day2.go#L120-L122test/e2e/vsphere/multi_vcenter_day2.go#L145-L145test/e2e/vsphere/multi_vcenter_day2.go#L166-L166test/e2e/vsphere/multi_vcenter_day2.go#L181-L181test/e2e/vsphere/multi_vcenter_day2.go#L206-L206
🤖 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 `@test/e2e/vsphere/multi_vcenter_day2.go` around lines 100 - 103, Sanitize test
logging in test/e2e/vsphere/multi_vcenter_day2.go: at lines 100-103, 145, 166,
181, and 206, log only the counts of the parsed vCenter server collections or
maps instead of their contents; at lines 120-122, omit cond.Message or redact it
before logging. Preserve the existing assertions and status information while
ensuring no internal hostnames or unfiltered operator output is emitted.
Source: Coding guidelines
|
@jcpowermac: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…tension
Adds an OTE-integrated e2e test that exercises adding a second vCenter
to a running cluster via the VmwareCloudFoundationMigration CR. The test
verifies the full cloud config pipeline: Infrastructure CR, cloud-provider-config
in openshift-config, kube-cloud-config in openshift-config-managed (CCCMO),
cloud-conf in openshift-cloud-controller-manager (CCM), and pod health.
The test is gated behind [OCPFeatureGate:VSphereMultiVCenterDay2] and
[platform:vsphere] so it only runs on vSphere clusters with the feature
gate enabled and a pre-created Migration CR.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit