-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add vSphere VIP support #2930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add vSphere VIP support #2930
Conversation
5801224 to
0beef59
Compare
Adds the optional install-config variables for APIVIP, IngressVIP, and DNSVIP to enable IPI deployment.
The bump was done using ```$ dep version dep: version : 0.5.1 build date : git hash : go version : go1.12 go compiler : gc platform : linux/amd64 features : ImportDuringSolve=false $ dep ensure -update -v github.com/openshift/api ```
0beef59 to
3df5eb8
Compare
|
/approve |
| openstacktypes "github.com/openshift/installer/pkg/types/openstack" | ||
| openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" | ||
| "github.com/openshift/installer/pkg/types/ovirt" | ||
| ovirttypes "github.com/openshift/installer/pkg/types/ovirt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally included here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rgolangh FYI if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was standardizing the imports to use *types. I can back that out if it is an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets leave it, just wanted to make sure that it wasn't an oversight
| baremetaltypes "github.com/openshift/installer/pkg/types/baremetal" | ||
| openstacktypes "github.com/openshift/installer/pkg/types/openstack" | ||
| openstackdefaults "github.com/openshift/installer/pkg/types/openstack/defaults" | ||
| ovirttypes "github.com/openshift/installer/pkg/types/ovirt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally included here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was standardizing the imports to use *types. I can back that out if it is an issue.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcpowermac, sdodson 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 |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
9 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
/test e2e-aws-upgrade |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/skip |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Looks like a lot of AWS issues /test e2e-aws-upgrade |
|
/skip |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
|
Keep getting the same AMI error: |
|
/hold cancel |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
|
/hold cancel |
|
/test e2e-aws-upgrade |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@mtnbikenc: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
This breaks vSphere UPI: generated URL for workers / masters is |
This reverts commit dcffc31. openshift#2930 breaks vSphere UPI, since generated worker.ign URLs are invalid
| case ovirt.Name: | ||
| case ovirttypes.Name: | ||
| ignitionHost = fmt.Sprintf("%s:22623", installConfig.Ovirt.APIVIP) | ||
| case vspheretypes.Name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtnbikenc @vrutkovs right here is where ignition is broken. Need to add a check for empty string APIVIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds the optional install-config variables for APIVIP, IngressVIP, and DNSVIP to enable IPI deployment.
https://issues.redhat.com/browse/CORS-1317
Needs: