Use buildOCIProcessArgs to calculate process.args considering
PauseCommand (if it was set) and the image's configured Entrypoint and
Cmd. This improves the previous implementation, which had been
ignoring Entrypoint, despite the image-spec recommendations to
consider it [1].
There's no reason to assume the pause image contains a /pause command,
and it seems more sane to assume that the pause container has
configured Entrypoint and Cmd appropriately without our having to
repair anything. Now that we're considering Entrypoint,
kubernetes/pause (which sets Entrypoint [2]) no longer needs a
PauseCommand "override".
I've dropped the podContainer.Config nil check because later code is
using it without a guard (e.g. podContainer.Config.Config.StopSignal)
and that hasn't caused problems. The check
was added in c0333b10 (Integrate containers/storage, 2016-10-18,
#189), but there was too much going on for a motivational comment in
the commit message.
When processArgs is empty, I'm setting Process nil (which the runtime
spec allows [3]) following createSandboxContainer. If 'start' is
called on those containers and errors (as the spec requires [4]), I
think that's an "image was broken" problem and not a "cri-o didn't
guess the right value" problem. I've shifted the process.args code
down within RunPodSandbox where the possible Process nil can clobber
the Process object created by SetProcessSelinuxLabel. I didn't see
any later configuration that looked like it would create Process
(which would require process.args [5]).
Down the road we might be able to use image-tools image -> runtime
config converter [6]. It's currently a private method with public
access restricted to full bundle unpacks.
[1]: https://github.com/opencontainers/image-spec/blob/v1.0.1/conversion.md#verbatim-fields
[2]: https://github.com/kubernetes/kubernetes/blob/v1.9.2/build/pause/Dockerfile#L18
[3]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/runtime.md#L119
[5]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[6]: https://github.com/opencontainers/image-tools/blob/v0.3.0/image/config.go#L68
Signed-off-by: W. Trevor King <wking@tremily.us>
OCI runtime callers (like CRI-O) are allowed to leave process unset
[1] for containers that they do not intend to 'start'. When we don't
have any process.args, we *must* leave process unset (because
process.args is required [2]). My personal preference would have been
to have both process and process.args optional [3], which would have
allowed for these settings to be decoupled, but that's not where the
spec ended up.
When we have no args and are clearing Process, we need to ensure that
we don't re-create an args-less structure later on by populating
process.user or similar. This commit collects later process-creating
calls (e.g. setupContainerUser, which populates process.user) into the
"we have some args" branch.
This commit leaves earlier process-creating calls
(e.g. SetProcessTerminal) where they were. Anything they do inside
Process will be clobbered later if we nil it, but that's fine.
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[3]: https://github.com/opencontainers/runtime-spec/pull/701#issue-210601101
Signed-off-by: W. Trevor King <wking@tremily.us>
Handling both the nil ociConfig check and kubeCommands length check on
the same line lets us drop one level of nesting. And that initial
zero-length check makes the internal kubeCommands nil check redundant,
so we can drop that too.
Also adjust the buildOCIProcessArgs argument from a *v1.Image to
*v1.ImageConfig, because we don't need anything else from Image here.
This moves the nil Image check from inside buildOCIProcessArgs to the
calling function, but I think that's worth the tighter scoping.
Signed-off-by: W. Trevor King <wking@tremily.us>
This makes fixing errors easier. Before this commit, errors looked
like [1]:
$ make gofmt
!!! 'gofmt -s' needs to be run on the following files:
./lib/config.go
make: *** [gofmt] Error 1
But that's not very helpful when your local gofmt thinks the file is
fine. With this commit, errors will look like:
$ make gofmt
find . -name '*.go' ! -path './vendor/*' -exec gofmt -s -w {} \+
git diff --exit-code
diff --git a/lib/config.go b/lib/config.go
index 1acca8c7..6a63b2b0 100644
--- a/lib/config.go
+++ b/lib/config.go
@@ -2,7 +2,7 @@ package lib
import (
"bytes"
-"io/ioutil"
+ "io/ioutil"
"github.com/BurntSushi/toml"
"github.com/kubernetes-incubator/cri-o/oci"
make: *** [Makefile:68: gofmt] Error 1
(or whatever, I just stuffed in a formatting error for demonstration
purposes).
Also remove the helper script in favor of direct Makefile calls,
because with Git handling difference reporting and exit status, this
becomes a simpler check. find's -exec, !, and -path arguments are
specified in POSIX [2].
[1]: https://travis-ci.org/kubernetes-incubator/cri-o/jobs/331949394#L1075
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/find.html
Signed-off-by: W. Trevor King <wking@tremily.us>
The last consumer was removed in 1bf6d203 (Remove kpod code after
repository move, 2017-11-02, #1111).
Signed-off-by: W. Trevor King <wking@tremily.us>
Having a separate function holding the details of this makes reading
createSandboxContainer easier.
While I was moving the code, I've also cleaned up two things:
* The nil capabilities check is now earlier, where before it had been
between the ALL handling and the non-ALL handling.
* I've added a capPrefixed variable to avoid having multiple
toCAPPrefixed calls per capability.
Signed-off-by: W. Trevor King <wking@tremily.us>
Avoid crashing 'make' with:
No package 'devmapper' found
by disabling the devmapper driver when the library it requires is not
installed. Also give the libdm_no_deferred_remove script a more
specific name to avoid confusion.
Signed-off-by: W. Trevor King <wking@tremily.us>
Catching up with the Dockerfile change from f51b0a10 (Dockerfile: move
to containernetworking/plugins, 2017-05-25, #536). The new plugins
commit from f51b0a10 is still the current Dockerfile entry.
This commit also replaces the previous 'go get' call with a git clone
to match the Dockerfile's approach. I've added an additional 'cd'
call so I don't have to repeat $GOPATH/... more than once, but other
than that, the example matches the current Dockerfile entry.
I've also removed some line-continuation slashes we've been dragging
around since the section landed 07ccda33 (tests: Install CNI
configuration files by default, 2017-04-06, #434). I'm guessing they
were a copy/paste bug from the Dockerfile, but this example has new
prompts for each command (so it doesn't need continuation) while the
Dockerfile is using && chaining (so it does).
Signed-off-by: W. Trevor King <wking@tremily.us>
The cri-o entries are stale vs. the content currently installed by the
Makefile. This commit drops them and just references the make call
before starting the table, which lets us stay DRY.
runc is not built from the cri-o repository. The docs have claimed it
was since 983aec63 (doc: Add instruction to run cri-o with kubernetes,
2017-01-31, #353), but it's independent like the CNI plugins.
The CNI plugins were moved to containernetworking/plugins in
containernetworking/cni@bc0d09e (plugins: moved to
containernetworking/plugins, 2017-05-17, containernetworking/cni#457).
I've added a link to the in-repo policy.json example. We probably
also want to link to the docs (for the version we vendor?) [1], but
I've left that alone for now.
The CNI config examples were removed from the project README in
9088a12c (contrib: cni: provide example CNI configurations,
2016-12-24, #295). I've adjusted the reference to point to the new
location, although again, I'd rather replace this with links to
upstream docs.
[1]: 3d0304a021/docs/policy.json.md
Signed-off-by: W. Trevor King <wking@tremily.us>
'make install' hasn't installed crio.conf since 8b632729 (Install to
/usr/local to avoid conflicts with vendor binaries, 2017-01-04, #304).
And Make output is usually not particularly interesting.
Signed-off-by: W. Trevor King <wking@tremily.us>
install.config has also installed rio-umount.conf since 51b225474
(Tell oci-umount where to remove mountpoints inside container, #937,
2017-09-21). And Make output is usually not particularly interesting.
Signed-off-by: W. Trevor King <wking@tremily.us>
Where the same user had multiple entries, I mostly went with whichever
entry had the most-recent non-merge commits.
The order is alphabetical according to Emacs' sort-lines.
Signed-off-by: W. Trevor King <wking@tremily.us>
And changed the name of their binary. This commit catches the docs up
with intel/cc-oci-runtime#1065 (merged 2017-09-25).
Signed-off-by: W. Trevor King <wking@tremily.us>
If a packager wants to be able to support addititional arguments on his
hook this will allow them to setup the configuration with these arguments.
For example this would allow a hook developer to add support for a --debug
flag to change the level of debugging in his hook.
In order to complete this task, I had to vendor in the latest
github.com://opencontainers/runtime-tools, which caused me to have to fix a
Mount and Capability interface calls
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Instead of compiling all of the *.go files each time, the
-i flag will cause them to be only compiled if they changed.
This will make developers much happier.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
