convert CRD webhook to apiextensions.k8s.io/v1 #964

kplimack · 2021-09-02T18:43:35Z

The webhook for CRD conversion of the metal3 resources was still formatted for apiextensions.k8s.io/v1beta1, while we were expecting it in apiextensions.k8s.io/v1. When enabling the webhooks for BMO, this error arose:

error validating data: ValidationError(CustomResourceDefinition.spec.conversion): unknown field "webhookClientConfig" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceConversion

Signed-off-by: Jake Plimack jplimack@tesla.com

Signed-off-by: Jake Plimack <jplimack@tesla.com>

metal3-io-bot · 2021-09-02T18:43:44Z

Hi @kplimack. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

kashifest · 2021-09-06T08:37:55Z

/ok-to-test

jplimack · 2021-09-07T16:25:11Z

/retest

jplimack · 2021-09-07T16:28:44Z

@kashifest / @fmuyassarov I'm unclear why the tests are failing, can you help me out?

fmuyassarov · 2021-09-07T18:35:17Z

@kashifest / @fmuyassarov I'm unclear why the tests are failing, can you help me out?

I think you need to run make generate

kashifest · 2021-09-07T18:37:29Z

@kashifest / @fmuyassarov I'm unclear why the tests are failing, can you help me out?

I think you need to run make generate

Yes precisely, you need to run make generate that would add the changes also in rendered capm3.yaml and that should make the CI happy

Signed-off-by: Jake Plimack <jplimack@tesla.com>

metal3-io-bot · 2021-09-07T18:44:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign stbenjam after the PR has been reviewed.
You can assign the PR to them by writing /assign @stbenjam in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

Signed-off-by: Jake Plimack <jplimack@tesla.com>

fmuyassarov · 2021-09-07T19:03:01Z

@jplimack you can also run those unit tests and generate tests locally if you would like to verify them

export CONTAINER_RUNTIME=docker
./hack/unit.sh 
./hack/generate.sh

jplimack · 2021-09-07T19:27:37Z

@fmuyassarov my unit tests pass when using hack/unit.sh, but when I run make unit, I arrive at the same TestProvision errors as prow
https://prow.apps.test.metal3.io/view/gs/metal3-prow/pr-logs/pull/metal3-io_baremetal-operator/964/unit/1435315738133925888

fmuyassarov · 2021-09-08T14:37:43Z

@fmuyassarov my unit tests pass when using hack/unit.sh, but when I run make unit, I arrive at the same TestProvision errors as prow
https://prow.apps.test.metal3.io/view/gs/metal3-prow/pr-logs/pull/metal3-io_baremetal-operator/964/unit/1435315738133925888

I'm actually seeing failure even when running `hack/unit.sh' after checking out to your branch.
EDIT: And why are the var substitutions commented out?

zaneb

Does anybody know if there is a later version of kubebuilder we can update to so that we don't continue to create new problems like this?

zaneb · 2021-09-08T13:20:50Z

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -278,7 +278,7 @@ type RAIDConfig struct {
 	// The list of logical disks for hardware RAID, if rootDeviceHints isn't used, first volume is root volume.
 	// You can set the value of this field to `[]` to clear all the hardware RAID configurations.
 	// +optional
-	HardwareRAIDVolumes []HardwareRAIDVolume `json:"hardwareRAIDVolumes"`
+	HardwareRAIDVolumes []HardwareRAIDVolume `json:"hardwareRAIDVolumes,omitempty"`


This is currently the subject of discussion in #962. Let's separate it out from this PR.

zaneb · 2021-09-08T13:33:01Z

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -290,7 +290,7 @@ type RAIDConfig struct {
 	// Software RAID will always be deleted.
 	// +kubebuilder:validation:MaxItems=2
 	// +optional
-	SoftwareRAIDVolumes []SoftwareRAIDVolume `json:"softwareRAIDVolumes"`
+	SoftwareRAIDVolumes []SoftwareRAIDVolume `json:"softwareRAIDVolumes,omitempty"`


zaneb · 2021-09-08T18:26:48Z

Does anybody know if there is a later version of kubebuilder we can update to so that we don't continue to create new problems like this?

Not until v3 by the looks of it. I don't know how feasible that update is right now.

metal3-io-bot · 2021-09-08T18:26:55Z

@kplimack: PR needs rebase.

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/test-infra repository.

fmuyassarov · 2021-09-08T19:16:30Z

Does anybody know if there is a later version of kubebuilder we can update to so that we don't continue to create new problems like this?

Not until v3 by the looks of it. I don't know how feasible that update is right now.

Yep, v3 is what we need probably. Kubebuilder book has migration instructions from v2 to v3. By quickly looking at the doc, it seems that migration should be straightforward. But I'm not sure if we want it now.

kashifest · 2021-12-20T12:05:32Z

/hold
Holding for a while. Changing the default branch reference to main from master. Once the transition is done I will unhold it.

metal3-io-bot · 2021-12-20T12:27:50Z

@kplimack: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
generate	`c3b89f5`	link	true	`/test generate`
gosec	`c3b89f5`	link	true	`/test gosec`
golint	`c3b89f5`	link	true	`/test golint`
unit	`c3b89f5`	link	true	`/test unit`
gomod	`c3b89f5`	link	true	`/test gomod`
gofmt	`c3b89f5`	link	true	`/test gofmt`
manifestlint	`c3b89f5`	link	true	`/test manifestlint`

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

kashifest · 2021-12-20T13:59:08Z

/hold cancel
The switch is done. BMO's default branch is now main

metal3-io-bot · 2022-03-20T14:46:21Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

metal3-io-bot · 2022-04-19T15:21:04Z

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

metal3-io-bot · 2022-04-19T15:21:07Z

@metal3-io-bot: Closed this PR.

In response to this:

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

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/test-infra repository.

convert CRD webhook to apiextensions.k8s.io/v1

38ce811

Signed-off-by: Jake Plimack <jplimack@tesla.com>

metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2021

metal3-io-bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 2, 2021

Merge branch 'master' into update-webhooks

b301f5b

metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2021

metal3-io-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 6, 2021

make generate

fbccf49

Signed-off-by: Jake Plimack <jplimack@tesla.com>

make build

c3b89f5

Signed-off-by: Jake Plimack <jplimack@tesla.com>

zaneb reviewed Sep 8, 2021

View reviewed changes

metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2021

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 20, 2021

fmuyassarov changed the base branch from master to main December 20, 2021 12:25

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 20, 2021

fmuyassarov mentioned this pull request Dec 23, 2021

📖 Update jenkins readme metal3-io/project-infra#298

Merged

metal3-io-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 20, 2022

metal3-io-bot closed this Apr 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert CRD webhook to apiextensions.k8s.io/v1 #964

convert CRD webhook to apiextensions.k8s.io/v1 #964

kplimack commented Sep 2, 2021

metal3-io-bot commented Sep 2, 2021

kashifest commented Sep 6, 2021

jplimack commented Sep 7, 2021

jplimack commented Sep 7, 2021 •

edited

Loading

fmuyassarov commented Sep 7, 2021

kashifest commented Sep 7, 2021

metal3-io-bot commented Sep 7, 2021

fmuyassarov commented Sep 7, 2021 •

edited

Loading

jplimack commented Sep 7, 2021

fmuyassarov commented Sep 8, 2021 •

edited

Loading

zaneb left a comment

zaneb Sep 8, 2021

zaneb Sep 8, 2021

zaneb commented Sep 8, 2021

metal3-io-bot commented Sep 8, 2021

fmuyassarov commented Sep 8, 2021 •

edited

Loading

kashifest commented Dec 20, 2021

metal3-io-bot commented Dec 20, 2021

kashifest commented Dec 20, 2021

metal3-io-bot commented Mar 20, 2022

metal3-io-bot commented Apr 19, 2022

metal3-io-bot commented Apr 19, 2022

convert CRD webhook to apiextensions.k8s.io/v1 #964

convert CRD webhook to apiextensions.k8s.io/v1 #964

Conversation

kplimack commented Sep 2, 2021

metal3-io-bot commented Sep 2, 2021

kashifest commented Sep 6, 2021

jplimack commented Sep 7, 2021

jplimack commented Sep 7, 2021 • edited Loading

fmuyassarov commented Sep 7, 2021

kashifest commented Sep 7, 2021

metal3-io-bot commented Sep 7, 2021

fmuyassarov commented Sep 7, 2021 • edited Loading

jplimack commented Sep 7, 2021

fmuyassarov commented Sep 8, 2021 • edited Loading

zaneb left a comment

Choose a reason for hiding this comment

zaneb Sep 8, 2021

Choose a reason for hiding this comment

zaneb Sep 8, 2021

Choose a reason for hiding this comment

zaneb commented Sep 8, 2021

metal3-io-bot commented Sep 8, 2021

fmuyassarov commented Sep 8, 2021 • edited Loading

kashifest commented Dec 20, 2021

metal3-io-bot commented Dec 20, 2021

kashifest commented Dec 20, 2021

metal3-io-bot commented Mar 20, 2022

metal3-io-bot commented Apr 19, 2022

metal3-io-bot commented Apr 19, 2022

jplimack commented Sep 7, 2021 •

edited

Loading

fmuyassarov commented Sep 7, 2021 •

edited

Loading

fmuyassarov commented Sep 8, 2021 •

edited

Loading

fmuyassarov commented Sep 8, 2021 •

edited

Loading