-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Graduate Ready Pods in Job status to Beta #3111
Conversation
|
||
#### Beta | ||
|
||
- Feature gate enabled by default. | ||
- Existing E2E and conformance tests passing. | ||
- Existing [E2E] and [conformance] tests passing. |
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.
I'd like to see some scalability tests results for beta graduation, since enabling this feature might increase in job status updates and having a clear information what the impact is is an important factor.
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.
There's a section What specific metrics should inform a rollback?
that would be especially important for the above question. The next question Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?
I'd like to see answered as well.
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.
@wojtek-t how do we go about scalability?
I could certainly add an scalability test. We have useful metrics from the job controller too.
Is there a prow job we can reuse?
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.
questions updated
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.
We're testing Jobs as part of our regular e2e scalability load test (not super extensively, but there are Jobs there).
I'm not sure I would be adding a dedicated CI test for that, but I certainly agree that seeing results from some manual test, that is being "bad case" for this feature would be desired.
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.
Added scalability tests (that we need to add) as graduation criteria.
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
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, 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 |
One-line PR description: Update milestone for Beta
Issue link: Track Ready Pods in Job status #2879
Other comments: