feat: support controller as etcd server #1803

AlinsRan · 2023-04-21T03:59:06Z

Type of change:

What this PR does / why we need it:

ref:

proposal: New architecture of Apache APISIX Ingress controller #610

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

codecov-commenter · 2023-04-23T07:49:22Z

Codecov Report

Merging #1803 (8836478) into master (e0a2b17) will decrease coverage by 3.61%.
Report is 3 commits behind head on master.
The diff coverage is 12.95%.

❗ Current head 8836478 differs from pull request most recent head 5433e2a. Consider uploading reports for the commit 5433e2a to get more accurate results

@@            Coverage Diff             @@
##           master    #1803      +/-   ##
==========================================
- Coverage   40.47%   36.87%   -3.61%     
==========================================
  Files          92       93       +1     
  Lines        7100     7851     +751     
==========================================
+ Hits         2874     2895      +21     
- Misses       3842     4568     +726     
- Partials      384      388       +4

Files Changed	Coverage Δ
pkg/apisix/apisix.go	`61.22% <ø> (ø)`
pkg/apisix/consumer.go	`26.06% <0.00%> (-11.35%)`	⬇️
pkg/apisix/global_rule.go	`26.06% <0.00%> (-11.35%)`	⬇️
pkg/apisix/plugin_metadata.go	`2.23% <0.00%> (-2.42%)`	⬇️
pkg/apisix/pluginconfig.go	`24.40% <0.00%> (-13.66%)`	⬇️
pkg/apisix/route.go	`25.38% <0.00%> (-14.47%)`	⬇️
pkg/apisix/ssl.go	`25.88% <0.00%> (-12.46%)`	⬇️
pkg/apisix/stream_route.go	`25.37% <0.00%> (-11.86%)`	⬇️
pkg/apisix/upstream.go	`31.62% <0.00%> (-18.75%)`	⬇️
pkg/apisix/utils.go	`0.92% <0.00%> (-0.30%)`	⬇️
... and 10 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

tao12345666333 · 2023-05-11T05:46:21Z

I want to put this to v1.8.

So we won't do a detailed review of this PR before the release of v1.7.

test/e2e/scaffold/apisix.go

leowmjw

Minor typo. Does this feature means the adpater is using btree in-mem?

leowmjw · 2023-08-29T06:00:50Z

pkg/providers/controller.go

@@ -390,6 +433,7 @@ func (c *Controller) run(ctx context.Context) {
 		KubeClient:          c.kubeClient,
 		MetricsCollector:    c.MetricsCollector,
 		Recorder:            c.recorder,
+		Eletor:              c.elector,


Typo - Elector

Thanks, Updated.

leowmjw · 2023-08-29T06:02:12Z

pkg/providers/apisix/apisix_upstream.go

@@ -280,7 +279,7 @@ updateStatus:
 }

 func (c *apisixUpstreamController) updateStatus(obj kube.ApisixUpstream, statusErr error) {
-	if obj == nil {
+	if obj == nil || c.Kubernetes.DisableStatusUpdates || !c.Eletor.IsLeader() {


Typo - c.Elector

pkg/providers/apisix/apisix_tls.go

pkg/providers/apisix/apisix_plugin_config.go

pkg/providers/apisix/apisix_consumer.go

pkg/config/config_test.go

tao12345666333

The main implementation in it, I think is enough for this PR. So once the other comments are all addressed I will give my LGTM

Gallardot · 2023-08-29T10:38:04Z

In a high availability architecture, a simple hang is meaningless for non-leader nodes, they still need to watch k8s resources continue to work, and are not allowed to hold write permissions until they become elected (write k8s resources, write apisix)

Great design, providing a clearer control plane and data plane architecture. However, I have a few questions:

Does the current architecture support high availability ?
Can a non-leader APISIX ingress controller be used as a viable ETCD server? How does APISIX avoid connecting to an unavailable ETCD server?
Some extreme edge cases may not be handled, such as when an APISIX ingress controller restarts before K8S CR resources such as APISIXRoute are synchronized to the corresponding ETCD server. Will the ETCD server come online prematurely and provide service? APISIX may obtain empty router data at this time, leading to failure.

AlinsRan · 2023-08-30T01:21:14Z

In a high availability architecture, a simple hang is meaningless for non-leader nodes, they still need to watch k8s resources continue to work, and are not allowed to hold write permissions until they become elected (write k8s resources, write apisix)

Great design, providing a clearer control plane and data plane architecture. However, I have a few questions:

Does the current architecture support high availability ?

Can a non-leader APISIX ingress controller be used as a viable ETCD server? How does APISIX avoid connecting to an unavailable ETCD server?

Some extreme edge cases may not be handled, such as when an APISIX ingress controller restarts before K8S CR resources such as APISIXRoute are synchronized to the corresponding ETCD server. Will the ETCD server come online prematurely and provide service? APISIX may obtain empty router data at this time, leading to failure.

Supported and implemented.
The non-leader node can still work, but it does not have write permission, but APISIX can still read data from it
In the current stage, DP and CP will run in one Pod, and if one of them is abnormal, the Pod will be restarted, and this problem does not exist. This problem only exists in the architecture where DP and CP are separated, which depends on the implementation of etcd-adapter, which can actually be solved, just ensure that etcd revision is always incremented, so when the CP plane is available, APISIX will still watch data from the CP.

AlinsRan · 2023-08-30T01:21:28Z

Minor typo. Does this feature means the adpater is using btree in-mem?

Yes.

tao12345666333 · 2023-08-30T01:46:08Z

Due to the fact that this feature has already been mostly completed and v1.7 has not yet been released, I think it's possible to move this feature to version v1.7.

tao12345666333 · 2023-08-30T01:46:38Z

Wait for CI.
Then we can merge this one.

AlinsRan added 3 commits April 17, 2023 14:48

feat: support controller as etcd server

878158f

fix e2e test

dc2aa6b

add store

c21e944

AlinsRan and others added 13 commits April 23, 2023 17:24

fix not found

461f158

fix

2bfe7ce

fix upstream

bb1aca7

add compare and delete verify

494a128

fix test

fcb839e

fix httprou

f1b1ff3

hmac

6250c13

upgrade etcd-adapter

d90b760

fix configmap

4012456

Merge branch 'apache:master' into feat/etcdserver

28b1aec

skip cases

90ebc83

update go mod

395b256

test ci

3ca9565

AlinsRan force-pushed the feat/etcdserver branch from 12ee095 to 3ca9565 Compare May 5, 2023 06:32

AlinsRan and others added 3 commits May 5, 2023 22:08

fix sd e2e

9c99f51

Merge branch 'apache:master' into feat/etcdserver

47da0cd

fix ssl

91270e7

AlinsRan force-pushed the feat/etcdserver branch from 30d831a to 91270e7 Compare May 5, 2023 19:01

skip some case

9f767a0

tao12345666333 added the area/controller label May 11, 2023

tao12345666333 added this to the 1.8.0 milestone May 11, 2023

Merge branch 'master' into feat/etcdserver

3dfa3e8

This was referenced Jul 24, 2023

request help: Do you have plan to remove etcd? #1896

Closed

proposal: New architecture of Apache APISIX Ingress controller #610

Closed

Merge branch 'master' into feat/etcdserver

af3b638

AlinsRan added 3 commits August 28, 2023 08:57

update yaml file

cd56477

update yaml file

fba0596

remove debug opt

1fefa4e

AlinsRan requested a review from tao12345666333 August 28, 2023 01:42

AlinsRan added 2 commits August 28, 2023 10:16

shutdown apisix tunnel

c2a9b9e

fix status

818e7d3

tao12345666333 reviewed Aug 28, 2023

View reviewed changes

test/e2e/scaffold/apisix.go Show resolved Hide resolved

AlinsRan added 2 commits August 29, 2023 02:16

perf leader election

38d0eab

Merge branch 'master' into feat/etcdserver

29e4531

AlinsRan requested a review from tao12345666333 August 29, 2023 05:56

leowmjw suggested changes Aug 29, 2023

View reviewed changes

tao12345666333 reviewed Aug 29, 2023

View reviewed changes

AlinsRan requested a review from Gallardot August 29, 2023 06:59

AlinsRan added 2 commits August 29, 2023 17:11

fix typo

fcdcf27

resolve conflict

c49c7ea

lingsamuel approved these changes Aug 29, 2023

View reviewed changes

update doc

5433e2a

AlinsRan requested review from leowmjw and tao12345666333 August 30, 2023 01:35

Gallardot approved these changes Aug 30, 2023

View reviewed changes

tao12345666333 approved these changes Aug 30, 2023

View reviewed changes

tao12345666333 modified the milestones: 1.8.0, v1.7.0 Aug 30, 2023

tao12345666333 merged commit 0bbdc4f into apache:master Aug 30, 2023
87 checks passed

tao12345666333 linked an issue Aug 31, 2023 that may be closed by this pull request

proposal: New architecture of Apache APISIX Ingress controller #610

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support controller as etcd server #1803

feat: support controller as etcd server #1803

AlinsRan commented Apr 21, 2023 •

edited

codecov-commenter commented Apr 23, 2023 •

edited

tao12345666333 commented May 11, 2023

leowmjw left a comment

leowmjw Aug 29, 2023

AlinsRan Aug 30, 2023

leowmjw Aug 29, 2023

AlinsRan Aug 30, 2023

tao12345666333 left a comment

Gallardot commented Aug 29, 2023

AlinsRan commented Aug 30, 2023 •

edited

AlinsRan commented Aug 30, 2023

tao12345666333 commented Aug 30, 2023

tao12345666333 commented Aug 30, 2023

feat: support controller as etcd server #1803

feat: support controller as etcd server #1803

Conversation

AlinsRan commented Apr 21, 2023 • edited

Type of change:

What this PR does / why we need it:

Pre-submission checklist:

codecov-commenter commented Apr 23, 2023 • edited

Codecov Report

tao12345666333 commented May 11, 2023

leowmjw left a comment

Choose a reason for hiding this comment

leowmjw Aug 29, 2023

Choose a reason for hiding this comment

AlinsRan Aug 30, 2023

Choose a reason for hiding this comment

leowmjw Aug 29, 2023

Choose a reason for hiding this comment

AlinsRan Aug 30, 2023

Choose a reason for hiding this comment

tao12345666333 left a comment

Choose a reason for hiding this comment

Gallardot commented Aug 29, 2023

AlinsRan commented Aug 30, 2023 • edited

AlinsRan commented Aug 30, 2023

tao12345666333 commented Aug 30, 2023

tao12345666333 commented Aug 30, 2023

AlinsRan commented Apr 21, 2023 •

edited

codecov-commenter commented Apr 23, 2023 •

edited

AlinsRan commented Aug 30, 2023 •

edited