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

rkt: fix panic on rm a non-existing pod with uuid-file #2679

Merged
merged 2 commits into from May 24, 2016

Conversation

Projects
None yet
3 participants
@iaguis
Member

iaguis commented May 24, 2016

We were trusting that the value in the uuid file referred to an existing
pod, which caused a panic if that wasn't the case.

In this commit we resolve the UUID to check if the pod actually exists.

Also, make rm's behavior consistent between caling it with a pod UUID
and with --uuid-file:

  • Print a similar error message
  • Set the exit status to 1 if we fail to remove any pod

There's no tests for rkt rm. I add some in #2655. I can add a test-case for this there.

Fixes #2678

@jonboulle

This comment has been minimized.

Show comment
Hide comment
@jonboulle

jonboulle May 24, 2016

Contributor

lgtm - test?

Contributor

jonboulle commented May 24, 2016

lgtm - test?

@iaguis

This comment has been minimized.

Show comment
Hide comment
@iaguis

iaguis May 24, 2016

Member

Added tests.

Member

iaguis commented May 24, 2016

Added tests.

iaguis added some commits May 24, 2016

rkt: fix panic on rm a non-existing pod with uuid-file
We were trusting that the value in the uuid file referred to an existing
pod, which caused a panic if that wasn't the case.

In this commit we resolve the UUID to check if the pod actually exists.

Also, make rm's behavior consistent between caling it with a pod UUID
and with `--uuid-file`:

* Print a similar error message
* Set the exit status to 1 if we fail to remove any pod

@iaguis iaguis added the needs/review label May 24, 2016

@iaguis

This comment has been minimized.

Show comment
Hide comment
@iaguis

iaguis May 24, 2016

Member

Rebased. PTAL.

Member

iaguis commented May 24, 2016

Rebased. PTAL.

@lucab

This comment has been minimized.

Show comment
Hide comment
@lucab

lucab May 24, 2016

Member

LGTM

Member

lucab commented May 24, 2016

LGTM

@lucab lucab added reviewed/lgtm and removed needs/review labels May 24, 2016

@lucab lucab merged commit 8f74d28 into rkt:master May 24, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment