You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/05/18 01:36:04 UTC

[GitHub] [apisix-ingress-controller] AlinsRan opened a new pull request, #1022: feat: sync CRD resource to apisix mechanism.

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

   <!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   <!-- Please delete options that are not relevant. -->
   
   - [ ] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   ### 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] lingsamuel commented on a diff in pull request #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r893002058


##########
cmd/ingress/ingress.go:
##########
@@ -171,6 +171,7 @@ For example, no available LB exists in the bare metal environment.`)
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterBaseURL, "default-apisix-cluster-base-url", "", "the base URL of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterAdminKey, "default-apisix-cluster-admin-key", "", "admin key used for the authorization of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterName, "default-apisix-cluster-name", "default", "name of the default apisix cluster")
+	cmd.PersistentFlags().DurationVar(&cfg.ApisixResourceSyncInterval.Duration, "apisix-resource-sync-interval", 300*time.Second, "sync-time of the default 300s")

Review Comment:
   IIRC cobra `--help` automatically shows the default value, so maybe we don't need to repeat it.



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 merged PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022


-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#issuecomment-1136682151

   re-run all jobs.


-- 
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 #1022: feat: sync CRD resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r875631265


##########
cmd/ingress/ingress.go:
##########
@@ -168,6 +168,7 @@ For example, no available LB exists in the bare metal environment.`)
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterBaseURL, "default-apisix-cluster-base-url", "", "the base URL of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterAdminKey, "default-apisix-cluster-admin-key", "", "admin key used for the authorization of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterName, "default-apisix-cluster-name", "default", "name of the default apisix cluster")
+	cmd.PersistentFlags().Int64Var(&cfg.ApisixCacheSyncInterval, "apisix-cache-sync-interval", 300, "sync-time of the default 300s")

Review Comment:
   Sync time from cache to APISIX cluster. Right?



-- 
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] lingsamuel commented on a diff in pull request #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r892998217


##########
conf/config-default.yaml:
##########
@@ -44,7 +44,7 @@ ingress_status_address: []   # when there is no available information on the Ser
                              # For example, no available LB exists in the bare metal environment.
 enable_profiling: true # enable profiling via web interfaces
                        # host:port/debug/pprof, default is true.
-
+apisix-resource-sync-interval: "300s" # The Kubernetes resources sync to apisix default interval time.

Review Comment:
   `Default interval for synchronizing Kubernetes resources to APISIX`



##########
cmd/ingress/ingress.go:
##########
@@ -171,6 +171,7 @@ For example, no available LB exists in the bare metal environment.`)
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterBaseURL, "default-apisix-cluster-base-url", "", "the base URL of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterAdminKey, "default-apisix-cluster-admin-key", "", "admin key used for the authorization of admin api / manager api for the default APISIX cluster")
 	cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterName, "default-apisix-cluster-name", "default", "name of the default apisix cluster")
+	cmd.PersistentFlags().DurationVar(&cfg.ApisixResourceSyncInterval.Duration, "apisix-resource-sync-interval", 300*time.Second, "sync-time of the default 300s")

Review Comment:
   `interval between syncs in seconds. Default value is 300s.`



##########
pkg/ingress/apisix_consumer.go:
##########
@@ -318,3 +318,29 @@ func (c *apisixConsumerController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("consumer", "delete")
 }
+
+func (c *apisixConsumerController) ResourceSync() {
+	objs := c.controller.apisixConsumerInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixConsumer sync failed, found ApisixConsumer resource with bad meta namespace key: %s", err)

Review Comment:
   same



##########
pkg/ingress/apisix_route.go:
##########
@@ -448,3 +448,25 @@ func (c *apisixRouteController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("route", "delete")
 }
+
+func (c *apisixRouteController) ResourceSync() {
+	objs := c.controller.apisixRouteInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixRoute sync failed, found ApisixRoute resource with bad meta namespace key: %s", err)

Review Comment:
   same



##########
pkg/ingress/controller.go:
##########
@@ -770,3 +774,54 @@ func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc context.
 		c.MetricsCollector.IncrCheckClusterHealth(c.name)
 	}
 }
+
+func (c *Controller) syncResourceAll() {

Review Comment:
   `syncAllResources`



##########
samples/deploy/configmap/apisix-ingress-cm.yaml:
##########
@@ -35,7 +35,7 @@ data:
    http_listen: ":8080"   # the HTTP Server listen address, default is ":8080"
    enable_profiling: true # enable profiling via web interfaces
                           # host:port/debug/pprof, default is true.
-
+   apisix-resource-sync-interval: 300s # Ingress sync to apisix default interval time.

Review Comment:
   same as above



##########
pkg/ingress/apisix_pluginconfig.go:
##########
@@ -366,3 +366,25 @@ func (c *apisixPluginConfigController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("PluginConfig", "delete")
 }
+
+func (c *apisixPluginConfigController) ResourceSync() {
+	objs := c.controller.apisixPluginConfigInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixPluginConfig sync failed, found ApisixPluginConfig resource with bad meta namespace key: %s", err)

Review Comment:
   same



##########
pkg/ingress/apisix_cluster_config.go:
##########
@@ -403,3 +403,29 @@ func (c *apisixClusterConfigController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("clusterConfig", "delete")
 }
+
+func (c *apisixClusterConfigController) ResourceSync() {
+	objs := c.controller.apisixClusterConfigInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixClusterConfig sync failed, found ApisixClusterConfig resource with bad meta namespace key: %s", err)

Review Comment:
   Please change to `Errorw`.



##########
pkg/ingress/apisix_tls.go:
##########
@@ -359,3 +359,29 @@ func (c *apisixTlsController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("TLS", "delete")
 }
+
+func (c *apisixTlsController) ResourceSync() {
+	objs := c.controller.apisixTlsInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixTls sync failed, found ApisixTls object with bad namespace/name: %s, ignore it", err)

Review Comment:
   same



##########
pkg/ingress/ingress.go:
##########
@@ -402,3 +402,25 @@ func (c *ingressController) isIngressEffective(ing kube.Ingress) bool {
 	}
 	return false
 }
+
+func (c *ingressController) ResourceSync() {
+	objs := c.controller.ingressInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("found ApisixConsumer resource with bad meta namespace key: %s", err)

Review Comment:
   `Errorw`



##########
pkg/ingress/controller.go:
##########
@@ -770,3 +774,54 @@ func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc context.
 		c.MetricsCollector.IncrCheckClusterHealth(c.name)
 	}
 }
+
+func (c *Controller) syncResourceAll() {
+	wg := sync.WaitGroup{}
+	goAttach := func(handler func()) {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			handler()
+		}()
+	}
+	goAttach(func() {
+		c.apisixConsumerController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixRouteController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixClusterConfigController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixPluginConfigController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixUpstreamController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixTlsController.ResourceSync()
+	})
+	goAttach(func() {
+		c.ingressController.ResourceSync()
+	})
+	wg.Wait()
+}
+
+func (c *Controller) resourceSyncLoop(ctx context.Context, interval time.Duration) {
+	// The interval shall not be less than 60 seconds.
+	if interval < _mininumApisixResourceSyncInterval {
+		interval = _mininumApisixResourceSyncInterval

Review Comment:
   We should add a log to notify this behavior



##########
pkg/ingress/apisix_upstream.go:
##########
@@ -301,3 +301,21 @@ func (c *apisixUpstreamController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("upstream", "delete")
 }
+
+func (c *apisixUpstreamController) ResourceSync() {
+	clusterConfigs := c.controller.apisixUpstreamInformer.GetIndexer().List()
+	for _, clusterConfig := range clusterConfigs {
+		key, err := cache.MetaNamespaceKeyFunc(clusterConfig)
+		if err != nil {
+			log.Errorf("ApisixUpstream sync failed, found ApisixUpstream resource with bad meta namespace key: %s", err)

Review Comment:
   same



##########
pkg/ingress/controller.go:
##########
@@ -64,6 +64,8 @@ const (
 	_resourceSyncAborted = "ResourceSyncAborted"
 	// _messageResourceFailed is used to report error
 	_messageResourceFailed = "%s synced failed, with error: %s"
+	// ingress sync to apsix mininum interval

Review Comment:
   `minimum interval for ingress sync to 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] AlinsRan commented on a diff in pull request #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883208500


##########
pkg/config/config_test.go:
##########
@@ -154,6 +157,7 @@ func TestConfigWithEnvVar(t *testing.T) {
 	"ingress_publish_service": "",
 	"ingress_status_address": [],
     "enable_profiling": true,
+	"apisix-cache-sync-interval": "200s",

Review Comment:
   ![image](https://user-images.githubusercontent.com/79972061/170611984-9f5b7716-aee9-4f25-a369-48b30e31a26d.png)
   



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883206811


##########
pkg/config/config_test.go:
##########
@@ -154,6 +157,7 @@ func TestConfigWithEnvVar(t *testing.T) {
 	"ingress_publish_service": "",
 	"ingress_status_address": [],
     "enable_profiling": true,
+	"apisix-cache-sync-interval": "200s",

Review Comment:
   Strangely, my local format is normal.



-- 
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 #1022: feat: sync CRD resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#issuecomment-1136641153

   ping @tao12345666333
   This CI error has nothing to do with this PR. Please help review this PR.


-- 
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 #1022: feat: sync CRD resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r875653197


##########
pkg/apisix/apisix.go:
##########
@@ -227,3 +232,32 @@ func (c *apisix) DeleteCluster(name string) {
 	// just delete its index.
 	delete(c.clusters, name)
 }
+
+func (c *apisix) syncCacheAll(ctx context.Context) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	wg := sync.WaitGroup{}
+	for _, cluster := range c.clusters {
+		wg.Add(1)
+		go func(clu Cluster) {
+			defer wg.Done()
+			clu.SyncCache(ctx)

Review Comment:
   This is synchronize the data in APISIX to the cache



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883205701


##########
test/e2e/suite-ingress/resourcesync.go:
##########
@@ -0,0 +1,216 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"time"
+
+	"github.com/onsi/ginkgo"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("suite-ingress: apisix resource sync", func() {
+	opts := &scaffold.Options{
+		Name:                  "default",
+		Kubeconfig:            scaffold.GetKubeconfig(),
+		APISIXConfigPath:      "testdata/apisix-gw-config.yaml",
+		IngressAPISIXReplicas: 1,
+		HTTPBinServicePort:    80,
+		APISIXRouteVersion:    "apisix.apache.org/v2beta3",
+	}
+	s := scaffold.NewScaffold(opts)
+	ginkgo.JustBeforeEach(func() {
+		backendSvc, backendPorts := s.DefaultHTTPBackend()
+		ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+   match:
+     hosts:
+     - httpbin.org
+     paths:
+       - /ip
+   backends:
+   - serviceName: %s
+     servicePort: %d
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ar))
+		err := s.EnsureNumApisixUpstreamsCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of upstreams")
+		err = s.EnsureNumApisixRoutesCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes")
+
+		ing := fmt.Sprintf(`
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  annotations:
+    kubernetes.io/ingress.class: apisix
+  name: ingress-extensions-v1beta1
+spec:
+  rules:
+  - host: local.httpbin.org
+    http:
+      paths:
+      - path: /headers
+        pathType: Exact
+        backend:
+          serviceName: %s
+          servicePort: %d
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ing))
+
+		err = s.ApisixConsumerKeyAuthCreated("foo", "foo-key")
+		assert.Nil(ginkgo.GinkgoT(), err)
+	})
+
+	ginkgo.It("for modified resource sync consistency", func() {
+		// crd resource sync interval
+		readyTime := time.Now().Add(60 * time.Second)
+
+		routes, _ := s.ListApisixRoutes()
+		assert.Len(ginkgo.GinkgoT(), routes, 2)
+
+		consumers, _ := s.ListApisixConsumers()
+		assert.Len(ginkgo.GinkgoT(), consumers, 1)
+
+		for _, route := range routes {
+			_ = s.CreateApisixRouteByApisixAdmin(id.GenID(route.Name), []byte(`
+{
+	"methods": ["GET"],
+	"uri": "/anything",
+	"plugins": {
+		"key-auth": {}
+	},
+	"upstream": {
+		"type": "roundrobin",
+		"nodes": {
+			"httpbin.org": 1
+		}
+	}
+}`))
+		}
+
+		for _, consumer := range consumers {
+			_ = s.CreateApisixConsumerByApisixAdmin([]byte(fmt.Sprintf(`
+{
+	"username": "%s",
+	"plugins": {
+		"key-auth": {
+			"key": "auth-one"
+		}
+	}
+}`, consumer.Username)))
+		}
+
+		_ = s.NewAPISIXClient().
+			GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			Expect().
+			Status(http.StatusNotFound)
+
+		_ = s.NewAPISIXClient().
+			GET("/headers").
+			WithHeader("Host", "local.httpbin.org").
+			Expect().
+			Status(http.StatusNotFound)
+
+		waitTime := time.Until(readyTime).Seconds()
+		time.Sleep(time.Duration(waitTime) * time.Second)
+
+		_ = s.NewAPISIXClient().
+			GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			Expect().
+			Status(http.StatusOK)
+
+		_ = s.NewAPISIXClient().
+			GET("/headers").
+			WithHeader("Host", "local.httpbin.org").
+			Expect().
+			Status(http.StatusOK)
+
+		consumers, _ = s.ListApisixConsumers()

Review Comment:
   Sure!



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#issuecomment-1141911745

   @AlinsRan please merge master, we have fixed e2e test cases. #1055 


-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r881190898


##########
pkg/config/config_test.go:
##########
@@ -154,6 +157,7 @@ func TestConfigWithEnvVar(t *testing.T) {
 	"ingress_publish_service": "",
 	"ingress_status_address": [],
     "enable_profiling": true,
+	"apisix-cache-sync-interval": "200s",

Review Comment:
   Maybe we can format it so that they are aligned



##########
pkg/config/config.go:
##########
@@ -115,15 +116,16 @@ type APISIXConfig struct {
 // default value.
 func NewDefaultConfig() *Config {
 	return &Config{
-		LogLevel:              "warn",
-		LogOutput:             "stderr",
-		HTTPListen:            ":8080",
-		HTTPSListen:           ":8443",
-		IngressPublishService: "",
-		IngressStatusAddress:  []string{},
-		CertFilePath:          "/etc/webhook/certs/cert.pem",
-		KeyFilePath:           "/etc/webhook/certs/key.pem",
-		EnableProfiling:       true,
+		LogLevel:                "warn",
+		LogOutput:               "stderr",
+		HTTPListen:              ":8080",
+		HTTPSListen:             ":8443",
+		IngressPublishService:   "",
+		IngressStatusAddress:    []string{},
+		CertFilePath:            "/etc/webhook/certs/cert.pem",
+		KeyFilePath:             "/etc/webhook/certs/key.pem",
+		EnableProfiling:         true,
+		ApisixCacheSyncInterval: types.TimeDuration{Duration: 6 * time.Second},

Review Comment:
   the default value shoud be 300s, right?



##########
pkg/ingress/controller.go:
##########
@@ -715,3 +719,54 @@ func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc context.
 		c.MetricsCollector.IncrCheckClusterHealth(c.name)
 	}
 }
+
+func (c *Controller) syncResourceAll() {
+	wg := sync.WaitGroup{}
+	goAttach := func(handler func()) {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			handler()
+		}()
+	}
+	goAttach(func() {
+		c.apisixConsumerController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixRouteController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixClusterConfigController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixPluginConfigController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixUpstreamController.ResourceSync()
+	})
+	goAttach(func() {
+		c.apisixTlsController.ResourceSync()
+	})
+	goAttach(func() {
+		c.ingressController.ResourceSync()
+	})
+	wg.Wait()
+}
+
+func (c *Controller) resourceSyncLoop(ctx context.Context, interval time.Duration) {
+	// The interval shall not be less than 60 minutes.

Review Comment:
   When defining this constant, you set it to a minimum of 60s, which is inconsistent here



##########
test/e2e/scaffold/ingress.go:
##########
@@ -268,6 +268,8 @@ spec:
             - debug
             - --log-output
             - stdout
+            - --apisix-cache-sync-interval
+            - 60s

Review Comment:
   should we enable this by default?



##########
pkg/ingress/apisix_cluster_config.go:
##########
@@ -277,3 +277,21 @@ func (c *apisixClusterConfigController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("clusterConfig", "delete")
 }
+
+func (c *apisixClusterConfigController) ResourceSync() {
+	objs := c.controller.apisixClusterConfigInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixClusterConfig sync failed, found ApisixClusterConfig resource with bad meta namespace key: %s", err)
+			continue
+		}
+		if !c.controller.isWatchingNamespace(key) {
+			continue
+		}
+		c.workqueue.Add(&types.Event{
+			Type:   types.EventAdd,

Review Comment:
   Why did you choose to use the `types.EventAdd` event type?



##########
test/e2e/suite-ingress/resourcesync.go:
##########
@@ -0,0 +1,216 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"time"
+
+	"github.com/onsi/ginkgo"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("suite-ingress: apisix resource sync", func() {
+	opts := &scaffold.Options{
+		Name:                  "default",
+		Kubeconfig:            scaffold.GetKubeconfig(),
+		APISIXConfigPath:      "testdata/apisix-gw-config.yaml",
+		IngressAPISIXReplicas: 1,
+		HTTPBinServicePort:    80,
+		APISIXRouteVersion:    "apisix.apache.org/v2beta3",
+	}
+	s := scaffold.NewScaffold(opts)
+	ginkgo.JustBeforeEach(func() {
+		backendSvc, backendPorts := s.DefaultHTTPBackend()
+		ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+   match:
+     hosts:
+     - httpbin.org
+     paths:
+       - /ip
+   backends:
+   - serviceName: %s
+     servicePort: %d
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ar))
+		err := s.EnsureNumApisixUpstreamsCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of upstreams")
+		err = s.EnsureNumApisixRoutesCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes")
+
+		ing := fmt.Sprintf(`
+apiVersion: extensions/v1beta1

Review Comment:
   please use `networking.k8s.io/v1` apiVersion



##########
test/e2e/suite-ingress/resourcesync.go:
##########
@@ -0,0 +1,216 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"time"
+
+	"github.com/onsi/ginkgo"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("suite-ingress: apisix resource sync", func() {
+	opts := &scaffold.Options{
+		Name:                  "default",
+		Kubeconfig:            scaffold.GetKubeconfig(),
+		APISIXConfigPath:      "testdata/apisix-gw-config.yaml",
+		IngressAPISIXReplicas: 1,
+		HTTPBinServicePort:    80,
+		APISIXRouteVersion:    "apisix.apache.org/v2beta3",
+	}
+	s := scaffold.NewScaffold(opts)
+	ginkgo.JustBeforeEach(func() {
+		backendSvc, backendPorts := s.DefaultHTTPBackend()
+		ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+   match:
+     hosts:
+     - httpbin.org
+     paths:
+       - /ip
+   backends:
+   - serviceName: %s
+     servicePort: %d
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ar))
+		err := s.EnsureNumApisixUpstreamsCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of upstreams")
+		err = s.EnsureNumApisixRoutesCreated(1)
+		assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes")
+
+		ing := fmt.Sprintf(`
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  annotations:
+    kubernetes.io/ingress.class: apisix
+  name: ingress-extensions-v1beta1
+spec:
+  rules:
+  - host: local.httpbin.org
+    http:
+      paths:
+      - path: /headers
+        pathType: Exact
+        backend:
+          serviceName: %s
+          servicePort: %d
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ing))
+
+		err = s.ApisixConsumerKeyAuthCreated("foo", "foo-key")
+		assert.Nil(ginkgo.GinkgoT(), err)
+	})
+
+	ginkgo.It("for modified resource sync consistency", func() {
+		// crd resource sync interval
+		readyTime := time.Now().Add(60 * time.Second)
+
+		routes, _ := s.ListApisixRoutes()
+		assert.Len(ginkgo.GinkgoT(), routes, 2)
+
+		consumers, _ := s.ListApisixConsumers()
+		assert.Len(ginkgo.GinkgoT(), consumers, 1)
+
+		for _, route := range routes {
+			_ = s.CreateApisixRouteByApisixAdmin(id.GenID(route.Name), []byte(`
+{
+	"methods": ["GET"],
+	"uri": "/anything",
+	"plugins": {
+		"key-auth": {}
+	},
+	"upstream": {
+		"type": "roundrobin",
+		"nodes": {
+			"httpbin.org": 1
+		}
+	}
+}`))
+		}
+
+		for _, consumer := range consumers {
+			_ = s.CreateApisixConsumerByApisixAdmin([]byte(fmt.Sprintf(`
+{
+	"username": "%s",
+	"plugins": {
+		"key-auth": {
+			"key": "auth-one"
+		}
+	}
+}`, consumer.Username)))
+		}
+
+		_ = s.NewAPISIXClient().
+			GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			Expect().
+			Status(http.StatusNotFound)
+
+		_ = s.NewAPISIXClient().
+			GET("/headers").
+			WithHeader("Host", "local.httpbin.org").
+			Expect().
+			Status(http.StatusNotFound)
+
+		waitTime := time.Until(readyTime).Seconds()
+		time.Sleep(time.Duration(waitTime) * time.Second)
+
+		_ = s.NewAPISIXClient().
+			GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			Expect().
+			Status(http.StatusOK)
+
+		_ = s.NewAPISIXClient().
+			GET("/headers").
+			WithHeader("Host", "local.httpbin.org").
+			Expect().
+			Status(http.StatusOK)
+
+		consumers, _ = s.ListApisixConsumers()

Review Comment:
   I think we should bind key-auth authentication for a route and then verify, what do you think?



-- 
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 #1022: feat: sync CRD resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#issuecomment-1133895324

   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?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 [#1022](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2369a3f) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/e2f19b563e672f21f9bc72c3890ed74908ddc9cc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2f19b5) will **decrease** coverage by `0.57%`.
   > The diff coverage is `20.98%`.
   
   > :exclamation: Current head 2369a3f differs from pull request most recent head 6a74f9a. Consider uploading reports for the commit 6a74f9a to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1022      +/-   ##
   ==========================================
   - Coverage   31.03%   30.45%   -0.58%     
   ==========================================
     Files          74       74              
     Lines        8185     8563     +378     
   ==========================================
   + Hits         2540     2608      +68     
   - Misses       5369     5677     +308     
   - Partials      276      278       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?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/ingress/apisix\_cluster\_config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X2NsdXN0ZXJfY29uZmlnLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_consumer.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X2NvbnN1bWVyLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_pluginconfig.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X3BsdWdpbmNvbmZpZy5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/controller.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvY29udHJvbGxlci5nbw==) | `0.81% <0.00%> (-0.11%)` | :arrow_down: |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `6.16% <0.00%> (-0.39%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `20.87% <0.00%> (-0.56%)` | :arrow_down: |
   | ... and [7 more](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2f19b5...6a74f9a](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1022?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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 commented on pull request #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#issuecomment-1163272417

   For the record only:
   
   Currently, we have timed synchronization of some major resources.
   
   But for the APISIX Ingress controller, the resources that need to be processed are not limited to this. For example, Gateway API related resources have not been processed.


-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883210663


##########
pkg/ingress/apisix_cluster_config.go:
##########
@@ -277,3 +277,21 @@ func (c *apisixClusterConfigController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("clusterConfig", "delete")
 }
+
+func (c *apisixClusterConfigController) ResourceSync() {
+	objs := c.controller.apisixClusterConfigInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixClusterConfig sync failed, found ApisixClusterConfig resource with bad meta namespace key: %s", err)
+			continue
+		}
+		if !c.controller.isWatchingNamespace(key) {
+			continue
+		}
+		c.workqueue.Add(&types.Event{
+			Type:   types.EventAdd,

Review Comment:
   * If ` types Eventupdate `, the controller needs to compare with the old objects during processing, which does not take effect in `apisixroute` `upstream`, and the logic needs to be modified.
   * For example, the object of apifix is deleted manually, or the resource conversion fails. The semantics is equivalent to retry. I think it is appropriate to use `types.eventadd`.



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883210663


##########
pkg/ingress/apisix_cluster_config.go:
##########
@@ -277,3 +277,21 @@ func (c *apisixClusterConfigController) onDelete(obj interface{}) {
 
 	c.controller.MetricsCollector.IncrEvents("clusterConfig", "delete")
 }
+
+func (c *apisixClusterConfigController) ResourceSync() {
+	objs := c.controller.apisixClusterConfigInformer.GetIndexer().List()
+	for _, obj := range objs {
+		key, err := cache.MetaNamespaceKeyFunc(obj)
+		if err != nil {
+			log.Errorf("ApisixClusterConfig sync failed, found ApisixClusterConfig resource with bad meta namespace key: %s", err)
+			continue
+		}
+		if !c.controller.isWatchingNamespace(key) {
+			continue
+		}
+		c.workqueue.Add(&types.Event{
+			Type:   types.EventAdd,

Review Comment:
   * If ` types.Eventupdate `, the controller needs to compare with the old objects during processing, which does not take effect in `ApisixRoute` `ApisixUpstream`, and the logic needs to be modified.
   * For example, the object of apifix is deleted manually, or the resource conversion fails. The semantics is equivalent to retry. I think it is appropriate to use `types.EventAdd`.



-- 
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 #1022: feat: sync CRD and ingress resource to apisix mechanism.

Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1022:
URL: https://github.com/apache/apisix-ingress-controller/pull/1022#discussion_r883206811


##########
pkg/config/config_test.go:
##########
@@ -154,6 +157,7 @@ func TestConfigWithEnvVar(t *testing.T) {
 	"ingress_publish_service": "",
 	"ingress_status_address": [],
     "enable_profiling": true,
+	"apisix-cache-sync-interval": "200s",

Review Comment:
   Strangely, my local format and IDE format is normal.



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