Skip to content
This repository was archived by the owner on Oct 10, 2020. It is now read-only.

[merged] Migrate script should now work since we are after docker-1.10#498

Closed
shishir-a412ed wants to merge 1 commit intoprojectatomic:masterfrom
shishir-a412ed:test_atomic_migrate
Closed

[merged] Migrate script should now work since we are after docker-1.10#498
shishir-a412ed wants to merge 1 commit intoprojectatomic:masterfrom
shishir-a412ed:test_atomic_migrate

Conversation

@shishir-a412ed
Copy link

Signed-off-by: Shishir Mahajan shishir.mahajan@redhat.com

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
@shishir-a412ed
Copy link
Author

This is a followup PR for #475. I have tested this on docker 1.12 and migrate tests run successfully on my local machine. Lets see if it passes jenky (docker 1.10) here.

This PR fixes 3 issues:

  1. With the introduction of dockerd (daemon binary) and docker (client binary), our existing mechanism of capturing dockerPid was failing. This PR fixes that.

  2. docker save doesn't allow to save / export empty images.
    https://github.com/projectatomic/atomic/blob/master/tests/test-images/Dockerfile.2 was creating an empty image. and When atomic migrate export tried to export it, we were getting this ERROR:
    Migrate script should now work since we are after docker-1.10 #475 (comment)

I checked with @willmtemple and since there were no references to this Dockerfile, I have removed it.

  1. docker has made changes to how it handles its {images/containers} layers. Now if I create/run a docker container and don't do any operations on this container i.e. docker diff is empty. Docker packs this with an entire rootfs. so something like.
 a) docker run -it fedora /bin/bash
 b) Exit the container 
 c) docker diff $container_ID should show `no results`.
 d) docker commit $(docker ps -lq) $Image_Name
 e) docker save $Image_Name > image.tar
 f) Untar image.tar
 g) We should see 1 layer with entire rootfs packed in it. 

When atomic migrate import tries to import this container it tried to apply the entire rootfs on the newly created container rootfs, which would obviously fail. So this actually gives us an opportunity to optimize atomic migrate export and import.

With this PR, atomic migrate export will only create container-diff.tar if there is an actual diff ELSE if will just skip all the steps needed to create container-diff.tar.

When importing if the container-diff.tar file is present, it will create a new container and apply the diff. If it is not present, it will just create a regular container and update the metadata.

Shishir

@shishir-a412ed
Copy link
Author

@rhatdan @jlebon Please review.

Shishir

@@ -1,4 +0,0 @@
FROM scratch
Copy link
Member

Choose a reason for hiding this comment

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

Why is this deleted?

Copy link
Author

Choose a reason for hiding this comment

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

Please see point (2) above.

Copy link
Member

Choose a reason for hiding this comment

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

Ok and no one was using this container? Does atomic migrate blow up with this situation? Why shouldn't this work?

Copy link
Author

Choose a reason for hiding this comment

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

docker doesn't allow saving empty images.
You can take this Dockerfile (the one I deleted), place it in /tmp and build an image.

a) docker build -t empty /tmp
b) docker save empty > empty_image.tar

When trying (b) you should get an error Error response from daemon: empty export - not implemented.

This is something docker doesn't provide. It has nothing to do with atomic migrate export

@rhatdan
Copy link
Member

rhatdan commented Jul 25, 2016

Awesome

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit 6e06772 has been approved by rhatdan

@rh-atomic-bot
Copy link

⌛ Testing commit 6e06772 with merge 7fcd49e...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: rhatdan
Pushing 7fcd49e to master...

@rh-atomic-bot rh-atomic-bot changed the title Migrate script should now work since we are after docker-1.10 [merged] Migrate script should now work since we are after docker-1.10 Jul 25, 2016
@giuseppe
Copy link
Collaborator

now test_migration fails for me like:

==== ./tests/integration/test_migrate.sh ====
+ set -euo pipefail
+ IFS='
    '
++ id -u
+ [[ 0 -ne 0 ]]
++ ps -q 1 -o comm=
+ init=systemd
+ '[' systemd '!=' systemd ']'
+ systemctl is-active docker
++ systemctl show -p MainPID docker.service
+ pid=MainPID=19679
++ echo 19679
+ dockerPid=19679
++ cat /proc/19679/cmdline
+ dockerCmdline=/usr/bin/dockerdaemon--exec-optnative.cgroupdriver=systemd--selinux-enabled
+ [[ /usr/bin/dockerdaemon--exec-optnative.cgroupdriver=systemd--selinux-enabled =~ -g= ]]
+ [[ /usr/bin/dockerdaemon--exec-optnative.cgroupdriver=systemd--selinux-enabled =~ -g/ ]]
+ [[ /usr/bin/dockerdaemon--exec-optnative.cgroupdriver=systemd--selinux-enabled =~ --graph ]]
+ '[' '!' -f /etc/sysconfig/docker ']'
+ '[' '!' -f /etc/sysconfig/docker-storage ']'
+ trap cleanup EXIT
+ atomic_storage_migrate
+ setup
++ /usr/bin/docker create --name test-migrate-1 -v /tmp fedora /bin/bash
+ CNT1=5a9dfcaab69aeb539fdf31478809b1315387d18d725f02459c40d749919a622c
++ /usr/bin/docker create --name test-migrate-2 -v /tmp busybox /bin/bash
+ CNT2=4dd485a71186a22ef5b8055af8d09b2c3ece3c762ef4d258385723f6636ffe53
+ echo y
++ pwd
+ /usr/bin/coverage run --source=./Atomic/ --branch --append atomic storage export --dir /home/gscrivano/src/atomic/migrate-dir
Exporting image: abe89eb35852
Exporting image: 6929dd943dec
Exporting image: 3d054f48e7dd
Exporting image: 8a4f8b9bc4fa
Exporting image: 73cc14ef2a33
Exporting image: 74a0a49d4fa7
Exporting image: 25efdd09cb66
Exporting image: 065a5e930c9b
Exporting image: 0d23b4ff1664
Exporting image: cf62323fa025
Exporting image: f9873d530588
Exporting image: 07d1216029a8
Exporting image: 50dae1ee8677
Exporting image: 2b8fd9751c4c
Exporting image: 0aea26a1f353
Exporting image: 44ef33aa689e
Exporting image: e5c8bcfbb6e4
Exporting image: 34bccec54793
Exporting image: f22df80bcef2
Exporting image: 7f3d53d72cb0
Exporting image: 89eca38e99b5
Exporting image: e42d9c8e1679
Exporting image: de5b81a43711
Exporting image: e6cd25aea7a9
Exporting image: 2ed032aef451
Exporting container: 4dd485a71186
Exporting container: 5a9dfcaab69a
Exporting container: aea8897ad83a
Exporting container: c9ce314d1e93
Exporting volumes
atomic export completed successfully
+ switch_docker_storage
+ systemctl stop docker
+ mkdir -p /var/lib/overlayfs
+ mount -o bind /var/lib/overlayfs /var/lib/docker
+ restorecon -R -v /var/lib/docker
restorecon reset /var/lib/docker context unconfined_u:object_r:var_lib_t:s0->unconfined_u:object_r:docker_var_lib_t:s0
+ systemctl start docker
+ echo y
++ pwd
+ /usr/bin/coverage run --source=./Atomic/ --branch --append atomic storage import --dir /home/gscrivano/src/atomic/migrate-dir
Importing image: 07d1216029a8
Importing image: abe89eb35852
Importing image: 6929dd943dec
Importing image: e6cd25aea7a9
Importing image: 0d23b4ff1664
Importing image: 89eca38e99b5
Importing image: 2b8fd9751c4c
Importing image: 34bccec54793
Importing image: 44ef33aa689e
Importing image: 73cc14ef2a33
Importing image: 2ed032aef451
Importing image: cf62323fa025
Importing image: 74a0a49d4fa7
Importing image: 8a4f8b9bc4fa
Importing image: 7f3d53d72cb0
Importing image: 065a5e930c9b
Importing image: f9873d530588
Error response from daemon: invalid argument

@yuqi-zhang
Copy link
Contributor

I get a similar result:

+ /usr/bin/coverage run --source=./Atomic/ --branch --append atomic storage import --dir /vagrant/atomic/migrate-dir
Importing image: 2239e93a0162
Importing image: 2b8fd9751c4c
Importing image: 0bd3bf69e0c4
Importing image: e4b679b71025
Importing image: 50dae1ee8677
Importing image: 95c34f735847
Error response from daemon: invalid argument

Importing image: a0fbb7ef4c4a
+ cleanup
+ systemctl stop docker

Also, I noticed that the "migrate-dir" currently gets generated in the source folder (i.e. /atomic/migrate-dir), and isn't cleaned up.

@shishir-a412ed
Copy link
Author

shishir-a412ed commented Jul 25, 2016

@giuseppe @yuqi-zhang That's strange. Can I SSH into one of your machine to take a look ?

@yuqi-zhang the migrate-dir didn't get cleaned up because atomic storage import was not completed successfully. the cleanup was part of atomic storage import. We need to clean it up even in case of failure. Can you raise a github issue and ping me there. I ll take a look and open a PR with the fix.

Shishir

@giuseppe
Copy link
Collaborator

I can reproduce on a fresh installed F24 VM.

I use this Ansible playbook to provision it and then just git clone atomic and run make check there:

https://gist.github.com/giuseppe/065818b9e6c962b91775be6104e2ca74

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants