Add 'extras' virtual package to fix kernel upgrade fallback from aufs to devicemapper #10860

Merged
merged 1 commit into from Mar 17, 2015

Conversation

Projects
None yet
8 participants
@mbentley
Contributor

mbentley commented Feb 17, 2015

Signed-off-by: Matt Bentley mbentley@mbentley.net

@mbentley

This comment has been minimized.

Show comment
Hide comment
@mbentley

mbentley Feb 17, 2015

Contributor

PR for discussion of #10859

Contributor

mbentley commented Feb 17, 2015

PR for discussion of #10859

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Feb 19, 2015

Contributor

hmmm ping @tianon idk what the virtual extras do

Contributor

jessfraz commented Feb 19, 2015

hmmm ping @tianon idk what the virtual extras do

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Feb 23, 2015

Member

https://packages.debian.org/sid/linux-image-extra-virtual is a 404 -- where does this package virtual come from? Is it Ubuntu-specific?

Member

tianon commented Feb 23, 2015

https://packages.debian.org/sid/linux-image-extra-virtual is a 404 -- where does this package virtual come from? Is it Ubuntu-specific?

@mbentley

This comment has been minimized.

Show comment
Hide comment
@mbentley

mbentley Feb 23, 2015

Contributor

@tianon - yes, it is Ubuntu specific but so are all of the linux-image-extra-* packages as far as I know.

*edit: http://packages.ubuntu.com/search?keywords=linux-image-extra-virtual

Contributor

mbentley commented Feb 23, 2015

@tianon - yes, it is Ubuntu specific but so are all of the linux-image-extra-* packages as far as I know.

*edit: http://packages.ubuntu.com/search?keywords=linux-image-extra-virtual

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Feb 23, 2015

Member

Oh right, the || true makes sure that works regardless.

However, this is potentially more dangerous than the other one is -- this package will install linux-image-generic (http://packages.ubuntu.com/vivid/linux-image-generic), which could then install a different kernel from what the user is currently using/intended to use. I guess we could argue that if they care about the way their system is configured, they shouldn't be running a random "install" script off the internet (which is what I'd rather encourage anyhow -- IMO this script needs to go away completely at some point).

Member

tianon commented Feb 23, 2015

Oh right, the || true makes sure that works regardless.

However, this is potentially more dangerous than the other one is -- this package will install linux-image-generic (http://packages.ubuntu.com/vivid/linux-image-generic), which could then install a different kernel from what the user is currently using/intended to use. I guess we could argue that if they care about the way their system is configured, they shouldn't be running a random "install" script off the internet (which is what I'd rather encourage anyhow -- IMO this script needs to go away completely at some point).

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Feb 23, 2015

Member

@unclejack what do you think?

Member

tianon commented Feb 23, 2015

@unclejack what do you think?

@unclejack

This comment has been minimized.

Show comment
Hide comment
@unclejack

unclejack Feb 23, 2015

Contributor

@tianon Perhaps we should prompt for the installation of the generic-extra kernel when detecting the virtual kernel without aufs on Ubuntu?

Contributor

unclejack commented Feb 23, 2015

@tianon Perhaps we should prompt for the installation of the generic-extra kernel when detecting the virtual kernel without aufs on Ubuntu?

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Feb 23, 2015

Member

That sounds reasonable to me! 👍

Member

tianon commented Feb 23, 2015

That sounds reasonable to me! 👍

@GordonTheTurtle

This comment has been minimized.

Show comment
Hide comment
@GordonTheTurtle

GordonTheTurtle Feb 28, 2015

Can you please sign your commits following these rules:

https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work

The easiest way to do this is to amend the last commit:

$ git clone -b "curl-install-fix" git@github.com:mbentley/docker.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

This will update the existing PR, so you do not need to open a new one.

Can you please sign your commits following these rules:

https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work

The easiest way to do this is to amend the last commit:

$ git clone -b "curl-install-fix" git@github.com:mbentley/docker.git somewhere
$ cd somewhere
$ git rebase -i HEAD~2
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

This will update the existing PR, so you do not need to open a new one.

@mbentley

This comment has been minimized.

Show comment
Hide comment
@mbentley

mbentley Feb 28, 2015

Contributor

I added some logic to check and see if the following were true:

  1. The current kernel was a -generic kernel
  2. That a linux-image-*-generic package was installed

This should verify that the packages linux-image-extra-$(uname -r) and linux-image-extra-virtual won't install a different kernel than what is currently running and ensure that the package is really installed and that someone just don't have -generic in the name of their custom compiled kernel.

If the kernel doesn't match, it will just give a warning and suggest installing linux-image-virtual (which will install the linux-image-generic package) and linux-image-extra-virtual for proper AUFS support.

Contributor

mbentley commented Feb 28, 2015

I added some logic to check and see if the following were true:

  1. The current kernel was a -generic kernel
  2. That a linux-image-*-generic package was installed

This should verify that the packages linux-image-extra-$(uname -r) and linux-image-extra-virtual won't install a different kernel than what is currently running and ensure that the package is really installed and that someone just don't have -generic in the name of their custom compiled kernel.

If the kernel doesn't match, it will just give a warning and suggest installing linux-image-virtual (which will install the linux-image-generic package) and linux-image-extra-virtual for proper AUFS support.

@tianon

View changes

project/install.sh
@@ -125,14 +125,21 @@ case "$lsb_dist" in
# aufs is preferred over devicemapper; try to ensure the driver is available.
if ! grep -q aufs /proc/filesystems && ! $sh_c 'modprobe aufs'; then
- kern_extras="linux-image-extra-$(uname -r)"
+ if echo $(uname -r) | grep -q '\-generic' && dpkg -l linux-image-*-generic | grep '^ii' > /dev/null 2>&1; then

This comment has been minimized.

@tianon

tianon Mar 3, 2015

Member

This could be simplified somewhat, couldn't it? (and catch the edge case of linux-image-*-generic somehow matching files in the user's current directory)

if uname -r | grep -q -- '-generic' && dpkg -l 'linux-image-*-generic' | grep -q '^ii' 2>/dev/null; then
@tianon

tianon Mar 3, 2015

Member

This could be simplified somewhat, couldn't it? (and catch the edge case of linux-image-*-generic somehow matching files in the user's current directory)

if uname -r | grep -q -- '-generic' && dpkg -l 'linux-image-*-generic' | grep -q '^ii' 2>/dev/null; then

This comment has been minimized.

@mbentley

mbentley Mar 3, 2015

Contributor

Hmm, indeed. Not sure what exactly I was doing with the echo there but yes, that's great, thanks for catching that!

@mbentley

mbentley Mar 3, 2015

Contributor

Hmm, indeed. Not sure what exactly I was doing with the echo there but yes, that's great, thanks for catching that!

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Mar 3, 2015

Member

👍 for the new logic! 😄

Member

tianon commented Mar 3, 2015

👍 for the new logic! 😄

@tianon

This comment has been minimized.

Show comment
Hide comment
Member

tianon commented Mar 5, 2015

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Mar 15, 2015

Member

@mbentley I think this needs a rebase; project/install.sh was moved to hack/install.sh

Member

thaJeztah commented Mar 15, 2015

@mbentley I think this needs a rebase; project/install.sh was moved to hack/install.sh

Add 'extras' virtual package to fix kernel upgrade fallback from aufs…
… to devicemapper

Added checks for the proper kernel support before blindly installing extras package

Fixes #10859

Signed-off-by: Matt Bentley <mbentley@mbentley.net>
@mbentley

This comment has been minimized.

Show comment
Hide comment
@mbentley

mbentley Mar 16, 2015

Contributor

thanks @thaJeztah. Rebased.

Contributor

mbentley commented Mar 16, 2015

thanks @thaJeztah. Rebased.

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Mar 17, 2015

Contributor

LGTM

Contributor

jessfraz commented Mar 17, 2015

LGTM

jessfraz pushed a commit that referenced this pull request Mar 17, 2015

Jessie Frazelle
Merge pull request #10860 from mbentley/curl-install-fix
Add 'extras' virtual package to fix kernel upgrade fallback from aufs to devicemapper

@jessfraz jessfraz merged commit ea25582 into moby:master Mar 17, 2015

1 of 2 checks passed

windows Jenkins build Windows-PRs 428 has failed
Details
janky Jenkins build Docker-PRs 3358 has succeeded
Details
@dredknight

This comment has been minimized.

Show comment
Hide comment
@dredknight

dredknight May 12, 2015

Same issue but the docker OS is centos7. How to fix it there?

Same issue but the docker OS is centos7. How to fix it there?

@mbentley mbentley deleted the mbentley:curl-install-fix branch May 12, 2015

@mbentley

This comment has been minimized.

Show comment
Hide comment
@mbentley

mbentley May 12, 2015

Contributor

Are you certain that you started with aufs support on CentOS 7? I wouldn't think so unless you compiled your own kernel as CentOS 7 and RHEL 7 are not shipping with aufs support to the best of my knowledge.

Contributor

mbentley commented May 12, 2015

Are you certain that you started with aufs support on CentOS 7? I wouldn't think so unless you compiled your own kernel as CentOS 7 and RHEL 7 are not shipping with aufs support to the best of my knowledge.

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