minishift / minishift-centos-iso

CentOS based ISO as an alternative for boot2docker ISO
GNU Lesser General Public License v3.0
40 stars 33 forks source link

Show a banner around YUM that it is not supported in Minishift VM #116

Closed gbraad closed 7 years ago

gbraad commented 7 years ago

Currently it is possible to install packages into the LiveRW filesystem of the LiveCD, however this can lead to exhaustion of the root / filesystem. An example of this can be seen here: https://github.com/minishift/minishift/issues/355#issuecomment-310972408

I suggest we replace the yum command with a wrapper that shows an UNSUPPORTED (or warning/information) banner. We still allow people to install, but strongly suggest this is not a good approach.

gbraad commented 7 years ago

Removal of rpm/yum needs to happen as follows:

$ rpm -e --nodeps rpm rpm-libs rpm-python rpm-build-libs yum yum-plugin-fastestmirror

Note: since yum removal is protected. you can't just run yum remove yum or yum remove rpm

[docker@minishift ~]$ sudo yum remove rpm       
Loaded plugins: fastestmirror
Resolving Dependencies
--> Running transaction check
---> Package rpm.x86_64 0:4.11.3-21.el7 will be erased
--> Processing Dependency: rpm for package: policycoreutils-2.5-11.el7_3.x86_64
--> Processing Dependency: rpm >= 4.4.2 for package: yum-3.4.3-150.el7.centos.noarch
--> Processing Dependency: rpm = 4.11.3-21.el7 for package: rpm-libs-4.11.3-21.el7.x86_64
--> Processing Dependency: rpm = 4.11.3-21.el7 for package: rpm-python-4.11.3-21.el7.x86_64
--> Running transaction check
---> Package policycoreutils.x86_64 0:2.5-11.el7_3 will be erased
--> Processing Dependency: policycoreutils = 2.5-11.el7_3 for package: policycoreutils-python-2.5-11.el7_3.x86_64
---> Package rpm-libs.x86_64 0:4.11.3-21.el7 will be erased
--> Processing Dependency: librpm.so.3()(64bit) for package: rpm-build-libs-4.11.3-21.el7.x86_64
--> Processing Dependency: librpm.so.3()(64bit) for package: 1:grub2-tools-2.02-0.44.el7.centos.x86_64
--> Processing Dependency: librpmio.so.3()(64bit) for package: rpm-build-libs-4.11.3-21.el7.x86_64
--> Processing Dependency: rpm-libs(x86-64) = 4.11.3-21.el7 for package: rpm-build-libs-4.11.3-21.el7.x86_64
---> Package rpm-python.x86_64 0:4.11.3-21.el7 will be erased
---> Package yum.noarch 0:3.4.3-150.el7.centos will be erased
--> Processing Dependency: yum >= 3.0 for package: yum-plugin-fastestmirror-1.1.31-40.el7.noarch
--> Running transaction check
---> Package grub2-tools.x86_64 1:2.02-0.44.el7.centos will be erased
---> Package policycoreutils-python.x86_64 0:2.5-11.el7_3 will be erased
---> Package rpm-build-libs.x86_64 0:4.11.3-21.el7 will be erased
---> Package yum-plugin-fastestmirror.noarch 0:1.1.31-40.el7 will be erased
--> Finished Dependency Resolution
Error: Trying to remove "yum", which is protected
hferentschik commented 7 years ago

Do you really want to remove it or just display a warning?

BTW, is there no way to allocate a bit more space to allow the installation of additional packages?

gbraad commented 7 years ago

The packages alone is not the big issue, because as you can see in the previous mentioned comment, people will also try to wget archives untar these inside /.

We haven't decided. I am for a warning message...

LalatenduMohanty commented 7 years ago

@gbraad Sorry do not understand your idea behind the solution. At this point It seems you want to remove Yum and then provide an alternate solution i.e. wrapper. If so then we need to have the solution before removing YUM.

gbraad commented 7 years ago

b2d images do have the issue of people installing packages or additional software, but it seems people often do with the CentOS image. @praveenkumar and I discussed what would be possible.

  1. wrapper to show a banner
  2. remove yum/rpm entirely

Since we are not sure about some of the dependencies when removing, like policycoreutils-python, we want to create a test ISO to investigate what we can do. The proposed PR is merely a WIP. I do not think we should disable it completely, as it can help us in debug situations too.

A banner/wrapper would be a simple:

#!/bin/sh
echo "Installing additional packages is not supported. Continue [y/n]"

function combine() {
    echo $@
}

if [[ $(combine $@) == *"-y"* ]]; then
  answer="y"
else
  read answer
fi
if echo "$answer" | grep -iq "^y" ;then
    yum-unsupported $@
fi
gbraad commented 7 years ago

No matter what is decided in https://github.com/minishift/minishift/issues/1076, I believe we need a solution for this that can work for the time being. WDYT @LalatenduMohanty @hferentschik

gbraad commented 7 years ago
#!/bin/sh
echo "Installing additional packages is not supported. Continue [y/n]"

if echo "$@" | grep -iq " -y" ; then
  answer="y"
else
  read answer
fi
if echo "$answer" | grep -iq "^y" ; then
    yum-unsupported $@
fi
$ minishift ssh "yum install -y vim"
Installing additional packages is not supported. Continue [y/n]
Loaded plugins: fastestmirror
You need to be root to perform this command.
$ minishift ssh "sudo yum install -y vim"
Installing additional packages is not supported. Continue [y/n]
Loaded plugins: fastestmirror
Loading mirror speeds from cached hostfile
 * base: mirrors.neusoft.edu.cn
 * extras: mirrors.neusoft.edu.cn
 * updates: centos.ustc.edu.cn
Package 2:vim-enhanced-7.4.160-1.el7_3.1.x86_64 already installed and latest version
Nothing to do
gbraad commented 7 years ago

Note: do not try to use escaping from the LiveCD-Creator template, as it will result in inconsistent results... You can escape $@ with \$@, but $answer would not escape correctly. Not even with \\\$answer. Therefore I eventually chose to use the same base64 encoding of the script... which I probably should have done from the beginning.

Anyways, please have a look at #119... and let me know what the banner should look like.

gbraad commented 7 years ago

Do we also need to do this for the RHEL-based image?

praveenkumar commented 7 years ago

Do we also need to do this for the RHEL-based image?

I would say yes we should have it part of RHEL-based image also.

gbraad commented 7 years ago

What do we want the message to be?

----------------------------------------------------------
Installing additional packages is not supported. For more information, please check:
    http://docs.openshift.org/latest/minishift/troubleshooting#livecd-root
----------------------------------------------------------
Continue [y/n]

/cc @LalatenduMohanty @hferentschik @praveenkumar @thatdocslady

thatdocslady commented 7 years ago

@gbraad looks good, the only thing is I'd replace "please check" with "see" (we don't need to say please most of the time, and "see" is the standard reference verb)

gbraad commented 7 years ago

@thatdocslady OK, I also added more information to the related task for the troubleshooting entry at https://github.com/minishift/minishift/issues/1152#issuecomment-316673386

gbraad commented 7 years ago
-----------------------------------------------------------------------------------------------------------------------------------------------------------

  Installing additional packages is not supported. For more information, see:

    https://docs.openshift.org/latest/minishift/using/troubleshooting.html#install-additional-software-not-supported

-----------------------------------------------------------------------------------------------------------------------------------------------------------
Continue [y/N]
LalatenduMohanty commented 7 years ago

@gbraad I still feel we can add little more information i.e. Installing additional packages is not supported because it will exhaust the root file system (/) and will lock the virtual machine. For more information, see:

As this communicates the consequences of the action and hopefully create the curiosity of the user to read more about it.

hferentschik commented 7 years ago

My take on this, which lies a bit in the middle:

"Installing additional packages is not supported, because it might exhaust the root file system. For more information, see"

It should be 'might' in this case, since it is not a given.

LalatenduMohanty commented 7 years ago

I am ok with @hferentschik's suggestion too.

gbraad commented 7 years ago

we are looking for a way to avoid using 'exhaust'... or is this understandable from your PoV?

hferentschik commented 7 years ago

Exhaust works just fine for me. I think this is the right term - "exhaust file system space"

gbraad commented 7 years ago

Merged: https://github.com/minishift/minishift-centos-iso/commit/8ef2db52bcca9076753840db827ef735f623d3b5