Update Dockerfile to use buster

[cpretzer]

Signed-off-by: Charles Pretzer charles@buoyant.io

This commit updates the Dockerfile to use the buster slim image debian:buster-20190910-slim

[siggy]

apologies for delayed review.

curious: is any change required around replacing iptables with iptables-legacy? i recall we hit a similar issue in the linkerd2 repo prior to moving proxy-init to this repo:
linkerd/linkerd2@14974e1#diff-99f27e036a094ea78b927b9c14cdb019R26-R29

[samlabs821]

debian buster uses nftables, which makes old rules incompatible

[joakimr-axis]

Should this perhaps be buster-20200514-slim in order to match the buster version used in the linkerd2 repo (https://github.com/linkerd/linkerd2/pull/4542/files)?

[joakimr-axis]

Try adding

RUN update-alternatives --set iptables /usr/sbin/iptables-legacy \
    && update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy

here to make current use of iptables work with buster.
(See linkerd/linkerd2#4567 for more info.)

[cpretzer]

Tested with buster-20200514-slim and the init containers started with the expected output.

The dashboard and linkerd command line output (stat, tap, etc.) all look correct.

[alpeb]

@cpretzer would you apply the iptables-legacy change suggested above? It's necessary for older k8s version, it seems.

[alpeb]

After the last change proxy-init is failing for me:

$ k -n linkerd logs linkerd-controller-65cfb4b59f-mwfth linkerd-init
2020/06/18 10:15:04 Tracing this script execution as [1592475304]
2020/06/18 10:15:04 State of iptables rules before run:
2020/06/18 10:15:04 > iptables -t nat -vnL
2020/06/18 10:15:04 < Fatal: can't open lock file /run/xtables.lock: Read-only file system

[cpretzer]

@alpeb I found the same error testing on multiple kubernetes 1.14, 1.16, and 1.18.

I did some digging and found an issue in the Kubernetes repository which describes similar behavior. In that PR they address the issue by creating a hostPath to write /run/xtables.lock.

kind also uses hostPath.

I don't think we want to use hostPath, so I tested by adding an emptyDir volume to a deployment that I manually injected the proxy and proxy-init into and that enabled the container to start.

If I understand what iptables is doing, it's using xtables.lock to block other iptables processes from updating the current iptables rules until it completes its work. The problem with using emptyDir is that other init containers might expect to use this as well and using an emptyDir basically just lets the proxy-init container write to memory so that it can execute its commands, which basically isolates the xtables.lock for the pod. As a result, if another container/process is calling iptables this lock is meaningless.

I see that we use hostPath volumes for the cni-plugin, but adding a hostPath volume for proxy-init feels like a pretty significant change to the functionality.

What do you think is the right approach here? Is there another option that I've overlooked?

[cpretzer]

Okay, I figured out what is going on: readOnlyRootFileSystem: true is set in the securityContext of the proxy-init container.

The integration tests haven't been updated to match the _proxy-init.tpl file, so they don't include readOnlyRootFileSystem among other settings.

Here is the output from the template in one of my service pods:

   initContainers:
...
        name: linkerd-init
...
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            add:
            - NET_ADMIN
            - NET_RAW
          privileged: false
          readOnlyRootFilesystem: true
          runAsNonRoot: false
          runAsUser: 0
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: FallbackToLogsOnError
...

And the iptablestest-lab.yaml has:

      initContainers:
      - name: linkerd-init
...
        securityContext:
          capabilities:
            add:
            - NET_ADMIN
          privileged: false
          runAsNonRoot: false
          runAsUser: 0
...

Using readOnlyRootFileSystem: true is the right thing to do, so we need to find a way to make /run writable, and I haven't thought of a better way other than the volumeMounts.

[alpeb]

Thanks for the extra research @cpretzer 👍

From my understanding, readOnlyRootFileSystem refers to the container's ability to write to its top layer, which is still separate from the host's file system (I might be wrong here, my understanding of containers is limited). In which case if we set that to true, the pod won't fail but the locking mechanism will be moot, just like with the emptyDir option you mentioned.
Although it turns out readOnlyRootFileSystem: false is the solution others are relying on.

It also seems to me the only way to do this while keeping proper locking is to rely on hostPath. We'd have to update our PSP to allow that. We can use the PSP allowedHostPath rule to limit the path that is writable, but I'm seeing a scary warning in the docs saying that's easily abused.

I'm realizing we're not honoring that lock file in CNI given we're relying on the default readOnlyRootFileSystem: false there. Perhaps the easiest is to do the same here?

CC @grampelberg

[alpeb]

As to why we're observing this behavior now even though we're still using iptables[-legacy], it's because we're also upgrading from iptables v1.6.0 to v1.8.2, and in v1.6.2 a change was introduced forcing the usage of that lock.

From this change:

Existing systems that depended on things working mostly correctly
even if there was no lock might be affected by this change

[alpeb]

@cpretzer This is running fine for me when I run the control plane in linkerd/linkerd2#4692 using the proxy-init image produced here. The integration test is hanging presumably because the volume changes from linkerd/linkerd2#4692 need to be applied here as well to the init-containers in the pods in iptablestest-lab.yaml in order to avoid the same /run/xtables.lock issue.

[cpretzer]

thanks @alpeb working on this now

[alpeb]

LGTM! 👍

[ihcsim]

LGTM!