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.

Sign up for GitHub

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

Jump to bottom

Upgrade operator-sdk to 1.9.0 #150

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Upgrade operator-sdk to 1.9.0 #150

merged 1 commit into from
Jul 8, 2021

Conversation

pliurh
Copy link
Collaborator

@pliurh pliurh commented Jun 25, 2021

Upgrade project to go.kubebuilder.io/v3

@pliurh pliurh requested review from zshi-redhat, adrianchiris and SchSeba and removed request for adrianchiris June 25, 2021 12:17
@pliurh pliurh force-pushed the upstream branch 4 times, most recently from d06b25e to 60910f5 Compare June 28, 2021 11:12
@pliurh pliurh changed the title WIP: Upgrade operator-sdk to 1.9.0 Upgrade operator-sdk to 1.9.0 Jun 28, 2021
zshi-redhat
zshi-redhat reviewed Jul 5, 2021
View reviewed changes
test/util/cluster/cluster.go Outdated
@@ -19,6 +19,28 @@ import (
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/pod"
)

func init() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we shall call the init from sriovv1 like this: https://github.com/k8snetworkplumbingwg/sriov-network-operator/pull/155/files#diff-fa9c480471555909f4af5c083bdc918e887e487b12ab7eb49cc27bf10757bb0cR53 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove init(), as it has been solved by #155.

zshi-redhat
zshi-redhat reviewed Jul 5, 2021
View reviewed changes
controllers/suite_test.go
cfg, err = testEnv.Start()
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())
cfg, err := testEnv.Start()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we change ToNot to NotTo? I see we have both in other places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was generated by kube-builder. Both do the same work. I don't think it matters.

@zshi-redhat
Copy link
Collaborator

Some CustomResourceDefinition still use v1beta1 api version:

./test/util/crds/machineconfiguration.openshift.io_machineconfigs_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1
./test/util/crds/machineconfiguration.openshift.io_machineconfigpools_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1
./test/util/crds/k8s.cni.cncf.io_networkattachmentdefinitions_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1

@pliurh
Copy link
Collaborator Author

pliurh commented Jul 5, 2021

Some CustomResourceDefinition still use v1beta1 api version:

./test/util/crds/machineconfiguration.openshift.io_machineconfigs_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1
./test/util/crds/machineconfiguration.openshift.io_machineconfigpools_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1
./test/util/crds/k8s.cni.cncf.io_networkattachmentdefinitions_crd.yaml:apiVersion: apiextensions.k8s.io/v1beta1

I'll fix this in another PR.

@zshi-redhat
Copy link
Collaborator

Are there any notes when running make generate/manifest/bundle etc after this change?
For example, the operator-sdk version etc?

Shall we add the notes in the commit message?

@pliurh
Upgrade operator-sdk to 1.9.0
bc40302 
Upgrade project to go.kubebuilder.io/v3
@pliurh
Copy link
Collaborator Author

pliurh commented Jul 5, 2021

Are there any notes when running make generate/manifest/bundle etc after this change?
For example, the operator-sdk version etc?

Shall we add the notes in the commit message?

How about creat a HACKING.md file that describes the details in another PR?

@pliurh pliurh merged commit 9d1cdd5 into k8snetworkplumbingwg:master Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zshi-redhat zshi-redhat zshi-redhat left review comments

@SchSeba SchSeba Awaiting requested review from SchSeba

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pliurh @zshi-redhat