vmware-archive / salt-pack

Salt Package Builder
Apache License 2.0
55 stars 23 forks source link

salt-common 2019.2.1+ds-1 drags in ifupdown (on Ubuntu 18.04 LTS, which use netplan) #702

Closed ewenmcneill closed 4 years ago

ewenmcneill commented 4 years ago

I'm pulling this out of https://github.com/saltstack/salt-pack/issues/682#issuecomment-535210585, posted by @smaragd as the referenced ticket is closed (and I have the same issue).


Was this dependency just added in the salt-common Ubuntu package for 2019.2.1+ds-1? Because when upgrading to the latest Salt version on our minions today, the ifupdown package got pulled into all our 18.04 systems that are happily running netplan and don't really need it otherwise.

Before, without ifupdown dependency:

Package: salt-common
Version: 2019.2.0+ds-1
Priority: extra
Section: admin
Source: salt
Maintainer: Debian Salt Team <pkg-salt-team@lists.alioth.debian.org>
Installed-Size: 28.6 MB
Depends: python-apt, python-dateutil, python-jinja2, python-msgpack (>= 0.4),
    python-pkg-resources, python-requests (>= 1.0.0), python-concurrent.futures,
    python-tornado (>= 4.2.1), python-yaml, python-systemd, python-psutil, python-gnupg,
    python:any (<< 2.8), python:any (>= 2.7.5-5~)

Now, with ifupdown dependency:

Package: salt-common
Version: 2019.2.1+ds-1
Priority: extra
Section: admin
Source: salt
Maintainer: Debian Salt Team <pkg-salt-team@lists.alioth.debian.org>
Installed-Size: 28.8 MB
Depends: python-apt, python-dateutil, python-jinja2, python-msgpack (>= 0.4),
    python-pkg-resources, python-requests (>= 1.0.0), python-concurrent.futures,
    python-tornado (>= 4.2.1), python-yaml, python-systemd, python-psutil, python-gnupg,
    ifupdown, python:any (<< 2.8), python:any (>= 2.7.5-5~)

On some 18.04 minions, where we still had an /etc/network/interfaces file from pre-18.04, the system went back to using ifupdown to configure the network interfaces after salt-common pulled in the previously purged ifupdown package.

What was the motivation for a hard-dependency of the ifupdown package in the salt-common Ubuntu package?

ewenmcneill commented 4 years ago

I see the same thing attempting to upgrade salt-common on an Ubuntu 18.04 LTS system installed from scratch on 18.04 LTS -- there is netplan managing the network interfaces, and ifupdown is not installed. AFAIK the recommendation is to only install ifupdown on Ubuntu 18.04 LTS systems if you want to use ifupdown instead of netplan. And I'm unclear whether installing ifupdown will interact, at boot, with netplan possibly resulting in a system that does not come back onto the network properly (always a concern with remote colo systems).

Looking into https://github.com/saltstack/salt-pack/issues/682, which added this dependency, it seems to have been added for https://github.com/saltstack/salt/issues/49078, as a work around for network.managed relying on ifupdown(see, eg, https://github.com/saltstack/salt/issues/49078#issuecomment-480973353 and https://github.com/saltstack/salt/issues/49078#issuecomment-485591028 and https://github.com/saltstack/salt/issues/49078#issuecomment-485952252).

But that seems a very blunt way of solving the problem for some users of one feature of salt, which potentially impacts the ability to boot other (server) systems that rely on netplan rather than ifupdown / salt network.managed (in my case I just use a template to generate a netplan configuration file).

Perhaps instead of Depends: it could be Recommends:? So that installation is optional, and there's less risk of breaking the booting of colo systems?

Ewen

ewen@naosr620:~$ cat /etc/issue
Ubuntu 18.04.3 LTS \n \l

ewen@naosr620:~$ sudo apt-get install salt-common
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following additional packages will be installed:
  ifupdown salt-minion
Suggested packages:
  ppp rdnssd python-augeas
The following NEW packages will be installed:
  ifupdown
The following packages will be upgraded:
  salt-common salt-minion
2 upgraded, 1 newly installed, 0 to remove and 6 not upgraded.
Need to get 6,524 kB of archives.
After this operation, 399 kB of additional disk space will be used.
Do you want to continue? [Y/n] n
Abort.
ewen@naosr620:~$  apt-cache show salt-common | grep Depends | grep ifupdown
Depends: python-apt, python-dateutil, python-jinja2, python-msgpack (>= 0.4), python-pkg-resources, python-requests (>= 1.0.0), python-concurrent.futures, python-tornado (>= 4.2.1), python-yaml, python-systemd, python-psutil, python-gnupg,ifupdown, python:any (<< 2.8), python:any (>= 2.7.5-5~)
ewen@naosr620:~$
ewenmcneill commented 4 years ago

