migrate to kubebuilder 2.3.1 by dhellmann · Pull Request #655 · metal3-io/baremetal-operator

dhellmann · 2020-09-27T15:05:41Z

This implements the work described in
https://github.com/metal3-io/metal3-docs/blob/master/design/baremetal-operator/kubebuilder-migration.md

It deviates a bit from the work list, because it turns out not to be
necessary to fork controller-tools now that they have added a flag to
support "unsafe" types like our float64 field.

Replaces #650
Fixes #651

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Replace build/Dockerfile with a symlink because we still have CI jobs and quay.io configurations using the file from that location. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

metal3-io-bot · 2020-09-27T15:06:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann

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:

~~OWNERS~~ [dhellmann]

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

dhellmann · 2020-09-27T15:06:45Z

@zaneb @maelk see if this version is easier to follow than #650

dhellmann · 2020-09-27T15:17:52Z

/test-integration

dhellmann · 2020-09-27T16:52:05Z

/test-integration

dhellmann · 2020-09-28T02:48:25Z

/test-integration

maelk

That's an amazing work! I have some details / nit picks but it overall looks good to me.

asalkeld · 2020-09-28T09:08:38Z

this seems really similar idea to k8s.io/apimachinery/pkg/util/wait

Interesting. @andfasano @zaneb I wonder if we would gain anything by using the apimachinery code?

Good to know! I had a quick look at the apimachinery code, but most of the implementation is based on timer, whereas we rely on the RequeueAfter re-enqueueing period field. Very likely the only reusable piece could the Backoff type, not sure if it's worth given the simplicity of the current implementation

dhellmann · 2020-09-28T11:32:11Z

/test-integration

zaneb · 2020-09-28T14:38:16Z

It looks like we now have two copies of the CRD (in deploy/crds/metal3.io_baremetalhosts_crd.yaml and config/crd/bases/metal3.io_baremetalhosts.yaml). Presumably that's required for testing, but I think we should replace the old one with a symlink to ensure we're testing the right thing.

dhellmann · 2020-09-28T16:31:54Z

It looks like we now have two copies of the CRD (in deploy/crds/metal3.io_baremetalhosts_crd.yaml and config/crd/bases/metal3.io_baremetalhosts.yaml). Presumably that's required for testing, but I think we should replace the old one with a symlink to ensure we're testing the right thing.

@maelk and I are going to look at cleaning up the kustomize files from the deploy and config directories as a separate PR after the repo is unblocked. We will eliminate the duplicate as part of that.

dhellmann · 2020-09-28T16:32:19Z

/test-integration

dhellmann · 2020-09-28T16:39:12Z

/test-integration

Move the files into the kubebuilder locations, without modifying them, to make tracking changes in the files easier. Remove auto-generated code that we won't be using after the migration. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Add the BareMetalHost API and controller. Move command line flags controlling which provisioner is used to main.go and make ProvisionerFactory a public member of the reconciler. Move logic for determining the concurrency level to the point at runtime where the reconciler is added to the manager. This lets us return an error instead of forcing an exit. Remove the suite_test.go file generated by kubebuilder, since we don't use that test framework. Fix up unit tests so they work. Minor changes: - Replace r.client calls. - Replace reconcile.Result with ctrl.Result. - Fix package name in controller code - Move controller test setup to one init() since it changes global state $ kubebuilder create api --group metal3.io --version v1alpha1 --kind BareMetalHost Create Resource [y/n] y Create Controller [y/n] y Writing scaffold for you to edit... apis/metal3.io/v1alpha1/baremetalhost_types.go controllers/metal3.io/baremetalhost_controller.go $ rm controllers/metal3.io/suite_test.go Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Use the latest release and pass the arguments we need to allow us to use the float64 field for clock speed and to generate output consistent with what we had from the operator-sdk. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

- Bring version numbers back to what they were before the migration. - Include a make target to tidy and verify them. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

generate.sh: files to watch have moved and expanded gofmt.sh: list go packages to find directories to scan gosec.sh: scan all local packages Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Since we don't have any webhooks, don't configure the server to listen. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Update the manager_auth_proxy_patch.yaml to set the metrics address to the same value used by default in main.go. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

We no longer need to tell people to install the operator-sdk, or to use it to run the tests. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

dhellmann · 2020-09-29T21:26:00Z

/test-integration

maelk · 2020-09-30T05:20:30Z

+ifeq (, $(shell which controller-gen))
+	@{ \
+	set -e ;\
+	CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
+	cd $$CONTROLLER_GEN_TMP_DIR ;\
+	go mod init tmp ;\
+	go get sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION) ;\
+	rm -rf $$CONTROLLER_GEN_TMP_DIR ;\
+	}
+CONTROLLER_GEN=$(GOBIN)/controller-gen
+else
+CONTROLLER_GEN=$(shell which controller-gen)
+endif


This does not ensure that we are running with a specific version of controller-gen. If the user has an outdated controller-gen installed on his machine, it might result in confusing behaviour. Should the executable be placed in a dedicated location, and referred as such, like Kustomize ?

We discussed this today in the community meeting and agreed that I will make this change as a follow-up PR.

zaneb · 2020-09-30T18:34:32Z

/lgtm

digambar15 · 2020-10-01T04:58:03Z

Looks good to me!

dhellmann added 5 commits September 27, 2020 08:40

remove old operator-sdk files

d4cc2c4

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

remove unused test files

fbf01ff

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

remove unused version file

e70ac96

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

move main.go to the kubebuilder location

c1e7fa4

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

move Dockerfile to its kubebuilder location

a30c3c0

Replace build/Dockerfile with a symlink because we still have CI jobs and quay.io configurations using the file from that location. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 27, 2020

dhellmann requested review from maelk and zaneb September 27, 2020 15:06

dhellmann mentioned this pull request Sep 27, 2020

rebuild using kubebuilder 2.3.1 #650

Closed

dhellmann force-pushed the kubebuilder-rewrite2 branch from ccc7854 to 027f718 Compare September 27, 2020 16:50

dhellmann force-pushed the kubebuilder-rewrite2 branch 3 times, most recently from 8a3c783 to b993195 Compare September 27, 2020 23:15

maelk mentioned this pull request Sep 28, 2020

[doc nit] Fix section header "install operator-sdk" #652

Closed

maelk reviewed Sep 28, 2020

View reviewed changes

Comment thread main.go

Comment thread main.go

Comment thread main.go Outdated

Comment thread hack/generate.sh Outdated

Comment thread Makefile Outdated

Comment thread Makefile Outdated

asalkeld reviewed Sep 28, 2020

View reviewed changes

Comment thread controllers/metal3.io/action_result.go Outdated

asalkeld reviewed Sep 28, 2020

View reviewed changes

dhellmann force-pushed the kubebuilder-rewrite2 branch from b993195 to 22513b3 Compare September 28, 2020 10:47

dhellmann force-pushed the kubebuilder-rewrite2 branch from 0d2ffe6 to c54c8a7 Compare September 28, 2020 16:50

dhellmann added 11 commits September 29, 2020 17:25

update controller tools version and arguments

2ac2985

Use the latest release and pass the arguments we need to allow us to use the float64 field for clock speed and to generate output consistent with what we had from the operator-sdk. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

make manifests generate

0579769

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

update go modules

7b05cbc

- Bring version numbers back to what they were before the migration. - Include a make target to tidy and verify them. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

fix hack scripts

91980b5

generate.sh: files to watch have moved and expanded gofmt.sh: list go packages to find directories to scan gosec.sh: scan all local packages Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

use go 1.14

d9479d9

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

disable webhook service

994e9ed

Since we don't have any webhooks, don't configure the server to listen. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

ensure metrics address settings match

5151562

Update the manager_auth_proxy_patch.yaml to set the metrics address to the same value used by default in main.go. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

update dev documentation

a941c0d

We no longer need to tell people to install the operator-sdk, or to use it to run the tests. Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

add entrypoint to dockerfile

ba7625e

Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

dhellmann force-pushed the kubebuilder-rewrite2 branch from e767c4c to ba7625e Compare September 29, 2020 21:25

maelk reviewed Sep 30, 2020

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2020

metal3-io-bot merged commit 6c0d397 into metal3-io:master Sep 30, 2020

Conversation

dhellmann commented Sep 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metal3-io-bot commented Sep 27, 2020

Uh oh!

dhellmann commented Sep 27, 2020

Uh oh!

dhellmann commented Sep 27, 2020

Uh oh!

dhellmann commented Sep 27, 2020

Uh oh!

dhellmann commented Sep 28, 2020

Uh oh!

maelk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asalkeld Sep 28, 2020

Choose a reason for hiding this comment

Uh oh!

dhellmann Sep 28, 2020

Choose a reason for hiding this comment

Uh oh!

andfasano Sep 28, 2020

Choose a reason for hiding this comment

Uh oh!

dhellmann commented Sep 28, 2020

Uh oh!

zaneb commented Sep 28, 2020

Uh oh!

dhellmann commented Sep 28, 2020

Uh oh!

dhellmann commented Sep 28, 2020

Uh oh!

dhellmann commented Sep 28, 2020

Uh oh!

dhellmann commented Sep 29, 2020

Uh oh!

maelk Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

dhellmann Sep 30, 2020

Choose a reason for hiding this comment

Uh oh!

zaneb commented Sep 30, 2020

Uh oh!

digambar15 commented Oct 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dhellmann commented Sep 27, 2020 •

edited

Loading

maelk left a comment •

edited

Loading