-
Notifications
You must be signed in to change notification settings - Fork 435
CNTRLPLANE-350: add NodePool minor version compatibility check #5873
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
CNTRLPLANE-350: add NodePool minor version compatibility check #5873
Conversation
|
@LiangquanLi930: This pull request references CNTRLPLANE-350 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 story to target the "4.19.0" version, but no target version was set. 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. |
|
@LiangquanLi930: This pull request references CNTRLPLANE-350 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 story to target the "4.19.0" version, but no target version was set. 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. |
|
@LiangquanLi930: This pull request references CNTRLPLANE-350 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 story to target the "4.19.0" version, but no target version was set. 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. |
|
@LiangquanLi930: This pull request references CNTRLPLANE-350 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 story to target the "4.19.0" version, but no target version was set. 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 verify |
c55b78b to
0f6c46b
Compare
|
@LiangquanLi930: This pull request references CNTRLPLANE-350 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 story to target the "4.19.0" version, but no target version was set. 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. |
|
/approve |
bb2ee50 to
4ae2a2c
Compare
|
/retest |
1 similar comment
|
/retest |
4ae2a2c to
3073679
Compare
| if err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, hcluster); err != nil { | ||
| // This is expected to happen when we create a cluster since there is no created HostedCluster CR to check the | ||
| // payload from. | ||
| logger.Info("WARNING: failed to get HostedCluster to check version compatibility") |
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.
but do you have to allow NodePool creation to complete before there's a HostedCluster? You could decide to block until the HostedCluster exists and have the create caller retry after that point. Or you could loop some number of times or for some duration here on the does-not-exist failure mode, waiting for the HostedCluster to start existing in parallel.
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.
When creating a HostedCluster using the HyperShift CLI, a default NodePool is also created. This is just CLI test code—we also add validation on the controller 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.
Sounds like that controller-side validation is future-plans, not current-implementation. But I'm fine assuming that there will be some controller-side backstops, and that this client code is just best-effort attempts to warn command-line users without bothering to go as far and slow as an API NodePool creation attempt, and letting these through here with the warning 👍
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.
#5931 is following up with the HyperShift-operator side validation discussed in this thread.
|
lgtm |
|
/lgtm |
|
/unhold |
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
This commit adds validation to ensure NodePool versions are compatible with the HostedCluster version: - For 4.even versions, allows y-2 difference - For 4.odd versions, allows y-1 difference - NodePool version cannot be higher than control plane version The validation is skipped in disconnected environments where registry access is not available.
cb8fa18 to
fa1623a
Compare
|
/retest |
|
/lgtm |
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/override "Red Hat Konflux / hypershift-operator-main-on-pull-request" |
|
/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: Red Hat Konflux / hypershift-operator-main-on-pull-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-sigs/prow repository. |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main 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. |
|
@LiangquanLi930: all tests passed! 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. |
f3154e0
into
openshift:main
What this PR does / why we need it:
This commit adds validation to ensure NodePool versions are compatible with the HostedCluster version:
The validation is skipped in disconnected environments where registry access is not available.
Jira story
https://issues.redhat.com/browse/CNTRLPLANE-330
https://issues.redhat.com/browse/CNTRLPLANE-350
Log
Checklist