kubernetes-sigs / cloud-provider-equinix-metal

Kubernetes Cloud Provider for Equinix Metal (formerly Packet Cloud Controller Manager)
https://deploy.equinix.com/labs/cloud-provider-equinix-metal
Apache License 2.0
74 stars 26 forks source link

The "cloud-provider-equinix-metal-kubernetes-external" service has an incorrect port #171

Closed clrxbl closed 3 years ago

clrxbl commented 3 years ago

The "cloud-provider-equinix-metal-kubernetes-external" service that the CCM creates when you enable the elastic IP for control plane nodes feature creates a service on port 443, not 6443. The EIP healthcheck checks on port 6443, so it will fail unless you manually edit the service.

  ports:
  - name: https
    nodePort: 32373
    port: 443
    protocol: TCP
    targetPort: 6443

port: 443 should be changed to 6443 by default.

deitch commented 3 years ago

The above service is created by taking the information directly from the native cluster default/kubernetes service. It just leverages what exists. I think if you do a kubectl get svc kubernetes -oyaml you should see identical. It actually would be interesting to post both.

You can override the endpoint that CPEM uses via the METAL_API_SERVER_PORT end var or the config.

However, it uses that only to be able to route it. I think your suggestion to change the default to match what the default is for default/kubernetes makes sense. But maybe it makes even more sense to determine the healthcheck port dynamically from the above?

clrxbl commented 3 years ago

Ah yeah, the default/kubernetes service does point to 443.

Name:              kubernetes
Namespace:         default
Labels:            component=apiserver
                   provider=kubernetes
Annotations:       <none>
Selector:          <none>
Type:              ClusterIP
IP Families:       <none>
IP:                172.17.0.1
IPs:               172.17.0.1
Port:              https  443/TCP
TargetPort:        6443/TCP
Endpoints:         10.0.0.101:6443,10.0.0.102:6443,10.0.0.103:6443
Session Affinity:  None
Events:            <none>
deitch commented 3 years ago

That default service is calculated automatically. It always sets the internal port to 443, and uses whatever port the apiserver is using.

The question now is what is the right port for healthcheck? Let's try and figure it out, so we can get it right.

CPEM does healthcheck in two distinct places:

  1. To the EIP. This currently uses the port set by env var METAL_API_SERVER_PORT, defaulting to 6443.
  2. To each node, both public and private IP. This currently uses the same port as above.

EIP

For the EIP, the routing should be to the EIP, which just hands it off to each controlplane host. The host routes it because of the added kube-system/cloud-provider-equinix-metal-kubernetes-external. Specifically, the port on that Service needs to match the port we use to do the healthcheck.

As you highlighted, we currently default to using 6443 for the healthcheck, but 443 for the Service.

Beyond that mismatch, this means that the port you use for the EIP (e.g. 443) is different than the port that you configured for actually connecting to an individual node (e.g. 6443). That works, but can be confusing.

Control Plane Nodes

The control plane nodes should be listening on whatever port the kube-apiserver is listening on. This can be determined via whatever the targetPort is for the default/kubernetes service.

Summary

As I read what I wrote, several conclusions seem clear to me. I would like to get another opinion on it (@clrxbl would you be so kind?):

Does this make sense?I am up for getting the change in, if we can have input from maintainers ( @displague and @gianarb, want to comment as well?) and a real-live customer @clrxbl :-)

clrxbl commented 3 years ago

I feel like it would make more sense to default to the targetPort / API server port for the EIP, and not the service port (443). Perhaps there are other use cases for the elastic IP that I'm not aware of that would make it seem odd to default to 6443. (I plan on just using it for my kubectl kube config).

Eitherway, an option to override the selected port would be fine for my usecase.

"Device checks should use whatever port the API server is listening on (determined from port on default/kubernetes service)" Shouldn't device checks check for targetPort? As the API server runs on port 6443 on each device.

deitch commented 3 years ago

Shouldn't device checks check for targetPort? As the API server runs on port 6443 on each device.

Yes, this - "determined from port on default/kubernetes service" - means "discovering it from the appropriate port setting (in this case targetPort) on the default/kubernetes service", and not the literal port setting. People might have set the API server onto 443 or 6443 or 8443 or 9876; default/kubernetes is how we discover it.

I realize the above wording might be confusing; I will edit it.

Going to wait until @displague and @gianarb comment here before plowing ahead.