-
Notifications
You must be signed in to change notification settings - Fork 1.4k
OCPBUGS-6508: Update Control Plane replica validation for Single Node OpenShift #9048
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
Conversation
@sadasu: This pull request references CORS-3456 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.18.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
@sadasu: This pull request references Jira Issue OCPBUGS-6508, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request. The bug has been updated to refer to the pull request using the external bug tracker. 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 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
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-sigs/prow repository. |
/test e2e-agent-sno-ipv6 |
/retest |
6befc4c
to
d5f71a6
Compare
/test e2e-agent-sno-ipv6 |
/retest-required |
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), pool.Replicas, "number of control plane replicas must be positive")) | ||
case *pool.Replicas == 1 && !supportedSingleNodePlatform(isSingleNodeOpenShift, platform): | ||
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), pool.Replicas, "Single Node OpenShift(SNO) not supported by this install method for this platform")) | ||
case *pool.Replicas != 3: |
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.
According to the docs (from what I could find), the default is 1 replica but it does not specify a maximum number or a constraint of 3.
Are we sure that 3 is the constraint 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.
Yes.
This has nothing to do with what is valid in the MachineSet API (which is not actually used for control plane Machines) and everything to do with what topologies of OpenShift are supported.
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.
@danmanor FYI - obviously we will expand the range here for 4/5 node control planes when you are ready to support that (if it has to be in this PR, then hopefully in a separate commit so that we could backport the !=3 check if necessary).
Sandhya, please co-ordinate with Dan on the timing and keep @rwsu in the loop on the agent side.
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.
@zaneb The diff was removed, what did I missed ?
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.
Previously it was checking that the number of replicas was either 1 or 3: 2145209
Since this didn't land before your changes that check has just been removed.
922e097
to
abf995e
Compare
39a7252
to
18bfea2
Compare
For ABI, for all the platforms ( baremetal, vsphere, none, external) installer/pkg/asset/agent/installconfig.go Lines 474 to 477 in ca4ca41
cc @zaneb |
LGTM from the perspective of platform support now. |
@cybertron based on the test https://github.com/openshift/installer/blob/main/pkg/asset/machines/master_test.go#L247, 1 master and 1 worker on BareMetal platform installed with IPI is a supported configuration. |
It looks like that was done just to save typing in a test designed to verify something else. |
14a72dc
to
772752a
Compare
Update test method TestBaremetalGeneratedAssetFiles() to create a valid Baremetal IPI install config.
@cybertron , @zaneb and @patrickdillon Could you PTAL? |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zaneb 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 |
/lgtm |
@sadasu: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
25b5d21
into
openshift:main
@sadasu: Jira Issue OCPBUGS-6508: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-6508 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 openshift-eng/jira-lifecycle-plugin repository. |
/cherrypick release-4.19 |
@sadasu: #9048 failed to apply on top of branch "release-4.19":
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-sigs/prow repository. |
@sadasu: Jira Issue OCPBUGS-6508 has been cloned as Jira Issue OCPBUGS-56811. Will retitle bug to link to clone. 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 openshift-eng/jira-lifecycle-plugin repository. |
@sadasu: Jira Issue OCPBUGS-56811: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-56811 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 openshift-eng/jira-lifecycle-plugin repository. |
[ART PR BUILD NOTIFIER] Distgit: ose-installer |
[ART PR BUILD NOTIFIER] Distgit: ose-baremetal-installer |
[ART PR BUILD NOTIFIER] Distgit: ose-installer-artifacts |
@sadasu: Jira Issue OCPBUGS-6508 is in an unrecognized state (ON_QA) and will not be 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 openshift-eng/jira-lifecycle-plugin repository. |
Installer can be used to provision Single Node OpenShift(SNO) only for AWS, GCP, Azure, PowerVS and Baremetal.
SNO is not supported on all on-prem platforms yet. Even on the platforms that support SNO installs, the install method currently supported are assisted/agent/ZTP.
Since, other install methods like ABI (agent based install) layer their install config validation over the Installer's own validation, this validation (that is sensitive to install method) was added while generating manifests in the IPI path.