Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cilium: improve host-port generated service names #11469

Merged
merged 1 commit into from
May 11, 2020

Conversation

borkmann
Copy link
Member

@borkmann borkmann commented May 11, 2020

See commit msg.

Closes: #11324

@borkmann borkmann added pending-review area/daemon Impacts operation of the Cilium daemon. release-note/misc This PR makes changes that have no direct user impact. labels May 11, 2020
@borkmann borkmann requested review from gandro, aanm and a team May 11, 2020 13:26
@borkmann
Copy link
Member Author

test-me-please

The service-name and namespace annotation does not have to be unique and
are mainly used by hubble [0]. However, lets improve the Cilium internal
naming scheme a bit into <pod-name>/host-port/<exposed-host-port>, so the
result looks as:

  # ./cilium/cilium service list -o json
  [...]
      "flags": {
        "name": "my-nginx-78cd9b665d-xmwp8/host-port/8080",
        "namespace": "default",
        "trafficPolicy": "Cluster",
        "type": "HostPort"
      },
  [...]

In any case, we can also _guarantee_ that we are never going to clash with
a regular service name since in k8s a service cannot be named this way:

  # kubectl expose deployment my-nginx --type=NodePort --port=30000 --name="my-nginx-78cd9b665d-xmwp8/host-port/8080"
  The Service "my-nginx-78cd9b665d-xmwp8/host-port/8080" is invalid: metadata.name: Invalid value: "my-nginx-78cd9b665d-xmwp8/host-port/8080": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')

The fact that we reuse our main BPF service handling internally is an
implementation detail - HostPort services are not accessible through DNS
this way anyway.

  [0] a2c49a5

Closes: #11324
Signed-off-by: Daniel Borkmann <[email protected]>
@borkmann borkmann force-pushed the pr/fix-host-port-naming branch from e9178ad to 9033d27 Compare May 11, 2020 13:55
@borkmann
Copy link
Member Author

test-me-please

@coveralls
Copy link

coveralls commented May 11, 2020

Coverage Status

Coverage decreased (-0.01%) to 37.872% when pulling 9033d27 on pr/fix-host-port-naming into fb8481a on master.

@borkmann
Copy link
Member Author

restart-ginkgo

@borkmann borkmann merged commit 1b8ba09 into master May 11, 2020
@borkmann borkmann deleted the pr/fix-host-port-naming branch May 11, 2020 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A Pod named "foo" with a hostPort defined overwrites datapath services with name "foo-host-port"
5 participants