tumblr / k8s-sidecar-injector

Kubernetes sidecar injection service

Apache License 2.0

345 stars 75 forks source link

Feature/inject host aliases #5

Closed iwilltry42 closed 5 years ago

iwilltry42 commented 5 years ago

What and why?

Add functionality to inject hostAliases, e.g. when certain pods (or injected containers) require access to some external services which are not in internal DNS. This works similar to the addVolumes functionality. Merging is not really required, as we just create new entries in /etc/hosts of the pod/containers. So simply adding the entries to the list of hostAliases, even if the same entries already exist is not a big problem (except that it adds a few lines to the YAML).

Testing Steps

added unittests as usual
tested with debian-debug in /examples/kubernetes

Reviewers

Required reviewers: @byxorna Request reviews from other people you want to review this PR in the "Reviewers" section on the right.

:warning: this PR must have at least 2 thumbs from the MAINTAINERS.md of the project before merging!

iwilltry42 commented 5 years ago

FYI @byxorna , the use-case here is related to the one in the PR for volumeMounts. If were injecting the kerberos sidecar into a pod, we want to access a Hadoop cluster. As the global DNS is not usable from one of our clusters, we need to be able to set hostAliases to resolve the Namenode FQDNs from inside this cluster.

alex-laties commented 5 years ago

LGTM pending some additional docs added to docs/sidecar-configuration-format.md

iwilltry42 commented 5 years ago

LGTM pending some additional docs added to docs/sidecar-configuration-format.md

@alex-laties , I just added documentation for hostAliases and volumeMounts.

iwilltry42 commented 5 years ago

@roymarantz , I added your test-cases. I also checked, what happens, if you actually apply those to a pod. Even with no hostnames specified, it works and the thing will just not be written to /etc/hosts :+1:

Contents of /etc/hosts in debian-debug pod with the hostAlias test-cases that are in the test-configmap now:

root@debian-debug:/# cat /etc/hosts
# Kubernetes-managed hosts file.
127.0.0.1       localhost
::1     localhost ip6-localhost ip6-loopback
fe00::0 ip6-localnet
fe00::0 ip6-mcastprefix
fe00::1 ip6-allnodes
fe00::2 ip6-allrouters
172.17.0.4      debian-debug

# Entries added by HostAliases.
1.2.3.4 some.domain.com
1.2.3.4 some.other-domain.com
4.3.2.1 another.domain.com
4.3.2.1 yetanother.domain.com
2.3.4.5 another.domain.com

byxorna commented 5 years ago

This should be good to merge, once a bit of documentation comments are added to docs/sidecar-configuration-format.md.

cc @komapa @defect @alex-laties for a final round. @tumblr/k8s

iwilltry42 commented 5 years ago

@byxorna I added missing docs for volumeMounts field :+1: