Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@rexagod: This pull request references OCPCLOUD-2715 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 "5.1.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. |
|
Hello @rexagod! Some important instructions when contributing to openshift/api: |
openshift-ci
Bot
added
the
size/XL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This adds optional node lifecycle timeout settings while preserving unset-versus-zero behavior and aligned CRD validation; the change is ready to merge. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| // attempting to drain the node until it succeeds. | ||
| // When not set, the behaviour is the same as a value of 0. This preserves the default Machine API | ||
| // drain behaviour and matches Cluster API. | ||
| // +kubebuilder:validation:Minimum=0 |
There was a problem hiding this comment.
Is there a sensible maximum we could impose for any of these fields?
There was a problem hiding this comment.
I looked at the controller logic to try to infer something but I don't think one exists, even from the perspective of OpenShift. This is also in-line with the fact that a zero-value for these will wait indefinitely (effectively going beyond whatever Maximum is set to).
Adds the following fields that mirror their counterparts from CAPI [1]: * nodeDrainTimeoutSeconds * nodeVolumeDetachTimeoutSeconds * nodeDeletionTimeoutSeconds These will be ingested by the machine-api-operator, as well as used for conversions between the CAPI and MAPI representations in cluster-capi-operator. Both operators will exhibit same usage behaviors in parity with upstream CAPI, however, for the last two fields, their nil cases are adhered to the existing MAO behavior for compatibility. [1]:https://github.com/kubernetes-sigs/cluster-api/blob/main/api/core/v1beta1/machine_types.go#L445-L460
5256275 to
4bdf0b9
Compare
|
/retest |
|
/test verify-feature-promotion
|
|
@rexagod: 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. |
3 participants
Adds the following fields that mirror their counterparts from CAPI 1:
These will be ingested by the machine-api-operator, as well as used for conversions between the CAPI and MAPI representations in cluster-capi-operator.
Both operators will exhibit same usage behaviors in parity with upstream CAPI, however, for the last two fields, their nil cases are adhered to the existing MAO behavior for compatibility.