Bug 1835438: move vSphere folder validation to provisioning #3597
Conversation
vSphere validation checks whether a folder exists when creating manifests or ignition configs. Creating the folder after the manifests or ignition configs is a valid UPI workflow so this check should only be run when provisioning infrastructure.
openshift-ci-robot
added
the
bugzilla/severity-unspecified
|
@patrickdillon: This pull request references Bugzilla bug 1835438, which is invalid:
Comment In response to this:
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. |
openshift-ci-robot
added
the
bugzilla/invalid-bug
|
/test e2e-vsphere |
|
|
For upi the template will need to be modified to: |
|
Explain to me why when users are setting a folder even for UPI... We allow them to create is afterwards? |
|
I was approaching this from the angle of questioning why would we prevent
users from doing it.
If we want to require users to have the folder created because it is safer,
that makes sense to me and we can close this.
If we do close this, @jcpowermac I think the problem in CI could be
temporarily mitigated by `sed`ing the cloud provider config manifest to
replace cluster ID with cluster name.
…On Wed, May 13, 2020, 4:42 PM Abhinav Dahiya ***@***.***> wrote:
Explain to me why when users are setting a folder even for UPI... We allow
them to create is afterwards?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3597 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABAVQYKXOKLZV2H5ZFRB7BDRRMA3PANCNFSM4NACX23A>
.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/bugzilla refresh |
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@jcpowermac: This pull request references Bugzilla bug 1835438, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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. |
openshift-ci-robot
removed
the
bugzilla/invalid-bug
|
/lgtm |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@patrickdillon: 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. 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@patrickdillon: All pull requests linked via external trackers have merged: openshift/installer#3597. Bugzilla bug 1835438 has been moved to the MODIFIED state. In response to this:
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. |
vSphere validation checks whether a folder exists when creating manifests or ignition configs. Creating the folder after the manifests or ignition configs is a valid UPI workflow so this check should only be run when provisioning infrastructure.
@abhinavdahiya we discussed this before, but it seems I answered your question incorrectly.
cc @jcpowermac did you see UPI documentation that needs to be changed to work with the new folder requirements? I am not seeing it in the repo. perhaps you were looking at official docs?