baremetal: add NetworkConfig field #5207

andfasano · 2021-09-13T10:58:43Z

This PR adds a new install-config field to store network configuration data (with validation) as described in the enhancement openshift/enhancements#817

pkg/types/baremetal/validation/platform.go

asalkeld

looks mostly good to me, but not an installer guru..

hardys · 2021-09-14T09:11:22Z

/label platform/baremetal

andfasano · 2021-09-20T08:46:18Z

/retest

kirankt · 2021-09-23T12:47:55Z

/retest

kirankt

Looks good to me, but lets wait for the enhancement to merge before committing this.
/hold

kirankt · 2021-09-23T13:03:58Z

/approve
Please remove the hold once the enhancement merges

imain · 2021-09-27T18:47:13Z

LGTM

andfasano · 2021-09-30T15:50:02Z

/retest

andfasano · 2021-10-04T08:11:19Z

/retest

kirankt · 2021-10-04T13:35:12Z

/retest

kirankt · 2021-10-11T15:42:29Z

/retest

romfreiman · 2021-10-18T07:37:37Z

@osherdp @eliorerz PTAL what happens to AI test

eliorerz · 2021-10-18T07:39:26Z

/test e2e-metal-assisted

andfasano · 2021-10-18T09:11:37Z

Do you have a reference to a specific failed mandatory build?

Why failed? :) I dont know, that's why I suggested to verify it prior to pushing

Because this PR did not yet merge, and the rehearsal jobs are used to catch any regression before that point - and it looks like there's an assisted test configured in the installer repo which passed until now, without showing any evidence related to such change.

andfasano · 2021-10-18T09:12:24Z

/retest

osherdp · 2021-10-18T09:17:43Z

@romfreiman @andfasano
@eliorerz is working on fixing assisted job

zaneb · 2021-11-05T03:05:00Z

/retest

andfasano · 2021-11-05T16:25:16Z

/retest

andfasano · 2021-11-08T17:40:09Z

/hold cancel

kirankt

/approve

sadasu · 2021-11-09T22:18:35Z

/retest

sadasu · 2021-11-09T22:18:41Z

/lgtm

staebler

/approve

pkg/types/baremetal/validation/platform.go

staebler · 2021-11-16T01:55:13Z

pkg/types/baremetal/platform.go

Please add documentation here for this field so that it shows up in the openshift-install explain output.

I saw that Host fields have been documented just here. Do you maybe mean adding a description also for all of them?

staebler · 2021-11-16T01:57:57Z

pkg/types/baremetal/platform.go

Are we sure that we want this to be a string rather than a map[string]interface{}? I would expect the user to want to enter in-line yaml rather than a text string.

The installer manages it essentially as a blob (it will be stored also in a secret, see PR 5247) and will pass it as it is to the image builder container (with a just a minimal validation).

Allow to specify desidered host networking settings yaml compatible with NMState as a new install-config field. Validation rule ensures that the string is at least a valid generic yaml.

andfasano · 2021-11-22T17:08:08Z

/retest-required

openshift-ci · 2021-11-22T19:59:55Z

@andfasano: 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
ci/prow/e2e-aws-workers-rhel7	47e1f2cff385d763d6c071d0dff4366a18a8c589	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-workers-rhel8	47e1f2cff385d763d6c071d0dff4366a18a8c589	link	false	`/test e2e-aws-workers-rhel8`
ci/prow/okd-e2e-aws-upgrade	47e1f2cff385d763d6c071d0dff4366a18a8c589	link	false	`/test okd-e2e-aws-upgrade`
ci/prow/e2e-crc	47e1f2cff385d763d6c071d0dff4366a18a8c589	link	false	`/test e2e-crc`

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.

kirankt

/lgtm

openshift-ci · 2021-11-30T17:13:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kirankt, staebler

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~~ [staebler]

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

openshift-bot · 2021-11-30T17:32:45Z

/retest-required

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

openshift-bot · 2021-11-30T18:08:50Z

/retest-required

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

andfasano changed the title ~~[WIPBaremetal networkconfig~~ [WIP] baremetal: add NetworkConfig field Sep 13, 2021

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2021

openshift-ci bot requested review from markmc and rna-afk September 13, 2021 11:00

andfasano force-pushed the baremetal-networkconfig branch 3 times, most recently from b514c0a to 32f08a1 Compare September 13, 2021 11:29

kirankt suggested changes Sep 13, 2021

View reviewed changes

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

asalkeld reviewed Sep 14, 2021

View reviewed changes

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

asalkeld reviewed Sep 14, 2021

View reviewed changes

openshift-ci bot added the platform/baremetal IPI bare metal hosts platform label Sep 14, 2021

andfasano force-pushed the baremetal-networkconfig branch from 32f08a1 to 4d25357 Compare September 15, 2021 16:45

andfasano force-pushed the baremetal-networkconfig branch from 4d25357 to c8b3f5c Compare September 23, 2021 09:00

andfasano changed the title ~~[WIP] baremetal: add NetworkConfig field~~ baremetal: add NetworkConfig field Sep 23, 2021

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2021

andfasano force-pushed the baremetal-networkconfig branch 2 times, most recently from 6ee4b61 to 7a9d18c Compare September 23, 2021 09:50

kirankt approved these changes Sep 23, 2021

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 23, 2021

andfasano mentioned this pull request Sep 27, 2021

Generate network config secrets for baremetal platform #5247

Merged

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2021

kirankt approved these changes Nov 8, 2021

View reviewed changes

openshift-ci bot assigned sadasu Nov 9, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2021

staebler reviewed Nov 16, 2021

View reviewed changes

openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2021

baremetal: add NetworkConfig field and validation

e367de3

Allow to specify desidered host networking settings yaml compatible with NMState as a new install-config field. Validation rule ensures that the string is at least a valid generic yaml.

andfasano force-pushed the baremetal-networkconfig branch from 47e1f2c to e367de3 Compare November 16, 2021 14:39

openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2021

kirankt approved these changes Nov 30, 2021

View reviewed changes

openshift-ci bot assigned kirankt Nov 30, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021

openshift-merge-robot merged commit f6c45dd into openshift:master Nov 30, 2021

baremetal: add NetworkConfig field #5207

baremetal: add NetworkConfig field #5207

Uh oh!

Conversation

andfasano commented Sep 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asalkeld left a comment

Choose a reason for hiding this comment

Uh oh!

hardys commented Sep 14, 2021

Uh oh!

andfasano commented Sep 20, 2021

Uh oh!

kirankt commented Sep 23, 2021

Uh oh!

kirankt left a comment

Choose a reason for hiding this comment

Uh oh!

kirankt commented Sep 23, 2021

Uh oh!

imain commented Sep 27, 2021

Uh oh!

andfasano commented Sep 30, 2021

Uh oh!

andfasano commented Oct 4, 2021

Uh oh!

kirankt commented Oct 4, 2021

Uh oh!

kirankt commented Oct 11, 2021

Uh oh!

romfreiman commented Oct 18, 2021

Uh oh!

eliorerz commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andfasano commented Oct 18, 2021

Uh oh!

andfasano commented Oct 18, 2021

Uh oh!

osherdp commented Oct 18, 2021

Uh oh!

zaneb commented Nov 5, 2021

Uh oh!

andfasano commented Nov 5, 2021

Uh oh!

andfasano commented Nov 8, 2021

Uh oh!

kirankt left a comment

Choose a reason for hiding this comment

Uh oh!

sadasu commented Nov 9, 2021

Uh oh!

sadasu commented Nov 9, 2021

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

staebler Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano commented Nov 22, 2021

Uh oh!

openshift-ci bot commented Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirankt left a comment

Choose a reason for hiding this comment

andfasano commented Sep 13, 2021 •

edited

Loading

eliorerz commented Oct 18, 2021 •

edited

Loading

openshift-ci bot commented Nov 22, 2021 •

edited

Loading