Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

fix: set containerPort to targetPort if defined #18

Merged
merged 3 commits into from
Nov 26, 2020

Conversation

Thunderbottom
Copy link
Contributor

containerPort gets set to the template port, and targetPort is ignored even when defined. This causes an issue where port actually points to the external ClusterIP port, and the targetPort is the port where the service is exposed from the container. Hence, in the case where targetPort is defined, containerPort should be set to targetPort and not the external ClusterIP port.

eg: defining ports in kubekutr config like:

ports:
  - port: 9000  # external cluster IP port
    targetPort: 8000  # actual internal container port

sets the containerPort to 9000 instead of 8000, which is wrong in this case because 9000 is the port which is exposed to the cluster, and 8000 is the port that the container is supposed to expose.

`containerPort` gets set to the template `port`, and `targetPort` is
ignored even when defined. This causes an issue where `port` actually
points to the external ClusterIP port, and the `targetPort` is the port
where the service is exposed from the container. Hence, in the case
where `targetPort` is defined, `containerPort` should be set to
`targetPort` and not the external ClusterIP port.

eg: defining ports in kubekutr config like:
```
ports:
  - port: 9000 # external cluster IP port
    targetPort: 8000 # actual internal container port
```
sets the containerPort to `9000` instead of `8000`, which is wrong in
this case because `9000` is the port which is exposed to the cluster,
and `8000` is the port that the container is supposed to expose.

Signed-off-by: Chinmay D. Pai <[email protected]>
templates/containers.tmpl Outdated Show resolved Hide resolved
@mr-karan mr-karan merged commit 0da6f2b into mr-karan:master Nov 26, 2020
@Thunderbottom Thunderbottom deleted the containerport-fix branch November 26, 2020 08:52
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.

2 participants