Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate kubebuilder layout to go/v4 #1746

Merged

Conversation

carlkyrillos
Copy link
Contributor

@carlkyrillos carlkyrillos commented Mar 14, 2025

Description

JIRA: RHOAIENG-21155

This PR migrates the project layout from kubebuilder to go/v3 to go/v4.

The changes were made in accordance with the kubebuilder migration documentation which can be found here.

An exhaustive list of major changes can be found in the kubebuilder documentation but the notable changes are:

  • Controllers were moved from controllers to internal/controller
  • Webhooks were moved from controllers/webhooks to internal/webhook
  • APIs were moved from apis to api
  • main.go was moved to cmd/main.go
  • The k8s.io/api/admission/v1beta1 package was deprecated and replaced with k8s.io/api/admission/v1
  • The operator-sdk CLI version was bumped from 1.31.0 to 1.33.0 to be compatible with the go/v4 layout

How Has This Been Tested?

These changes were tested by building operator, bundle, and catalog images, then verifying that the DSC and DSCI CRs can be created and that they successfully reconcile.

The following images were used for testing and can be used for verification:

quay.io/ckyrillo/opendatahub-operator:rhoaieng-21155-5
quay.io/ckyrillo/opendatahub-operator-bundle:rhoaieng-21155-5
quay.io/ckyrillo/opendatahub-operator-catalog:rhoaieng-21155-5

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@openshift-ci openshift-ci bot requested review from CFSNM and ugiordan March 14, 2025 13:56
@carlkyrillos
Copy link
Contributor Author

/assign @lburgazzoli

@zdtsw
Copy link
Member

zdtsw commented Mar 14, 2025

can we wait till ODH 2.26.0 code freeze done to merge this one? and not rush into RHOAI 2.19.0

@ugiordan
Copy link
Member

ugiordan commented Mar 14, 2025

@carlkyrillos Given the number of files to review, just to confirm—most of the changes are related to moving controllers to internal/controller and updating apis to api paths, correct?

can we wait till ODH 2.26.0 code freeze done to merge this one? and not rush into RHOAI 2.19.0
I also agree with @zdtsw, we should wait before merging this.

@carlkyrillos
Copy link
Contributor Author

carlkyrillos commented Mar 14, 2025

can we wait till ODH 2.26.0 code freeze done to merge this one? and not rush into RHOAI 2.19.0

@zdtsw Yes, definitely. I'll put a hold on the PR

@carlkyrillos
Copy link
Contributor Author

Holding until the ODH 2.26.0 code freeze as these changes aren't required for 2.19.0

/hold

@carlkyrillos
Copy link
Contributor Author

@carlkyrillos Given the number of files to review, just to confirm—most of the changes are related to moving controllers to internal/controller and updating apis to api paths, correct?

can we wait till ODH 2.26.0 code freeze done to merge this one? and not rush into RHOAI 2.19.0
I also agree with @zdtsw, we should wait before merging this.

@ugiordan That's correct, the vast majority of the changes are related to moving existing files to api and internal and updating import statements across the codebase to the new paths. There are some small changes to Dockerfiles/Dockerfile, Makefile, PROJECT, and some cmd/component-codegen files but those are also just path updates.

@zdtsw zdtsw added the odh-2.27 label Mar 14, 2025
Copy link

codecov bot commented Mar 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 24.14%. Comparing base (b4ab7d5) to head (89f983d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1746      +/-   ##
==========================================
- Coverage   24.54%   24.14%   -0.41%     
==========================================
  Files         172      171       -1     
  Lines       11874    11782      -92     
==========================================
- Hits         2915     2845      -70     
+ Misses       8679     8661      -18     
+ Partials      280      276       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@carlkyrillos carlkyrillos force-pushed the RHOAIENG-21155 branch 2 times, most recently from 555f90f to bccaede Compare March 21, 2025 14:33
@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

2 similar comments
@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

1 similar comment
@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

@carlkyrillos
Copy link
Contributor Author

/unhold

@carlkyrillos carlkyrillos force-pushed the RHOAIENG-21155 branch 2 times, most recently from e29d1e5 to d7e9e99 Compare March 24, 2025 13:10
@carlkyrillos
Copy link
Contributor Author

/test opendatahub-operator-e2e

@lburgazzoli
Copy link
Member

LGTM

@openshift-ci openshift-ci bot added the lgtm label Mar 24, 2025
Copy link

openshift-ci bot commented Mar 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ugiordan

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

The pull request process is described here

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

Copy link
Member

@zdtsw zdtsw left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-merge-bot openshift-merge-bot bot merged commit c7bb96c into opendatahub-io:main Mar 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants