-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Bug 2109538: nutanix: allow creating manifests without Prism Central connection #6149
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
Bug 2109538: nutanix: allow creating manifests without Prism Central connection #6149
Conversation
|
@vrutkovs: This pull request references Bugzilla bug 2109538, 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
No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request. DetailsIn 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. |
|
/cherry-pick release-4.11 |
|
@vrutkovs: once the present PR merges, I will cherry-pick it on top of release-4.11 in a new PR and assign it to you. DetailsIn 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. |
|
@vrutkovs: An error was encountered adding this pull request to the external tracker bugs for bug 2109538 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details. Full error message.
response code 400 not 200
Please contact an administrator to resolve this issue, then request a bug refresh with DetailsIn 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. |
|
/cc @thunderboltsid |
|
@vrutkovs: This pull request references Bugzilla bug 2109538. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. DetailsIn 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. |
|
@vrutkovs: GitHub didn't allow me to request PR reviews from the following users: thunderboltsid. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
|
My only concern is that if someone has a misconfigured PE on install-config, they won’t get an error on running create manifests but only on create cluster . |
thunderboltsid
left a comment
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.
LGTM
|
/retest |
1 similar comment
|
/retest |
|
Still no luck with upi-installer build: /retest |
In Assisted Installer we create manifests and generate Ignition in the cloud and pass ignition files to the agents. For platform integration we can't request credentials from users, instead we fill these in with fake credentials and let them change these later. This commit moves validations to ValidateForProvisioning, so that `openshift-install create manifests` phase would pass with fake credentials
In Assisted Installer we can't contact Prism Central to resolve Prism Element name via API. Instead we allow setting it to dummy value and let users update Infrastructure object later. This field is optional and would use API call if not set
47b1bb1 to
02a6ad4
Compare
|
/retest |
1 similar comment
|
/retest |
|
/test e2e-nutanix |
|
/test e2e-nutanix |
1 similar comment
|
/test e2e-nutanix |
| return field.InternalError(field.NewPath("platform", "nutanix", "prismElements"), errors.Wrapf(err, "prism element UUID %s does not correspond to a valid prism element in Prism", pe.UUID)) | ||
| } | ||
| } | ||
|
|
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.
Now that you getting the "Name" of the PrismElement as an optional parameter, do you need to validate if that exists also? And I am assuming that the Name of the PrismElement should correspond to the same UUID in the Nutanix cluster.
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.
Yes, the name should match the UUID, it is being validated in create cluster phase - or filled in if its unset.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon, thunderboltsid 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 |
|
/lgtm |
|
/skip |
|
@vrutkovs: 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. |
|
/skip |
|
/retest-required |
|
@vrutkovs: All pull requests linked via external trackers have merged: Bugzilla bug 2109538 has been moved to the MODIFIED state. DetailsIn 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. |
|
@vrutkovs: new pull request created: #6251 DetailsIn 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. |
In Assisted Installer we create manifests and generate Ignition in the cloud and pass ignition files to the agents. For platform integration we can't request credentials from users, instead we fill these in with fake credentials and let them change these later.
This commit moves validations to ValidateForProvisioning, so that
openshift-install create manifestsphase would pass with fake credentials