Skip to content

Conversation

@chrischdi
Copy link
Contributor

@chrischdi chrischdi commented Dec 10, 2025

Fixup the function introduced in #423 to not use error wrapping and align with openshift/library-go#2064

The first error wrapping would have lead to wrapping also on nil cases.

Summary by CodeRabbit

  • Refactor
    • Improved error handling in the API extensions installation process for more direct error reporting.

Note: This is a technical improvement with no user-facing changes.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci-robot
Copy link

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

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Walkthrough

Modified error handling in applyCustomResourceDefinitionV1Improved to return raw errors instead of wrapping them with fmt.Errorf in the NotFound and generic error branches, while preserving nolint:wrapcheck directives.

Changes

Cohort / File(s) Summary
Error handling refactoring
pkg/controllers/capiinstaller/apiextensions.go
Removed fmt.Errorf wrapping from two error return paths in applyCustomResourceDefinitionV1Improved (NotFound branch and generic error branch), now returning raw errors directly while maintaining nolint:wrapcheck directives

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward error handling refactoring affecting two localized return statements
  • No control flow or logic changes involved
  • Pattern is consistent and repetitive across both modifications

Poem

🐰 Errors unwrapped, now bare and true,
No fmt.Errorf fog in morning dew,
Raw returns dance through the air,
Simplicity reigns with nary a care! 🌿

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title 'capiinstaller: fix error wrapping' directly and accurately describes the main change: removing error wrapping in the capiinstaller package's apiextensions.go file.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@openshift-ci openshift-ci bot requested review from RadekManak and damdo December 10, 2025 06:48
Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@damdo damdo changed the title capiinstaller: fix error wrapping NO-JIRA: capiinstaller: fix error wrapping Dec 10, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 10, 2025
@openshift-ci-robot
Copy link

@chrischdi: This pull request explicitly references no jira issue.

Details

In response to this:

Fixup the function introduced in #423 to not use error wrapping and align with openshift/library-go#2064

The first error wrapping would have lead to wrapping also on nil cases.

Summary by CodeRabbit

  • Refactor
  • Improved error handling in the API extensions installation process for more direct error reporting.

Note: This is a technical improvement with no user-facing changes.

✏️ Tip: You can customize this high-level summary in your review settings.

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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2025
@openshift-ci-robot
Copy link

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-techpreview
/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
Copy link
Contributor

openshift-ci bot commented Dec 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo

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

The pull request process is described here

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

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2025
@damdo
Copy link
Member

damdo commented Dec 10, 2025

@miyadav the regression error is a known issue IIRC, are you able to confirm? Thanks!

@miyadav
Copy link
Member

miyadav commented Dec 10, 2025

@damdo yes , the test looks good on older version ( for gcp we removed webhook check , for aws we need it , will update that ). No issues related to PR.

@miyadav
Copy link
Member

miyadav commented Dec 10, 2025

/verified by @miyadav
ci tests looks good , failed one need update.

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Dec 10, 2025
@openshift-ci-robot
Copy link

@miyadav: This PR has been marked as verified by @miyadav.

Details

In response to this:

/verified by @miyadav
ci tests looks good , failed one need update.

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.

@chrischdi
Copy link
Contributor Author

/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Dec 10, 2025
@chrischdi
Copy link
Contributor Author

/tide refresh

@chrischdi
Copy link
Contributor Author

/override e2e-openstack-ovn-techpreview
/override regression-clusterinfra-aws-ipi-techpreview-capi

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 10, 2025

@chrischdi: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • e2e-openstack-ovn-techpreview
  • regression-clusterinfra-aws-ipi-techpreview-capi

Only the following failed contexts/checkruns were expected:

  • CodeRabbit
  • ci/prow/build
  • ci/prow/e2e-aws-capi-techpreview
  • ci/prow/e2e-aws-ovn
  • ci/prow/e2e-aws-ovn-serial-1of2
  • ci/prow/e2e-aws-ovn-serial-2of2
  • ci/prow/e2e-aws-ovn-techpreview
  • ci/prow/e2e-aws-ovn-techpreview-upgrade
  • ci/prow/e2e-azure-capi-techpreview
  • ci/prow/e2e-azure-ovn-techpreview
  • ci/prow/e2e-azure-ovn-techpreview-upgrade
  • ci/prow/e2e-gcp-capi-techpreview
  • ci/prow/e2e-gcp-ovn-techpreview
  • ci/prow/e2e-metal3-capi-techpreview
  • ci/prow/e2e-openstack-capi-techpreview
  • ci/prow/e2e-openstack-ovn-techpreview
  • ci/prow/e2e-vsphere-capi-techpreview
  • ci/prow/images
  • ci/prow/lint
  • ci/prow/okd-scos-images
  • ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi
  • ci/prow/unit
  • ci/prow/vendor
  • ci/prow/verify-deps
  • pull-ci-openshift-cluster-capi-operator-main-build
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial-1of2
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial-2of2
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-techpreview-upgrade
  • pull-ci-openshift-cluster-capi-operator-main-e2e-azure-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-azure-ovn-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-azure-ovn-techpreview-upgrade
  • pull-ci-openshift-cluster-capi-operator-main-e2e-gcp-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-gcp-ovn-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-metal3-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-openstack-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-openstack-ovn-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-e2e-vsphere-capi-techpreview
  • pull-ci-openshift-cluster-capi-operator-main-images
  • pull-ci-openshift-cluster-capi-operator-main-lint
  • pull-ci-openshift-cluster-capi-operator-main-okd-scos-images
  • pull-ci-openshift-cluster-capi-operator-main-regression-clusterinfra-aws-ipi-techpreview-capi
  • pull-ci-openshift-cluster-capi-operator-main-unit
  • pull-ci-openshift-cluster-capi-operator-main-vendor
  • pull-ci-openshift-cluster-capi-operator-main-verify-deps
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

Details

In response to this:

/override e2e-openstack-ovn-techpreview
/override regression-clusterinfra-aws-ipi-techpreview-capi

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.

@chrischdi
Copy link
Contributor Author

/override ci/prow/e2e-openstack-ovn-techpreview
/override ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi

As mentioned above, looks good

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 10, 2025

@chrischdi: Overrode contexts on behalf of chrischdi: ci/prow/e2e-openstack-ovn-techpreview, ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi

Details

In response to this:

/override ci/prow/e2e-openstack-ovn-techpreview
/override ci/prow/regression-clusterinfra-aws-ipi-techpreview-capi

As mentioned above, looks good

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 10, 2025

@chrischdi: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 8ee10a1 into openshift:main Dec 10, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants