Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Use port number instead of name for k8s service port #3256

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

Xinayder
Copy link
Contributor

@Xinayder Xinayder commented Nov 2, 2023

I've found an issue when deploying Dendrite's Helm chart on my local cluster. The template for generating an Ingress resource tries to find the service port using a name (http), but the template that generates the Service resource, instead, identifies the resource with a port number.

According to the Kubernetes ServiceSpec, ports.targetPort can be either a number or a string; if it's the latter, it will be looked up as a named port in the pod's container ports.

Pull Request Checklist

@Xinayder Xinayder requested a review from a team as a code owner November 2, 2023 20:24
@Xinayder
Copy link
Contributor Author

Xinayder commented Nov 2, 2023

I did some more testing and it turns out that even if you set the targetPort property on the Service definition, the ingress controller won't find the pod.

However, if you set the service.port property on the ingress resource to the port number, it works. If it's set to http, it doesn't.

@InezMc
Copy link

InezMc commented Nov 7, 2023

Confirm that private signoff recieved.

@Xinayder
Copy link
Contributor Author

Xinayder commented Nov 7, 2023

Still haven't received an email back from Matrix regarding my private signoff request.

@Xinayder Xinayder changed the title Use name instead of number for k8s service port Use port number instead of name for k8s service port Nov 7, 2023
@Xinayder
Copy link
Contributor Author

Sign-off confirmed.

Copy link
Contributor

@S7evinK S7evinK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants