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

If Service Type is ClusterIP set spec.addresses.value into ClusterIP #2166

Closed
akhenakh opened this issue Nov 8, 2023 · 10 comments · Fixed by #2582
Closed

If Service Type is ClusterIP set spec.addresses.value into ClusterIP #2166

akhenakh opened this issue Nov 8, 2023 · 10 comments · Fixed by #2582
Assignees
Labels
kind/enhancement New feature or request

Comments

@akhenakh
Copy link
Contributor

akhenakh commented Nov 8, 2023

When creating a GW using ClusterIP:

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: EnvoyProxy
metadata:
  name: proxy-config-internal
  namespace: envoy-gateway-system
spec:
  provider:
    type: Kubernetes
    kubernetes:
      envoyService:
        type: ClusterIP

There is no way to set the IP address to be used, if we use spec.adresses it will set the externalIPs not the clusterIP:

apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: eg-internal
  namespace: envoy-gateway-system
spec:
  addresses:
    - type: IPAddress
      value: 10.43.146.30
NAME          CLASS         ADDRESS        PROGRAMMED   AGE
eg-internal   eg-internal   10.43.146.30   True         11m

But is in fact this will produce:

spec:
  clusterIP: 10.43.65.18
  clusterIPs:
  - 10.43.65.18
  externalIPs:
  - 10.43.146.30
  internalTrafficPolicy: Cluster

In a ClusterIP case, setting the clusterIP with spec.addresses.value would probably be more useful.

@akhenakh akhenakh added the kind/enhancement New feature or request label Nov 8, 2023
@arkodg
Copy link
Contributor

arkodg commented Nov 8, 2023

we should be setting the service ClusterIP when service type is ClusterIP

@arkodg arkodg added good first issue Good for newcomers help wanted Extra attention is needed labels Nov 8, 2023
@akhenakh
Copy link
Contributor Author

akhenakh commented Nov 8, 2023

You mean as a fix you won't set externalIPs anymore when it's a clusterIP, but what about a way to let the end user set the clusterIP?

same as

apiVersion: v1
kind: Service
spec:
  clusterIP: 10.96.0.10
  type: ClusterIP

I think I got what you are saying

@arkodg
Copy link
Contributor

arkodg commented Nov 10, 2023

the fix here is to set gateway.spec.addresses to service.spec.clusterIPs
we should probably not set externalIPs here, unless there is a good reason to do so

@LZiHaN
Copy link

LZiHaN commented Dec 18, 2023

Hey, is this up for grabs? I'd love to take a shot at it.:)

@shawnh2 shawnh2 removed the help wanted Extra attention is needed label Dec 18, 2023
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label Jan 17, 2024
@akhenakh
Copy link
Contributor Author

akhenakh commented Feb 1, 2024

Anyone working on that one?

@arkodg arkodg removed good first issue Good for newcomers stale labels Feb 1, 2024
@zirain
Copy link
Contributor

zirain commented Feb 2, 2024

@LZiHaN are you still working on this?

@LZiHaN
Copy link

LZiHaN commented Feb 2, 2024

@LZiHaN are you still working on this?

sorry, I don't have bandwidth to continue working on this. perhaps someone else colud give it a try.

@zirain
Copy link
Contributor

zirain commented Feb 2, 2024

I'd like to unsign you first.

@zirain zirain added the help wanted Extra attention is needed label Feb 2, 2024
@ShyunnY
Copy link
Contributor

ShyunnY commented Feb 3, 2024

I'm interested in this. Can it be assigned to me?

@shawnh2 shawnh2 removed the help wanted Extra attention is needed label Feb 3, 2024
ShyunnY added a commit to ShyunnY/gateway that referenced this issue Feb 9, 2024
ShyunnY added a commit to ShyunnY/gateway that referenced this issue Feb 10, 2024
zirain pushed a commit that referenced this issue Feb 12, 2024
…IP (#2582)

when service type = clusterIP set the address to service.clusterIP (#2166)

Signed-off-by: ShyunnY <1147212064@qq.com>
vixns pushed a commit to vixns/gateway that referenced this issue Feb 18, 2024
…IP (envoyproxy#2582)

when service type = clusterIP set the address to service.clusterIP (envoyproxy#2166)

Signed-off-by: ShyunnY <1147212064@qq.com>
Signed-off-by: Stéphane Cottin <stephane.cottin@vixns.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants