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.

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

Add a preinst script to fail if aufs is in use #4

Merged
merged 3 commits into from
Mar 2, 2021

Conversation

jawn-smith
Copy link

The aufs storage driver is deprecated. If any containers are using aufs, we should block the upgrade of the docker package so that the users can make the necessary changes to get off of aufs.

The existence of /var/lib/docker/aufs is the best indicator for whether any containers may be using aufs. This is how docker's backend code checks the valid storage drivers, and avoids any complications of having multiple docker daemons running simultaneously.

@tianon
Copy link
Owner

tianon commented Feb 24, 2021

IMO this would be worth a NEWS item (probably both here and on src:aufs-tools which provides auplink which all AUFS users should have installed and be using but might not be, hence why it's useful in both).

Hitting src:aufs-tools with a NEWS entry also will have a higher chance of catching non-Docker users of AUFS (if there are many of those left 😬).

3. Update to src:linux >= 4.1.1-1~exp1 ("aufs: Apply patches to enable
building aufs out-of-tree"), and then compile the aufs modules out-of-tree
(a package for doing this module compilation automatically doesn't yet
exist at the time of this writing, but might in the future).
Copy link
Owner

Choose a reason for hiding this comment

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

Hah! Since I wrote this, aufs-dkms is now a thing -- is that going to be "supported" on Ubuntu to the same level it is on Debian, or are even the "AUFS enabling" patches being removed from src:linux* in Ubuntu?

Copy link
Owner

Choose a reason for hiding this comment

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

(If not, it may be worth calling that out in the above NEWS entry, but if so then it's probably fine to just leave as-is and make users actually dig if they want to stay on AUFS for some reason I cannot fathom.)

Copy link
Owner

Choose a reason for hiding this comment

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

Having done some more digging, aufs-dkms is in bad shape, and in Ubuntu is currently provided by the kernel package (and due to the basket case AUFS is, can't really be sustainable/maintainable for the wide swath of kernels Ubuntu supports), so it's probably worth making the new NEWS entry explicitly call out that option three here is not actually worth users time trying to chase down and they do need to migrate (or maintain their own kernel builds :trollface:).

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the NEWS entry. Thanks for the feedback!

Copy link
Owner

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Other than clarifying around aufs-dkms, I think this looks fine. 👍

Copy link
Owner

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Looks sane to me 👍

Is there anyone else on the Canonical side who ought to review this?

(Also, did you want to do the upload/update dance post-merge or pre-merge?)

@jawn-smith
Copy link
Author

I believe @mwhudson is going to take a look and do the upload/merge later. Thanks for reviewing!

@mwhudson mwhudson merged commit cb8b6b3 into tianon:ubuntu Mar 2, 2021
@jawn-smith jawn-smith deleted the ubuntu branch March 2, 2021 14:52
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.

None yet

3 participants