Add --all and --latest to checkpoint/restore #1662
Conversation
openshift-ci-robot
added
size/L
needs-ok-to-test
| Name: "all, a", | ||
| Usage: "checkpoint all running containers", | ||
| }, | ||
| LatestFlag, |
There was a problem hiding this comment.
Not for this PR, but if we're going to define LatestFlag in common.go, should we do the same for the All flag?
There was a problem hiding this comment.
I actually thought about it, but the verb ('running' in this case), would be different, so it has to be a function. As the definition of the flags happens outside of a function, it would also need to be moved inside the function, I think, and it sounded overly complicated in this case.
There was a problem hiding this comment.
Oh good point on the running bit, that verbiage is probably different amongst the different podman commands.
|
/approve |
openshift-ci-robot
added
the
approved
|
/ok-to-test |
openshift-ci-robot
removed
the
needs-ok-to-test
|
Code LGTM |
|
LGTM |
|
@adrianreber Mind rebasing to pick up CI fixes? |
adrianreber
force-pushed
the
all-and-latest
branch
from
be7bf88 to
12b909a
Compare
|
/retest |
adrianreber
force-pushed
the
all-and-latest
branch
2 times, most recently
from
2eae3fb to
d6eacd4
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, rhatdan 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 |
|
☔ The latest upstream changes (presumably #1639) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@adrianreber Needs a rebase. |
adrianreber
force-pushed
the
all-and-latest
branch
from
d6eacd4 to
81cbd71
Compare
|
/lgtm |
adrianreber
force-pushed
the
all-and-latest
branch
from
81cbd71 to
4322df9
Compare
|
Sorry, rebasing broke the import statements. Fixed and force pushed. |
|
bot, retest this please |
|
/retest images |
The check about the --all and --latest option is used and repeated and some commands. Factor it out and put it into common. Signed-off-by: Adrian Reber <[email protected]>
Just as the checkAllAndLatest() function the new code in getAllOrLatestContainers() is used in some commands and duplicated. This factors out this code to be used in other places without duplicating it. Signed-off-by: Adrian Reber <[email protected]>
Instead of duplicating the same code in multiple commands this uses the newly added function checkAllAndLatest() instead. Signed-off-by: Adrian Reber <[email protected]>
This removes duplicate code paths which has been previously factored out as getAllOrLatestContainers(). Signed-off-by: Adrian Reber <[email protected]>
This add the convenience options --all and --latest to the subcommands checkpoint and restore. Signed-off-by: Adrian Reber <[email protected]>
adrianreber
force-pushed
the
all-and-latest
branch
from
4322df9 to
e8d6903
Compare
|
I was testing in the wrong directory and was pushing broken code. Not good. This time I verified that I actually tested it correctly. Sorry for all the noise here. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
This started as a simple change to add
--alland--latestto the checkpoint and restore commands.While adding these features I saw that the same code to handle
--alland--latestcan be found in multiple places. So I extended this PR to include common code for--alland--latesthandling and replaced the code with the newly added common code at a few places. Overall this PR removes more lines of code than it adds.I have run the test suite after each commit locally and the results looked good.
I initially included the changes also in
port.gobut as the port command allows a port to be listed after-lthe newly added common logic cannot be used inport.go.