Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different service target port #279

Closed
vbehar opened this issue Aug 21, 2019 · 6 comments · Fixed by #327
Closed

Different service target port #279

vbehar opened this issue Aug 21, 2019 · 6 comments · Fixed by #327
Labels
kind/feature Feature request

Comments

@vbehar
Copy link
Contributor

vbehar commented Aug 21, 2019

Hi,

the current behaviour when "duplicating" services is to use the canary.Spec.Service.Port for both the service port and the target port.

In all our apps we currently configure our services on port 80, and on container ports on something else (like 8080 for example).

I'd like to create a PR to allow a TargetPort in the canary spec, but before I wanted to check with you if there is a reason for using the same port for both the service and the container.

thanks

@stefanprodan
Copy link
Member

stefanprodan commented Aug 21, 2019

The reason is simplicity. Flagger also does port discovery and maps all container ports in the ClusterIP service. I'm ok with adding service.targetPort as an optional field but please make sure this change doesn't break AppMesh, Linkerd and all the other integrations.

@vbehar
Copy link
Contributor Author

vbehar commented Aug 21, 2019

ok, thanks

@nil-scan
Copy link
Contributor

nil-scan commented Oct 2, 2019

We also need this and I'd be happy to help testing the AppMesh side of things

@hollinwilkins
Copy link

Happy to test out with Linkerd here

@stefanprodan stefanprodan added the kind/feature Feature request label Oct 6, 2019
@stefanprodan
Copy link
Member

stefanprodan commented Oct 6, 2019

I've implemented this in #327 and added e2e tests for Istio, Linkerd, Gloo, NGINX and Kubernetes CNI.

I'm pretty confident using target port breaks App Mesh due to virtual nodes and virtual routers

To test this use weaveworks/flagger:target-port-2e79817 image and apply the CRD with:

kubectl apply -f https://raw.githubusercontent.com/weaveworks/flagger/target-port/kustomize/base/flagger/crd.yaml

@vbehar
Copy link
Contributor Author

vbehar commented Oct 8, 2019

great, thanks @stefanprodan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants