Skip to content

Handle abs path for rootfs in oci hook#2418

Merged
stevvooe merged 1 commit intocontainerd:masterfrom
crosbymichael:hook-root
Jul 17, 2018
Merged

Handle abs path for rootfs in oci hook#2418
stevvooe merged 1 commit intocontainerd:masterfrom
crosbymichael:hook-root

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

Fixes #2412

Signed-off-by: Michael Crosby [email protected]

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2418 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2418   +/-   ##
=======================================
  Coverage   45.03%   45.03%           
=======================================
  Files          92       92           
  Lines        9407     9407           
=======================================
  Hits         4236     4236           
  Misses       4488     4488           
  Partials      683      683
Flag Coverage Δ
#linux 49.26% <ø> (ø) ⬆️
#windows 41.3% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f15c3be...e239f65. Read the comment docs.

@dmcgowan
Copy link
Copy Markdown
Member

LGTM

@alban
Copy link
Copy Markdown
Contributor

alban commented Jun 27, 2018

I am confused how to use this with the ctr containers create CLI tool. Could you give an example showing how to use the ctr CLI flags to specify the config.json and the rootfs? The doc says:

NAME:
   ctr containers create - create container

USAGE:
   ctr containers create [command options] [flags] Image|RootFS CONTAINER

But when I give a RootFS in the command line, it seems to be interpreted as an image and I get an error like ctr: image "/path/to/rootfs": not found.

@estesp
Copy link
Copy Markdown
Member

estesp commented Jul 11, 2018

@alban from the original issue report, you need the --rootfs flag to force parsing the "image reference" as a root filesystem path instead of an image reference for both ctr containers create or ctr run:

$ ctr run --rm --gpus 0 --rootfs ~/runc/ubuntu test /bin/bash

@dmcgowan
Copy link
Copy Markdown
Member

ping @alban were you able to get this working?

@alban
Copy link
Copy Markdown
Contributor

alban commented Jul 16, 2018

@estesp @dmcgowan Thanks! I got it working :)

For the record, here are the notes of my test:

Details I was looking for a way to give both `rootfs` and `config.json` on the command line.

The use case is for runtime-tools: it creates a unique temporary directory such as /tmp/ocitest800635031 and populates it with:

  • /tmp/ocitest242104745/config.json
  • /tmp/ocitest242104745/runtimetest
  • etc.

The rootfs is . so there is no rootfs subdirectory and the container process has access to the config.json directly, so it can check what configuration it is supposed to have and whether it matches the reality.

	"root": {
		"path": "."
	},
        "process": {
                "args": [
                        "/runtimetest",

If ctr sends the config.json content over gRPC without context, containerd cannot know how to interpret the relative path ..

The command that worked was:

$ ctr -n oci-tests containers create --config /tmp/ocitest673856730/config.json --rootfs /tmp/ocitest673856730 39742f98-2908-47e1-8f3f-7da79acaf5c2

Copy link
Copy Markdown
Member

@stevvooe stevvooe left a comment

Choose a reason for hiding this comment

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

Lgtm

@stevvooe stevvooe merged commit dfde5ec into containerd:master Jul 17, 2018
@crosbymichael crosbymichael deleted the hook-root branch July 19, 2018 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants