You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "AlinsRan (via GitHub)" <gi...@apache.org> on 2023/04/21 03:59:08 UTC

[GitHub] [apisix-ingress-controller] AlinsRan opened a new pull request, #1803: feat: support controller as etcd server

AlinsRan opened a new pull request, #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803

   <!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   <!-- Please delete options that are not relevant. -->
   
   <!-- Select all the options from below that matches the type your PR best -->
   
   - [ ] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   - [ ] Documentation
   - [ ] Refactor
   - [ ] Chore
   - [ ] CI/CD or Tests
   
   ### What this PR does / why we need it:
   
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   ### Pre-submission checklist:
   
   <!--
   Please follow the requirements:
   1. Use Draft if the PR is not ready to be reviewed
   2. Test is required for the feat/fix PR, unless you have a good reason
   3. Doc is required for the feat PR
   4. Use a new commit to resolve review instead of `push -f`
   5. Use "request review" to notify the reviewer once you have resolved the review
   -->
   
   - [ ] 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](https://github.com/apache/apisix-ingress-controller#community) first**
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1698352992

   > > 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:
   > 
   > 1. Does the current architecture support high availability ?
   > 2. Can a non-leader APISIX ingress controller be used as a viable ETCD server?  How does APISIX avoid connecting to an unavailable ETCD server?
   > 3. 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.
   
   1. Supported and implemented.
   2. The non-leader node can still work, but it does not have write permission, but APISIX can still read data from it
   3. 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, then APISIX will still read from DP when the DP plane is available Get the data.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1698368934

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1309503524


##########
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,

Review Comment:
   Thanks, Updated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1698369363

   Wait for CI.
   Then we can merge this one. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1305095446


##########
conf/apisix-schema.json:
##########


Review Comment:
   Maintenance can be done through diff [https://apisix.apache.org/zh/docs/apisix/control-api/#get-v1schema ](https://apisix.apache.org/zh/docs/apisix/control-api/#get-v1schema) .
   Using files instead of APIs, there are 3 considerations
   1. DP API only supports getting schema, does not support validate, still need to verify in CP.
   In a separate schema for DP and CP, get the json schema to validate the CRD, which will cause CP to depend on DP.
   2. Since the json schema cannot modify instances and inject some default values, there are some schemas that 3. cannot be used directly, including the limit-count plugin.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1309504662


##########
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() {

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1692719919

   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)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1543374029

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1307423527


##########
test/e2e/scaffold/apisix.go:
##########
@@ -46,7 +46,7 @@ spec:
       app: apisix-deployment-e2e-test
   strategy:
     rollingUpdate:
-      maxSurge: 50%
+      maxSurge: 50%%

Review Comment:
   typo?



##########
test/e2e/scaffold/apisix.go:
##########
@@ -46,7 +46,7 @@ spec:
       app: apisix-deployment-e2e-test
   strategy:
     rollingUpdate:
-      maxSurge: 50%
+      maxSurge: 50%%

Review Comment:
   typo?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] Gallardot commented on pull request #1803: feat: support controller as etcd server

Posted by "Gallardot (via GitHub)" <gi...@apache.org>.
Gallardot commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1697188039

   > 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: 
   
   1. Does the current architecture support high availability ?
   2. Can a non-leader APISIX ingress controller be used as a viable ETCD server?  How does APISIX avoid connecting to an unavailable ETCD server?
   3. 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1306733144


##########
Dockerfile:
##########
@@ -41,5 +41,6 @@ RUN yum -y install ca-certificates libc6-compat \
 
 COPY --from=build-env /usr/share/zoneinfo/Hongkong /etc/localtime
 COPY --from=build-env /build/apisix-ingress-controller .
+COPY --from=build-env /build/conf/apisix-schema.json ./conf/apisix-schema.json

Review Comment:
   We can just copy it from local file 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #1803: feat: support controller as etcd server

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1518984059

   ## [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1803](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bf4c617) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98ff8e525365b3f1d25a404d63b5da2195f6fe54?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (98ff8e5) will **decrease** coverage by `2.85%`.
   > The diff coverage is `3.83%`.
   
   > :exclamation: Current head bf4c617 differs from pull request most recent head c21e944. Consider uploading reports for the commit c21e944 to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1803      +/-   ##
   ==========================================
   - Coverage   39.30%   36.46%   -2.85%     
   ==========================================
     Files          91       91              
     Lines        8073     8598     +525     
   ==========================================
   - Hits         3173     3135      -38     
   - Misses       4494     5051     +557     
   - Partials      406      412       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [pkg/apisix/consumer.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9jb25zdW1lci5nbw==) | `26.06% <0.00%> (-11.35%)` | :arrow_down: |
   | [pkg/apisix/global\_rule.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9nbG9iYWxfcnVsZS5nbw==) | `26.06% <0.00%> (-11.35%)` | :arrow_down: |
   | [pkg/apisix/plugin\_metadata.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9wbHVnaW5fbWV0YWRhdGEuZ28=) | `2.51% <0.00%> (-2.14%)` | :arrow_down: |
   | [pkg/apisix/pluginconfig.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9wbHVnaW5jb25maWcuZ28=) | `26.84% <0.00%> (-11.22%)` | :arrow_down: |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `24.87% <0.00%> (-14.98%)` | :arrow_down: |
   | [pkg/apisix/ssl.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9zc2wuZ28=) | `27.86% <0.00%> (-10.48%)` | :arrow_down: |
   | [pkg/apisix/stream\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC9zdHJlYW1fcm91dGUuZ28=) | `26.42% <0.00%> (-10.81%)` | :arrow_down: |
   | [pkg/apisix/upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwaXNpeC91cHN0cmVhbS5nbw==) | `32.53% <0.00%> (-17.84%)` | :arrow_down: |
   | [...kg/providers/apisix/translation/apisix\_consumer.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X2NvbnN1bWVyLmdv) | `65.21% <0.00%> (-1.95%)` | :arrow_down: |
   | [pkg/providers/apisix/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `31.14% <0.00%> (ø)` | |
   | ... and [6 more](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ... and [1 file with indirect coverage changes](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1803/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 merged pull request #1803: feat: support controller as etcd server

Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 merged PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1305095446


##########
conf/apisix-schema.json:
##########


Review Comment:
   Maintenance can be done through diff [https://apisix.apache.org/zh/docs/apisix/control-api/#get-v1schema ](https://apisix.apache.org/zh/docs/apisix/control-api/#get-v1schema) .
   Using files instead of APIs, there are 3 considerations
   1. DP API only supports getting schema, does not support validate, still need to verify in CP.
   2. In a separate schema for DP and CP, get the json schema to validate the CRD, which will cause CP to depend on DP.
   3. Since the json schema cannot modify instances and inject some default values, there are some schemas that  cannot be used directly, including the limit-count plugin.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1308248340


##########
test/e2e/scaffold/apisix.go:
##########
@@ -46,7 +46,7 @@ spec:
       app: apisix-deployment-e2e-test
   strategy:
     rollingUpdate:
-      maxSurge: 50%
+      maxSurge: 50%%

Review Comment:
   No, if you want to output a string using format characters, you need to use %% to represent the percentage sign.
   ![image](https://github.com/apache/apisix-ingress-controller/assets/79972061/92cb98c6-76df-4d36-92c6-f81a87e80eef)
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] AlinsRan commented on pull request #1803: feat: support controller as etcd server

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#issuecomment-1698353144

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] leowmjw commented on a diff in pull request #1803: feat: support controller as etcd server

Posted by "leowmjw (via GitHub)" <gi...@apache.org>.
leowmjw commented on code in PR #1803:
URL: https://github.com/apache/apisix-ingress-controller/pull/1803#discussion_r1308252372


##########
pkg/providers/apisix/apisix_tls.go:
##########
@@ -195,7 +195,7 @@ updateStatus:
 }
 
 func (c *apisixTlsController) updateStatus(obj kube.ApisixTls, statusErr error) {
-	if obj == nil {
+	if obj == nil || c.Kubernetes.DisableStatusUpdates || !c.Eletor.IsLeader() {

Review Comment:
   Typo - c.Elector



##########
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,

Review Comment:
   Typo - Elector



##########
pkg/providers/apisix/apisix_consumer.go:
##########
@@ -363,7 +363,7 @@ func (c *apisixConsumerController) ResourceSync(interval time.Duration) {
 }
 
 func (c *apisixConsumerController) updateStatus(obj kube.ApisixConsumer, statusErr error) {
-	if obj == nil {
+	if obj == nil || c.Kubernetes.DisableStatusUpdates || !c.Eletor.IsLeader() {

Review Comment:
   Typo - Elector



##########
pkg/config/config_test.go:
##########
@@ -187,7 +204,13 @@ func TestConfigWithEnvVar(t *testing.T) {
         "default_cluster_base_url": "{{.DEFAULT_CLUSTER_BASE_URL}}",
         "default_cluster_admin_key": "{{.DEFAULT_CLUSTER_ADMIN_KEY}}",
         "default_cluster_name": "{{.DEFAULT_CLUSTER_NAME}}"
-    }
+    },
+	"etcdserver": {
+		"enalbed": false,

Review Comment:
   typo - "enabled"



##########
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() {

Review Comment:
   Typo - c.Elector



##########
pkg/providers/apisix/apisix_plugin_config.go:
##########
@@ -222,7 +222,7 @@ updatestatus:
 }
 
 func (c *apisixPluginConfigController) updateStatus(obj kube.ApisixPluginConfig, statusErr error) {
-	if obj == nil {
+	if obj == nil || c.Kubernetes.DisableStatusUpdates || !c.Eletor.IsLeader() {

Review Comment:
   Typo - c.Elector



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org