powervs: allow VPC, Cloud connection, and NW re-use #6217
Conversation
|
Relies on PR #6210 |
clnperez
force-pushed
the
vpc_ccon_sharing
branch
from
8106183 to
8fd2a0a
Compare
because Power VS has some networking limitations, we have been restricted to one cluster/zone for IPI on Power VS. this PR allows re-using an existing VPC, Cloud connection (the GRE tunnel wrapper that allows us to communicate with IBM Cloud components from the Power VS DCs), and a Power Network. It should be noted that the intended use case is that the user pre-creates the items. If resources from a previous cluster are re-used, and destroy is called on that cluster, the destroy would fail before completion, and the Cloud connection would be deleted for the other clusters. We'll document what is supported in official documentation. Signed-off-by: Christy Norman <[email protected]>
this is being added as a seperate PR, and whehen it merges, i'll remove this commit. this is included so that the tests run Signed-off-by: Christy Norman <[email protected]>
clnperez
force-pushed
the
vpc_ccon_sharing
branch
from
8fd2a0a to
6941459
Compare
|
reconciling some doc/code comment descriptions and fixed the tf formatting. i think this is good to go for review. @r4f4 anyone else i need to get to look at it on the installer team? |
|
weirdness with prow... /retest-required |
Do you mind if we stick a
I'd say @rna-afk if he has some free cycles this sprint, given his background of reviewing PowerVS PRs. |
|
@bkhadars @miyamotoh would appreciate a review from you both since this touches code you've submitted or modified in the past |
|
Proposed changes make sense, and no obvious red flags I see by just looking at the diffs. So, LGTM, as long as it's tested working. |
|
/retest-required |
|
lots of |
|
@r4f4 this seems stuck. any ideas? |
|
/retest |
|
@clnperez: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
It went through now. I guess it had to do with the Prow builder upgrade. |
| variable "pvs_network_name" { | ||
| type = string | ||
| description = "The name of a pre-created Power VS DHCP Network." | ||
| default = "" |
There was a problem hiding this comment.
lets remove the default fields if there is nothing to set.
There was a problem hiding this comment.
maybe this is a terraform knoweldge gap for me, but if these arent set to an empty string, they're null, and i thought i'd get a type mismatch later when trying to check if it equals nil or not. i do have to check those to set some resource counts later on
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rna-afk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
powervs: allow VPC, Cloud connection, and NW re-use
5 participants
because Power VS has some networking limitations, we have been restricted to one cluster/zone for IPI on Power VS. this PR allows re-using an existing VPC, Cloud connection (the GRE tunnel wrapper that allows us to communicate with IBM Cloud components from the Power VS DCs), an a Power Network. It should be noted that the intended use case is that these are from previously-installed OpenShift clusters, not created by hand.
Signed-off-by: Christy Norman [email protected]