-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
sig-k8s-infra: Add cleanup job for disk snapshots #23770
sig-k8s-infra: Add cleanup job for disk snapshots #23770
Conversation
10c47cb
to
aa1b9dd
Compare
testgrid-alert-email: ameukam@gmail.com | ||
testgrid-num-failures-to-alert: '3' | ||
spec: | ||
serviceAccountName: boskos-janitor |
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.
@spiffxp Is it ok to use this job or do I need to create a separate SA with custom role ?
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.
The default service account should also have rights to do this
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.
If you add a TODO that we should revisit this that's good enough for now for me
testgrid-alert-email: ameukam@gmail.com | ||
testgrid-num-failures-to-alert: '3' | ||
spec: | ||
serviceAccountName: boskos-janitor |
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.
The default service account should also have rights to do this
testgrid-alert-email: ameukam@gmail.com | ||
testgrid-num-failures-to-alert: '3' | ||
spec: | ||
serviceAccountName: boskos-janitor |
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.
If you add a TODO that we should revisit this that's good enough for now for me
There's a part of me that asks why the jobs aren't doing this by default, but if this is how it's already done I'm fine proceeding this way |
Related: - Ref: kubernetes/k8s.io#2241 - Part of kubernetes/k8s.io#1469 Add a cleanup job that will delete disk snapshots older than 30 days created by sig-scalability periodic job ci-golang-tip-k8s-1-18-canary. Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
aa1b9dd
to
89746ee
Compare
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ameukam, spiffxp 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 |
@ameukam: Updated the
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/test-infra repository. |
Related:
Add a cleanup job that will delete disk snapshots older than 30 days created by sig-scalability periodic
job ci-golang-tip-k8s-1-18-canary.
Signed-off-by: Arnaud Meukam ameukam@gmail.com