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

support multicluster service for endpointSlices #9908

Closed
BSWANG opened this issue Apr 28, 2023 · 7 comments
Closed

support multicluster service for endpointSlices #9908

BSWANG opened this issue Apr 28, 2023 · 7 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@BSWANG
Copy link

BSWANG commented Apr 28, 2023

What happened:
Using Multi Cluster Service in alibabacloud, associate the Multi Cluster Service in Ingress rule, cannot access ingress with 503 error code. Dig into the ingress-controller log, found the it can not found active endpoint from endpointslices:

W0427 20:58:02.264980       7 endpointslices.go:82] Error obtaining Endpoints for Service "default/amcs-nginx": no object matching key "default/amcs-nginx" in local store
W0427 20:58:02.265003       7 controller.go:1179] Service "default/amcs-nginx" does not have any active Endpoint.

However the Multi Cluster Service ClusterIP can be access, the kube-proxy works as expected on the Multi Cluster Service.

# kubectl get serviceimport
NAME    TYPE           IP                    AGE
nginx   ClusterSetIP   ["192.168.118.171"]   15h

# kubectl get service
amcs-nginx   ClusterIP   192.168.118.171   <none>        80/TCP    15h

# kubectl get endpointslice
imported-c0c4a2c9339e44235a5228d0b0ff2259c-nginx-vnlnd   IPv4          80      10.0.2.4,10.0.2.5       15h
imported-ce962c460ae284243a3161d6f2c7b4cf7-nginx-d8rr5   IPv4          80      10.0.2.221,10.0.2.236   15h
# EndpointSlices has `kubernetes.io/service-name: amcs-nginx` label to associate to `amcs-nginx`.

The Multi Cluster Service's EndpointSlice be filtered by ingress-nginx-controller by Service name prefix .

for _, listKey := range s.ListKeys() {
if len(key) < (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key) {
continue
}
// generated endpointslices names has truncated svc name as prefix when svc name is too long, we compare only non truncated part
// https://github.com/kubernetes/ingress-nginx/issues/9240
if len(key) >= (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key[:apiNames.MaxGeneratedNameLength+keyNsLen-1]) {
continue
}

So the Endpoint named imported-c0c4a2c9339e44235a5228d0b0ff2259c-nginx-vnlnd cannot add to amcs-nginx active endpoint.

What you expected to happen:
Don't filter endpointslice with servicename prefix, selection should work as kube-proxy do.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

v1.6.4-aliyun.1

Kubernetes version (use kubectl version):
v1.24.6-aliyun.1

Environment:
Alibaba Cloud
ACK
AliyunLinux2 with 4.19 kernel.`

How to reproduce this issue:
Using Service by manual create&associate endpointslice named by non-prefix of Service in Ingress rule

Anything else we need to know:

@BSWANG BSWANG added the kind/bug Categorizes issue or PR as related to a bug. label Apr 28, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Apr 28, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@longwuyuan
Copy link
Contributor

/remove-kind bug
/kind feature

Te service-name is integral to the conventional approach of handling this. Changing to another approach may help to make some progress towards addressing the special use cases like multicluster service, but the larger impact is not described or studied.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/bug Categorizes issue or PR as related to a bug. labels Apr 28, 2023
@BSWANG
Copy link
Author

BSWANG commented Apr 28, 2023

@longwuyuan In kube-proxy implement, there is no prefix check of endpointslice name.
IMO, the endpointslice selection should be same mechanism across multiply kubernetes controllers.

https://github.com/kubernetes/kubernetes/blob/28247d53d22bbea9344305ac3db53c473c6d6278/pkg/proxy/endpointslicecache.go#L390-L398

func endpointSliceCacheKeys(endpointSlice *discovery.EndpointSlice) (types.NamespacedName, string, error) {
	var err error
	serviceName, ok := endpointSlice.Labels[discovery.LabelServiceName]
	if !ok || serviceName == "" {
		err = fmt.Errorf("no %s label set on endpoint slice: %s", discovery.LabelServiceName, endpointSlice.Name)
	} else if endpointSlice.Namespace == "" || endpointSlice.Name == "" {
		err = fmt.Errorf("expected EndpointSlice name and namespace to be set: %v", endpointSlice)
	}
	return types.NamespacedName{Namespace: endpointSlice.Namespace, Name: serviceName}, endpointSlice.Name, err
}

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 4, 2023
@longwuyuan
Copy link
Contributor

I don't think there is any intention to change endpointSlices for multicluster support. There are no resources to work on this issue.

Since there is no action item on the project here, I am closing the issue.

/close

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Closing this issue.

In response to this:

I don't think there is any intention to change endpointSlices for multicluster support. There are no resources to work on this issue.

Since there is no action item on the project here, I am closing the issue.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@longwuyuan
Copy link
Contributor

/retitle support multicluster service for endpointSlices

@k8s-ci-robot k8s-ci-robot changed the title EndpointSlice selection shouldn't filter by service name prefix support multicluster service for endpointSlices Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

3 participants