connect-init command breaks workaround for supporting multiple ports in sidecar proxy

[fouadsemaan]

As of consul-helm chart v0.32.0 which runs consul 1.10 and consul-k8s 0.26.0 we can no longer use our workaround to support multiple ports in sidecar proxy as suggested in open issue hashicorp/consul#5388 (comment). Our workaround was functional as of Consul 1.9.4 (consul-helm 0.31.1).

The consul upgrade to 1.10 is integrated with Kubernetes objects. Kubernetes services are now part of the standard deployment of any pod that is ran on the mesh.

Each pod is now tied to a Kubernetes service and only one Kubernetes service is expected since a pod can only expose one port due to Consul's limitation to supporting one port per pod. In consul-k8s, the new connect-init function makes that assumption and breaks the current workaround we have in place to support multiple ports.

It is made clear in the code that no more than two services are expected (https://github.com/hashicorp/consul-k8s/blob/v0.26.0/subcommand/connect-init/command.go#L166). Since it doesn't log the services that are returned, I presume that it's returning all the services associated with that pod.

filter := fmt.Sprintf("Meta[%q] == %q and Meta[%q] == %q", connectinject.MetaKeyPodName, c.flagPodName, connectinject.MetaKeyKubeNS, c.flagPodNamespace) (https://github.com/hashicorp/consul-k8s/blob/v0.26.0/subcommand/connect-init/command.go#L159)

I think adding the service name to the filter will bring back exactly the Kubernetes services tied to the service name.

Our workaround is tied to manually adding sidecars to the main application container. Here is an example:

POD A --->service 1 (port 9888) with associated Kubernetes Service 1
--->service 2 (port 9889) with associated Kubernetes Service 2

We have POD A which exposes two ports each representing a different service on POD A. We created the corresponding Kubernetes service for each of the services exposed in POD A.

In this example I think the filter for Pod A for service 1 is returning Kubernetes Services 1 and 2 since the filter is only applied to the pod/namespace and not pod/namespace/service.

[david-yu]

Hi @fouadsemaan thank you for filing this issue and bringing this to our attention. I was discussing this internally within the team. First we don't filter by service name since that is not available to us inside of the pod. The reason we perform this searching logic is because it’s not trivial to figure out the k8s service name from the pod.

We probably need to understand your workaround in more detail in order to see if its viable to make your multi-port workaround work with the refactor we performed around service registration and de-registration for our work in Consul 1.10.

[tiwarisanjay]

@david-yu what if we add service name in filter block as following. Does filter support the same:

		filter := fmt.Sprintf("Meta[%q] == %q and Meta[%q] == %q and Meta[%q] == %q", connectinject.MetaKeyPodName, c.flagPodName, connectinject.MetaKeyKubeNS, c.flagPodNamespace, connectinject.MetaKeyKubeServiceName, c.flagServiceName)
		serviceList, err := consulClient.Agent().ServicesWithFilter(filter)
		c.logger.Info("List of services for pod %s in namepsace %s are %s", c.flagPodName, c.flagPodNamespace, serviceList)

Referring to following
https://github.com/hashicorp/consul-k8s/blob/v0.26.0/subcommand/connect-init/command.go#L159

@fouadsemaan

[fouadsemaan]

@david-yu looking at the example here https://learn.hashicorp.com/tutorials/consul/service-mesh-application-secure-networking?in=consul/kubernetes#define-the-frontend-service the k8s service name, the service account name and pod name are all consistent. Are you saying it is not necessary to have the k8s service name consistent with every other component?

[fouadsemaan]

@david-yu I tried to rebuild the consul-k8s binary. I see that it has the hashicorp/go-discovery dependency https://github.com/hashicorp/consul-k8s/blob/master/go.mod#L16. It in turn has a dependency on tencentcloud/tencentcloud-sdk-go https://github.com/hashicorp/go-discover/blob/master/go.mod#L33. I don't see that specific version (v3.0.83) in github.

[ishustava]

Hey @tiwarisanjay @fouadsemaan

c.flagServiceName is only passed in if you have service name annotation on your pod:

consul-k8s/connect-inject/container_init.go

Line 121 in 81c05aa

data.ServiceName = pod.Annotations[annotationService]

If you don't have an annotation, then that flag will be empty.

Are you saying it is not necessary to have the k8s service name consistent with every other component?

Correct we don't have a requirement that the pod name and the k8s service name are the same. If you have acls enabled, then we only require that the service account name and the k8s service name are the same.

I tried to rebuild the consul-k8s binary.

The easiest way to build the binary is with make dev or make dev-docker. Do either of those work for you?

I think it'd be helpful to have some steps for us to reproduce the workflow you are trying so we can either fix it or have some recommendation.

[fouadsemaan]

Hi @ishustava

In order to support consul connect for multiple ports, we essentially replicate what the connect inject controller does manually. We don't use the consul annotations since we set the sidecars ourselves.

We do have ACLs turned. Since we create a Kubernetes headless service for every service (sidecar) running on the pod, each of the Kubernetes headless services point to the same pod. Therefore I believe the filter is returning multiple Kubernetes services given that the query filte is only based on podname/namespace and not podname/namespace/servicename.

If you don't have an annotation, then that flag will be empty.

So for example instead of
consul-k8s connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \ -acl-auth-method="consul-k8s-auth-method" \ -service-account-name="product-api" \ -service-name="" \
We would do
consul-k8s connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \ -acl-auth-method="consul-k8s-auth-method" \ -service-account-name="product-api" \ -service-name="product-api" \

I would like to make a change in the consul-k8s connect-init function add the service-name parameter to the query.

I suggest that if the serviceName is not empty then add it as a query parameter.

[ndhanushkodi]

Do you have an individual connect-init init container per Kubernetes headless service that points to the same Pod? So you only need the one service-name flag to the connect-init container?

We could modify the filter based on the flag, something like:

filter := fmt.Sprintf("Meta[%q] == %q and Meta[%q] == %q", connectinject.MetaKeyPodName, c.flagPodName, connectinject.MetaKeyKubeNS, c.flagPodNamespace)
if c.flagServiceName != "" {
  filter = filter + fmt.Sprintf(" and Meta[%q] == %q", connectinject.MetaKeyKubeServiceName, c.flagServiceName)
}

[fouadsemaan]

Yes and yes.

Do you have an individual connect-init init container per Kubernetes headless service that points to the same Pod? So you only need the one service-name flag to the connect-init container?

Fantastic

We could modify the filter based on the flag, something like:

[ndhanushkodi]

@fouadsemaan want us to go ahead and add this or did you want to make the change yourselves?

[fouadsemaan]

@ndhanushkodi Yes please go ahead. Will you tag it as v0.26.1?

[david-yu]

@fouadsemaan I assume you are using Consul Helm to deploy consul-k8s? If so, how are you getting those multiple services on the same pod registered?

[fouadsemaan]

@david-yu

As I mentioned to @ndhanushkodi and @ishustava we manually setup the sidecars. We manually replicate what the connect-inject does for multiple ports. We use the consul helm chart for everything else (servers, agents, controllers, service intentions et al.)

[fouadsemaan]

@david-yu @ndhanushkodi Do you have a timeline to incorporate the change?

[david-yu]

Hi @fouadsemaan our plan is to create a PR and provide an dev image for you test. As far as timing, I believe this should happen shortly as they are working on the PR right now. I'm hopeful we can have something today but I need to check with the team.

[fouadsemaan]

@david-yu great thank you. Or a dev binary (and not dev-image)?

[david-yu]

The dev container image should include the binary which you can copy to your local filesystem like so:

docker cp <containerId>:/bin/consul-k8s /host/path/target

[ndhanushkodi]

Hi @fouadsemaan I still have a few unit test tweaks to make, but in the meanwhile you can pull the image hashicorpdev/consul-k8s@sha256:0de6573eacb0aa1782f14044d13cddcdb8ddbf258d01a103a8b95869bdbf0257 and run the step @david-yu mentioned above to grab the binary from it. The new flag is called -k8s-service-name, rather than -service-name because the latter flag already exists and is used for setting a Consul service name that's different from the K8s service name. You can use the -k8s-service-name flag as you described in this comment. Can you let us know if this works for you as expected in your setup?

[david-yu]

Hi @fouadsemaan you can also pull that image by using

docker pull hashicorpdev/consul-k8s:2d2b303

[fouadsemaan]

@david-yu @ndhanushkodi I am getting this error: "flag provided but not defined: -k8s-service-name".

I don't see the flah when running the command below:

consul-k8s connect-init -h

#####edit#####

Actually I see it in this image: hashicorpdev/consul-k8s@sha256:0de6573eacb0aa1782f14044d13cddcdb8ddbf258d01a103a8b95869bdbf0257 and not hashicorpdev/consul-k8s:2d2b303

[david-yu]

@fouadsemaan Ok go ahead and use that sha it looks like I was mistaken in the link I provided.

[fouadsemaan]

Correct we don't have a requirement that the pod name and the k8s service name are the same. If you have acls enabled, then we only require that the service account name and the k8s service name are the same.

@ishustava I got around to testing this and it didn't work. Is it just the k8s service name and service account that need to match? Please note that because we are using 1 pod with the 3 sidecars we obviously can only use one service account since it's at the pod level. The way we got around that in consul 1.9 is by creating 3 service accounts and mounting the token associated with each of the service accounts into the sidecar proxy. Does the pod name or container name have any impact?

Here is the error I am getting:

2021-08-05T19:18:11.943Z [INFO] Check to ensure a Kubernetes service has been created for this application. If your pod is not starting also check the connect-inject deployment logs.
2021-08-05T19:18:12.945Z [INFO] Unable to find registered services; retrying

@david-yu @ndhanushkodi

[david-yu]

Closing as our initial release on multi-port was merged in #1012, docs will shortly follow this or next week. Stay tuned!