fix: Use port number instead of name in targetPort

[labaneilers]

When creating ports in a Service for a Deployment, the chart currently uses the port name (instead of port number) for the targetPort property value. This works in Kubernetes with Services and Ingresses, because it knows how to map port names to numbers.

However, third-party controllers, including in my case, the AWS Load Balancer Controller: https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/, may not properly map port names to numbers when translating service ports into load balancer listener configurations. This means that this chart cannot create a functional Ingress using an AWS Application Load Balancer.

This fix is to simply use the port number for the service port. This shouldn't affect any existing Services created by the chart, because the port number works in Kubernetes equally as well as the name. It also shouldn't affect any externally created Ingresses that refer to the port, since the port name remains available and unchanged.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: labaneilers / name: Laban Eilers (c34a3ba134fff78603a686fced44a5ffba96c56c)

[laban-eilers-simplisafe]

@labaneilers thanks for the contribution. Please bump the chart's minor version number and run make generate-examples. Have you tested this change, specifically as part of an upgrade flow?

@TylerHelmuth I just want to double check you mean to bump the minor version, not patch, correct? I'm not 100% certain what would be the difference in terms of a helm chart, but my understanding was that this is more of a bug fix, which wouldn't affect the helm chart's "API", or the behavior for anyone who isn't being affected by the bug. I thought that would mean you'd want a "patch" bump. Can you please confirm?

[laban-eilers-simplisafe]

@TylerHelmuth As for testing the change as part of an upgrade flow: I've re-installed my collector Deployment via helm upgrade, pointing at my local copy of the charts repo, with my fix. The Ingress, which was previously creating a broken load balancer (with a listener pointing to port 1), immediately created a healthy load balancer (pointing at the correct port). Is that what you mean?

Thanks for your attention and patience!

[TylerHelmuth]

@simpli-laban @labaneilers If the change affects users we'll do a Minor bump (since we don't have a Major version yet). But if the change doesn't affect anyone we could do a patch bump.

When you tested did you use the AWS Load Balancer Controller or a standard k8s cluster? I want to make sure this change does affect users using standard k8s controllers.

[labaneilers]

@simpli-laban @labaneilers If the change affects users we'll do a Minor bump (since we don't have a Major version yet). But if the change doesn't affect anyone we could do a patch bump.

When you tested did you use the AWS Load Balancer Controller or a standard k8s cluster? I want to make sure this change does affect users using standard k8s controllers.

@TylerHelmuth I'm not 100% sure this is what you're asking, but: The cluster I'm testing with has both aws-load-balancer-controller and nginx controllers installed. Ingresses created with the nginx controller worked both before and after my fix. Ingresses using aws-load-balancer-controller work after my fix.

Oh, and both before and after my fix, accessing the Service locally within the cluster works.

[labaneilers]

lol, sorry I switched to my work account by accident- I was in the wrong chrome profile :)

This is meant to be a contribution via my personal github account.

[TylerHelmuth]

@labaneilers please handle merge conflicts (its probably the chart version)

[labaneilers]

@labaneilers please handle merge conflicts (its probably the chart version)

Done. It was just the chart version and generated examples.