Skip to content

OCPBUGS-88037: Add e2e test verifying CAPI GCP instance ownership label#596

Open
matzew wants to merge 1 commit into
openshift:mainfrom
matzew:OCPBUGS-88037-gcp-ownership-label-test
Open

OCPBUGS-88037: Add e2e test verifying CAPI GCP instance ownership label#596
matzew wants to merge 1 commit into
openshift:mainfrom
matzew:OCPBUGS-88037-gcp-ownership-label-test

Conversation

@matzew

@matzew matzew commented Jun 11, 2026

Copy link
Copy Markdown
Member

Verify that the GCPCluster object carries the
kubernetes-io-cluster-=owned label that openshift-install destroy cluster relies on to identify and clean up cloud resources.

CAPG propagates AdditionalLabels from the GCPCluster to all managed resources. This test confirms the label is present without any explicit code changes.

Summary by CodeRabbit

  • Tests
    • Enhanced GCP E2E test validation to verify cluster infrastructure ownership and configuration.

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 11, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@matzew: This pull request references Jira Issue OCPBUGS-88037, which is invalid:

  • expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Verify that the GCPCluster object carries the
kubernetes-io-cluster-=owned label that openshift-install destroy cluster relies on to identify and clean up cloud resources.

CAPG propagates AdditionalLabels from the GCPCluster to all managed resources. This test confirms the label is present without any explicit code changes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 30a3ded7-b73d-42f2-b477-3d5aff3f1cd2

📥 Commits

Reviewing files that changed from the base of the PR and between 05c113e and 2e6a5c9.

📒 Files selected for processing (2)
  • e2e/gcp_helpers.go
  • e2e/gcp_test.go

Walkthrough

This pull request adds E2E test verification for GCP cluster ownership labels. A new helper function validates that created CAPI instances have the expected kubernetes-io-cluster-<infraName> label set to "owned", and integrates this check into the existing machine creation test.

Changes

GCP Cluster Ownership Label Verification

Layer / File(s) Summary
Ownership label verification helper and test integration
e2e/gcp_helpers.go, e2e/gcp_test.go
New verifyCAPIInstanceOwnershipLabel(infraName string) helper validates that the GCPCluster for a given infrastructure name exists in the CAPI namespace and contains the correct ownership label with value "owned". The test calls this helper after MachineSet creation to verify the label was properly set.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning Helper function verifyCAPIInstanceOwnershipLabel() uses synchronous cl.Get() without timeout/retry. Other e2e tests wrap such calls in Eventually() blocks (webhook_test.go). This violates the t... Wrap cl.Get() in Eventually() with framework timeout constants (e.g., WaitShort) to handle eventual consistency of API objects, following patterns in webhook_test.go and framework DeleteObjects().
Microshift Test Compatibility ⚠️ Warning New GCP e2e test uses machine.openshift.io APIs (mapiv1beta1.GCPMachineProviderSpec) without MicroShift protection mechanisms ([Skipped:MicroShift], [apigroup:machine.openshift.io], or IsMicroShift... Add [apigroup:machine.openshift.io] tag to test name (line 61) or add exutil.IsMicroShiftCluster() check in BeforeAll to skip on MicroShift.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding an E2E test that verifies CAPI GCP instance ownership labels.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All Ginkgo test names are stable and deterministic with no dynamic information; test titles are static descriptive strings and dynamic values are properly confined to test bodies.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR adds a helper function for label verification and modifies an existing test. No new Ginkgo test blocks (It/Describe/Context/When) were added. The helper only validates GCPCluster metadata,...
Topology-Aware Scheduling Compatibility ✅ Passed PR adds only E2E test code (gcp_helpers.go, gcp_test.go) with no deployment manifests, operator code, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed Code contains no process-level stdout writes; all code is properly contained in test blocks using Ginkgo/Gomega framework which intercepts test output.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Test contains no IPv4 assumptions or external connectivity requirements. It only queries cluster-internal Kubernetes API objects (GCPCluster, MachineSet) to verify label properties.
No-Weak-Crypto ✅ Passed PR contains no cryptographic algorithm usage (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), no custom crypto implementations, and no secret comparisons. Code is pure E2E test verification logic.
Container-Privileges ✅ Passed The PR adds only Go test files (e2e/gcp_helpers.go and e2e/gcp_test.go) with no container/Kubernetes manifests. No privileged security settings were detected in the PR changes.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data (passwords, tokens, API keys, PII, session IDs, hostnames, customer data) exposed in logs. The code only logs cluster name and label keys, which are standard infrastructure identi...

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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

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

This repository is configured in: LGTM mode

@openshift-ci openshift-ci Bot requested review from RadekManak and racheljpg June 11, 2026 15:24
@openshift-ci

openshift-ci Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

Verify that the GCPCluster has the kubernetes-io-cluster-<infraID>=owned
label in AdditionalLabels, which CAPI propagates to all managed resources.
The installer's destroy logic uses this label to find and clean up
CAPI-created GCP resources.

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
@matzew matzew force-pushed the OCPBUGS-88037-gcp-ownership-label-test branch from df314de to 2e6a5c9 Compare June 11, 2026 15:35
@mdbooth

mdbooth commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mdbooth

mdbooth commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-disconnected-techpreview
/test e2e-aws-capi-techpreview
/test e2e-aws-capi-techpreview-post-install
/test e2e-aws-ovn
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-techpreview
/test e2e-aws-ovn-techpreview-upgrade
/test e2e-azure-capi-techpreview
/test e2e-azure-ovn-techpreview
/test e2e-azure-ovn-techpreview-upgrade
/test e2e-gcp-capi-techpreview
/test e2e-gcp-ovn-techpreview
/test e2e-metal3-capi-techpreview
/test e2e-openstack-capi-techpreview
/test e2e-openstack-ovn-techpreview
/test e2e-vsphere-capi-techpreview
/test regression-clusterinfra-aws-ipi-techpreview-capi

@openshift-ci

openshift-ci Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

@matzew: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack-ovn-techpreview 2e6a5c9 link false /test e2e-openstack-ovn-techpreview
ci/prow/e2e-aws-capi-techpreview-post-install 2e6a5c9 link true /test e2e-aws-capi-techpreview-post-install
ci/prow/e2e-gcp-capi-techpreview 2e6a5c9 link true /test e2e-gcp-capi-techpreview
ci/prow/e2e-aws-ovn-techpreview 2e6a5c9 link true /test e2e-aws-ovn-techpreview

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants