USHIFT-1225: Remove backups for no longer existing deploys #2049
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pmtk: This pull request references USHIFT-1225 which is a valid jira issue. 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. |
openshift-ci
bot
added
do-not-merge/hold
openshift-merge-robot
added
the
needs-rebase
|
@pmtk: GitHub didn't allow me to request PR reviews from the following users: pmtk. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
pmtk
changed the title
openshift-ci
bot
added
the
approved
pmtk
force-pushed
the
1225-remove-backups-for-non-existent-deploys
branch
3 times, most recently
from
873d7ba
to
453bef8
Compare
openshift-merge-robot
removed
the
needs-rebase
pmtk
changed the title
|
/unhold |
|
/cc @dhellmann @eggfoobar |
openshift-ci
bot
removed
do-not-merge/work-in-progress
pkg/admin/prerun/backups.go
Outdated
| // and returns a list of backups that do not belong to any of these deployments | ||
| func (bs Backups) getDangling(deploymentIDs []string) Backups { | ||
| backupsToRemove := []data.BackupName{} | ||
| sansDeploy := []data.BackupName{} |
There was a problem hiding this comment.
What do you think of changing this to unknownDeployments
pkg/admin/prerun/backups.go
Outdated
|
|
||
| if len(sansDeploy) > 0 { | ||
| // Expecting to be "4.13" | ||
| klog.InfoS("Found backups not belonging to any deployment - they need to deleted manually", "backups", sansDeploy) |
There was a problem hiding this comment.
Small nit:
they need to deleted manually > they need to be deleted manually
| return backups, nil | ||
| } | ||
|
|
||
| func (bs Backups) filter(pred func(data.BackupName) bool) Backups { |
There was a problem hiding this comment.
Maybe we remove this, in favor of https://github.com/openshift/microshift/pull/1998/files#diff-e78e17834c76663b756393d04ebe7aceb8ea4059d44b7d2ae3ee4e9999aead6eR394 when it's merged in?
There was a problem hiding this comment.
I actually intended to do other way round: remove the one from FDO PR; have this as a new impl
pmtk
force-pushed
the
1225-remove-backups-for-non-existent-deploys
branch
from
453bef8
to
c937953
Compare
openshift-merge-robot
added
the
needs-rebase
pmtk
force-pushed
the
1225-remove-backups-for-non-existent-deploys
branch
from
c937953
to
cfd29a7
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
pmtk
force-pushed
the
1225-remove-backups-for-non-existent-deploys
branch
from
cfd29a7
to
418e968
Compare
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eggfoobar, pmtk 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 |
|
@pmtk: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
/cc
/hold