In case it helps anyone else, for now I've put the salt packages on my Ubuntu 18.04 systems on hold to avoid dragigng in ifupdown with:

ewen@naosr620:~$ dpkg -l | awk '/salt-/ { print $2, "hold"; }' | sudo dpkg --set-selections
ewen@naosr620:~$ dpkg -l | grep salt-
hi  salt-common                           2019.2.0+ds-1                               all          shared libraries that salt requires for all packages
hi  salt-minion                           2019.2.0+ds-1                               all          client package for salt, the distributed remote execution system
ewen@naosr620:~$ 

Ewen

smaragd commented 4 years ago

Thanks for turning this into a new open issue. Right after I had posted the comment in the other thread, my fear was that this would probably go unnoticed. I completely agree with your concerns about having both netplan and ifupdown on an 18.04 system, and how that might behave together. Many of our minions are VMs, managed by the client, where getting emergency console access is very painful if the minion drops off the network

For my work-around, until this is decided or solved, I just locked in my ~600 minions by pointing them to the explicit archive repo for 2019.2.0 in my (Salt-managed) /etc/apt/sources.list.d/saltstack.list because I don't know of a Salt-managed way to control package holds:

deb [arch=amd64] http://repo.saltstack.com/apt/ubuntu/18.04/amd64/archive/2019.2.0 bionic main

instead of

deb [arch=amd64] http://repo.saltstack.com/apt/ubuntu/18.04/amd64/latest bionic main

I would favor a Recommends dependency as well, so on systems that have and use ifupdown, one can use network.managed, and on systems that don't have ifupdown, an attempt to use the network.managed state could fail gracefully with a warning saying that ifupdown is needed.

ewenmcneill commented 4 years ago

Perhaps instead of Depends: it could be Recommends:? So that installation is optional, and there's less risk of breaking the booting of colo systems?

For reference, Debian guidance to the usage of Depends: / Recommends: / Suggests::

https://www.debian.org/doc/debian-policy/ch-relationships.html#binary-dependencies-depends-recommends-suggests-enhances-pre-depends

Basically Depends: is intended for "impossible to use the package without this",Recommends: is intended for "to get the most out of this package you probably also want this" and Suggests: is intended for "you'd probably find this other package enhances your experience" / "you might need this for some features". Typically Recommends: are installed by default, but can be overridden, and Suggests: are not installed by default but can be optionally included.

Here salt-common to ifupdown feels like a Suggests: relationship to me. But I'd be okay with it being Recommends: if you're concerned users won't notice/install the suggestions (ie, so long as I'm not forced to install ifupdown on a netplan system).

Ewen

smaragd commented 4 years ago

Thanks for clarifying the three options' meanings. Suggests: might be the more fitting option then. If you manage your 18.04 server with ifupdown instead of netplan, you most likely already installed ifupdown at server install or launch time or via an explicit pkg.install state earlier, since vanilla 18.04 server comes without.

dmurphy18 commented 4 years ago

reopened saltstack/salt-pack#682 and shall make it Suggests:

dmurphy18 commented 4 years ago

https://github.com/saltstack/salt-pack-py3/issues/161

dmurphy18 commented 4 years ago

https://github.com/saltstack/salt-pack-py3/pull/162

This should appear in the next point release which should be soon due a number of other issues in Salt 2019.2.1.

ewenmcneill commented 4 years ago

This should appear in the next point release which should be soon due a number of other issues in Salt 2019.2.1.

@dmurphy18 Thanks very much for the prompt response, and the update from Depends: to Suggests:. I'll keep salt-common / salt-minion on hold on my 18.04 systems until I see the next point release appear on the others, then upgrade.

Ewen

ewenmcneill commented 4 years ago

The version with Suggests: ifupdown got released in 2019.2.2 AFAICT, and I was able to install it without having ifupdown dragged in. So that solves my problem, and I've taken salt-common/salt-minion off hold on the affected systems.

Ewen

ewen@naosr620:~$ dpkg -l | grep salt
ii  salt-common                           2019.2.2+ds-1                                   all          shared libraries that salt requires for all packages
ii  salt-minion                           2019.2.2+ds-1                                   all          client package for salt, the distributed remote execution system
ewen@naosr620:~$ apt-cache show salt-common | grep ifupdown
Suggests: ifupdown
ewen@naosr620:~$ dpkg -l | grep ifupdown
ewen@naosr620:~$ 
otrapeznikov commented 4 years ago

This issue is back with salt 2018.3.5+ds-1

Package: salt-common
Priority: extra
Section: admin
Installed-Size: 26484
Maintainer: Debian Salt Team <pkg-salt-team@lists.alioth.debian.org>
Architecture: all
Source: salt
Version: 2018.3.5+ds-1
Depends: python-apt, python-dateutil, python-jinja2, python-msgpack (>= 0.4), python-pkg-resources, python-requests (>= 1.0.0), python-concurrent.futures, python-tornado (>= 4.2.1), python-yaml, python-systemd, python-psutil, python-gnupg, ifupdown, python:any (<< 2.8), python:any (>= 2.7.5-5~)
Recommends: lsb-release, python-mako
Breaks: python-mako (<< 0.7.0)
Filename: pool/extras/s/salt/salt-common_2018.3.5+ds-1_all.deb
Size: 6088964
MD5sum: 140842fd1cf22214cfb53d498af8f45d
SHA1: d8a6a4b4ad13f181bf8e271d7d60bc7914a19a94
SHA256: 333c1d727c5e95dbea24e6b276e5f1983dd0dd7832d56b121297e9ebbc32c37f
SHA512: e447b970bd6ccfe1e680d7fea4508c1b1795ec233982c8d4213151e3abc39e886d0da1a1e101d8b2a7cbd586868471e8ea8e4ae29fdd63f6421a42697197bd3e
Description: shared libraries that salt requires for all packages
Description-md5: aaa76a7bda4c75e02b81764c1dc6e91a
Homepage: http://saltstack.org/
dmurphy18 commented 4 years ago

@otrapeznikov The 2018.3 branch of releases are CVE-only, please upgrade to v2019.2.3 or v3000 (Neon) just released, where the issue you have is fixed.

Note only change between 2018.3.4 and 2018.3.5 was for a CVE fix.

otrapeznikov commented 4 years ago

@dmurphy18 If the release is CVE only, why the new dependency was introduced? Release notes are missing details on changes that break systems with netplan.

dmurphy18 commented 4 years ago

@otrapeznikov The problem existed in earlier versions of the 2018.3 branch, it is there in 2019.2.0 and 2018.3.4, but got fixed in 2019.2.1 and carried forward to 2019.2.2, 2019.2.3 and 3000. The fix was not back-ported to 2018.3.5 since that was a CVE only fix. Hopefully that clears the matter up. The branch 2018.3 only has CVE fixes applied and actually CVE support for 2018.3 ended last Oct so only Extended Life Support until Oct 2020.

otrapeznikov commented 4 years ago

@dmurphy18 ifupdown dependency wasn't present in 2018.3.4, as I've mentioned in https://github.com/saltstack/salt-pack/issues/725

Package: salt-common
Priority: extra
Section: admin
Installed-Size: 26328
Maintainer: Debian Salt Team <pkg-salt-team@lists.alioth.debian.org>
Architecture: all
Source: salt
Version: 2018.3.4+ds-1
Depends: python-apt, python-dateutil, python-jinja2, python-msgpack (>= 0.4), python-pkg-resources, python-requests (>= 1.0.0), python-concurrent.futures, python-tornado (>= 4.2.1), python-yaml, python-systemd, python-psutil, python-gnupg, python:any (<< 2.8), python:any (>= 2.7.5-5~)
Recommends: lsb-release, python-mako
Breaks: python-mako (<< 0.7.0)
Filename: pool/extras/s/salt/salt-common_2018.3.4+ds-1_all.deb
Size: 5929216
MD5sum: c1409199a9e169e82c6aeaa26dbb1e6e
SHA1: ed9956f01803b343513f81161fa3e13cd371b03b
SHA256: 1f719cfd0f849d7c4b678bf302c2e59100c36ffe49baf60809fe1a0c8e7aea13
SHA512: e8c49e3a317130131e94da92746b4117d4459953beb6b6110918f7b18add6c3cdf51c49f1e283aee68b4afd54b3412400aaa7c04fbbc261ec5fe595290cf5201
Description: shared libraries that salt requires for all packages
Description-md5: aaa76a7bda4c75e02b81764c1dc6e91a
Homepage: http://saltstack.org/