remove multi-arch.bash but use skopeo to get images directly #2508

XiaodongLoong · 2020-07-07T07:22:36Z

When I test runc on mips64le, I found that the integration and rootlessintegration test only support arm64 and amd64 cpu architecture.
I developed a new way to execute the integration test which can support more cpu arch.

Signed-off-by: Xiaodong Liu liuxiaodong@loongson.cn

cyphar · 2020-07-07T07:46:52Z

#2486 is trying to clean up this issue (and should also allow us to remove multi-arch.sh). I don't really like this approach since using and requiring Docker just to get images is overkill. umoci and skopeo should be able to pull multi-arch images without issue. But if you need MIPS support then we might have to hold off on switching to openSUSE since it doesn't support MIPS...

XiaodongLoong · 2020-07-07T08:29:28Z

#2486 is trying to clean up this issue (and should also allow us to remove multi-arch.sh). I don't really like this approach since using and requiring Docker just to get images is overkill. umoci and skopeo should be able to pull multi-arch images without issue.

Sorry, I ignored the "overkill" regulation and only simplify the process.

But if you need MIPS support then we might have to hold off on switching to openSUSE since it doesn't support MIPS...

Yes, I need MIPS support indeed.

AkihiroSuda · 2020-07-07T10:01:34Z

Makefile

 	$(CONTAINER_ENGINE) build $(CONTAINER_ENGINE_BUILD_FLAGS) -t $(RUNC_IMAGE) .

+.hello-world:
+	docker export $(shell docker create hello-world) -o $(CURDIR)/tests/integration/testdata/hello-world.tar


docker -> $(CONTAINER_ENGINE)

OK, I will fix it.

XiaodongLoong · 2020-07-08T01:20:50Z

@cyphar I might have no "overkill" problem, I used docker in Makefile before starting container, it is same as executing integration test with docker.

Thanks!

kolyshkin · 2020-07-09T03:46:45Z

Please clean this PR up (you probably did you do git pull without --rebase?), it should not contain any extra commits.

kolyshkin · 2020-07-13T19:13:55Z

Needs a rebase

XiaodongLoong · 2020-07-15T10:00:37Z

This PR can make the test cases run on more CPU architecture using the ability from docker.
@cyphar @AkihiroSuda @kolyshkin PTAL Thanks!

kolyshkin · 2020-07-16T00:43:58Z

Makefile

+.debian:
+	$(CONTAINER_ENGINE) export $(shell $(CONTAINER_ENGINE) create debian) -o $(CURDIR)/tests/integration/testdata/debian.tar
+	chmod a+r $(CURDIR)/tests/integration/testdata/debian.tar
+	touch "$@"


You need to add something like rm -f .debian .busybox .hello-world to clean target. The downloaded images, too, I guess.

Also, it might be helpful to do

ln -sf $(CURDIR)/tests/integration/testdata/debian.tar $@

instead of just touch, as in the case a tarball will be removed, make will redo the target thanks to a broken symlink.

Using $(CURDIR) is redundant I think.

These three rules look similar, and it should be easy to generalize and merge those into one single rule.

Maybe move this code out to a separate Makefile in tests/integration/testdata, and just call it from the main Makefile.

kolyshkin · 2020-07-16T00:45:09Z

Makefile

 test: unittest integration rootlessintegration

-localtest: localunittest localintegration localrootlessintegration
+localtest: | all .busybox .hello-world .debian


This repetition can be avoided by having something like

TEST_IMAGES ?= debian busybox hello-world GET_IMAGE_TARGETS := $(addprefix ., $(TEST_IMAGES)) ... localtest: | $(GET_IMAGE_TARGETS)

Also, specifying these prerequisites here is redundant, as every one of (localunittest, localintegration, localrootlessintegration) already have those.

kolyshkin · 2020-07-16T00:46:19Z

Makefile

 test: unittest integration rootlessintegration

-localtest: localunittest localintegration localrootlessintegration
+localtest: | all .busybox .hello-world .debian
+	make localunittest localintegration localrootlessintegration


No need to call make here.

kolyshkin · 2020-07-16T00:46:59Z

Makefile

@@ -72,7 +88,7 @@ unittest: runcimage
 		-v $(CURDIR):/go/src/$(PROJECT) \
 		$(RUNC_IMAGE) make localunittest TESTFLAGS=$(TESTFLAGS)

-localunittest: all
+localunittest: | all .busybox .hello-world .debian


Shouldn't it be localunittest: all | .busybox .hello-world .debian?

kolyshkin · 2020-07-16T00:49:17Z

Vagrantfile.centos7

+    systemctl start docker
+    mkdir /busybox /debian
+    docker export $(docker create busybox) | tar -C /busybox/ -xvf -
+    docker export $(docker create debian) | tar -C /debian/ -xvf -


Is there a way to maybe use something lighter than docker to just download/extract images from the repo? I frankly don't know

there's https://raw.githubusercontent.com/moby/moby/master/contrib/download-frozen-image-v2.sh script

https://github.com/containers/skopeo

It's just I don't like much the idea of needing to run dockerd inside vagrant for the sole purpose of obtaining busybox (or the like) image.

What needs to be done instead is to download the images before running vagrant. Something like (in .travis.yml):

- ln -sf Vagrantfile.fedora32 Vagrantfile + - make download-test-images - sudo vagrant up && sudo mkdir -p /root/.ssh && sudo sh -c "vagrant ssh-config >> /ro

(and then vagrant up will copy those as it copies everything from the current directory into /vagrant

skopeo + umoci is what #2486 uses and (personal biases aside) I would much prefer that over using Docker just for the purposes of downloading like 3 images.

I also want us to remove the need for three different images for our tests. Since it looks like folks want super esoteric architectures, we can just use Debian in #2486 and work around the missing packages.

@cyphar
I don't know umoci can or can not download images for the CPU architecture supporting debian images. But umoci only support the ARM64 and AMD64 in the view of runc master branch.
At first, I only want to make the test cases run on more CPU architecture.

skopeo does the downloading and it does support multi-arch. umoci is a Go program and doesn't have any real architecture restrictions (aside from "you must be able to build a Go program on it"). Heck, you can even run umoci on MacOS.

All I'm suggesting is that this:

% docker export $(docker create busybox) | tar -C /busybox/ -xvf -

Becomes this:

% skopeo copy docker://busybox:latest oci:/tmp/busybox-image:latest % umoci raw unpack --image /tmp/busybox-image:latest /busybox

That's it. And the latter will be more performant because docker export $(docker create) | tar xvf - requires extracting and generating tar archives multiple times (not to mention transferring the archive and all the rest of it)!

OK, I got it.

kolyshkin · 2020-07-16T01:24:53Z

I like the idea in general, the implementation can be improved though

XiaodongLoong · 2020-07-17T10:04:32Z

@cyphar @kolyshkin Thanks for your advice. I have pushed a new commit for this PR.
There are still three test images( hello-world, busybox, debian), I suggest that we can open a new issue to run test using debian only.

Signed-off-by: liuxiaodong <liuxiaodong@loongson.cn>

kolyshkin · 2020-08-01T01:12:24Z

tests/integration/hooks.bats

@@ -14,6 +14,7 @@ function setup() {

 	teardown_debian
 	setup_debian
+	update_config '.root.readonly |= false'


as I noted earlier, it should be done

in a very test case that requires it

as a separate patch describing why it is done (since the linker in hooks test likes to create files)

If this PR don't have this patch, CI verification will fail. Can I add some notes ?

I skiped the hook test case, but add some TODO notes for an new PR to fix this issue.

XiaodongLoong · 2020-08-05T07:20:08Z

@kolyshkin
I failed to install skopeo with Permission denied error.

$ echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/  /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list
277/home/travis/.travis/functions: line 109: /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list: Permission denied
278The command "echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/  /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list" failed and exited with 1 during .

kolyshkin · 2020-08-10T06:49:55Z

I failed to install skopeo with Permission denied error.

Please see the .travis.yml file a few lines before your changes -- it calls apt, too, so you don't have to call it again.

Also, as you can see, it uses sudo and you should too.

echo ... | sudo cat > /etc/apt/....

Signed-off-by: Xiaodong Liu <liuxiaodong@loongson.cn>

XiaodongLoong · 2020-08-19T09:37:26Z

updated , @kolyshkin PTAL

kolyshkin · 2020-09-18T07:53:37Z

@cyphar do you think we should proceed with this one? Overall I like it.

XiaodongLoong force-pushed the support-more-cpu-arch branch from 3678286 to 0ab2dce Compare July 7, 2020 07:53

AkihiroSuda reviewed Jul 7, 2020

View reviewed changes

XiaodongLoong force-pushed the support-more-cpu-arch branch 2 times, most recently from 56cc22d to 67aa77e Compare July 13, 2020 10:05

XiaodongLoong force-pushed the support-more-cpu-arch branch 9 times, most recently from 2882345 to cda7423 Compare July 15, 2020 08:40

XiaodongLoong requested a review from AkihiroSuda July 15, 2020 10:19

kolyshkin reviewed Jul 16, 2020

View reviewed changes

XiaodongLoong force-pushed the support-more-cpu-arch branch 2 times, most recently from 70025dc to cbf0a2d Compare July 17, 2020 09:36

XiaodongLoong added a commit to XiaodongLoong/runc that referenced this pull request Jul 17, 2020

https://github.com/opencontainers/runc/pull/2508

374664d

Signed-off-by: liuxiaodong <liuxiaodong@loongson.cn>

kolyshkin reviewed Aug 1, 2020

View reviewed changes

XiaodongLoong force-pushed the support-more-cpu-arch branch 4 times, most recently from 7a661dc to 922e507 Compare August 5, 2020 06:22

XiaodongLoong force-pushed the support-more-cpu-arch branch 3 times, most recently from 2455aae to db6d0bf Compare August 13, 2020 01:27

XiaodongLoong changed the title ~~remove multi-arch.bash but use docker to get images directly~~ remove multi-arch.bash but use skopeo to get images directly Aug 13, 2020

XiaodongLoong force-pushed the support-more-cpu-arch branch 12 times, most recently from 2aa26a4 to 9a9e527 Compare August 17, 2020 09:27

remove multi-arch.bash but use skopeo to get images directly

fdc6311

Signed-off-by: Xiaodong Liu <liuxiaodong@loongson.cn>

XiaodongLoong force-pushed the support-more-cpu-arch branch from 9a9e527 to fdc6311 Compare August 17, 2020 11:01

XiaodongLoong requested a review from kolyshkin August 26, 2020 02:03

XiaodongLoong closed this Sep 18, 2020

kolyshkin mentioned this pull request Jan 21, 2021

ci: untangle getting test images #2741

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove multi-arch.bash but use skopeo to get images directly #2508

remove multi-arch.bash but use skopeo to get images directly #2508

XiaodongLoong commented Jul 7, 2020

cyphar commented Jul 7, 2020 •

edited

Loading

XiaodongLoong commented Jul 7, 2020

AkihiroSuda Jul 7, 2020

XiaodongLoong Jul 8, 2020

XiaodongLoong commented Jul 8, 2020

kolyshkin commented Jul 9, 2020

kolyshkin commented Jul 13, 2020

XiaodongLoong commented Jul 15, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

kolyshkin Jul 16, 2020

cyphar Jul 16, 2020

cyphar Jul 16, 2020

XiaodongLoong Jul 16, 2020

cyphar Jul 16, 2020

XiaodongLoong Jul 16, 2020

kolyshkin commented Jul 16, 2020

XiaodongLoong commented Jul 17, 2020 •

edited

Loading

kolyshkin Aug 1, 2020

XiaodongLoong Aug 4, 2020 •

edited

Loading

XiaodongLoong Aug 17, 2020

XiaodongLoong commented Aug 5, 2020 •

edited

Loading

kolyshkin commented Aug 10, 2020

XiaodongLoong commented Aug 19, 2020

kolyshkin commented Sep 18, 2020

remove multi-arch.bash but use skopeo to get images directly #2508

remove multi-arch.bash but use skopeo to get images directly #2508

Conversation

XiaodongLoong commented Jul 7, 2020

cyphar commented Jul 7, 2020 • edited Loading

XiaodongLoong commented Jul 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaodongLoong commented Jul 8, 2020

kolyshkin commented Jul 9, 2020

kolyshkin commented Jul 13, 2020

XiaodongLoong commented Jul 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kolyshkin commented Jul 16, 2020

XiaodongLoong commented Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

XiaodongLoong Aug 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaodongLoong commented Aug 5, 2020 • edited Loading

kolyshkin commented Aug 10, 2020

XiaodongLoong commented Aug 19, 2020

kolyshkin commented Sep 18, 2020

cyphar commented Jul 7, 2020 •

edited

Loading

XiaodongLoong commented Jul 17, 2020 •

edited

Loading

XiaodongLoong Aug 4, 2020 •

edited

Loading

XiaodongLoong commented Aug 5, 2020 •

edited

Loading