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.

Sign up for GitHub

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

Jump to bottom

Aufs to overlay2 migration utility #168

Merged
4 commits merged into from
Apr 15, 2021
Merged

Aufs to overlay2 migration utility #168

4 commits merged into from
Apr 15, 2021

Conversation

robertgzr
Copy link
Contributor

Migrate image store, containers and daemon configs to overlayfs (overlay2 graphdriver)

@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from 9bbc785 to c87c269 Compare June 3, 2019 17:12
@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from bc6cd2d to 3e70c79 Compare June 14, 2019 17:07
@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from 5c9e031 to 9088b6a Compare July 1, 2019 15:48
@robertgzr robertgzr mentioned this pull request Jul 4, 2019
Closed
4 tasks
@robertgzr robertgzr force-pushed the a2o-migrate branch 3 times, most recently from 08b758c to c6eb51b Compare September 24, 2019 13:23
@robertgzr robertgzr changed the title [WIP] aufs to overlay2 migration utility aufs to overlay2 migration utility Sep 30, 2019
@robertgzr robertgzr changed the title aufs to overlay2 migration utility Aufs to overlay2 migration utility Sep 30, 2019
@robertgzr robertgzr marked this pull request as ready for review September 30, 2019 18:09
roman-mazur
roman-mazur reviewed Oct 8, 2019
View reviewed changes
Copy link
Contributor

@roman-mazur roman-mazur left a comment

Choose a reason for hiding this comment

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

We don't seem to have a test that would illustrate a rollback scenario.
IMHO it's worth adding to catch other errors like those usages of "overlay2" string instead of the value passed by a user.

cmd/a2o-migrate/a2o/commit.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/a2o/migrate.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/a2o/utils.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/a2o/utils.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/aufsutil/aufsutils.go Outdated
@@ -0,0 +1,80 @@
// TODO: do we need to handle .wh..wh.plnk layer hardlinks?
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an answer to the question? :)

cmd/a2o-migrate/cli/main.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/test/a2o-migrate.sh Outdated Show resolved Hide resolved
cmd/a2o-migrate/test/integration.sh Outdated Show resolved Hide resolved
cmd/a2o-migrate/test/Dockerfile Outdated Show resolved Hide resolved
vendor/github.com/containerd/containerd/cmd/containerd-shim/main_unix.go Outdated Show resolved Hide resolved
roman-mazur
roman-mazur reviewed Oct 12, 2019
View reviewed changes
cmd/a2o-migrate/a2o/commit.go Outdated Show resolved Hide resolved
cmd/a2o-migrate/test/integration.sh Outdated Show resolved Hide resolved
contrib/beind/Dockerfile Outdated Show resolved Hide resolved
@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from 4c3c293 to 57a16a2 Compare October 14, 2019 11:31
@robertgzr robertgzr mentioned this pull request Oct 29, 2019
Merged
@robertgzr
Copy link
Contributor Author

@roman-mazur I have finally squashed this down :)

roman-mazur
roman-mazur reviewed Oct 29, 2019
View reviewed changes
Copy link
Contributor

@roman-mazur roman-mazur left a comment

Choose a reason for hiding this comment

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

Looks good to me in general. However, it would be great to have a test that tries to simulate a failure and rollback (just to make sure we cover basic path).

For instance, having an integration-test-failure.sh:

./a2o-migrate -version
./a2o-migrate -debug -migrate
// Maybe also do some files corruption (rewriting/deleting some of the migrated files)
./a2o-migrate -debug -rollback

@roman-mazur
Copy link
Contributor

Is it ok travis build fails? Should we fix it or disable it?

@robertgzr
Copy link
Contributor Author

@roman-mazur fixing the travis build here: #189

@robertgzr
Copy link
Contributor Author

simulate a failure and rollback

sounds good. will work on that tomorrow

@robertgzr
Copy link
Contributor Author

@roman-mazur should we close this for now and reopen when we move the migration logic to the engine too?

@robertgzr
Copy link
Contributor Author

I created #205 to track the motivation and progress of this, should we decide to postpone the work.

@robertgzr robertgzr mentioned this pull request Jun 2, 2020
Closed
@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from c7b1ce9 to d17f992 Compare June 11, 2020 13:08
@robertgzr
Copy link
Contributor Author

robertgzr commented Jun 11, 2020 •
edited
Loading

updated this with the new approach. moving the a2o code under pkg/storagemigration and implementing it to run when the engine starts.

Right now it's guarded by a check if the storage driver is set to overlay2 and looks for an env var BALENA_MIGRATE_OVERLAY. The logic is mostly identical to what we had before...

It will attempt a migration. If that fails run the fail-cleanup routine and if not commit the changes.

Of course this can't check if the migration was actually successful in the sense that we can run anything...

I think it makes more sense to have the integration testing done using a custom meta-balena image, since the previous vagrant-based test needs a custom OS anyway

@saintaardvark saintaardvark mentioned this pull request Sep 16, 2020
Open
@robertgzr robertgzr force-pushed the a2o-migrate branch 2 times, most recently from 7b71bff to 7a56167 Compare September 17, 2020 16:50
@robertgzr
Export GenerateID and IDLength from overlay2 graphdriver
eea708c 
We use this from a2o-migrate to get valid layer IDs

Signed-off-by: Robert Günzler <robertg@balena.io>
@ghost
Copy link

ghost commented Jan 25, 2021

Your landr site preview has been successfully deployed to https://landr-balena-os-repo-balena-engine-preview-168.netlify.app

Deployed with Landr 6.13.1

@robertgzr robertgzr removed the request for review from petrosagg January 26, 2021 10:29
@robertgzr robertgzr marked this pull request as ready for review January 28, 2021 09:58
klutchell
klutchell reviewed Feb 1, 2021
View reviewed changes
daemon/daemon.go Outdated Show resolved Hide resolved
@pdcastro pdcastro mentioned this pull request Mar 26, 2021
Merged
@robertgzr robertgzr mentioned this pull request Apr 12, 2021
Closed
12 tasks
@robertgzr
Add aufs to overlay2 migrator
b3a976b 
The main logic is under pkg/storagemigration. This is able to seamlessly
migrate images and containers from AUFS to overlay2.

Change-type: patch
Signed-off-by: Robert Günzler <robertg@balena.io>
@robertgzr
pkg/storagemigration: add unit tests
9a7db57 
Signed-off-by: Robert Günzler <robertg@balena.io>
@robertgzr
pkg/storagemigration: add integration test
01b1943 
Signed-off-by: Robert Günzler <robertg@balena.io>
@robertgzr
Copy link
Contributor Author

@klutchell I think I want to merge this one pretty soon. Most of the work here has gone through pretty intense manual testing (on the big platforms anyway) and should we identify any follow-up problems they can always be fixed in subsequent PRs

For the most part merging this would allow me to open the meta-balena part of this as a PR pointing to engine master :)

@robertgzr
Copy link
Contributor Author

@balena-ci rebase

@klutchell
Copy link
Contributor

@balena-ci rebase

klutchell
klutchell approved these changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@klutchell klutchell left a comment

Choose a reason for hiding this comment

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

LGTM!

@ghost ghost merged commit b6b8d35 into master Apr 15, 2021
@robertgzr robertgzr deleted the a2o-migrate branch April 15, 2021 13:42
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-mazur roman-mazur roman-mazur left review comments

@klutchell klutchell klutchell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@robertgzr @roman-mazur @klutchell