-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
suspended jobs: graduate to beta #2646
Conversation
- Metric name: `job_sync_duration_seconds` gets a new label named `action` | ||
to allow operators to filter Job sync latency by the action performed. | ||
The possible values for this label are: | ||
- `reconciling` when the Job's pod creation/deletion expectations are |
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.
is this mutually exclusive with pods_created
and pods_deleted
?
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:
Each sample of this metric will have exactly one of the above values for the
action
label.
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.
PRR is missing here
@adtac you have to update the existing file in https:/kubernetes/enhancements/tree/master/keps/prod-readiness/sig-apps |
4db92e0
to
28604b9
Compare
latest commit looks good to me |
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.
/lgtm
/approve
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.
Just two minor comments - and please rebase
@@ -385,78 +390,86 @@ field. | |||
|
|||
* **Are there any tests for feature enablement/disablement?** No. |
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.
Please add some unit tests for it.
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.
actually, we do have integration tests that test feature enablement exhaustively. updated the KEP.
While the above list isn't exhaustive, they're signals in favour of rollbacks. | ||
|
||
* **Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path | ||
tested?** <!-- I'll answer this after implementation. |
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.
You can do the same thing I did here:
https:/kubernetes/enhancements/pull/2538/files
Signed-off-by: Adhityaa Chandrasekar <[email protected]>
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.
squashed & rebased
@@ -385,78 +390,86 @@ field. | |||
|
|||
* **Are there any tests for feature enablement/disablement?** No. |
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.
actually, we do have integration tests that test feature enablement exhaustively. updated the KEP.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adtac, soltysh, wojtek-t 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 |
Hello @adtac 👋, 1.22 Docs Shadow here. Please follow the steps detailed in the documentation to open a PR against dev-1.22 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Fri July 9, 11:59 PM PDT. Also, take a look at Documenting for a release to familiarize yourself with the docs requirement for the release. Thank you! 🙏 |
ref: #2232
/sig apps
/cc @ahg-g @alculquicondor @soltysh @wojtek-t