Skip to content

Conversation

@jlebon
Copy link
Member

@jlebon jlebon commented Jan 29, 2019

Factor out the code that actually pulls and rebases so that we have the
same out path in both changed and unchanged cases in Execute(). This
will become useful for an upcoming patch where we'll introduce a systemd
service conditioned on a file, though one minor thing this fixes right
now is that we always try to delete the image, even if we didn't just
pull it.

Add a new wrapper that just ignores command errors. Then use it for
`podman kill` and `podman rm`. The advantage of this is that we now log
the invocations as well as any errors from them.
Factor out the code that actually pulls and rebases so that we have the
same out path in both changed and unchanged cases in `Execute()`. This
will become useful for an upcoming patch where we'll introduce a systemd
service conditioned on a file, though one minor thing this fixes right
now is that we always try to delete the image, even if we didn't just
pull it.
With the previous patch, we now always try to delete the image (unless
`--keep` was passed), instead of only if we pulled it in this session.
As a result, we no longer want to fail if we can't delete the image
because it doesn't exist already. I opened an RFE on podman for this,
though really, even if it failed because of another error, we don't
really want want to fail the whole operation for this.
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 29, 2019
Copy link
Member

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ashcrow
Copy link
Member

ashcrow commented Jan 30, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2019
@ashcrow ashcrow merged commit f1ed664 into openshift:master Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants