Baremetal: Re-arrange validations for provisioning network at cluster creation time #3851

kirankt · 2020-07-07T16:38:11Z

This PR re-arranges all provisioning network validations until the create target. This enables end users to run other targets such as ignition-configs, manifests without specifying any config related to the provisioning network.

This is an implementation of openshift/enhancements#388.

Co-Authored-By: @andfasano

kirankt · 2020-07-07T17:22:55Z

/cc @dhellmann, @yevgeny-shnaidman, @romfreiman
/assign @stbenjam

stbenjam

Can you link the enhancement in the commit message/PR description? openshift/enhancements#388

Other than that just one question and a nit. Also I'm curious what @andfasano thinks since he did a lot of work on the validations code.

pkg/asset/installconfig/platformprovisioncheck.go

pkg/types/baremetal/validation/platform.go

stbenjam · 2020-07-07T17:44:27Z

/label platform/baremetal

stbenjam · 2020-07-07T17:45:04Z

@abhinavdahiya: we'll need an installer team approval on this one. This is an implementation of openshift/enhancements#388.

pkg/asset/installconfig/baremetal/validation.go

pkg/types/baremetal/validation/platform.go

abhinavdahiya · 2020-07-07T17:50:04Z

/approve

stbenjam · 2020-07-07T18:21:52Z

/hold

Just holding for now so it doesn't merge until we get the right stakeholders to look.

andfasano

From a general point of view, I think that if we had a validation system completely tag based then it would have been simpler to partition the applied rules by scope (through a custom validation rule and context). In this way the flow is a little bit more complex to follow

pkg/types/baremetal/validation/platform.go

kirankt · 2020-07-08T17:22:29Z

/test e2e-ovirt
/test e2e-openstack
/test e2e-aws-scaleup-rhel7
/test e2e-metal-ipi

kirankt · 2020-07-08T23:07:34Z

/retest

openshift-ci-robot · 2020-07-09T07:16:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, yevgeny-shnaidman

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya]

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

stbenjam · 2020-07-09T13:23:16Z

/hold cancel

stbenjam · 2020-07-09T16:29:52Z

I did some testing on this -- it works great! All of the provisioning checks are deferred until create cluster -- except one.

I just wanted to get clarification on the BMC's. I apologize if we already discussed this, my brain is a bit foggy at the end of the sprint. However, I think we expected assisted-installer folks to provide hosts without BMC creds, right? Looks like those validations are still running during create ignition-configs:

FATAL failed to fetch Kubeconfig Admin Client: failed to load asset "Install Config": invalid "install-config.yaml" file: [platform.baremetal.hosts[0].BMC.Username: Required value: missing Username, platform.baremetal.hosts[0].BMC.Password: Required value: missing Password, platform.baremetal.hosts[0].BMC.Address: Required value: missing Address, platform.baremetal.hosts[1].BMC.Username: Required value: missing Username, platform.baremetal.hosts[1].BMC.Password: Required value: missing Password, platform.baremetal.hosts[1].BMC.Address: Required value: missing Address, platform.baremetal.hosts[2].BMC.Username: Required value: missing Username, platform.baremetal.hosts[2].BMC.Password: Required value: missing Password, platform.baremetal.hosts[2].BMC.Address: Required value: missing Address]

dhellmann · 2020-07-09T16:43:55Z

I did some testing on this -- it works great! All of the provisioning checks are deferred until create cluster -- except one.

I just wanted to get clarification on the BMC's. I apologize if we already discussed this, my brain is a bit foggy at the end of the sprint. However, I think we expected assisted-installer folks to provide hosts without BMC creds, right? Looks like those validations are still running during create ignition-configs:

That's right, the assisted installer will create hosts without any BMC details. I think that means just their name and MAC address will be set.

kirankt · 2020-07-09T17:24:09Z

I did some testing on this -- it works great! All of the provisioning checks are deferred until create cluster -- except one.
I just wanted to get clarification on the BMC's. I apologize if we already discussed this, my brain is a bit foggy at the end of the sprint. However, I think we expected assisted-installer folks to provide hosts without BMC creds, right? Looks like those validations are still running during create ignition-configs:

That's right, the assisted installer will create hosts without any BMC details. I think that means just their name and MAC address will be set.

My mistake. I didn/t look closely inside the BMC struct. Will fix this in the next commit.

kirankt · 2020-07-09T23:42:48Z

/hold

kirankt · 2020-07-10T02:36:12Z

/retest

openshift-bot · 2020-07-11T03:39:54Z

/retest

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

openshift-bot · 2020-07-11T04:18:52Z

/retest

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

openshift-bot · 2020-07-11T04:44:51Z

/retest

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

openshift-bot · 2020-07-11T05:10:53Z

/retest

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

openshift-bot · 2020-07-11T05:23:52Z

/retest

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

openshift-bot · 2020-07-11T05:49:51Z

/retest

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

openshift-bot · 2020-07-11T06:15:53Z

/retest

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

openshift-bot · 2020-07-11T06:28:55Z

/retest

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

openshift-bot · 2020-07-11T06:41:51Z

/retest

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

openshift-bot · 2020-07-11T07:07:58Z

/retest

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

openshift-bot · 2020-07-11T07:33:54Z

/retest

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

openshift-bot · 2020-07-11T07:46:52Z

/retest

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

openshift-bot · 2020-07-11T08:00:10Z

/retest

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

openshift-bot · 2020-07-11T08:25:52Z

/retest

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

openshift-bot · 2020-07-11T08:38:51Z

/retest

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

openshift-bot · 2020-07-11T08:51:52Z

/retest

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

openshift-bot · 2020-07-11T09:04:53Z

/retest

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

openshift-bot · 2020-07-11T09:43:52Z

/retest

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

openshift-bot · 2020-07-11T09:57:04Z

/retest

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

openshift-bot · 2020-07-11T10:09:51Z

/retest

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

openshift-bot · 2020-07-11T10:22:53Z

/retest

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

openshift-bot · 2020-07-11T10:35:52Z

/retest

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

openshift-bot · 2020-07-11T11:01:51Z

/retest

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

openshift-bot · 2020-07-11T11:14:53Z

/retest

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

openshift-bot · 2020-07-11T11:40:51Z

/retest

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

openshift-bot · 2020-07-11T11:53:51Z

/retest

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

openshift-ci-robot · 2020-07-11T12:09:45Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-openstack	`80152b4`	link	`/test e2e-openstack`
ci/prow/e2e-ovirt	`80152b4`	link	`/test e2e-ovirt`

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-07-11T12:22:06Z

/retest

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

openshift-ci-robot requested review from celebdor and jstuever July 7, 2020 16:38

openshift-ci-robot assigned stbenjam Jul 7, 2020

stbenjam reviewed Jul 7, 2020

View reviewed changes

pkg/asset/installconfig/platformprovisioncheck.go Outdated Show resolved Hide resolved

pkg/types/baremetal/validation/platform.go Outdated Show resolved Hide resolved

openshift-ci-robot added the platform/baremetal IPI bare metal hosts platform label Jul 7, 2020

abhinavdahiya reviewed Jul 7, 2020

View reviewed changes

pkg/asset/installconfig/baremetal/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Jul 7, 2020

View reviewed changes

pkg/types/baremetal/validation/platform.go Outdated Show resolved Hide resolved

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

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 7, 2020

andfasano reviewed Jul 8, 2020

View reviewed changes

pkg/types/baremetal/validation/platform.go Outdated Show resolved Hide resolved

yevgeny-shnaidman approved these changes Jul 9, 2020

View reviewed changes

hardys mentioned this pull request Jul 9, 2020

baremetal: Disable network interfaces and BMH validations when needed #3794

Closed

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2020

dhellmann mentioned this pull request Jul 9, 2020

baremetal: relax network interface and BMH validations when generating ignition-configs #3788

Closed

kirankt changed the title ~~Baremetal: Re-arrange validations for provisioning network at cluster creationg time~~ Baremetal: Re-arrange validations for provisioning network at cluster creation time Jul 9, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2020

openshift-merge-robot merged commit ec72961 into openshift:master Jul 11, 2020

stbenjam mentioned this pull request Dec 4, 2020

openstack: Allow to skip pre-flight validations #4452

Merged

Baremetal: Re-arrange validations for provisioning network at cluster creation time #3851

Baremetal: Re-arrange validations for provisioning network at cluster creation time #3851

Uh oh!

Conversation

kirankt commented Jul 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirankt commented Jul 7, 2020

Uh oh!

stbenjam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stbenjam commented Jul 7, 2020

Uh oh!

stbenjam commented Jul 7, 2020

Uh oh!

Uh oh!

Uh oh!

abhinavdahiya commented Jul 7, 2020

Uh oh!

stbenjam commented Jul 7, 2020

Uh oh!

andfasano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kirankt commented Jul 8, 2020

Uh oh!

kirankt commented Jul 8, 2020

Uh oh!

openshift-ci-robot commented Jul 9, 2020

Uh oh!

stbenjam commented Jul 9, 2020

Uh oh!

stbenjam commented Jul 9, 2020

Uh oh!

dhellmann commented Jul 9, 2020

Uh oh!

kirankt commented Jul 9, 2020

Uh oh!

kirankt commented Jul 9, 2020

Uh oh!

kirankt commented Jul 10, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

openshift-bot commented Jul 11, 2020

Uh oh!

kirankt commented Jul 7, 2020 •

edited

Loading

openshift-ci-robot commented Jul 11, 2020 •

edited

Loading