Skip to content
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

runc cli: nits #3214

Merged
merged 3 commits into from
Sep 15, 2021
Merged

runc cli: nits #3214

merged 3 commits into from
Sep 15, 2021

Conversation

kolyshkin
Copy link
Contributor

Separated out of #3131 and reworked

  1. Remove unneeded newlines from error messages.
  2. Simplify startContainer call, fix runc restore not checking --pid-file argument.
  3. Wrap runc create and runc run errors.

Error messages should not usually contain newlines.

Testing shows that the error runc delete prints is the same before and
after this commit:

	[kir@kir-rhat runc-tst]$ sudo ../runc/runc delete xx3
	ERRO[0000] cannot delete container xx3 that is not stopped: running
	[kir@kir-rhat runc-tst]$

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All three callers* of startContainer call revisePidFile and createSpec
before calling it, so it makes sense to move those calls to inside of
the startContainer, and drop the spec argument.

* -- in fact restore does not call revisePidFile, but it should.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As the error may contain anything, it may not be clear to a user that
the whole (create or run) operation failed. Amend the errors.

Also, change the code flow in create to match that of run, so we don't
have to add the fake "return nil" at the end.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

LGTM.

@cyphar cyphar merged commit dbc294f into opencontainers:master Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants