[OCPCLOUD-882] Add defaulting and validation via Webhook for Azure #607

JoelSpeed · 2020-06-03T14:17:13Z

Adds a defaulting and validating function for Azure provider specs and registers it with the existing webhooks introduced in #601

openshift-ci-robot · 2020-06-03T14:17:29Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

enxebre · 2020-06-08T14:21:40Z

/approve

JoelSpeed · 2020-06-09T09:26:11Z

/retest

JoelSpeed · 2020-06-09T12:05:14Z

/retest

JoelSpeed · 2020-06-09T13:55:17Z

/retest

JoelSpeed · 2020-06-10T11:03:29Z

/retest

alexander-demicev

/lgtm

Danil-Grigorev · 2020-06-10T11:33:20Z

pkg/apis/machine/v1beta1/machine_webhook.go

+
+	var errs []error
+	providerSpec := new(azure.AzureMachineProviderSpec)
+	if err := yaml.Unmarshal(m.Spec.ProviderSpec.Value.Raw, &providerSpec); err != nil {


Not a big deal, but it might be better to maintain a single location for unmarshaling spec in here: https://github.com/openshift/cluster-api-provider-azure/blob/9034812d3a88fae165bdc359d2c0a04c43f61c7a/pkg/cloud/azure/actuators/machine_scope.go#L218 so it could be removed later, when the providers will have it's own resources.

Danil-Grigorev · 2020-06-10T11:50:56Z

pkg/apis/machine/v1beta1/machine_webhook_test.go

+	}{
+		{
+			testCase: "with no location it fails",
+			providerSpec: &azure.AzureMachineProviderSpec{


Most of test cases are using valid provider spec, but then altering a single field. Could we just patch full spec with changes we need to test, to improve readability?

Danil-Grigorev · 2020-06-10T11:51:31Z

pkg/apis/machine/v1beta1/machine_webhook_test.go

+		{
+			testCase:     "it defaults defaultable fields",
+			providerSpec: &azure.AzureMachineProviderSpec{},
+			expectedProviderSpec: &azure.AzureMachineProviderSpec{


Most of test cases are using valid provider spec, but then altering a single field. Could we just patch full spec with changes we need to test, to improve readability?

JoelSpeed · 2020-06-10T12:00:59Z

/hold

Going to look into Danil's suggestions

Danil-Grigorev · 2020-06-10T12:30:30Z

/lgtm

openshift-bot · 2020-06-10T14:43:14Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T14:56:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T16:30:12Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T16:40:29Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T17:21:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T17:32:39Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T17:58:44Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T19:03:58Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T19:41:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T20:49:25Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-10T21:25:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T02:50:44Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T04:34:42Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T06:05:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T06:36:06Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T07:36:49Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T07:49:46Z

/retest

Please review the full test history for this PR and help us cut down flakes.

JoelSpeed · 2020-06-11T09:28:09Z

/retest

openshift-bot · 2020-06-11T09:46:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

JoelSpeed · 2020-06-11T10:23:15Z

/test e2e-aws-upgrade

openshift-bot · 2020-06-11T10:39:17Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T11:04:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T18:52:11Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-06-11T20:27:47Z

@JoelSpeed: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-azure-operator	`b0f86d2`	link	`/test e2e-azure-operator`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

openshift-bot · 2020-06-11T21:14:40Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-06-11T21:40:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 3, 2020

openshift-ci-robot requested review from elmiko and enxebre June 3, 2020 14:17

JoelSpeed force-pushed the azure-validation branch 4 times, most recently from a466439 to 562403b Compare June 5, 2020 09:31

JoelSpeed marked this pull request as ready for review June 5, 2020 09:31

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 5, 2020

JoelSpeed force-pushed the azure-validation branch from 562403b to 0dd61cd Compare June 5, 2020 12:32

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2020

Revendor to bring in azure provider

1fdf03a

JoelSpeed force-pushed the azure-validation branch from 0dd61cd to ee1f69e Compare June 8, 2020 12:36

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 8, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2020

alexander-demicev approved these changes Jun 10, 2020

View reviewed changes

openshift-ci-robot assigned alexander-demicev Jun 10, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2020

Danil-Grigorev reviewed Jun 10, 2020

View reviewed changes

openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Jun 10, 2020

Danil-Grigorev approved these changes Jun 10, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2020

openshift-merge-robot merged commit 51ae5ff into openshift:master Jun 11, 2020

enxebre mentioned this pull request Jun 15, 2020

Allow the Azure UserDataSecret Namespace to be omitted #616

Merged

JoelSpeed deleted the azure-validation branch June 15, 2020 11:26

[OCPCLOUD-882] Add defaulting and validation via Webhook for Azure #607

[OCPCLOUD-882] Add defaulting and validation via Webhook for Azure #607

Conversation

JoelSpeed commented Jun 3, 2020

openshift-ci-robot commented Jun 3, 2020

enxebre commented Jun 8, 2020

JoelSpeed commented Jun 9, 2020

JoelSpeed commented Jun 9, 2020

JoelSpeed commented Jun 9, 2020

JoelSpeed commented Jun 10, 2020

alexander-demicev left a comment

Choose a reason for hiding this comment

Danil-Grigorev Jun 10, 2020

Choose a reason for hiding this comment

Danil-Grigorev Jun 10, 2020

Choose a reason for hiding this comment

Danil-Grigorev Jun 10, 2020

Choose a reason for hiding this comment

JoelSpeed commented Jun 10, 2020

Danil-Grigorev commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 10, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

JoelSpeed commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

JoelSpeed commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-ci-robot commented Jun 11, 2020 • edited

openshift-bot commented Jun 11, 2020

openshift-bot commented Jun 11, 2020

openshift-ci-robot commented Jun 11, 2020 •

edited