hashicorp / consul-k8s

First-class support for Consul Service Mesh on Kubernetes
https://www.consul.io/docs/k8s
Mozilla Public License 2.0
669 stars 323 forks source link

If container does not specify port, port 0 is registered #195

Open lkysow opened 4 years ago

lkysow commented 4 years ago

Edit (July 11, 2022): now that we have tproxy, we should default to the targetPort of the kube service instead of looking at the pod ports array

If a pod doesn't have a port set, e.g.

spec:
  containers:
    # This name will be the service name in Consul.
    - name: static-server
      image: hashicorp/http-echo:latest
      args:
        - -text="hello world"
        - -listen=:8080
#      ports:
#        - containerPort: 8080
#           name: http

Then we register it as running on port 0. This breaks connect because the envoy proxy can't route to port 0 but there are no error logs so it's hard to debug.

Instead we should either return an error during the mutating webhook call for that pod so it doesn't get scheduled and users can use kubectl describe to see a status event that states the error (pod has no containers with ports set).

lkysow commented 4 years ago

Note that not all services need to have ports. For example if it's only a client.

ghost commented 3 years ago

What i noticed in this example:

apiVersion: v1
kind: ServiceAccount
metadata:
  name: static-server
---
apiVersion: v1
kind: Pod
metadata:
  name: static-server
  annotations:
    'consul.hashicorp.com/connect-inject': 'true'
spec:
  containers:
    # This name will be the service name in Consul.
    - name: static-server
      image: hashicorp/http-echo:latest
      args:
        - -text="hello world"
        - -listen=:8080
      ports:
        - containerPort: 8080
          name: http
    # If ACLs are enabled, the serviceAccountName must match the Consul service name.
  serviceAccountName: static-server

When ports is enabled its trying to execute the following command in the consul-connect-inject-init init container:

  ......
  # Generate the envoy bootstrap code /bin/consul connect envoy \ -proxy-id="${PROXY_SERVICE_ID}" \ -prometheus-scrape-path="/metrics" \ -prometheus-backend-port="20100" \ -token-file="/consul/connect-inject/acl-token" \ -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml # Copy the Consul binary cp /bin/consul /consul/connect-inject/consul

Please take note of the -prometheus-scrape-path this is triggered by the ports definition. And in the logs of the init container it fails with: flag provided but not defined: -prometheus-scrape-path

Any idea why it's adding it to that command?

kschoche commented 3 years ago

Hi @tkwy-rick - The prometheus scrape path is part of a new feature (metrics) that was released in the latest consul-k8s release. Have you enabled metrics or any of it's sub-features? If so you'll need to have consul 1.9.0+, or if you've enabled metrics merging you'll need to use consul:1.10.0-alpha https://github.com/hashicorp/consul-helm/blob/master/CHANGELOG.md Can you confirm if updating consul or disabling metrics resolves the problem? EDIT: I should also clarify that the alpha release should only be used in a dev environment! :)

ghost commented 3 years ago

aah off-course. That's it. I will disable it for now. Looking forward to that feature though!

Any idea when the 1.10 release is planned?

kschoche commented 3 years ago

Excellent, I'm glad that worked for you! We don't have a public GA date for 1.10 right now, but we're targeting sometime around May. Hope this helps!

vladimir-shopov commented 2 years ago

I see that this is still open. What were the plans to handle such situations, because I've stumbled on an issue where a containerPort isn't specified in a particular Pod, which leads to the inability to communicate with the service in this Pod via the Service Mesh?

Isn't it more appropriate to obtain information about the port from the Service rather than from the Pod, having in mind that Note: As of consul-k8s v0.26.0 and Consul Helm v0.32.0, having a Kubernetes service is required to run services on the Consul Service Mesh.? By the way, from Kubernetes perspective the containerPort has no significance whatsoever as stated in their documentation: List of ports to expose from the container. Exposing a port here gives the system additional information about the network connections a container uses, but is primarily informational. Not specifying a port here DOES NOT prevent that port from being exposed. Any port which is listening on the default "0.0.0.0" address inside a container will be accessible from the network. Cannot be updated.

lkysow commented 2 years ago

@vladimir-shopov yes agreed, this issue is quite old and the description was written prior to our tproxy support. Now we should just default to the targetPort of the service. I'll update the description to match.