bug: When k8s service is deleted, the APISIX Upstream nodes field is not cleared

[Donghui0]

Issue description

When k8s service is deleted, the APISIX Upstream nodes field is not cleared

Environment

• your apisix-ingress-controller version: v1.2
• your Kubernetes cluster version: v1.16

Minimal test code / Steps to reproduce the issue

1. delete k8s service.
2. query APISIX upstream nodes field

What's the actual result? (including assertion message & call stack if applicable)

remaining history of upstream node

What's the expected result?

upstream nodes cleared

[tokers]

This is a known behavior, see

apisix-ingress-controller/pkg/ingress/controller.go

Line 529 in 1e1be74

if k8serrors.IsNotFound(err) {

.

From the proxy point of view, the result is similar, requests will be failed no matter whether we clean the nodes.

Of course, we do need to make some reactions for the Services/Endpoints delete events, PR's welcome to optimize it.

[Donghui0]

This is a known behavior, see

apisix-ingress-controller/pkg/ingress/controller.go

Line 529 in 1e1be74

if k8serrors.IsNotFound(err) {

.
From the proxy point of view, the result is similar, requests will be failed no matter whether we clean the nodes.

Of course, we do need to make some reactions for the Services/Endpoints delete events, PR's welcome to optimize it.

There will be problems in special scenarios: the node ip is allocated by other containers.

[tokers]

This is a known behavior, see

apisix-ingress-controller/pkg/ingress/controller.go

Line 529 in 1e1be74

if k8serrors.IsNotFound(err) {

.
From the proxy point of view, the result is similar, requests will be failed no matter whether we clean the nodes.
Of course, we do need to make some reactions for the Services/Endpoints delete events, PR's welcome to optimize it.

There will be problems in special scenarios: the node ip is allocated by other containers.

Fair enough, PR's welcome to fix this.

[tao12345666333]

Thanks for your feedback, it is not difficult to fix it, welcome to submit a PR

[gxthrj]

I hope this can have an overall solution and then coding.

Upstream management in the ingress controller has become more complicated, it will be affected in many ways，for example, the resources of k8s svc , ApisixUpstream and ApisixRoute ，There are subset-related configurations in ApisixRoute and ApisixUpstream.

[tokers]

I hope this can have an overall solution and then coding.

Upstream management in the ingress controller has become more complicated, it will be affected in many ways，for example, the resources of k8s svc , ApisixUpstream and ApisixRoute ，There are subset-related configurations in ApisixRoute and ApisixUpstream.

It doesn't affect ApisixRoute, but it will affect multiple Upstream objects in APISIX due to subset mechanics.

[AlinsRan]

I'm trying to solve this problem, I have two implementation schemes, which need to be confirmed:

1. Add cache (go-memdb) to controller and maintain a table upstream_services {service_name --> []upstream_id}. The upstreamController is responsible for write, When [] is 0, this record is killed. The endpoint is responsible for reading and updating the node field of apifix when the services are updated.
2. This status info is maintained by endpoint or upstreamController. Mutual communication and high coupling are required.

I prefer the first, any better proposal?

[tao12345666333]

@AlinsRan thanks.

Currently we use the following methods to generate the upstream name.

apisix-ingress-controller/pkg/types/apisix/v1/types.go

Lines 425 to 450 in 49dd015

	// ComposeUpstreamName uses namespace, name, subset (optional) and port info to compose
	// the upstream name.
	func ComposeUpstreamName(namespace, name, subset string, port int32) string {
	pstr := strconv.Itoa(int(port))
	// FIXME Use sync.Pool to reuse this buffer if the upstream
	// name composing code path is hot.
	var p []byte
	if subset == "" {
	p = make([]byte, 0, len(namespace)+len(name)+len(pstr)+2)
	} else {
	p = make([]byte, 0, len(namespace)+len(name)+len(subset)+len(pstr)+3)
	}
	buf := bytes.NewBuffer(p)
	buf.WriteString(namespace)
	buf.WriteByte('_')
	buf.WriteString(name)
	buf.WriteByte('_')
	if subset != "" {
	buf.WriteString(subset)
	buf.WriteByte('_')
	}
	buf.WriteString(pstr)
	return buf.String()
	}

Then generate its ID.

apisix-ingress-controller/pkg/id/idgen.go

Lines 25 to 39 in 49dd015

	// GenID generates an ID according to the raw material.
	func GenID(raw string) string {
	if raw == "" {
	return ""
	}
	sh := &reflect.SliceHeader{
	Data: (*reflect.StringHeader)(unsafe.Pointer(&raw)).Data,
	Len: len(raw),
	Cap: len(raw),
	}
	p := *(*[]byte)(unsafe.Pointer(sh))
	res := crc32.ChecksumIEEE(p)
	return fmt.Sprintf("%x", res)
	}

Maintaining the mapping relationship between Service and Upstream, I think it is a better choice.

Here we need to clear the node field of Upstream in APISIX after discovering that the service has been deleted

[AlinsRan]

@AlinsRan thanks.

Currently we use the following methods to generate the upstream name.

apisix-ingress-controller/pkg/types/apisix/v1/types.go

Lines 425 to 450 in 49dd015

	// ComposeUpstreamName uses namespace, name, subset (optional) and port info to compose
	// the upstream name.
	func ComposeUpstreamName(namespace, name, subset string, port int32) string {
	pstr := strconv.Itoa(int(port))
	// FIXME Use sync.Pool to reuse this buffer if the upstream
	// name composing code path is hot.
	var p []byte
	if subset == "" {
	p = make([]byte, 0, len(namespace)+len(name)+len(pstr)+2)
	} else {
	p = make([]byte, 0, len(namespace)+len(name)+len(subset)+len(pstr)+3)
	}
	buf := bytes.NewBuffer(p)
	buf.WriteString(namespace)
	buf.WriteByte('_')
	buf.WriteString(name)
	buf.WriteByte('_')
	if subset != "" {
	buf.WriteString(subset)
	buf.WriteByte('_')
	}
	buf.WriteString(pstr)
	return buf.String()
	}

Then generate its ID.

apisix-ingress-controller/pkg/id/idgen.go

Lines 25 to 39 in 49dd015

	// GenID generates an ID according to the raw material.
	func GenID(raw string) string {
	if raw == "" {
	return ""
	}
	sh := &reflect.SliceHeader{
	Data: (*reflect.StringHeader)(unsafe.Pointer(&raw)).Data,
	Len: len(raw),
	Cap: len(raw),
	}
	p := *(*[]byte)(unsafe.Pointer(sh))
	res := crc32.ChecksumIEEE(p)
	return fmt.Sprintf("%x", res)
	}

Maintaining the mapping relationship between Service and Upstream, I think it is a better choice.

Here we need to clear the node field of Upstream in APISIX after discovering that the service has been deleted

I understand, After the SVC is deleted, it should also be written to apisix upstream nods filed when it is created.

The question is how to maintain this relationship.

[tao12345666333]

#1064 has been merged. I will close this one. thanks