baremetal: make provisioning network optional #3929

stbenjam · 2020-07-20T17:43:41Z

This implements the enhancement request that makes the provisioning
network optional. It introduces a new platform configuration called
"provisioningNetwork" that takes 3 possible values: managed, unmanaged,
and disabled. The "unmanaged" option replaces the external DHCP flag
that was previously used.

When the network is set to "disabled," it is expected that users use
virtualmedia configuration. We will deploy no DHCP/TFTP provisioning
services for a user to use.

openshift-ci-robot · 2020-07-20T17:43:57Z

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

stbenjam · 2020-07-20T17:47:14Z

/cc @hardys

Interested in getting some early feedback on this. There's some unresolved issues, but control plane deploys most of the way. More notes in openshift-metal3/dev-scripts#1069 about using this.

data/data/baremetal/bootstrap/main.tf

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

stbenjam · 2020-07-20T18:02:27Z

data/data/manifests/openshift/baremetal-provisioning-config.yaml.template

openshift/machine-api-operator#632 adds this

pkg/asset/installconfig/baremetal/baremetal.go

hardys · 2020-08-05T14:59:04Z

Small nit noticed but otherwise lgtm

hardys · 2020-08-05T16:25:50Z

/lgtm

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

data/data/install.openshift.io_installconfigs.yaml

abhinavdahiya · 2020-08-05T19:48:33Z

docs/user/metal/install_ipi.md

Personaly i think Unmanaged mode should have been names TFTPOnly instead because Umanaged imples tftp and no dhcp is very apparent.

Since the enhancement is already merged and seems like other APIs are also merged I do not know if we can back it out now but I would have liked more clearer names.

Unmanaged is clear to me, but I think the naming ship has sailed, with a merged enhancement and various changes already done.

pkg/asset/installconfig/baremetal/baremetal.go

pkg/types/baremetal/validation/platform.go

pkg/types/baremetal/validation/platform_test.go

pkg/types/baremetal/validation/platform.go

pkg/types/conversion/installconfig.go

hardys · 2020-08-06T15:46:12Z

/lgtm

abhinavdahiya

/approve

openshift-ci-robot · 2020-08-06T16:54:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya

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~~ [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-08-11T00:39:53Z

/test e2e-metal-ipi

stbenjam · 2020-08-11T00:41:04Z

/test e2e-metal-ipi

stbenjam · 2020-08-11T00:51:49Z

/retest

hardys · 2020-08-11T07:34:55Z

e2e-metal-ipi passed and openshift/machine-api-operator#632 landed so I think this is good to go - I checked the prow history for the failing jobs and they seem to be unstable atm, e.g there were issues with openstack after the latest RHCOS bump etc.

/hold cancel

openshift-bot · 2020-08-11T08:57:33Z

/retest

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

stbenjam · 2020-08-11T10:43:53Z

/hold

hardys · 2020-08-11T12:24:06Z

/lgtm

This implements the enhancement request that makes the provisioning network optional. It introduces a new platform configuration called "provisioningNetwork" that takes 3 possible values: managed, unmanaged, and disabled. The "unmanaged" option replaces the external DHCP flag that was previously used. When the network is set to "disabled," it is expected that users use virtualmedia configuration. We will deploy no DHCP/TFTP provisioning services for a user to use.

stbenjam · 2020-08-11T12:55:24Z

Wait, let's make sure e2e-metal-ipi passes.

/hold

hardys · 2020-08-11T13:31:44Z

/lgtm

openshift-ci-robot · 2020-08-11T14:04:02Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-libvirt	ef1bf05127aab0d0026bdb4954c9de9a44279a7f	link	`/test e2e-libvirt`
ci/prow/e2e-crc	ef1bf05127aab0d0026bdb4954c9de9a44279a7f	link	`/test e2e-crc`
ci/prow/e2e-openstack	`25bc23e`	link	`/test e2e-openstack`
ci/prow/e2e-ovirt	`25bc23e`	link	`/test e2e-ovirt`
ci/prow/e2e-aws-fips	`25bc23e`	link	`/test e2e-aws-fips`

Full PR test history. Your PR dashboard.

Details

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.

stbenjam · 2020-08-11T14:30:47Z

/hold cancel

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 Jul 20, 2020

openshift-ci-robot requested review from celebdor and hardys July 20, 2020 17:43

stbenjam mentioned this pull request Jul 20, 2020

Optional provisioning network openshift-metal3/dev-scripts#1069

Merged

stbenjam commented Jul 20, 2020

View reviewed changes

data/data/baremetal/bootstrap/main.tf Outdated Show resolved Hide resolved

stbenjam commented Jul 20, 2020

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

stbenjam commented Jul 20, 2020

View reviewed changes

data/data/manifests/openshift/baremetal-provisioning-config.yaml.template Outdated

Copy link

Member Author

stbenjam Jul 20, 2020

Choose a reason for hiding this comment

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

openshift/machine-api-operator#632 adds this

stbenjam changed the title ~~[WIP] baremetal: make provisioning network optional~~ baremetal: make provisioning network optional Jul 31, 2020

stbenjam marked this pull request as ready for review July 31, 2020 10:23

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 Jul 31, 2020

stbenjam force-pushed the optional-provisioning branch 3 times, most recently from 04d0b99 to 892e098 Compare July 31, 2020 11:08

hardys reviewed Aug 5, 2020

View reviewed changes

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

stbenjam force-pushed the optional-provisioning branch 3 times, most recently from a1e18c8 to 629a519 Compare August 5, 2020 16:00

openshift-ci-robot assigned hardys Aug 5, 2020

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

abhinavdahiya reviewed Aug 5, 2020

View reviewed changes

stbenjam force-pushed the optional-provisioning branch from 629a519 to b056cff Compare August 6, 2020 14:40

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2020

stbenjam force-pushed the optional-provisioning branch 3 times, most recently from 8a5d1fb to ef1bf05 Compare August 6, 2020 15:06

abhinavdahiya reviewed Aug 6, 2020

View reviewed changes

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

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 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 Aug 11, 2020

stbenjam force-pushed the optional-provisioning branch from ef1bf05 to fe2803e Compare August 11, 2020 12:15

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020

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

stbenjam force-pushed the optional-provisioning branch from fe2803e to 3bc7b9f Compare August 11, 2020 12:43

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2020

stbenjam force-pushed the optional-provisioning branch from 3bc7b9f to 25bc23e Compare August 11, 2020 12:46

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 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 Aug 11, 2020

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

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

openshift-merge-robot merged commit 7f8e8ea into openshift:master Aug 11, 2020

stbenjam deleted the optional-provisioning branch August 11, 2020 14:36

stbenjam mentioned this pull request Aug 14, 2020

Bug 1867165: baremetal: allow configuring bootstrap mac addresses #4052

Merged

baremetal: make provisioning network optional #3929

baremetal: make provisioning network optional #3929

Uh oh!

Conversation

stbenjam commented Jul 20, 2020

Uh oh!

openshift-ci-robot commented Jul 20, 2020

Uh oh!

stbenjam commented Jul 20, 2020

Uh oh!

Uh oh!

Uh oh!

stbenjam Jul 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hardys commented Aug 5, 2020

Uh oh!

hardys commented Aug 5, 2020

Uh oh!

Uh oh!

Uh oh!

abhinavdahiya Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

stbenjam Aug 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hardys commented Aug 6, 2020

Uh oh!

abhinavdahiya left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Aug 6, 2020

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

hardys commented Aug 11, 2020

Uh oh!

openshift-bot commented Aug 11, 2020

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

hardys commented Aug 11, 2020

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

hardys commented Aug 11, 2020

Uh oh!

openshift-ci-robot commented Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stbenjam commented Aug 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openshift-ci-robot commented Aug 11, 2020 •

edited

Loading