IBMCloud: BYON Enablement - InstallConfig by cjschaef · Pull Request #6050 · openshift/installer

cjschaef · 2022-06-27T18:19:29Z

Setup enablement for Bring Your Own Network support on IBM Cloud
using IPI. This portion focuses on adding support for the
required values in the InstallConfig, and performing simple
validation of those resources provided.

cjschaef · 2022-06-27T19:38:56Z

This requires #6046 changes

Update: That change is now merged

pkg/asset/installconfig/ibmcloud/client.go

jeffnowicki · 2022-07-01T15:05:08Z

/cc @rvanderp3

cjschaef · 2022-07-01T15:38:22Z

After rebasing off the Regional URL fix, I am still missing a regional url call

ERROR failed to fetch Master Machines: failed to load asset "Install Config": failed to create install config: [platform.ibmcloud.controlPlaneSubnets: Not found: "existing-network-1-us-east-1", platform.ibmcloud.controlPlaneSubnets: Not found: "existing-network-1-us-east-2", platform.ibmcloud.controlPlaneSubnets: Not found: "existing-network-1-us-east-3", platform.ibmcloud.computeSubnets: Not found: "existing-compute-network-1-us-east-1", platform.ibmcloud.computeSubnets: Not found: "existing-compute-network-1-us-east-2", platform.ibmcloud.computeSubnets: Not found: "existing-compute-network-1-us-east-3"]

I'll see about getting that fixed in this PR

/hold

cjschaef · 2022-07-01T16:16:39Z

I believe I resolved the remaining issues after the rebase off #6046

# bin/openshift-install version
bin/openshift-install unreleased-master-6173-g273656b43019522d2eeebcd5f367c50e473a7f4b
built from commit 273656b43019522d2eeebcd5f367c50e473a7f4b
release image registry.ci.openshift.org/origin/release:4.11
release architecture amd64

# yq '.platform.ibmcloud' cluster-deploys/cjs-test-107/install-config.yaml 
region: us-east
resourceGroupName: existing-network-1
vpcName: existing-network-1-vpc
controlPlaneSubnets:
  - existing-network-1-us-east-1
  - existing-network-1-us-east-2
  - existing-network-1-us-east-3
computeSubnets:
  - existing-compute-network-1-us-east-1
  - existing-compute-network-1-us-east-2
  - existing-compute-network-1-us-east-3

# bin/openshift-install create manifests --dir cluster-deploys/cjs-test-107
INFO Consuming Install Config from target directory 
INFO Manifests created in: cluster-deploys/cjs-test-107/manifests and cluster-deploys/cjs-test-107/openshift

/unhold

rvanderp3

found a few nits, otherwise lgtm

rvanderp3 · 2022-07-05T16:47:07Z

pkg/asset/installconfig/ibmcloud/validation.go

do we want to break out of the loop if we've found the desired VPC?

Yep, let me add that post validateExistingSubnets

rvanderp3 · 2022-07-05T17:00:34Z

pkg/asset/installconfig/ibmcloud/validation.go

is a nil check redundant here?

Yeah, I guess we do validate this in platform
https://github.com/openshift/installer/pull/6050/files#diff-da80c878bba406f82e10ab78858cbaff8bb20a72f7aed6ce10051c4ab550441fR47

I can remove the nil check.

rvanderp3 · 2022-07-05T17:00:47Z

pkg/asset/installconfig/ibmcloud/validation.go

is a nil check redundant here?

Yeah, same as above, but for Compute.
https://github.com/openshift/installer/pull/6050/files#diff-da80c878bba406f82e10ab78858cbaff8bb20a72f7aed6ce10051c4ab550441fR50

I can remove the nil check

cjschaef · 2022-07-12T14:26:42Z

Sorry, will get to these soon, have been on other non-IPI duties

Setup enablement for Bring Your Own Network support on IBM Cloud using IPI. This portion focuses on adding support for the required values in the InstallConfig, and performing simple validation of those resources provided.

rvanderp3 · 2022-07-13T16:57:22Z

/lgtm
/assign @patrickdillon

patrickdillon · 2022-07-13T22:41:03Z

/approve

openshift-ci · 2022-07-13T22:42:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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

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

openshift-ci-robot · 2022-07-13T23:33:53Z

/retest-required

Remaining retests: 2 against base HEAD 9256b2e and 8 for PR HEAD 60aeec4 in total

jeffnowicki · 2022-07-14T13:27:55Z

/retest

jeffnowicki · 2022-07-14T14:55:49Z

/retest

openshift-ci · 2022-07-14T21:35:14Z

@cjschaef: 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-libvirt	`60aeec4`	link	false	`/test e2e-libvirt`
ci/prow/e2e-ibmcloud	`60aeec4`	link	false	`/test e2e-ibmcloud`

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.

…options IBMCloud: BYON Enablement - InstallConfig

cjschaef · 2022-08-29T13:49:59Z

/rettitle CORS-2263: IBMCloud: BYON Enablement - InstallConfig

openshift-ci bot requested review from r4f4 and rna-afk June 27, 2022 18:20

cjschaef force-pushed the ibmcloud_byon_config_options branch 5 times, most recently from 90b2f2c to d902b35 Compare June 27, 2022 19:14

cjschaef mentioned this pull request Jun 27, 2022

IBMCloud: BYON Enablement - Terraform #6051

Merged

cjschaef commented Jul 1, 2022

View reviewed changes

pkg/asset/installconfig/ibmcloud/client.go Outdated Show resolved Hide resolved

openshift-ci bot requested a review from rvanderp3 July 1, 2022 15:05

cjschaef force-pushed the ibmcloud_byon_config_options branch from d902b35 to cbdfc76 Compare July 1, 2022 15:18

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

cjschaef force-pushed the ibmcloud_byon_config_options branch from cbdfc76 to 273656b Compare July 1, 2022 16:10

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

rvanderp3 reviewed Jul 5, 2022

View reviewed changes

IBMCloud: BYON Enablement - InstallConfig

60aeec4

Setup enablement for Bring Your Own Network support on IBM Cloud using IPI. This portion focuses on adding support for the required values in the InstallConfig, and performing simple validation of those resources provided.

cjschaef force-pushed the ibmcloud_byon_config_options branch from 273656b to 60aeec4 Compare July 13, 2022 15:22

openshift-ci bot assigned patrickdillon and rvanderp3 Jul 13, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2022

openshift-ci bot merged commit a6700b0 into openshift:master Jul 14, 2022

cjschaef deleted the ibmcloud_byon_config_options branch August 10, 2022 18:04

cjschaef pushed a commit to cjschaef/installer that referenced this pull request Aug 10, 2022

Merge pull request openshift#6050 from cjschaef/ibmcloud_byon_config_…

e7d4ddc

…options IBMCloud: BYON Enablement - InstallConfig

Conversation

cjschaef commented Jun 27, 2022

Uh oh!

cjschaef commented Jun 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jeffnowicki commented Jul 1, 2022

Uh oh!

cjschaef commented Jul 1, 2022

Uh oh!

cjschaef commented Jul 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rvanderp3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rvanderp3 Jul 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjschaef commented Jul 12, 2022

Uh oh!

rvanderp3 commented Jul 13, 2022

Uh oh!

patrickdillon commented Jul 13, 2022

Uh oh!

openshift-ci bot commented Jul 13, 2022

Uh oh!

openshift-ci-robot commented Jul 13, 2022

Uh oh!

jeffnowicki commented Jul 14, 2022

Uh oh!

jeffnowicki commented Jul 14, 2022

Uh oh!

openshift-ci bot commented Jul 14, 2022

Uh oh!

cjschaef commented Aug 29, 2022 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cjschaef commented Jun 27, 2022 •

edited

Loading

cjschaef commented Jul 1, 2022 •

edited

Loading

rvanderp3 Jul 5, 2022 •

edited

Loading

cjschaef commented Aug 29, 2022 •

edited by openshift-ci bot

Loading