OCPBUGS-4466: Add check for compact-cluster install on GCP, AWS & Azure#8226
Conversation
|
@sadasu: This pull request references Jira Issue OCPBUGS-4466, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/jira refresh |
|
@sadasu: This pull request references Jira Issue OCPBUGS-4466, which is invalid:
Comment 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. |
|
/test ? |
|
@sadasu: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
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. |
|
/jira refresh |
|
@sadasu: This pull request references Jira Issue OCPBUGS-4466, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
bc1cef3 to
aefd976
Compare
|
/test e2e-gcp-ovn |
|
/test e2e-gcp-ovn |
pkg/asset/cluster/tfvars/tfvars.go
Outdated
There was a problem hiding this comment.
So we cannot check this during install-config validation because this happens after the manifests have been generated and (possibly) edited by users.
If this becomes a pattern, we should consider adding a manifest validation stage.
There was a problem hiding this comment.
@r4f4 yes. I was surprised too that we allow customers to edit the manifests post creation but don't check if it has been modified in a sane way.
Also, this fix is added in the code path to generate terraform inputs. Let us invest the time to figure out how to add a manifest validation stage within the CAPI flow so we detect modified manifests and validate them.
|
/retest-required |
1 similar comment
|
/retest-required |
patrickdillon
left a comment
There was a problem hiding this comment.
/approve
This LGTM, but left a suggestion. Let me know if you need another tag.
fyi we can handle pre-provisioning validation in https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformprovisioncheck.go
but i think handling this here is perfectly fine.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon 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 |
I think this location has the advantage that it validates any updates to manifests too. |
r4f4
left a comment
There was a problem hiding this comment.
So you've decided to add the check for aws, azure, and gcp?
Yes, although the bug was raised only for GCP. |
|
/retest |
r4f4
left a comment
There was a problem hiding this comment.
// 2. workers = 0, masters schedulable, valid compact cluster but currently unsupported on GCP
Are we sure this statement also applies to AWS and Azure?
Worker machinesets generated, during create manifests can be deleted before creating the cluster. Detect this case and take action based on whether masters are marked as schedulable. This fixes compact cluster installs on GCP, AWS and Azure.
Yes. This bug was raised when https://issues.redhat.com/browse/CORS-2420 was tested. The associated feature https://issues.redhat.com/browse/OCPSTRAT-341 has epics for AWS, GCP, Azure and vSphere. Support for compact clusters was not implemented and hence this is a good way error out and inform the customer (instead of crashing). |
|
@sadasu: This pull request references Jira Issue OCPBUGS-4466, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/test e2e-gcp-ovn |
|
/test e2e-gcp-ovn |
|
/retest-required |
|
@sadasu: 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/test-infra repository. I understand the commands that are listed here. |
|
@sadasu: Jira Issue OCPBUGS-4466: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-4466 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Fix included in accepted release 4.16.0-0.nightly-2024-04-29-222758 |
Add additional checks around detecting 0 worker machinesets when generating terraform.tfvars for compact clusters.
This bug was raised when https://issues.redhat.com/browse/CORS-2420 was tested. The associated feature https://issues.redhat.com/browse/OCPSTRAT-341 had epics for AWS, GCP, Azure and vSphere. Support for compact clusters was not implemented and hence this is a good way error out of the install with an informational message to the customer (instead of crashing).