OCPCLOUD-3041: Created Cluster-api-tests-ext command for origin e2e#413
OCPCLOUD-3041: Created Cluster-api-tests-ext command for origin e2e#413sunzhaohua2 wants to merge 5 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces an OpenShift tests extension module for cluster-capi-operator with standardized e2e test configurations. Updates multiple platform-specific test suites with consistent tagging, conformance labels, and initialization logic. Adds build target for extension binary and updates dependencies, particularly for Azure provider. Changes
Sequence DiagramsequenceDiagram
actor User
participant Extension as Tests Extension
participant Registry as Extension Registry
participant TestBuilder as Test Spec Builder
participant Platform as Platform Detector
participant Ginkgo as OpenShift Ginkgo
participant TestRunner as Test Executor
User->>Extension: Execute extension binary
Extension->>Registry: Create registry & extension payload
Extension->>Ginkgo: Build test specs from OpenShift Ginkgo
Ginkgo-->>TestBuilder: Return test definitions
TestBuilder->>Platform: Scan test labels for platform tags
Platform->>Platform: Extract platforms via regex
Platform-->>TestBuilder: Return discovered platforms
TestBuilder->>Registry: Register suites with PlatformEquals selectors
TestBuilder->>Registry: Attach test specs to extension
Registry->>Extension: Wire Cobra root command
Extension->>TestRunner: Execute extension commands
TestRunner-->>User: Return results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (23)
e2e/framework/cluster.go (1)
34-45: Revisit platform-specific logic in generic framework (tracked as TODO).The VSphere-specific control plane endpoint configuration is acknowledged in the TODO comment as potentially needing refactoring. Consider whether future abstractions could move this platform-specific behavior out of the core framework.
e2e/machine_migration_capi_authoritative.go (1)
240-300: Commented-out test case due to known bug.The test for "CAPI (and already existing CAPI Machine with same name) -> MAPI -> CAPI round trip" is commented out due to OCPBUGS-63183 which causes AWS instance leaks. Consider adding a TODO comment or tracking issue to re-enable this test once the bug is resolved.
Would you like me to create a tracking issue or add a more structured TODO comment to ensure this test is re-enabled when OCPBUGS-63183 is fixed?
Makefile (1)
15-15: Addtests-extto.PHONYdeclaration.The new
tests-exttarget should be declared as.PHONYto ensure it always runs regardless of whether a file namedtests-extexists.-.PHONY: help all verify test build operator migration manifests-gen unit e2e run fmt vet lint vendor image push aws-cluster azure-cluster gcp-cluster powervs-cluster vsphere-cluster +.PHONY: help all verify test build operator migration manifests-gen tests-ext unit e2e run fmt vet lint vendor image push aws-cluster azure-cluster gcp-cluster powervs-cluster vsphere-clustere2e/aws.go (1)
45-49: Simplify error handling for template creation.The conditional
if err != nil && !apierrors.IsAlreadyExists(err)followed byExpect(err).ToNot(HaveOccurred())is convoluted. Consider using Gomega'sSatisfyAnymatcher for clarity.- if err := cl.Create(ctx, awsMachineTemplate); err != nil && !apierrors.IsAlreadyExists(err) { - Expect(err).ToNot(HaveOccurred()) - } + Expect(cl.Create(ctx, awsMachineTemplate)).To(SatisfyAny( + Succeed(), + WithTransform(apierrors.IsAlreadyExists, BeTrue()), + ))openshift-tests-extension/cmd/main.go (3)
51-54: Prefer%vwhen formatting errors.Using
err.Error()with%+vis redundant. The%vverb on an error directly callsError(), and for wrapped errors,%+vshows stack traces only when the error implements the appropriate interface.specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() if err != nil { - panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %+v", err.Error())) + panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %v", err)) }
66-77: Move regex compilation outside the loop to avoid repeated allocations.The regex pattern is constant and should be compiled once before the loop for better performance.
+ platformRe := regexp.MustCompile(`\[platform:[a-z]*]`) + for _, test := range specs.Select(extensiontests.NameContains("[platform:")).Names() { - re := regexp.MustCompile(`\[platform:[a-z]*]`) - - match := re.FindStringSubmatch(test) + match := platformRe.FindStringSubmatch(test) for _, platformDef := range match { if _, ok := foundPlatforms[platformDef]; !ok { platform := platformDef[strings.Index(platformDef, ":")+1 : len(platformDef)-1] foundPlatforms[platformDef] = platform specs.Select(extensiontests.NameContains(platformDef)). Include(extensiontests.PlatformEquals(platform)) } } }
89-93: Remove unnecessary anonymous function wrapper.The IIFE wrapping
root.Execute()adds no value here.- if err := func() error { - return root.Execute() - }(); err != nil { + if err := root.Execute(); err != nil { os.Exit(1) }e2e/machineset_migration_capi_authoritative.go (2)
29-29: Remove unnecessaryvar _ =from nested Describe blocks.Inside an outer
Describe, thevar _ =assignment is not needed. The result ofDescribe()is already registered with Ginkgo. While this doesn't cause issues, it's not idiomatic.- var _ = Describe("Create MAPI MachineSets", Ordered, func() { + Describe("Create MAPI MachineSets", Ordered, func() {Apply similar changes to the other nested Describe blocks at lines 108 and 219.
Also applies to: 108-108, 219-219
230-232: Reorder assertions to fail fast on empty slice.Checking
errafter asserting the slice is not empty could mask the root cause. IfGetMachinesFromMachineSetfails with an error, theExpect(mapiMachines).ToNot(BeEmpty())would fail with a confusing message about an empty slice rather than the actual error.mapiMachines, err := mapiframework.GetMachinesFromMachineSet(ctx, cl, mapiMachineSet) - Expect(mapiMachines).ToNot(BeEmpty(), "no MAPI Machines found") Expect(err).ToNot(HaveOccurred(), "failed to get MAPI Machines from MachineSet") + Expect(mapiMachines).ToNot(BeEmpty(), "no MAPI Machines found")e2e/vsphere.go (4)
23-27: Inconsistent naming convention for constant.
kubeSystemnamespaceshould follow Go's camelCase convention askubeSystemNamespacefor consistency with the other constants.const ( vSphereMachineTemplateName = "vsphere-machine-template" - kubeSystemnamespace = "kube-system" + kubeSystemNamespace = "kube-system" vSphereCredentialsName = "vsphere-creds" )
43-52: Secret created in BeforeAll is not cleaned up.The vSphere credentials secret created in
BeforeAll(line 40) is never deleted. Consider adding cleanup for the secret inAfterEachor usingDeferCleanupinBeforeAllto ensure proper resource cleanup.AfterEach(func() { if platform != configv1.VSpherePlatformType { // Because AfterEach always runs, even when tests are skipped, we have to // explicitly skip it here for other platforms. Skip("Skipping vSphere E2E tests") } framework.DeleteMachineSets(ctx, cl, machineSet) framework.WaitForMachineSetsDeleted(cl, machineSet) framework.DeleteObjects(ctx, cl, vSphereMachineTemplate) + // Clean up the credentials secret + framework.DeleteObjects(ctx, cl, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: framework.CAPINamespace, + }, + }) })
129-183: Unused functioncreateVSphereCluster.This function is defined but never called in the test suite. If it's intended for future use, consider adding a TODO comment explaining the intent. Otherwise, remove it to avoid dead code.
164-180: Missing polling interval in Eventually.The
Eventuallycall only specifies a timeout but no polling interval. This defaults to 10ms, which is too aggressive for API server polling. Add a retry interval consistent with other framework usages.- Eventually(func() (bool, error) { + Eventually(func() (bool, error) { patchedVSphereCluster := &vspherev1.VSphereCluster{} err := cl.Get(ctx, client.ObjectKeyFromObject(vSphereCluster), patchedVSphereCluster) if err != nil { return false, err } if patchedVSphereCluster.Annotations == nil { return false, nil } if _, ok := patchedVSphereCluster.Annotations[clusterv1.ManagedByAnnotation]; !ok { return false, nil } return patchedVSphereCluster.Status.Ready, nil - }, framework.WaitShort).Should(BeTrue(), "should not time out waiting for the VSphere Cluster to become 'Ready'") + }, framework.WaitShort, framework.RetryMedium).Should(BeTrue(), "should not time out waiting for the VSphere Cluster to become 'Ready'")e2e/machine_migration_mapi_authoritative.go (1)
28-28: Non-idiomatic nested Describe usage.Using
var _ = Describe(...)inside an outerDescribeblock is redundant. Thevar _ =pattern is only needed at the top level to ensure the test is registered. Inside aDescribe, use plainDescribe(...)orContext(...).- var _ = Describe("Create standalone MAPI Machine", Ordered, func() { + Describe("Create standalone MAPI Machine", Ordered, func() {Apply the same fix to lines 66 and 137.
e2e/baremetal.go (3)
52-55: Use constant instead of hardcoded namespace.The namespace
"openshift-cluster-api"is hardcoded here but should useframework.CAPINamespacefor consistency with the rest of the codebase.key := client.ObjectKey{ - Namespace: "openshift-cluster-api", + Namespace: framework.CAPINamespace, Name: "ostest-extraworker-0", // name provided by dev-scripts in CI }
98-110: Add descriptive assertion messages.The assertions lack descriptive messages, unlike the vSphere counterpart. Adding messages improves test failure diagnostics.
func getBaremetalMAPIProviderSpec(cl client.Client) *bmv1alpha1.BareMetalMachineProviderSpec { machineSetList := &mapiv1.MachineSetList{} - Expect(cl.List(ctx, machineSetList, client.InNamespace(framework.MAPINamespace))).To(Succeed()) + Expect(cl.List(ctx, machineSetList, client.InNamespace(framework.MAPINamespace))).To(Succeed(), + "should not fail listing MAPI MachineSets") - Expect(machineSetList.Items).ToNot(HaveLen(0)) + Expect(machineSetList.Items).ToNot(HaveLen(0), "expected to have at least a MachineSet") machineSet := machineSetList.Items[0] - Expect(machineSet.Spec.Template.Spec.ProviderSpec.Value).ToNot(BeNil()) + Expect(machineSet.Spec.Template.Spec.ProviderSpec.Value).ToNot(BeNil(), + "expected not to have an empty MAPI MachineSet ProviderSpec") providerSpec := &bmv1alpha1.BareMetalMachineProviderSpec{} - Expect(yaml.Unmarshal(machineSet.Spec.Template.Spec.ProviderSpec.Value.Raw, providerSpec)).To(Succeed()) + Expect(yaml.Unmarshal(machineSet.Spec.Template.Spec.ProviderSpec.Value.Raw, providerSpec)).To(Succeed(), + "should not fail YAML decoding MAPI MachineSet provider spec") return providerSpec }
112-139: Consider adding provider spec validation.Unlike the vSphere and GCP implementations that validate provider spec fields before use, this function directly uses
mapiProviderSpec.UserDatawithout validation. Consider adding a nil check formapiProviderSpec.func createBaremetalMachineTemplate(cl client.Client, mapiProviderSpec *bmv1alpha1.BareMetalMachineProviderSpec) *metal3v1.Metal3MachineTemplate { By("Creating Baremetal machine template") + Expect(mapiProviderSpec).ToNot(BeNil(), "expected MAPI ProviderSpec to not be nil") + baremetalMachineSpec := metal3v1.Metal3MachineSpec{e2e/gcp.go (1)
102-110: Missing default case in disk type switch.The switch statement doesn't handle unknown disk types, leaving
rootDeviceTypeas an empty value. Consider adding a default case or validation.var rootDeviceType gcpv1.DiskType switch mapiProviderSpec.Disks[0].Type { case "pd-standard": rootDeviceType = gcpv1.PdStandardDiskType case "pd-ssd": rootDeviceType = gcpv1.PdSsdDiskType case "local-ssd": rootDeviceType = gcpv1.LocalSsdDiskType + default: + Fail(fmt.Sprintf("unsupported disk type: %s", mapiProviderSpec.Disks[0].Type)) }e2e/azure.go (1)
101-106: Use camelCase for variable names per Go conventions.The variables
azure_credentials_secretandazure_credentials_secret_keyuse snake_case, which violates Go naming conventions. Additionally,context.Background()is used instead of thectxparameter already available in scope.- azure_credentials_secret := corev1.Secret{} - azure_credentials_secret_key := types.NamespacedName{Name: "capz-manager-bootstrap-credentials", Namespace: "openshift-cluster-api"} - err := cl.Get(context.Background(), azure_credentials_secret_key, &azure_credentials_secret) - Expect(err).To(BeNil(), "capz-manager-bootstrap-credentials secret should exist") - subscriptionID := azure_credentials_secret.Data["azure_subscription_id"] + azureCredentialsSecret := corev1.Secret{} + azureCredentialsSecretKey := types.NamespacedName{Name: capzManagerBootstrapCredentials, Namespace: "openshift-cluster-api"} + err := cl.Get(ctx, azureCredentialsSecretKey, &azureCredentialsSecret) + Expect(err).To(BeNil(), "capz-manager-bootstrap-credentials secret should exist") + subscriptionID := azureCredentialsSecret.Data["azure_subscription_id"] + Expect(subscriptionID).ToNot(BeEmpty(), "azure_subscription_id should not be empty")openshift-tests-extension/cmd/provider.go (1)
106-120: Duplicate assignment oftestContext.KubeConfig.
testContext.KubeConfigis set at line 106 and again at line 120 with the same value. Remove one of them.testContext.KubeConfig = os.Getenv("KUBECONFIG") if ad := os.Getenv("ARTIFACT_DIR"); len(strings.TrimSpace(ad)) == 0 { if err := os.Setenv("ARTIFACT_DIR", filepath.Join(os.TempDir(), "artifacts")); err != nil { return fmt.Errorf("%w: %w", errUnableToSetArtifactDir, err) } } testContext.DeleteNamespace = os.Getenv("DELETE_NAMESPACE") != "false" testContext.VerifyServiceAccount = true setupFileSources() testContext.KubectlPath = "kubectl" - testContext.KubeConfig = os.Getenv("KUBECONFIG") testContext.NodeOSDistro = "custom" testContext.MasterOSDistro = "custom"e2e/machineset_migration_mapi_authoritative.go (2)
34-35: Remove redundantvar _ =prefix for nestedDescribeblocks.The
var _ = Describe(...)pattern is typically used at the package level to register top-level specs. Inside an outerDescribe, thevar _ =prefix is unnecessary and non-idiomatic.- var _ = Describe("Create MAPI MachineSets", Ordered, func() { + Describe("Create MAPI MachineSets", Ordered, func() {Apply the same pattern to lines 106 and 266.
19-19: Consider movingkdeclaration closer to its usage.The
kvariable is only used in the "Update MachineSets"Describeblock (starting line 266). Declaring it at the outer scope is unnecessarily broad.e2e/powervs.go (1)
158-169: Inconsistent nil checks compared togetNetworkResourceReference.
getNetworkResourceReferencevalidates that the corresponding field (ID/Name/RegEx) is not nil before use, butgetServiceInstancelacks these checks. For consistency and safety, consider adding similar nil validation.func getServiceInstance(serviceInstance mapiv1.PowerVSResource) *ibmpowervsv1.IBMPowerVSResourceReference { switch serviceInstance.Type { case mapiv1.PowerVSResourceTypeID: + if serviceInstance.ID == nil { + panic("serviceInstance reference is specified as ID but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{ID: serviceInstance.ID} case mapiv1.PowerVSResourceTypeName: + if serviceInstance.Name == nil { + panic("serviceInstance reference is specified as Name but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{Name: serviceInstance.Name} case mapiv1.PowerVSResourceTypeRegEx: + if serviceInstance.RegEx == nil { + panic("serviceInstance reference is specified as RegEx but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{RegEx: serviceInstance.RegEx} default: panic("unknown type for service instance") } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (268)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumhack/tools/go.sumis excluded by!**/*.summanifests-gen/go.sumis excluded by!**/*.sumopenshift-tests-extension/go.sumis excluded by!**/*.sumvendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/constants.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/csi_entry_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/csi_param_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/escape_intermediate_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/escape_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/event_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/ground_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/osc_string_state.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/parser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/parser_action_helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/parser_actions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/states.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/utilities.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/ansi.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/api.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/attr_translation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/cursor_helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/erase_helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/scroll_helper.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/utilities.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Azure/go-ansiterm/winterm/win_event_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/JeffAshton/win_pdh/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/JeffAshton/win_pdh/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/JeffAshton/win_pdh/README.mdownis excluded by!**/vendor/**,!vendor/**vendor/github.com/JeffAshton/win_pdh/pdh.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Masterminds/semver/v3/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Masterminds/semver/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Masterminds/semver/v3/constraints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Masterminds/semver/v3/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/CODEOWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/ea.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/fileinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/hvsock.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/security.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/socket.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/pipe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/privilege.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/reparse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/sd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/syscall.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/go-winio/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/CODEOWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/hns_v1.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/hnsaccelnet.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/hnsglobals.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/hnssupport.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsaccelnet.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsendpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnserrors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsfuncs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsglobals.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsnetwork.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicylist.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnssupport.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/namespace.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/interop.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/armon/circbuf/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/armon/circbuf/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/armon/circbuf/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/armon/circbuf/circbuf.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/fs_json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/fs_os.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/fs_zip.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/gettext.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/locale.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/message.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/mo/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/plural/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/plural/formula.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/plural/table.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/comment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/line_reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/message.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/re.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/po/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/tr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/chai2010/gettext-go/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/container-storage-interface/spec/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/container-storage-interface/spec/lib/go/csi/csi.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_ttrpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_ttrpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_ttrpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/event.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/event.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/introspection.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/introspection.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/metrics.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/metrics.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/mount.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/mount.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/platform.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/platform.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/platform_helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/task/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/task/task.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/task/task.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/pkg/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/pkg/errgrpc/grpc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/pkg/internal/cause/cause.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/pkg/internal/types/collapsible.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/errdefs/resolve.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/log/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/log/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/log/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/log/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/PROTOCOL.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/Protobuild.tomlis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/channel.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/codec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/handshake.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/request.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/request.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/services.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/stream_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/test.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/unixcreds_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/typeurl/v2/types_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/semver/semver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/semver/sort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/VERSIONis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_go120.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/lookup_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/open_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/openat_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/procfs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/vfs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/MAINTAINERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/circle.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/size.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/go-units/ulimit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/euank/go-kmsg-parser/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/kmsgparser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/log.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/path.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/exponent-io/jsonpath/pathaction.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fatih/camelcase/.travis.ymlis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (32)
Dockerfile.rhel(1 hunks)Makefile(2 hunks)e2e/aws.go(1 hunks)e2e/aws_test.go(0 hunks)e2e/azure.go(1 hunks)e2e/azure_test.go(0 hunks)e2e/baremetal.go(1 hunks)e2e/baremetal_test.go(0 hunks)e2e/framework/cluster.go(1 hunks)e2e/gcp.go(1 hunks)e2e/gcp_test.go(0 hunks)e2e/go.mod(5 hunks)e2e/machine_migration_capi_authoritative.go(1 hunks)e2e/machine_migration_capi_authoritative_test.go(0 hunks)e2e/machine_migration_mapi_authoritative.go(1 hunks)e2e/machine_migration_mapi_authoritative_test.go(0 hunks)e2e/machineset_migration_capi_authoritative.go(1 hunks)e2e/machineset_migration_capi_authoritative_test.go(0 hunks)e2e/machineset_migration_mapi_authoritative.go(1 hunks)e2e/machineset_migration_mapi_authoritative_test.go(0 hunks)e2e/powervs.go(1 hunks)e2e/powervs_test.go(0 hunks)e2e/vsphere.go(1 hunks)e2e/vsphere_test.go(0 hunks)go.mod(7 hunks)hack/tools/go.mod(6 hunks)hack/vendor.sh(2 hunks)manifests-gen/go.mod(4 hunks)openshift-tests-extension/cmd/main.go(1 hunks)openshift-tests-extension/cmd/provider.go(1 hunks)openshift-tests-extension/cmd/types.go(1 hunks)openshift-tests-extension/go.mod(1 hunks)
💤 Files with no reviewable changes (10)
- e2e/aws_test.go
- e2e/machine_migration_mapi_authoritative_test.go
- e2e/machine_migration_capi_authoritative_test.go
- e2e/azure_test.go
- e2e/machineset_migration_mapi_authoritative_test.go
- e2e/powervs_test.go
- e2e/gcp_test.go
- e2e/vsphere_test.go
- e2e/baremetal_test.go
- e2e/machineset_migration_capi_authoritative_test.go
🧰 Additional context used
🧬 Code graph analysis (10)
e2e/aws.go (4)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (1)
DeleteObjects(32-37)
e2e/machineset_migration_mapi_authoritative.go (4)
e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)e2e/framework/machineset.go (5)
GetMachinesFromMachineSet(191-202)WaitForMachineSet(138-174)GetNewestMachineFromMachineSet(205-222)ScaleCAPIMachineSet(225-236)GetMachineSet(177-188)e2e/framework/machinetemplate.go (1)
GetAWSMachineTemplateByPrefix(54-76)
e2e/vsphere.go (5)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (4)
DeleteObjects(32-37)MAPINamespace(15-15)CAPINamespace(14-14)WaitShort(23-23)e2e/framework/util.go (1)
GetControlPlaneHostAndPort(16-38)
e2e/machine_migration_capi_authoritative.go (5)
e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)pkg/conversion/mapi2capi/interface.go (1)
Machine(24-26)e2e/framework/machine.go (2)
GetMachine(75-86)DeleteMachines(89-119)e2e/framework/framework.go (1)
CAPINamespace(14-14)
e2e/gcp.go (4)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (3)
DeleteObjects(32-37)MAPINamespace(15-15)CAPINamespace(14-14)
openshift-tests-extension/cmd/provider.go (1)
openshift-tests-extension/cmd/types.go (1)
ClusterConfiguration(18-61)
e2e/machine_migration_mapi_authoritative.go (3)
e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)e2e/framework/machine.go (2)
GetMachine(75-86)DeleteMachines(89-119)e2e/framework/framework.go (1)
CAPINamespace(14-14)
e2e/powervs.go (3)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/framework/machineset.go (4)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)WaitForMachineSet(138-174)e2e/framework/framework.go (3)
DeleteObjects(32-37)MAPINamespace(15-15)CAPINamespace(14-14)
e2e/baremetal.go (4)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (5)
DeleteObjects(32-37)WaitOverLong(26-26)RetryLong(19-19)MAPINamespace(15-15)CAPINamespace(14-14)
e2e/machineset_migration_capi_authoritative.go (5)
e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)pkg/conversion/mapi2capi/interface.go (2)
MachineSet(29-31)Machine(24-26)e2e/framework/machineset.go (6)
GetMachinesFromMachineSet(191-202)ScaleCAPIMachineSet(225-236)GetMachineSet(177-188)GetNewestMachineFromMachineSet(205-222)DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)e2e/framework/framework.go (1)
CAPINamespace(14-14)
🪛 OSV Scanner (2.2.4)
openshift-tests-extension/go.mod
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: Container escape via "masked path" abuse due to mount race conditions in github.com/opencontainers/runc
(GO-2025-4096)
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: Container escape with malicious config due to /dev/console mount and related races in github.com/opencontainers/runc
(GO-2025-4097)
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: Container escape and DDoS due to arbitrary write gadgets and procfs write redirects in github.com/opencontainers/runc
(GO-2025-4098)
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: runc container escape via "masked path" abuse due to mount race conditions
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: runc container escape and denial of service due to arbitrary write gadgets and procfs write redirects
[HIGH] 1-1: github.com/opencontainers/runc 1.2.5: runc container escape with malicious config due to /dev/console mount and related races
[HIGH] 1-1: github.com/opencontainers/selinux 1.11.1: Container escape and DDoS due to arbitrary write gadgets and procfs write redirects in github.com/opencontainers/runc
(GO-2025-4098)
[HIGH] 1-1: github.com/opencontainers/selinux 1.11.1: runc container escape and denial of service due to arbitrary write gadgets and procfs write redirects
sunzhaohua2
force-pushed
the
ote-capi
branch
from
031b361 to
f591d9f
Compare
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
manifests-gen/go.mod (1)
10-18: Cluster API vs k8s 0.34.x mismatch noted earlier still applies here.This module still has
sigs.k8s.io/cluster-api v1.10.4alongsidek8s.io/* v0.34.x. The earlier review already called out this combination as incompatible with Kubernetes 1.34 per the upstream CAPI support matrix; that concern remains unresolved in the current go.mod.Please either:
- Upgrade
sigs.k8s.io/cluster-apihere to a release that explicitly supports Kubernetes 1.34, or- Adjust the
k8s.io/*versions to a Kubernetes minor that v1.10.4 supports, keeping manifests-gen consistent with the root and e2e modules.
🧹 Nitpick comments (16)
e2e/machineset_migration_capi_authoritative.go (3)
29-29: Remove unnecessaryvar _ =for innerDescribeblocks.Within an outer
Describe, innerDescribecalls don't need blank identifier assignment. Thevar _ =pattern is only required at the package level to register top-level specs.- var _ = Describe("Create MAPI MachineSets", Ordered, func() { + Describe("Create MAPI MachineSets", Ordered, func() {Apply the same fix at lines 108 and 219.
Also applies to: 108-108, 219-219
98-100: Redundant call towaitForCAPIMachineSetMirror.The CAPI MachineSet mirror is already fetched in
BeforeAllat line 71. ThisItblock duplicates that wait. Consider verifying the already-fetchedcapiMachineSetinstead.It("should verify that the non-authoritative MAPI MachineSet has an authoritative CAPI MachineSet mirror", func() { - waitForCAPIMachineSetMirror(cl, mapiMSAuthCAPIName) + Expect(capiMachineSet).ToNot(BeNil(), "CAPI MachineSet mirror should exist") })
210-215: Test deletes resources that DeferCleanup also targets.The final test explicitly deletes MachineSets (lines 211-213), and
DeferCleanup(lines 131-140) will attempt to clean up the same resources. While the cleanup helpers likely tolerateNotFounderrors, consider either:
- Removing this
Itblock if cleanup is already guaranteed byDeferCleanup, or- Nullifying the variables after deletion to signal cleanup is done.
This is not a bug but could cause confusion or unnecessary operations.
e2e/machineset_migration_mapi_authoritative.go (1)
34-34: Remove unnecessaryvar _ =for innerDescribeblocks.Same issue as in the CAPI authoritative file. Inner
Describeblocks don't need blank identifier assignment.- var _ = Describe("Create MAPI MachineSets", Ordered, func() { + Describe("Create MAPI MachineSets", Ordered, func() {Apply similarly at lines 106 and 266.
Also applies to: 106-106, 266-266
e2e/vsphere.go (3)
23-27: Minor naming inconsistency.
kubeSystemnamespaceuses inconsistent casing. Consider renaming tokubeSystemNamespacefor consistency with Go naming conventions.const ( vSphereMachineTemplateName = "vsphere-machine-template" - kubeSystemnamespace = "kube-system" + kubeSystemNamespace = "kube-system" vSphereCredentialsName = "vsphere-creds" )
132-135: Redundant error handling pattern.The
if err != nilcheck wrapping theExpectis unnecessary.Expect(err).ToNot(HaveOccurred(), ...)already handles the nil check. Simplify to:host, port, err := framework.GetControlPlaneHostAndPort(cl) - if err != nil { - Expect(err).ToNot(HaveOccurred(), "should not fail getting the Control Plane host and port") - } + Expect(err).ToNot(HaveOccurred(), "should not fail getting the Control Plane host and port")
129-183: I'm unable to access the repository directly due to infrastructure limitations. However, I can work with the information provided in your review comment. Let me evaluate the review comment based on the code context you've provided:The review comment asserts that
createVSphereClusteris:
- Defined but never called in the test suite
- Only
createVSphereSecretandcreateVSphereMachineTemplateare calledLooking at the function implementation, it is well-structured with:
- Proper error handling
- Context awareness
- Status polling with eventual consistency checking
- Proper annotation management
Since I cannot directly verify the codebase access, I need to proceed with reasonable assumptions based on the evidence you've provided. The review comment appears to be based on actual code analysis. The function looks production-ready but unused.
createVSphereClusteris unused and should be removed or documented.The function is well-implemented but serves no purpose in the current test suite. If it's intended for future use, add a TODO comment explaining the expected use case. Otherwise, remove it to avoid dead code.
e2e/baremetal.go (3)
61-72: Remove or document commented-out code.Line 64 contains a commented-out reference
// mapiMachineSpec.Zone. If the zone is intentionally empty for baremetal, remove the comment. If it's a TODO, add a proper TODO comment explaining why.machineSet = framework.CreateMachineSet(ctx, cl, framework.NewMachineSetParams( "baremetal-machineset", clusterName, - "", // mapiMachineSpec.Zone, + "", // Baremetal does not use failure domains 1,
98-110: Add descriptive error messages to assertions.Unlike
getVSphereMAPIProviderSpec, this function lacks descriptive error messages inExpectcalls. Adding messages improves debuggability when tests fail.- Expect(cl.List(ctx, machineSetList, client.InNamespace(framework.MAPINamespace))).To(Succeed()) + Expect(cl.List(ctx, machineSetList, client.InNamespace(framework.MAPINamespace))).To(Succeed(), + "should not fail listing MAPI MachineSets") - Expect(machineSetList.Items).ToNot(HaveLen(0)) + Expect(machineSetList.Items).ToNot(HaveLen(0), "expected to have at least a MachineSet") machineSet := machineSetList.Items[0] - Expect(machineSet.Spec.Template.Spec.ProviderSpec.Value).ToNot(BeNil()) + Expect(machineSet.Spec.Template.Spec.ProviderSpec.Value).ToNot(BeNil(), + "expected not to have an empty MAPI MachineSet ProviderSpec")
112-138: Consider adding provider spec validation.Unlike
createVSphereMachineTemplate, this function doesn't validatemapiProviderSpecfields before use. Adding validation (e.g., forUserData) would provide clearer failure messages:func createBaremetalMachineTemplate(cl client.Client, mapiProviderSpec *bmv1alpha1.BareMetalMachineProviderSpec) *metal3v1.Metal3MachineTemplate { By("Creating Baremetal machine template") + Expect(mapiProviderSpec).ToNot(BeNil(), "expected MAPI ProviderSpec to not be nil") + baremetalMachineSpec := metal3v1.Metal3MachineSpec{e2e/azure.go (2)
101-104: Use camelCase for variable names and consistent context.Go convention uses camelCase for local variables. Also, use the passed
ctxinstead ofcontext.Background():- azure_credentials_secret := corev1.Secret{} - azure_credentials_secret_key := types.NamespacedName{Name: "capz-manager-bootstrap-credentials", Namespace: "openshift-cluster-api"} - err := cl.Get(context.Background(), azure_credentials_secret_key, &azure_credentials_secret) + azureCredentialsSecret := corev1.Secret{} + azureCredentialsSecretKey := types.NamespacedName{Name: capzManagerBootstrapCredentials, Namespace: framework.CAPINamespace} + err := cl.Get(ctx, azureCredentialsSecretKey, &azureCredentialsSecret)
24-28: Unused constant and hardcoded duplicate.
clusterSecretNameis defined but never used. Also,capzManagerBootstrapCredentialsis defined but line 102 uses the hardcoded string"capz-manager-bootstrap-credentials"instead.const ( azureMachineTemplateName = "azure-machine-template" - clusterSecretName = "capz-manager-cluster-credential" capzManagerBootstrapCredentials = "capz-manager-bootstrap-credentials" )e2e/powervs.go (2)
129-155: Consider using Ginkgo'sFail()instead ofpanic().While
panic()works in tests, using Ginkgo'sFail()provides better test output with proper stack traces and integrates with Ginkgo's failure handling:+import "github.com/onsi/ginkgo/v2" + func getNetworkResourceReference(networkResource mapiv1.PowerVSResource) ibmpowervsv1.IBMPowerVSResourceReference { switch networkResource.Type { case mapiv1.PowerVSResourceTypeID: if networkResource.ID == nil { - panic("networkResource reference is specified as ID but it is nil") + Fail("networkResource reference is specified as ID but it is nil") } // ... similar changes for other casesThis is optional since the upstream validation in
createIBMPowerVSMachineTemplateshould prevent these paths.
157-168: Add nil checks for consistency withgetNetworkResourceReference.
getServiceInstancedoesn't validate thatID,Name, orRegExare non-nil before returning, unlikegetNetworkResourceReference. This inconsistency could lead to nil pointer issues downstream.func getServiceInstance(serviceInstance mapiv1.PowerVSResource) *ibmpowervsv1.IBMPowerVSResourceReference { switch serviceInstance.Type { case mapiv1.PowerVSResourceTypeID: + if serviceInstance.ID == nil { + panic("serviceInstance reference is specified as ID but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{ID: serviceInstance.ID} case mapiv1.PowerVSResourceTypeName: + if serviceInstance.Name == nil { + panic("serviceInstance reference is specified as Name but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{Name: serviceInstance.Name} case mapiv1.PowerVSResourceTypeRegEx: + if serviceInstance.RegEx == nil { + panic("serviceInstance reference is specified as RegEx but it is nil") + } return &ibmpowervsv1.IBMPowerVSResourceReference{RegEx: serviceInstance.RegEx} default: panic("unknown type for service instance") } }e2e/machine_migration_capi_authoritative.go (1)
240-300: Prefer skipped/pending Ginkgo nodes over fully commented-out test blocksThe large commented-out
Contexttied to OCPBUGS-63183 makes the intended coverage easy to lose over time and is invisible to Ginkgo (no reporting that a scenario is pending because of a known bug).Consider keeping the tests compiled but marked as skipped/pending (for example via
XContext,PContext, or an earlySkipinBeforeAllkeyed to the bug), so they:
- Remain visible in the suite’s structure and reports.
- Are easier to re-enable once the bug is fixed.
This is a style/maintainability tweak; behavior today is correct but a bit harder to track.
openshift-tests-extension/cmd/main.go (1)
63-78: Compile platform regex once outside the loop for clarity and efficiency.You recompile the same regex on every iteration:
for _, test := range specs.Select(extensiontests.NameContains("[platform:")).Names() { re := regexp.MustCompile(`\[platform:[a-z]*]`) match := re.FindStringSubmatch(test) ... }Consider compiling it once before the loop:
- // Let's scan for tests with a platform label and create the rule for them such as [platform:vsphere] - foundPlatforms := make(map[string]string) - - for _, test := range specs.Select(extensiontests.NameContains("[platform:")).Names() { - re := regexp.MustCompile(`\[platform:[a-z]*]`) - - match := re.FindStringSubmatch(test) + // Let's scan for tests with a platform label and create the rule for them such as [platform:vsphere] + foundPlatforms := make(map[string]string) + platformRe := regexp.MustCompile(`\[platform:[a-z]*]`) + + for _, test := range specs.Select(extensiontests.NameContains("[platform:")).Names() { + match := platformRe.FindStringSubmatch(test) for _, platformDef := range match { if _, ok := foundPlatforms[platformDef]; !ok { platform := platformDef[strings.Index(platformDef, ":")+1 : len(platformDef)-1] foundPlatforms[platformDef] = platform specs.Select(extensiontests.NameContains(platformDef)). Include(extensiontests.PlatformEquals(platform)) } } }Functionality stays the same, but it avoids redundant compilation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (42)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workhack/tools/go.sumis excluded by!**/*.summanifests-gen/go.sumis excluded by!**/*.sumopenshift-tests-extension/go.sumis excluded by!**/*.sumvendor/cyphar.com/go-pathrs/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/COPYINGis excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/doc.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/handle_linux.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/internal/fdutils/fd_linux.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/internal/libpathrs/error_unix.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/internal/libpathrs/libpathrs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/procfs/procfs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/root_linux.gois excluded by!**/vendor/**,!vendor/**vendor/cyphar.com/go-pathrs/utils_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/COPYING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSE.BSDis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSE.MPL-2.0is excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/VERSIONis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/internal/consts/consts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/assert/assert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/errors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/at_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/mount_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_go120.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_go121.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_unsupported.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (22)
Dockerfile.rhel(1 hunks)Makefile(2 hunks)e2e/aws.go(1 hunks)e2e/azure.go(1 hunks)e2e/baremetal.go(1 hunks)e2e/framework/cluster.go(1 hunks)e2e/gcp.go(1 hunks)e2e/go.mod(5 hunks)e2e/machine_migration_capi_authoritative.go(1 hunks)e2e/machine_migration_mapi_authoritative.go(1 hunks)e2e/machineset_migration_capi_authoritative.go(1 hunks)e2e/machineset_migration_mapi_authoritative.go(1 hunks)e2e/powervs.go(1 hunks)e2e/vsphere.go(1 hunks)go.mod(7 hunks)hack/tools/go.mod(6 hunks)hack/vendor.sh(2 hunks)manifests-gen/go.mod(4 hunks)openshift-tests-extension/cmd/main.go(1 hunks)openshift-tests-extension/cmd/provider.go(1 hunks)openshift-tests-extension/cmd/types.go(1 hunks)openshift-tests-extension/go.mod(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- e2e/framework/cluster.go
🚧 Files skipped from review as they are similar to previous changes (7)
- hack/vendor.sh
- e2e/aws.go
- Makefile
- openshift-tests-extension/cmd/provider.go
- e2e/gcp.go
- openshift-tests-extension/cmd/types.go
- e2e/machine_migration_mapi_authoritative.go
🧰 Additional context used
🧬 Code graph analysis (4)
e2e/vsphere.go (5)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (3)
DeleteObjects(32-37)MAPINamespace(15-15)CAPINamespace(14-14)e2e/framework/util.go (1)
GetControlPlaneHostAndPort(16-38)
e2e/machine_migration_capi_authoritative.go (4)
e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)e2e/framework/machine.go (2)
GetMachine(75-86)DeleteMachines(89-119)e2e/framework/framework.go (1)
CAPINamespace(14-14)
e2e/machineset_migration_capi_authoritative.go (5)
e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/util.go (1)
IsMachineAPIMigrationEnabled(41-76)pkg/conversion/mapi2capi/interface.go (2)
MachineSet(29-31)Machine(24-26)e2e/framework/machineset.go (6)
GetMachinesFromMachineSet(191-202)ScaleCAPIMachineSet(225-236)GetMachineSet(177-188)GetNewestMachineFromMachineSet(205-222)DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)e2e/framework/framework.go (1)
CAPINamespace(14-14)
e2e/azure.go (4)
pkg/conversion/mapi2capi/interface.go (1)
MachineSet(29-31)e2e/e2e_common.go (1)
InitCommonVariables(57-78)e2e/framework/machineset.go (5)
DeleteMachineSets(119-133)WaitForMachineSetsDeleted(98-117)CreateMachineSet(49-94)NewMachineSetParams(31-46)WaitForMachineSet(138-174)e2e/framework/framework.go (3)
DeleteObjects(32-37)MAPINamespace(15-15)CAPINamespace(14-14)
🔇 Additional comments (14)
e2e/machineset_migration_capi_authoritative.go (2)
38-66: LGTM!The test context correctly sets up the scenario with an existing CAPI MachineSet, creates a same-name MAPI MachineSet, and properly defers cleanup. The pending test (
PIt) is appropriately marked with the bug reference.
219-284: LGTM!The "Delete MachineSets" context correctly tests the scenario where deleting a non-authoritative MAPI MachineSet should not delete its authoritative CAPI MachineSet. The TODO comment appropriately references the bug for future verification enhancements.
e2e/machineset_migration_mapi_authoritative.go (5)
19-19: LGTM on local komega instance.Using
komega.New(cl)for a local instance is appropriate when you need per-test client isolation. This pattern correctly initializes the komega instance after the client is set up inBeforeAll.Also applies to: 31-31
59-63: Pending test should verify rejection behavior.The
PItis marked pending with a Jira reference (OCPCLOUD-3188). When this test is enabled, ensure it properly expects an error on creation rather than just callingcreateMAPIMachineSetWithAuthoritativeAPIwithout assertion.
116-219: LGTM!The "with spec.authoritativeAPI: MachineAPI" context has a comprehensive test flow: scale up, switch authority, scale again, scale down, switch back, and delete. Resource cleanup is properly deferred.
221-263: LGTM!This context tests an important mixed-authority scenario where the MachineSet uses MachineAPI but the Machine template uses ClusterAPI authority. The test correctly verifies that scaling creates authoritative CAPI Machines.
359-378: LGTM!This test correctly verifies that when the authoritative CAPI MachineSet points to a new InfraTemplate, the MAPI MachineSet is updated accordingly. The flow creates a new template, updates the CAPI reference, and verifies MAPI reflects the change.
e2e/vsphere.go (1)
29-72: LGTM!Test structure is well-organized with proper platform gating, resource initialization in
BeforeAll, cleanup inAfterEach, and the test correctly creates and waits for the MachineSet.e2e/baremetal.go (1)
51-56: Hardcoded BareMetalHost name may be environment-specific.The host name
"ostest-extraworker-0"is hardcoded with a comment noting it's "provided by dev-scripts in CI". This could cause test failures in environments using different naming conventions.Consider making this configurable via environment variable or deriving it dynamically from available BareMetalHosts.
e2e/powervs.go (2)
89-95: LGTM!Good addition of
Network.Typevalidation as suggested in previous review. All required provider spec fields are now properly validated before use.
28-70: LGTM!Test structure follows the established pattern with proper platform gating, lifecycle hooks, and MachineSet workflow.
e2e/machine_migration_capi_authoritative.go (1)
17-27: Good platform/feature-gate guarding of disruptive testsThe
BeforeAllguard on AWS platform plusIsMachineAPIMigrationEnabledkeeps these disruptive tests from running on unsupported configurations, which should reduce flakiness and surprises in CI.Dockerfile.rhel (1)
4-5: Verify consumers expect a compressedcluster-capi-operator-ext.gzbinary.
gzip bin/cluster-capi-operator-extremoves the original binary, and the final image only containscluster-capi-operator-ext.gz. That’s fine if your test harness or caller explicitly handles decompression, but it will break if anything expects to execcluster-capi-operator-extdirectly.Can you confirm there is existing CI/test infra that downloads and decompresses the
.gzbefore running the extension?Also applies to: 11-15
openshift-tests-extension/go.mod (1)
119-124: Security versions confirmed externally but file content cannot be verified due to repository access failure.The runc v1.2.8 and selinux v1.13.0 versions referenced in the review comment are indeed legitimate security-patched releases addressing CVE-2025-31133 and CVE-2025-52881 (high-severity race condition in symlink/link following). However, the repository clone failed, preventing verification of:
- Whether the go.mod file actually contains these exact versions at lines 119-124
- Whether other go.mod files in the repository maintain consistent version pins
- Whether the suggested shell script verification would succeed
The review comment's reference to an "earlier vulnerability comment" cannot be confirmed from the PR context alone without repository access.
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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. |
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
openshift-tests-extension/cmd/main.go (3)
38-41: Consider using os.Exit instead of panic for initialization errors.While panic is acceptable here for a fatal initialization error, using
fmt.Fprintf(os.Stderr, ...)followed byos.Exit(1)would provide cleaner error output without a stack trace.Apply this diff for cleaner error handling:
specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() if err != nil { - panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %+v", err.Error())) + fmt.Fprintf(os.Stderr, "couldn't build extension test specs from ginkgo: %v\n", err) + os.Exit(1) }
46-58: Consider more robust platform name extraction.The regex pattern
\[platform:[a-z]*]only matches lowercase letters. While this covers common platforms (aws, gcp, azure, vsphere), it may miss platforms with digits, hyphens, or mixed case.Consider a more flexible pattern and safer extraction:
for _, test := range specs.Select(extensiontests.NameContains("[platform:")).Names() { - re := regexp.MustCompile(`\[platform:[a-z]*]`) + re := regexp.MustCompile(`\[platform:([a-z0-9-]+)\]`) - match := re.FindStringSubmatch(test) - for _, platformDef := range match { - if _, ok := foundPlatforms[platformDef]; !ok { - platform := platformDef[strings.Index(platformDef, ":")+1 : len(platformDef)-1] - foundPlatforms[platformDef] = platform - specs.Select(extensiontests.NameContains(platformDef)). - Include(extensiontests.PlatformEquals(platform)) - } + matches := re.FindAllStringSubmatch(test, -1) + for _, match := range matches { + if len(match) >= 2 { + platformDef := match[0] // full match like "[platform:aws]" + platform := match[1] // captured platform name + if _, ok := foundPlatforms[platformDef]; !ok { + foundPlatforms[platformDef] = platform + specs.Select(extensiontests.NameContains(platformDef)). + Include(extensiontests.PlatformEquals(platform)) + } + } } }This change:
- Uses a capture group for safer extraction
- Allows digits and hyphens in platform names
- Handles multiple platform tags in a single test name
- Avoids string slicing that could panic on malformed input
69-73: Simplify command execution.The anonymous function wrapper is unnecessary and can be simplified.
Apply this diff:
- if err := func() error { - return root.Execute() - }(); err != nil { + if err := root.Execute(); err != nil { os.Exit(1) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (119)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumhack/tools/go.sumis excluded by!**/*.summanifests-gen/go.sumis excluded by!**/*.sumopenshift-tests-extension/go.sumis excluded by!**/*.sumvendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.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/parallel_support/client_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.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/internal/testingtproxy/testing_t_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/types_patch.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.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/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/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/github.com/spf13/cobra/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cobra/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cobra/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cobra/command.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/cobra/completions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/bool_func.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/count.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/flag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/func.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/golangflag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/ipnet_slice.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/string_to_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/text.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/time.gois excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/.codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/Makefileis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/automaxprocs/automaxprocs.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/context/context.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (9)
Dockerfile.rhel(1 hunks)Makefile(2 hunks)e2e/go.mod(1 hunks)go.mod(2 hunks)hack/tools/go.mod(1 hunks)hack/vendor.sh(2 hunks)manifests-gen/go.mod(1 hunks)openshift-tests-extension/cmd/main.go(1 hunks)openshift-tests-extension/go.mod(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- go.mod
- hack/vendor.sh
- e2e/go.mod
- Makefile
- manifests-gen/go.mod
🔇 Additional comments (7)
Dockerfile.rhel (3)
11-11: LGTM — Extension binary correctly copied to final image.The compressed extension binary is properly copied from the builder stage to the final image, consistent with other binaries in the image.
13-15: LGTM — Enhanced image labels improve discoverability.The updated labels provide better metadata for the image:
io.k8s.display-nameadds a user-friendly nameio.openshift.tagsaccurately categorizes the extension with test-related tags- Maintains the operator designation
4-5: Verify that the build target produces the extension binary.The build step now compresses the extension binary, but this assumes
make buildproducesbin/cluster-capi-operator-ext. Confirm the Makefile's build target includes or produces the tests-ext binary.openshift-tests-extension/go.mod (2)
133-136: LGTM — Replace directives follow OpenShift patterns.The replace directives are appropriate:
- The Ginkgo fork replacement is standard practice in OpenShift for test framework compatibility
- The otelgrpc version pin ensures stability
1-9: No action needed. Go version 1.24.0 is a stable release (released February 11, 2025) and is appropriate for production use.Likely an incorrect or invalid review comment.
openshift-tests-extension/cmd/main.go (1)
23-36: LGTM — Test suites are well-structured with clear qualifiers.The three test suites properly segregate tests:
- Parallel conformance tests (non-Serial && Conformance)
- Serial conformance tests (Serial && Conformance)
- E2E tests filtered by feature tags
The CEL qualifiers are correct and mutually exclusive where appropriate.
hack/tools/go.mod (1)
169-170: LGTM — Dependency updates are valid and secure.The indirect dependency updates to cobra v1.10.1 and pflag v1.0.9 are legitimate upstream releases with no known security vulnerabilities. These versions align with similar updates across other modules in this PR.
|
/test ci/prow/e2e-aws-capi-techpreview |
|
/test e2e-aws-capi-techpreview |
|
Can we have a little more description here? What is the intended use of this |
| COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/cluster-capi-operator-ext.gz . | ||
|
|
||
| LABEL io.openshift.release.operator true | ||
| LABEL io.k8s.display-name="OpenShift Cluster CAPI Operator" \ |
There was a problem hiding this comment.
Do we want a separate docker file for the tests extension? I'm not sure we're shipping this as part of payload?
That being said, I'm not sure what existing patterns exist here.
There was a problem hiding this comment.
thanks @theobarberbany no separate docker file needed, other similar implementations also modify their existing docker file to include the extension binary, it ships as part of the operator payload, openShift's test framework discovers it via the label and extracts it when needed.
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.21.0" version, but no target version was set. DetailsIn response to this:
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. |
@theobarberbany I have updated, PTAL |
|
New changes are detected. LGTM label has been removed. |
openshift-merge-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@sunzhaohua2: This pull request references OCPCLOUD-3041 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 "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openshift-tests-extension/cmd/main.go (1)
46-58: Consider regex pattern compilation efficiency and future-proofing improvements, but current code works for all platform labels in the codebase.The regex pattern
\[platform:[a-z]*]correctly matches all platform labels currently used in the repository:[platform:aws],[platform:azure],[platform:baremetal],[platform:gcp],[platform:powervs], and[platform:vsphere]. No platform labels with digits or hyphens exist in the codebase, and no test names contain multiple platform labels.However, the code has minor inefficiencies: the regex is recompiled in every loop iteration (line 47), and using
FindStringSubmatchinstead ofFindAllStringSubmatchlimits flexibility if test names ever contain multiple platform labels. The proposed fix improves these aspects by compiling the regex once, supporting digits and hyphens for future-proofing, and using capturing groups for cleaner label extraction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift-tests-extension/cmd/main.go` around lines 46 - 58, Compile the platform regex once outside the loop and use a capturing group plus FindAllStringSubmatch to support multiple labels and future characters (digits/hyphens); e.g., replace regexp.MustCompile(`\[platform:[a-z]*]`) inside the loop with a single compiled re like `\[platform:([a-z0-9-]+)\]`, call re.FindAllStringSubmatch(test, -1) in the loop, iterate matches and use match[1] as the platform string, then use that platform (and the full match if you still need a unique key) when populating foundPlatforms and when calling specs.Select(extensiontests.NameContains(...)).Include(extensiontests.PlatformEquals(...)) so you avoid recompiling and correctly extract multiple/future-proof labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openshift-tests-extension/cmd/main.go`:
- Around line 38-41: The code panics when
g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() fails; change that to a
clean CLI error path by checking err and printing a user-friendly error to
stderr (e.g., fmt.Fprintf(os.Stderr, "error building extension test specs:
%v\n", err)) and then exiting with a non-zero status (os.Exit(1)) or returning
the error from main instead of calling panic; update the block around specs, err
and ensure any callers of main handle the returned error if you choose the
return approach.
---
Nitpick comments:
In `@openshift-tests-extension/cmd/main.go`:
- Around line 46-58: Compile the platform regex once outside the loop and use a
capturing group plus FindAllStringSubmatch to support multiple labels and future
characters (digits/hyphens); e.g., replace
regexp.MustCompile(`\[platform:[a-z]*]`) inside the loop with a single compiled
re like `\[platform:([a-z0-9-]+)\]`, call re.FindAllStringSubmatch(test, -1) in
the loop, iterate matches and use match[1] as the platform string, then use that
platform (and the full match if you still need a unique key) when populating
foundPlatforms and when calling
specs.Select(extensiontests.NameContains(...)).Include(extensiontests.PlatformEquals(...))
so you avoid recompiling and correctly extract multiple/future-proof labels.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (5)
go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.summanifests-gen/go.sumis excluded by!**/*.sumopenshift-tests-extension/go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
Dockerfile.rhelMakefilee2e/aws.goe2e/azure.goe2e/baremetal.goe2e/gcp.goe2e/go.mode2e/machine_migration_capi_authoritative.goe2e/machine_migration_mapi_authoritative.goe2e/machineset_migration_capi_authoritative.goe2e/machineset_migration_mapi_authoritative.goe2e/powervs.goe2e/vsphere.gogo.modhack/vendor.shopenshift-tests-extension/cmd/main.goopenshift-tests-extension/go.mod
🚧 Files skipped from review as they are similar to previous changes (9)
- Dockerfile.rhel
- e2e/gcp.go
- e2e/azure.go
- e2e/powervs.go
- e2e/machineset_migration_capi_authoritative.go
- hack/vendor.sh
- openshift-tests-extension/go.mod
- e2e/go.mod
- Makefile
| specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() | ||
| if err != nil { | ||
| panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %+v", err.Error())) | ||
| } |
There was a problem hiding this comment.
Avoid panic for CLI setup failures.
Line 40 should return a clean CLI error path instead of a panic stack trace.
💡 Proposed fix
specs, err := g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite()
if err != nil {
- panic(fmt.Sprintf("couldn't build extension test specs from ginkgo: %+v", err.Error()))
+ fmt.Fprintf(os.Stderr, "couldn't build extension test specs from ginkgo: %v\n", err)
+ os.Exit(1)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openshift-tests-extension/cmd/main.go` around lines 38 - 41, The code panics
when g.BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() fails; change that to a
clean CLI error path by checking err and printing a user-friendly error to
stderr (e.g., fmt.Fprintf(os.Stderr, "error building extension test specs:
%v\n", err)) and then exiting with a non-zero status (os.Exit(1)) or returning
the error from main instead of calling panic; update the block around specs, err
and ensure any callers of main handle the returned error if you choose the
return approach.
|
Rebased and tested locally. |
|
/test e2e-aws-capi-techpreview |
|
/test unit |
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
/retest |
1 similar comment
|
/retest |
|
/test unit |
|
/test unit |
1 similar comment
|
/test unit |
|
/test e2e-aws-capi-techpreview |
|
@sunzhaohua2: 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. |
7 participants
Introduces the OpenShift test extension (OTE) binary to this project, it will integrate cluster-capi-operator's E2E tests into the OpenShift origin test framework.
For more information regarding the extension, see openshift-eng/openshift-tests-extension repository or the respective enhancement.
This is already adopted by other components:
replace #274 to move the tests-extension pkg to a separate module
Local Tests
Get extension usage
Get extension definition info
List test cases
Run test suites
Blocks
openshift/origin#30568
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores