Skip to content

vsphere: add multiple datacenter and clusters#918

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jcpowermac:vsphere-mdcnc
Oct 5, 2022
Merged

vsphere: add multiple datacenter and clusters#918
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jcpowermac:vsphere-mdcnc

Conversation

@jcpowermac
Copy link
Contributor

@jcpowermac jcpowermac commented Oct 4, 2021

No description provided.

@openshift-ci 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 Oct 4, 2021
@openshift-ci openshift-ci bot requested review from ironcladlou and russellb October 4, 2021 16:43
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Oct 4, 2021

cc:
@rvanderp3
@bostrt
@jstuever
@staebler

staebler
staebler reviewed Oct 19, 2021
View reviewed changes
jcpowermac
jcpowermac commented Oct 21, 2021
View reviewed changes
@jcpowermac jcpowermac changed the title [wip] vsphere: add multiple datacenter and clusters vsphere: add multiple datacenter and clusters Oct 21, 2021
@openshift-ci 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 21, 2021
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Oct 21, 2021

/assign @aravindhp

aravindhp
aravindhp reviewed Oct 21, 2021
View reviewed changes
@jcpowermac jcpowermac force-pushed the vsphere-mdcnc branch 2 times, most recently from da72181 to 6ecbbdf Compare October 25, 2021 14:36
staebler
staebler reviewed Oct 25, 2021
View reviewed changes
@aravindhp
Copy link
Contributor

aravindhp commented Oct 25, 2021

/cc @openshift/openshift-team-cloud

For cloud config review

@jcpowermac jcpowermac force-pushed the vsphere-mdcnc branch 2 times, most recently from ec940ee to 5171952 Compare October 26, 2021 16:40
JoelSpeed
JoelSpeed reviewed Oct 28, 2021
View reviewed changes
enhancements/installer/vsphere-ipi-multiple-datacenter-clusters.md Outdated
Comment on lines 305 to 774
- The out-of-tree CCM is required for this work. It will need to be enabled at
installation time.
Copy link
Contributor

@JoelSpeed JoelSpeed Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be available using a feature gate soon, so development work can start before we get to GA

Copy link
Contributor

@JoelSpeed JoelSpeed Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will definitely not GA before 4.12, it might GA in 4.12 but last I heard the CSI driver was not in a good state and we depend on that

Copy link
Contributor Author

@jcpowermac jcpowermac Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vsphere regions/zones will be greenfield only. afaik the issue was migration.

This was referenced Oct 28, 2021
Closed
Closed
Closed
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Sep 14, 2022

/priority important-soon

@openshift-ci openshift-ci bot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Sep 14, 2022
@rvanderp3
Copy link
Contributor

rvanderp3 commented Sep 14, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2022
JoelSpeed
JoelSpeed reviewed Sep 16, 2022
View reviewed changes
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enhancement still talks about deployment zones which we got rid of, so we should get that updated

Secondly, there are a few comments of mine from the last review round that haven't been responded to or resolved

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Sep 19, 2022

The enhancement still talks about deployment zones which we got rid of, so we should get that updated

Secondly, there are a few comments of mine from the last review round that haven't been responded to or resolved

sorry about that @JoelSpeed should now be resolved.

@JoelSpeed
Copy link
Contributor

JoelSpeed commented Oct 3, 2022

/approve
/lgtm

/hold @jcpowermac Are we expecting anyone else to review this before merging? I'm happy with the content for now, do we need an ack from @gnufied for the storage side?

@openshift-ci 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 3, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 3, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 3, 2022
@gnufied
Copy link
Member

gnufied commented Oct 5, 2022 •
edited
Loading

From storage POV, it looks good. I have some concerns about same field being present in multiple places (but only for installer APIs), for example - A user can specify network in FailureDomain topology and inside NodeNetworking field. Is there a mapping between 2? But I am not from networking team, so apologies for talking through my hat.

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Oct 5, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2022

@jcpowermac: all tests passed!

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-merge-robot openshift-merge-robot merged commit f6b33eb into openshift:master Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoelSpeed JoelSpeed JoelSpeed left review comments

@ironcladlou ironcladlou Awaiting requested review from ironcladlou

@russellb russellb Awaiting requested review from russellb

+9 more reviewers

@gnufied gnufied gnufied left review comments

@fabianofranz fabianofranz fabianofranz left review comments

@elmiko elmiko elmiko left review comments

@bostrt bostrt bostrt left review comments

@lobziik lobziik lobziik left review comments

@patrickdillon patrickdillon patrickdillon left review comments

@aravindhp aravindhp aravindhp left review comments

@staebler staebler staebler left review comments

@rvanderp3 rvanderp3 rvanderp3 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@JoelSpeed JoelSpeed

@aravindhp aravindhp

@rvanderp3 rvanderp3

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.