Merge ~raharper/cloud-init:fix-lp1619423 into cloud-init:master

[Ryan Harper]

debian: control.in add explicit package deps

Some builds systems (like Ubuntu-core) use --no-install-recommends and miss a number
of core dependency packages which breaks some core function of cloud-init when the
required binaries are not present. Add these packages as explicit dependencies to
the debian package.

[Scott Moser (smoser)]

On Fri, 14 Oct 2016, Ryan Harper wrote:

> Ryan Harper has proposed merging ~raharper/cloud-init:fix-lp1619423 into cloud-init:master.
>
> Requested reviews:
> cloud init development team (cloud-init-dev)
> Related bugs:
> Bug #1619423 in cloud-init: "snappy does not include ssh-import-id preventing cloud-init user-data from importing ssh-keys"
> https://bugs.launchpad.net/cloud-init/+bug/1619423
>
> For more details, see:
> https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/308555
>
> debian: control.in add explicit package deps
>
> Some builds systems (like Ubuntu-core) use --no-install-recommends and miss a number
> of core dependency packages which breaks some core function of cloud-init when the
> required binaries are not present. Add these packages as explicit dependencies to
> the debian package.

This seems broken.
I'm pretty certain you're not supposed to have to depend on any thing
'essential' which includes at least dash and mount.

i only took a quick look.

> --
> You are subscribed to branch cloud-init:master.
>

[Oliver Grawert (ogra)]

yeah, this pretty much duplicates the ubuntu-core seed (plus adding some MB of deps i guess), growing the core snap by megabytes while taking away the control of the seed is surely not a solution (see comment #1 on the bug it is supposed to solve).

[Ryan Harper (raharper)]

I'm highly confused.

Our thread there in the bug was:

>> @ryan is there any reason why cloud-init does not actually depend on the
>> package if it needs it to be functional ?
>
>
>I believe it's part of the ubuntu-server seed, so it's expected to be part
>of the server image
>Or at least the cloud-image, which of course includes cloud-init.

@ryan: well, if it breaks core functionality of cloud-init to not have it, then not having it as dependency is per definition a packaging bug ...

So, here I am putting the explicit dependencies on the package; Scott says this is wrong; you say I'm taking away the control of the seed.

Please tell me what I need to do so that when we include cloud-init it has the commands it needs to deliver it's core function (which I'm defining by the set of config modules it will run by default as well as tools needed to acquire data sources etc).

a66e60e... by Ryan Harper on 2016-10-17

Update deps; skip packages marked Essential

7a59fa8... by Ryan Harper on 2016-10-17

Rework dep list to drop any package marked 'Essential'

Closer inspection of config modules to add missing commands
that are part of cloud-init core setup and config.

- Dropped any package tagged as 'Essential'

[Scott Moser (smoser)]

$ cat go.sh
#!/bin/sh
deps="
apt cloud-guest-utils coreutils dash debconf e2fsprogs gnupg2 hostname
ifupdown iproute2 locales lsb-release mount net-tools passwd procps sed
ssh-import-id systemd udev
"
for dep in $deps; do
   apt-cache show $dep | grep -qi "^Essential:" && r=essential || r=not
   printf "%-25s %s\n" "$dep" "$r"
done

$ sh ./go.sh
apt not
cloud-guest-utils not
coreutils essential
dash essential
debconf not
e2fsprogs essential
gnupg2 not
hostname essential
ifupdown not
iproute2 not
locales not
lsb-release not
mount essential
net-tools not
passwd not
procps not
sed essential
ssh-import-id not
systemd not
udev not

So, todo here is:
a.) anything 'essential' does not have to be listed as a depends. Its assumed part of a Ubuntu system.
b.) That leaves the others...
Some that I think we should add for sure:
 - net-tools (and then subsequently remove it by dropping use of ifconfig in favor of ip)
 - iproute2: definitely should list this.
apt : seems like this should not be essential given snappy. We use it if its there.
 - systemd: we should jsut not depend on this, but use the right init system wherever we are (could add systemd-init|upstart-init... but probably there *is* an init system).
 - udev: i guess depend on it if we use it.

[Ryan Harper (raharper)]

On Tue, Oct 18, 2016 at 2:49 PM, Scott Moser <email address hidden> wrote:

> $ cat go.sh
> #!/bin/sh
> deps="
> apt cloud-guest-utils coreutils dash debconf e2fsprogs gnupg2 hostname
> ifupdown iproute2 locales lsb-release mount net-tools passwd procps sed
> ssh-import-id systemd udev
> "
> for dep in $deps; do
> apt-cache show $dep | grep -qi "^Essential:" && r=essential || r=not
> printf "%-25s %s\n" "$dep" "$r"
> done
>
> $ sh ./go.sh
> apt not
> cloud-guest-utils not
> coreutils essential
> dash essential
> debconf not
> e2fsprogs essential
> gnupg2 not
> hostname essential
> ifupdown not
> iproute2 not
> locales not
> lsb-release not
> mount essential
> net-tools not
> passwd not
> procps not
> sed essential
> ssh-import-id not
> systemd not
> udev not
>
> So, todo here is:
> a.) anything 'essential' does not have to be listed as a depends. Its
> assumed part of a Ubuntu system.
>

ACK

> b.) That leaves the others...
> Some that I think we should add for sure:
> - net-tools (and then subsequently remove it by dropping use of ifconfig
> in favor of ip)
>

Do we want to require the dropping of ifconfig to this PR? If so, that's
fine but I want to be clear on the TODO.

> - iproute2: definitely should list this.
>

ACK

> apt : seems like this should not be essential given snappy. We use it if
> its there.
>

ACK, move to recommends (as well as recommend snapd?)

> - systemd: we should jsut not depend on this, but use the right init
> system wherever we are (could add systemd-init|upstart-init... but probably
> there *is* an init system).
>

ACK

> - udev: i guess depend on it if we use it.
>

ACK: cc_disk_setup uses udevadm

>
> --
> https://code.launchpad.net/~raharper/cloud-init/+git/
> cloud-init/+merge/308555
> You are the owner of ~raharper/cloud-init:fix-lp1619423.
>

a8495d1... by Ryan Harper on 2016-10-18

apt is not required; ifupdown is replaced with iproute2

[Ryan Harper (raharper)]

On Tue, Oct 18, 2016 at 3:03 PM, Ryan Harper <email address hidden>
wrote:

>
>
> On Tue, Oct 18, 2016 at 2:49 PM, Scott Moser <email address hidden> wrote:
>
>> $ cat go.sh
>> #!/bin/sh
>> deps="
>> apt cloud-guest-utils coreutils dash debconf e2fsprogs gnupg2 hostname
>> ifupdown iproute2 locales lsb-release mount net-tools passwd procps sed
>> ssh-import-id systemd udev
>> "
>> for dep in $deps; do
>> apt-cache show $dep | grep -qi "^Essential:" && r=essential || r=not
>> printf "%-25s %s\n" "$dep" "$r"
>> done
>>
>> $ sh ./go.sh
>> apt not
>> cloud-guest-utils not
>> coreutils essential
>> dash essential
>> debconf not
>> e2fsprogs essential
>> gnupg2 not
>> hostname essential
>> ifupdown not
>> iproute2 not
>> locales not
>> lsb-release not
>> mount essential
>> net-tools not
>> passwd not
>> procps not
>> sed essential
>> ssh-import-id not
>> systemd not
>> udev not
>>
>> So, todo here is:
>> a.) anything 'essential' does not have to be listed as a depends. Its
>> assumed part of a Ubuntu system.
>>
>
> ACK
>
>
>> b.) That leaves the others...
>> Some that I think we should add for sure:
>> - net-tools (and then subsequently remove it by dropping use of ifconfig
>> in favor of ip)
>>
>
> Do we want to require the dropping of ifconfig to this PR? If so, that's
> fine but I want to be clear on the TODO.
>

Note that net-tools is compatible with freebsd ... so, dropping net-tools
requires replacement of uses of route, ifconfig and others
and adding runtime checks to pick the right tool and additional parsers;
the old parser for ifconfig/route/netstat output and a
new one for ip.

I'd like to defer dropping this I think; rather we need to have some sort
of distro object lookup for the right "tools"

Ryan

[Scott Moser (smoser)]

yes. deferring is fine. i meant at some point in the future.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:a8495d1e40eada17de3e8a22eff5694c9b239264
https://jenkins.ubuntu.com/server/job/cloud-init-ci/74/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/74
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/74/console
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/74
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-amd64/74
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/74

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/74/rebuild

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

moved this to work in progress.
i think that is where we are right now..
feel free to re-submit if needed.
h

Unmerged commits

a8495d1... by Ryan Harper on 2016-10-18

apt is not required; ifupdown is replaced with iproute2

7a59fa8... by Ryan Harper on 2016-10-17

Rework dep list to drop any package marked 'Essential'

Closer inspection of config modules to add missing commands
that are part of cloud-init core setup and config.

- Dropped any package tagged as 'Essential'

a66e60e... by Ryan Harper on 2016-10-17

Update deps; skip packages marked Essential

a55ae1f... by Ryan Harper on 2016-10-14

debian: add explicit dependencies for required binaries

diff --git a/packages/debian/control.in b/packages/debian/control.in
index b58561e..10491ae 100644
--- a/packages/debian/control.in
+++ b/packages/debian/control.in
@@ -21,8 +21,26 @@ Architecture: all
 Depends: procps,
          ${python},
          ${misc:Depends},
-         ${${python}:Depends}
-Recommends: eatmydata, sudo, software-properties-common, gdisk
+         ${${python}:Depends},
+         ca-certificates,
+         cloud-guest-utils,
+         debconf,
+         gdisk,
+         gnupg1 | gnupg2,
+         iproute2,
+         locales,
+         lsb-release,
+         net-tools,
+         passwd,
+         procps,
+         software-properties-common,
+         ssh-import-id,
+         sudo,
+         tzdata,
+         udev,
+         upstart | systemd,
+         util-linux
+Recommends: eatmydata
 XB-Python-Version: ${python:Versions}
 Description: Init scripts for cloud instances
  Cloud instances need special scripts to run during initialisation