-
Notifications
You must be signed in to change notification settings - Fork 253
HIVE-2777: Implement hive Nutanix provisioning #2573
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
HIVE-2777: Implement hive Nutanix provisioning #2573
Conversation
|
@eliorerz: This pull request references HIVE-2777 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
@eliorerz: GitHub didn't allow me to request PR reviews from the following users: eliorerz. 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-sigs/prow repository. |
dc89df3 to
c68f669
Compare
|
@eliorerz: This pull request references HIVE-2777 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cc @2uasimojo |
|
@eliorerz: This pull request references HIVE-2777 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@eliorerz: This pull request references HIVE-2777 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Going to get started on this. One question about the install-config: I noticed the prismElements and subnetUUIDs are duplicated at the top level and under failureDomains. We talked yesterday about the ClusterDeployment.Spec.Platform.Nutanix schema not containing this redundancy. I assume it's still supported via install-config, but it's not necessary, right? IOW your PoC still works if you omit the top-level copy? |
Thanks, Regarding your question, the problem is that |
|
/test e2e e2e-pool Weird flakery probably due to some upstream bug that seems to be fixed now. |
2uasimojo
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.
This is great! With a couple of minor tweaks, I think it's ready to go. (Except that, as we discussed, we're going to need to clone the appropriate install-config fields into cd.Spec.Platform.Nutanix in anticipation of MachinePools before we can actually "release" :( )
Reminders:
- We're going to need doc updates.
- Let's not forget to look into ClusterPools (will probably need to use Inventory). Note that the work required for ClusterPools overlaps quite a bit with that needed for
hiveutil create-cluster, although the latter is optional as we've discussed.
| ) | ||
|
|
||
| // ConfigureCreds loads secrets designated by the environment variables CLUSTERDEPLOYMENT_NAMESPACE, | ||
| // CREDS_SECRET_NAME, and CERTS_SECRET_NAME and configures Nutanix credential environment variables |
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.
Not using certs currently. Does the PC connection require/support certs? Probably want to check upstream whether we need to include that support right away. (In which case we'll want the certs in the CD as well -- see e.g. vsphere.)
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 added it in my previous PR but I removed it for now. I'm still waiting for an answer on that.
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 think this is the last pending issue on this PR. It seems like we don't need it to move forward, given that you've been able to deploy successfully as written. So, assuming you haven't yet gotten your answer, we can change this comment to a TODO: certs? and defer to a subsequent PR.
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.
Still need to either add certs options or make the comment agree with the functionality.
pkg/controller/clusterdeployment/installconfigvalidation_test.go
Outdated
Show resolved
Hide resolved
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
Show resolved
Hide resolved
c68f669 to
c092981
Compare
c092981 to
c17e725
Compare
bd300f9 to
3908c64
Compare
b9b80e8 to
66218d8
Compare
5ed8108 to
4eeb0f8
Compare
Add Nutanix deprovisioning support.
HIVE-2779: Add Nutanix hiveutil support. # Conflicts: # apis/go.mod
…oyment and hiveutil
Inject certificate from platform.certificatesSecretRef to install-config AdditionalTrustBundle (if not exist)
Add a setUnsupportedConfigurationCondition helper to bubble user-facing errors in the Nutanix actuator. When data disk discovery or RHCOS image retrieval fails, the MachinePool status is updated instead of returning an error.
2uasimojo
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.
This is ready to go... after the rebase 😇
| if updateErr := a.setUnsupportedConfigurationCondition(pool, logger, "InvalidDataDisk", "data source must specify a UUID"); updateErr != nil { | ||
| return nil, false, updateErr | ||
| } | ||
| return nil, false, nil |
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.
nit, this could be pared down to
| if updateErr := a.setUnsupportedConfigurationCondition(pool, logger, "InvalidDataDisk", "data source must specify a UUID"); updateErr != nil { | |
| return nil, false, updateErr | |
| } | |
| return nil, false, nil | |
| return nil, false, a.setUnsupportedConfigurationCondition(pool, logger, "InvalidDataDisk", "data source must specify a UUID") |
Similar below. But not really worth fixing.
4eeb0f8 to
bb2de74
Compare
|
@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/e2e-vsphere 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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo, eliorerz 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 |
1 similar comment
|
@eliorerz: The following test 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-sigs/prow repository. I understand the commands that are listed here. |
Yeah? Well... do it again. /override ci/prow/e2e-vsphere |
|
@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/e2e-vsphere 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-sigs/prow repository. |
This is a partial implementation of #2550 PR, the final code will (probably) extend
ClusterDeploymentCR and will add support withMachinePools.This PR adds support for provisioning OpenShift clusters on Nutanix using the OpenShift Installer's IPI installation method within Hive.
Key changes include:
install-confignutanix platform credentials from secretnutanix-creds(seepasteInProviderCredentialsmethod)Implementation:
Secrets