Power VS: Create Remaining TF Resources by clnperez · Pull Request #5780 · openshift/installer

clnperez · 2022-04-06T20:13:36Z

Add the creation of the VPC in IBM Cloud to connect in to the load
balancers, the DHCP Network in Power VS, and the Cloud Connection (a GRE
tunnel wrapper) to connect the two.

This PR also adds time as a Terraform provider because of the way the
IP addresses are retrieved for the server instances. With traditional
Power VS networks, Power VS assigns a static IP. With v1 of the DHCP
service created for IPI, the DHCP server on the private network assigns
the IP. As a result, there is a small amount of time needed to wait between
the instance creation and the IP retrieval from the list of leases.

It also reorganizes some of the resource creation due to this difference in
behavior from the network we did early development with.

Signed-off-by: Christy Norman christy@linux.vnet.ibm.com

clnperez · 2022-04-06T20:14:13Z

depends on #5614 and #5615

Prashanth684 · 2022-04-06T20:51:25Z

splitting the vendoring into a separate commit would help

clnperez · 2022-04-07T20:10:55Z

i'd like to get a 2nd opinion on that from @rna-afk . it's vendoring but its also a new tf provider's vendoring -- as opposed to the normal vendoring for the installer binary

clnperez · 2022-04-07T20:11:35Z

or maybe even from @staebler since he's done most of the tf changes wrt all that

rna-afk · 2022-04-07T20:56:55Z

I think it's best to move them to another commit. Would make it easier to read the tf files since terraform changes are just mostly vendor changes

staebler · 2022-04-07T21:15:23Z

or maybe even from @staebler since he's done most of the tf changes wrt all that

I am 100% in support of splitting this PR into multiple commits, with the vendor being one contained commit. At a glance, this is the minimum that I would do.

Commit for adding the new terraform provider.
Commit for vendoring the dependencies of the new provider.
Commit for the tf files.
Commit for the rest.

patrickdillon · 2022-04-11T15:35:48Z

+1. Having vendoring in separate commits makes reviewing much easier.

clnperez · 2022-04-11T18:28:52Z

just what i was looking for @staebler. thanks and will do

clnperez · 2022-04-12T18:59:54Z

got those split up and will fix the merge conflicts. heading out the door for a bit but wanted to get the first half done

clnperez · 2022-04-18T19:38:41Z

@rna-afk @patrickdillon i don't know why verify-vendor always seems to fail for me. any suggestions? i'm still using go 1.17, and no matter whether i run go mod tidy/vendor using the container or not, nothing changes.

clnperez · 2022-04-18T19:49:19Z

ah. rebasing and re-pushing worked even though it didn't change anything i could see 🤷🏻‍♀️

clnperez · 2022-04-18T20:33:23Z

actually that didn't work either. i was looking at verify-codegen, not verify-vendor. there was only an issue with one repo, so i just manually moved the dependency version in go.mod:

github.com/vmware/govmomi v0.24.0 => github.com/vmware/govmomi v0.27.4

clnperez · 2022-04-19T02:05:42Z

PR was rebased. PTAL @patrickdillon @rna-afk

clnperez · 2022-04-25T18:27:46Z

/test e2e-aws

patrickdillon

I am fine with merging this with the understanding that a later PR will add a separate bootstrap phase.

Can you remove the file .swp file: data/data/powervs/cluster/master/lb/.variables.tf.swp

Then I will approve, lgtm

clnperez · 2022-04-28T14:12:05Z

removed that file and re-pushed. @miyamotoh fyi

patrickdillon · 2022-04-28T15:43:25Z

/approve

will LGTM after tests

openshift-ci · 2022-04-28T15:44:39Z

[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

patrickdillon · 2022-04-28T17:28:39Z

/retest

patrickdillon · 2022-04-28T23:28:52Z

/lgtm

openshift-bot · 2022-04-29T01:24:03Z

/retest-required

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

in order to wait for Power VS machines to have DHCP addresses assigned Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

Create Network, VPC, Cloud Connection Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

differentiating from the vendor commit in the same patch series that is adding a new terraform provider (time) Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

clnperez · 2022-04-29T14:47:59Z

@patrickdillon a nutanix commit caused a merge conflict. fixed but needs a new approve, etc.

rna-afk · 2022-04-29T16:52:24Z

/lgtm

openshift-bot · 2022-04-29T16:58:14Z

/retest-required

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

openshift-bot · 2022-04-29T17:37:15Z

/retest-required

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

openshift-ci · 2022-04-29T19:07:26Z

@clnperez: 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/okd-e2e-aws	`a1a6498`	link	false	`/test okd-e2e-aws`
ci/prow/e2e-metal-ipi	`a1a6498`	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-ibmcloud	`a1a6498`	link	false	`/test e2e-ibmcloud`
ci/prow/e2e-openstack	`a1a6498`	link	false	`/test e2e-openstack`
ci/prow/e2e-crc	`a1a6498`	link	false	`/test e2e-crc`
ci/prow/e2e-azure	`a1a6498`	link	false	`/test e2e-azure`
ci/prow/e2e-libvirt	`a1a6498`	link	false	`/test e2e-libvirt`
ci/prow/e2e-gcp	`a1a6498`	link	false	`/test e2e-gcp`

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.

openshift-ci Bot requested review from jhixson74 and jstuever April 6, 2022 20:14

clnperez mentioned this pull request Apr 6, 2022

Add IBM Power VS: tf data #5614

Merged

clnperez force-pushed the upstream-vpc-nw branch from f38cea1 to 85d7418 Compare April 12, 2022 18:59

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 12, 2022

clnperez force-pushed the upstream-vpc-nw branch from 85d7418 to 52ddb0d Compare April 12, 2022 21:57

openshift-ci Bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 12, 2022

clnperez force-pushed the upstream-vpc-nw branch from 52ddb0d to b4dd721 Compare April 14, 2022 13:18

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 14, 2022

clnperez force-pushed the upstream-vpc-nw branch 3 times, most recently from 3375d26 to 5f9dc3e Compare April 18, 2022 19:37

clnperez force-pushed the upstream-vpc-nw branch from 5f9dc3e to 7525d26 Compare April 18, 2022 19:57

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2022

clnperez force-pushed the upstream-vpc-nw branch from 7525d26 to d0d45b5 Compare April 19, 2022 01:53

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 19, 2022

patrickdillon reviewed Apr 28, 2022

View reviewed changes

clnperez force-pushed the upstream-vpc-nw branch from c2af3b3 to 124980d Compare April 28, 2022 14:11

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2022

openshift-ci Bot assigned patrickdillon Apr 28, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2022

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 29, 2022

clnperez added 5 commits April 29, 2022 09:25

new terraform provider: time

5a90c5b

in order to wait for Power VS machines to have DHCP addresses assigned Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

time provider vendoring

a4f9c32

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

Power VS: Remaining TF resources

efbac9f

Create Network, VPC, Cloud Connection Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

Power VS: go files for remaining TF resources

2d9c4fd

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

main vendor commit

a1a6498

differentiating from the vendor commit in the same patch series that is adding a new terraform provider (time) Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

clnperez force-pushed the upstream-vpc-nw branch from 124980d to a1a6498 Compare April 29, 2022 14:46

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 Apr 29, 2022

openshift-ci Bot assigned rna-afk Apr 29, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 29, 2022

openshift-merge-robot merged commit 05b1a10 into openshift:master Apr 29, 2022

miyamotoh mentioned this pull request May 12, 2022

Power VS: Separating bootstrap resources to own stage for clean destroy #5901

Merged

Conversation

clnperez commented Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clnperez commented Apr 6, 2022

Uh oh!

Prashanth684 commented Apr 6, 2022

Uh oh!

clnperez commented Apr 7, 2022

Uh oh!

clnperez commented Apr 7, 2022

Uh oh!

rna-afk commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staebler commented Apr 7, 2022

Uh oh!

patrickdillon commented Apr 11, 2022

Uh oh!

clnperez commented Apr 11, 2022

Uh oh!

clnperez commented Apr 12, 2022

Uh oh!

clnperez commented Apr 18, 2022

Uh oh!

clnperez commented Apr 18, 2022

Uh oh!

clnperez commented Apr 18, 2022

Uh oh!

clnperez commented Apr 19, 2022

Uh oh!

clnperez commented Apr 25, 2022

Uh oh!

patrickdillon left a comment

Choose a reason for hiding this comment

Uh oh!

clnperez commented Apr 28, 2022

Uh oh!

patrickdillon commented Apr 28, 2022

Uh oh!

openshift-ci Bot commented Apr 28, 2022

Uh oh!

patrickdillon commented Apr 28, 2022

Uh oh!

patrickdillon commented Apr 28, 2022

Uh oh!

openshift-bot commented Apr 29, 2022

Uh oh!

clnperez commented Apr 29, 2022

Uh oh!

rna-afk commented Apr 29, 2022

Uh oh!

openshift-bot commented Apr 29, 2022

Uh oh!

openshift-bot commented Apr 29, 2022

Uh oh!

openshift-ci Bot commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

clnperez commented Apr 6, 2022 •

edited

Loading

rna-afk commented Apr 7, 2022 •

edited

Loading