Generate network config secrets for baremetal platform #5247

andfasano · 2021-09-27T16:54:01Z

This patch adds the generation of secrets to store the network configuration defined per host in the baremetal platform.

Currently based on PR #5207

andfasano · 2021-09-29T15:15:49Z

/label platform/baremetal

openshift-ci · 2021-09-29T15:15:49Z

@andfasano: The label(s) /label plartform/baremetal cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, backport-risk-assessed, cherry-pick-approved

Details

In response to this:

/label plartform/baremetal

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.

andfasano · 2021-10-04T11:21:09Z

/label platform/baremetal

andfasano · 2021-10-04T11:21:38Z

/hold

Requires #5207 to land first, and also metal3-io/baremetal-operator#936

ardaguclu · 2021-10-08T10:21:42Z

/lgtm

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

/retest

zaneb · 2021-12-01T17:33:35Z

I think all the prerequisites for this have landed. @andfasano could you rebase?

/hold cancel

andfasano · 2021-12-02T15:55:49Z

/hold
Requires #5441

zaneb · 2021-12-02T19:32:12Z

@andfasano @ardaguclu given that both this and #5441 require tests to pass for All of The Things, I think we should submit them both in the same PR. That should result in this merging a lot faster.

andfasano · 2021-12-03T11:56:12Z

/hold cancel

I've imported BMO bump changes and added the missing part related to linking BMH with the newly create network secret. @zaneb @ardaguclu it's now ready for another review

zaneb · 2021-12-03T15:19:19Z

/lgtm
/cc @kirankt
/retest

staebler · 2021-12-03T22:33:22Z

pkg/asset/machines/master.go

Do not use reflection as a means to avoid code duplication. Either create a []interface{} to pass into the function or have the function operator on each element individually. I realize that there will still be quite a bit of boilerplate code with either approach, but that is go.

Unfortunately it's not possible to use []interface{} as function parameter as disallowed by the language. I've thus kept the conversion code within the generateAsset boundaries to minimize the noise outside, but that small portion cannot be simplified further, so I guess this could be a reasonable compromise between not using reflection and removing code duplication (while operating on each element individually appeared to make the code more complicated). I guess that in future Type Parameters could help for that.

That is exactly what I was trying to suggest with "create a []interface{} to pass into the function".

staebler · 2021-12-03T22:46:18Z

pkg/asset/machines/master.go

Do not overload the term asset, which has a specific meaning in the context of the installer.

Suggested change

