-
Notifications
You must be signed in to change notification settings - Fork 251
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
Upgrade layout from Kubebuilder v2 to Kubebuilder v3.1 #147
Conversation
0cfde06
to
156c821
Compare
Can you add some simple descriptions of the upgrade steps? Since the changed files are huge to have a look at. |
Follow https://book.kubebuilder.io/migration/v2vsv3.html to upgrade fluentbit-operator.
|
cd $$TMP_DIR ;\ | ||
go mod init tmp ;\ | ||
echo "Downloading $(2)" ;\ | ||
GOBIN=$(PROJECT_DIR)/bin go get $(2) ;\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we use go install? For more information, see https://golang.org/doc/go-get-install-deprecation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is automatically generated by kubebuilderv3.1.For the sake of uniformity, it may be not to modify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is automatically generated by kubebuilderv3.1.For the sake of uniformity, it may be not to modify.
Starting in Go 1.17, installing executables with go get is deprecated. go install may be used instead. However, this case would not block this pr.
@@ -24,11 +25,11 @@ import ( | |||
) | |||
|
|||
var ( | |||
// SchemeGroupVersion is group version used to register these objects | |||
SchemeGroupVersion = schema.GroupVersion{Group: "logging.kubesphere.io", Version: "v1alpha2"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to make these changes. Pls revert it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See next comment. Need to fix here or api/generated/clientset/versioned/typed/fluentbitoperator/v1alpha2/fluentbitoperator_client.go:94:8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks,fixed
|
Maybe we need the github-actions-CI test to check it. :) |
@zhu733756 Good point, would you investigate adding CI to this project? |
Ok. Later, will post a pr to add it. |
@bojand @keladhruv @nicktate @patrick-stephens We're upgrading kubebuilder to v3.1, you can take a look at this PR and help to review it if you're interested |
/lgtm |
Makefile changed in pr #152 |
@wenchajun Pls rebase your PR because #152 has been merged. |
Since v3.0+ kubebuilder not include files like v2.3+, which will lead to the test files like suite_test.go being failed. See this fix: |
4ed9080
to
9d0dbdb
Compare
.github/workflows/main.yaml
Outdated
@@ -37,8 +37,8 @@ jobs: | |||
|
|||
- name: Install kubebuilder-2.3.2 | |||
run: | | |||
curl -L "https://github.com/kubernetes-sigs/kubebuilder/releases/download/v2.3.2/kubebuilder_2.3.2_linux_amd64.tar.gz" | tar -xz -C /tmp/ | |||
sudo mv /tmp/kubebuilder_2.3.2_linux_amd64 /usr/local/kubebuilder | |||
curl -L -o kubebuilder https://go.kubebuilder.io/dl/latest/$(go env GOOS)/$(go env GOARCH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd better use a fixed version instead of latest?
aec65bb
to
2789bd6
Compare
Kubebuilderv2 has some differences from v3. The same is true for E2E workflows. Maybe there are some changes here. I modify some E2E files . please review it . |
@wenchajun #154 has been merged, would you resolve the conflict again? |
@zhu733756 would you review Elon's change |
Ok. |
Signed-off-by: chengdehao <[email protected]>
Signed-off-by: chengdehao <[email protected]>
/lgtm See zhu733756#3 |
Follow https://book.kubebuilder.io/migration/v2vsv3.html to upgrade fluentbit-operator
Signed-off-by: root [email protected]