Simplify tzdata noninteractive installation

[rotu]

based on osrf/docker_images#480

1. Remove hardcoded list of tzdata Ubuntu distributions. Instead use the --no-upgrade option to skip installing tzdata if it is already installed.
2. Instead of editing /etc/timezone and /etc/localtime, use DEBIAN_FRONTEND=non-interactive to suppress interactive configuration prompts for tzdata

[mikaelarguedas]

Thanks @rotu for looking into improving the ros docker images!

This looks very similar to the original approach from #34 (de0a6fb).

The advantage of keeping the list of Ubuntu distros is that it's easy to maintain (as it's a list of past / EOL distros it doesnt change over time) and prevent adding unnecessary layers to images that dont need it.

I don't recall why the symlink approach was chosen over the noninteractive suggested in the first commit of the PR though..

Overall it doesn't seem to me that this PR is necessary or fixing an existing problem is it ?

[rotu]

@mikaelarguedas The reason I went down this road is because this code found its way into ros-tooling code. https://github.com/ros-tooling/setup-ros/blob/08652fc1d324cf9b9cca3e3858b0416c9d2418f3/src/setup-ros-linux.ts#L82-L90

I think it is more explicit and what the setup script is doing and doesn't hardcode the assumption about certain Ubuntu distributions, so it will work with even niche distributions.

So no, it's not a necessary PR - it's just making the code's intention clearer.

[mikaelarguedas]

I think it is more explicit and what the setup script is doing

Agreed, I'd just like to be sure why we moved away from debían noninteractive in the original PR to not revert it without understanding

doesn't hardcode the assumption about certain Ubuntu distributions, so it will work with even niche distributions.

I'm not sure I get that point. Do you have an example (real or imaginary) of a niche distribution that would work with this change and not without?

I'm still in favor of keeping the list if there is no compelling argument to change it. Current advantages of the list:

• no additional layers in images that don't need it
• EOL images don't diverge from the template

[rotu]

Agreed, I'd just like to be sure why we moved away from debían noninteractive in the original PR to not revert it without understanding

You made that change, so you should know better than I. I'm guessing it was as a followup to this comment, either a misunderstanding about setting DEBIAN_FRONTEND=noninteractive globally or following the steps from this blog post.

I'm not sure I get that point. Do you have an example (real or imaginary) of a niche distribution that would work with this change and not without?

If I understand this correctly, a bare Debian installation would need this, since tzdata is not in aptitude search ~prequired -F"%p". (we'd also need to relax the os_name condition too, a change that I didn't make)

I'm also assuming that this script did not intend to reinstall tzdata in bionic and focal, but that is the effect of the script now, since it's not on the distro blacklist.

The argument for this change is basically the same as the argument for preferring feature detection over browser detection.

• no additional layers in images that don't need it

I don't understand what you mean by this.

[mikaelarguedas]

If I understand this correctly, a bare Debian installation would need this, since tzdata is not in aptitude search ~prequired -F"%p". (we'd also need to relax the os_name condition too, a change that I didn't make)

This template is used for all images generated including the images based on Debian. As all debian images come with tzdata installed and configured to Etc/UTC, there is no need to perform any extra steps for those.

I'm also assuming that this script did not intend to reinstall tzdata in bionic and focal, but that is the effect of the script now, since it's not on the distro blacklist.

This script does not reinstall tzdata. It installs it on the ubuntu images that don't have it installed and need a prompt at install time (all the ones not in the blacklist). Can you clarify why you think it is getting reinstalled on bionic and focal ?

I don't understand what you mean by this.

Any RUN statement adds a layer to the docker image. It was/is recommended to limit the number of layers, especially if they don't bring any additional value to a given image. This is the motivation for not adding this layer for images where it is not necessary (all the debian ones and all the ubuntu ones listed in the blacklist).

I'm guessing it was as a followup to this comment, either a misunderstanding about setting DEBIAN_FRONTEND=noninteractive globally or following the steps from this blog post.

I didnt get which comment this linked to as it landed me on a commit.

From rereading the thread from #34 and the docker guidelines there doesn't seem to be a strong recommendation against using DEBIAN_FRONTEND=noninteractive just for a specific apt invocation so I'm ok with going back to the invocation from de0a6fb i.e.:

RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
    tzdata \
    && rm -rf /var/lib/apt/lists/*

IMO the logic to do add that RUN statement only to images needing it to pass the build without prompt should be kept.

[mikaelarguedas]

/cc @tfoote @ruffsl

[rotu]

As all debian images come with tzdata installed

How do you figure? Ubuntu is a subset of Debian and clearly some Ubuntu distributions don't have tzdata.

This script does not reinstall tzdata. It installs it on the ubuntu images that don't have it installed and need a prompt at install time (all the ones not in the blacklist). Can you clarify why you think it is getting reinstalled on bionic and focal ?

I'm actually not sure how I came to that conclusion... it seems like a basic misunderstanding of mine. I think I saw tzdata as a preinstalled package here or in some other listing and wrongly inferred that it's part of focal.

Any RUN statement adds a layer to the docker image. It was/is recommended to limit the number of layers, especially if they don't bring any additional value to a given image.

Oh the sheer number of assumptions I make! I just assumed that the Dockerfile was much like a shell script and that it's executed about as imperatively as it looks. I still don't know whether minimizing layers is an actual performance concern or premature optimization, but that at least explains why it may have been a performance problem at one time.

If it's not an actual performance concern then I still think we should be installing tzdata with --no-upgrade. It makes it very explicit that "tzdata is now installed", whereas that's not at all obvious if we skip it depending on enumerated distribution codenames.

[tfoote]

IMO the logic to do add that RUN statement only to images needing it to pass the build without prompt should be kept.

Agreed. The number of layers and complexity is a concern. This should be primary. We specifically have implemented it so that this workaround only appears on the older platforms that didn't have support before. When that full list of targets is out of support we can just drop the whole workaround. If we change to this "simpler" solution it will be in every newer image until some day when we have to make a "change" and remove it.

In the place that you're pointing to in ros-tooling I would strongly suggest that the same pattern we're using here be used to only deal with this issue on the older platforms which need the workaround.

We are effectively configuring the tzdata when we set this up. Preventing it from upgrading and forcing non-interactive mode I suspect may lead to later invocations triggering the setup configuration or requiring always passing --no-upgrade on future invocations of install or upgrade. And also how will this effect programs that expect the tzdata to be setup?

The cleaner solution as outlined in the original PR #34 is to use debconf-set-selections but we avoided that to keep avoid adding more dependencies and doing it with simple tools. I agree that I don't see this different approach as cleaner or beneficial compared to the current implementation.