func generateAsset(objects interface{}, fileName string) (assets []*asset.File, err error) {

func generateAssetFile(objects interface{}, fileName string) (assets []*asset.File, err error) {

pkg/asset/machines/baremetal/hosts.go

Latest BMO version exposes a new type that is required by the current patch

pkg/asset/machines/master.go

ardaguclu · 2021-12-06T15:38:38Z

/lgtm

staebler

Looks good. Just a few nits.

pkg/asset/machines/master.go

andfasano · 2021-12-06T17:33:36Z

/retest-required

openshift-ci · 2021-12-06T19:03:07Z

@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-metal-ipi-ovn-dualstack	f8ec6828490dc3d659cceabc43ea07bc671a4ba0	link	false	`/test e2e-metal-ipi-ovn-dualstack`
ci/prow/okd-e2e-aws	`b42680f`	link	false	`/test okd-e2e-aws`
ci/prow/e2e-libvirt	`b42680f`	link	false	`/test e2e-libvirt`
ci/prow/e2e-ovirt	`b42680f`	link	false	`/test e2e-ovirt`
ci/prow/e2e-aws-fips	`b42680f`	link	false	`/test e2e-aws-fips`
ci/prow/e2e-metal-ipi-virtualmedia	`b42680f`	link	false	`/test e2e-metal-ipi-virtualmedia`
ci/prow/e2e-aws-workers-rhel7	`b42680f`	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-single-node	`b42680f`	link	false	`/test e2e-aws-single-node`
ci/prow/e2e-crc	`b42680f`	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.

staebler

/lgtm
/approve

openshift-ci · 2021-12-06T19:46:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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-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 27, 2021

openshift-ci bot requested review from ardaguclu and celebdor September 27, 2021 16:54

andfasano force-pushed the baremetal-networkconfig-secret branch 4 times, most recently from 4200559 to a136531 Compare October 4, 2021 11:18

andfasano changed the title ~~[WIP] Generate network config secrets for baremetal platform~~ Generate network config secrets for baremetal platform Oct 4, 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 Oct 4, 2021

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

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

openshift-ci bot assigned ardaguclu Oct 8, 2021

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

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

andfasano force-pushed the baremetal-networkconfig-secret branch from a136531 to ef35557 Compare October 18, 2021 07:24

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 Oct 18, 2021

andfasano mentioned this pull request Nov 16, 2021

baremetal: add NetworkConfig field #5207

Merged

hardys mentioned this pull request Dec 1, 2021

[Baremetal] Support For Deploying with ISOs for Baremetal IPI #5425

Closed

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Dec 1, 2021

andfasano force-pushed the baremetal-networkconfig-secret branch from ef35557 to fc00bdf Compare December 2, 2021 07:47

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

ardaguclu mentioned this pull request Dec 3, 2021

Bump baremetal operator to latest #5441

Closed

openshift-ci bot requested a review from kirankt December 3, 2021 15:19

openshift-ci bot assigned zaneb Dec 3, 2021

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

staebler reviewed Dec 3, 2021

View reviewed changes

Bump baremetal-operator dependency to 20211201170610-92ffa60c683d

02e47c0

Latest BMO version exposes a new type that is required by the current patch

andfasano force-pushed the baremetal-networkconfig-secret branch from 48b5d77 to f8ec682 Compare December 6, 2021 09:29

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2021

ardaguclu reviewed Dec 6, 2021

View reviewed changes

pkg/asset/machines/master.go Outdated Show resolved Hide resolved

andfasano force-pushed the baremetal-networkconfig-secret branch from f8ec682 to 6ca765b Compare December 6, 2021 14:19

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

staebler reviewed Dec 6, 2021

View reviewed changes

pkg/asset/machines/master.go Outdated Show resolved Hide resolved

pkg/asset/machines/master.go Outdated Show resolved Hide resolved

pkg/asset/machines/master.go Outdated Show resolved Hide resolved

baremetal: generate network config secrets

b42680f

andfasano force-pushed the baremetal-networkconfig-secret branch from 6ca765b to b42680f Compare December 6, 2021 16:19

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2021

staebler reviewed Dec 6, 2021

View reviewed changes

openshift-ci bot assigned staebler Dec 6, 2021

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

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2021

openshift-merge-robot merged commit cd69a4b into openshift:master Dec 6, 2021

zaneb mentioned this pull request Dec 16, 2021

Baremetal: Deploy using CoreOS ISO from the release payload #5473

Merged

	func generateAsset(objects interface{}, fileName string) (assets []*asset.File, err error) {
	func generateAssetFile(objects interface{}, fileName string) (assets []*asset.File, err error) {

Generate network config secrets for baremetal platform #5247

Generate network config secrets for baremetal platform #5247

Uh oh!

Conversation

andfasano commented Sep 27, 2021

Uh oh!

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

Uh oh!

Uh oh!

openshift-ci bot commented Sep 29, 2021

Uh oh!

andfasano commented Oct 4, 2021

Uh oh!

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

Uh oh!

Uh oh!

ardaguclu commented Oct 8, 2021

Uh oh!

andfasano commented Nov 5, 2021

Uh oh!

zaneb commented Dec 1, 2021

Uh oh!

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

Uh oh!

Uh oh!

zaneb commented Dec 2, 2021

Uh oh!

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

Uh oh!

Uh oh!

zaneb commented Dec 3, 2021

Uh oh!

staebler Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ardaguclu commented Dec 6, 2021

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andfasano commented Dec 6, 2021

Uh oh!

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

Uh oh!

Uh oh!

staebler left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Dec 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andfasano commented Sep 29, 2021 •

edited

Loading

andfasano commented Oct 4, 2021 •

edited

Loading

andfasano commented Dec 2, 2021 •

edited

Loading

andfasano commented Dec 3, 2021 •

edited

Loading

openshift-ci bot commented Dec 6, 2021 •

edited

Loading