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/03/13 03:37:11 UTC

[GitHub] [apisix-ingress-controller] cmssczy opened a new pull request #919: fix: watch all namespaces by default

cmssczy opened a new pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919


   <!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   <!-- Please delete options that are not relevant. -->
   
   - [x] 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. -->
   fix https://github.com/apache/apisix-ingress-controller/issues/908
   ### 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] tao12345666333 commented on pull request #919: fix: watch all namespaces by default

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


   It is related to the running resource of GitHub Action.


-- 
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] gxthrj commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1073863015


   Should check the failed case.
   ```
   • Failure [71.433 seconds]
   namespacing filtering
   /home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/ingress/namespace.go:39
     without namespace_selector
     /home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/ingress/namespace.go:104
       all resources will be watched [It]
       /home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/ingress/namespace.go:125
   
       
       	Error Trace:	namespace.go:177
       	            				runner.go:113
       	            				runner.go:64
       	            				it_node.go:26
       	            				spec.go:215
       	            				spec.go:138
       	            				spec_runner.go:200
       	            				spec_runner.go:170
       	            				spec_runner.go:66
       	            				suite.go:79
       	            				ginkgo_dsl.go:238
       	            				ginkgo_dsl.go:213
       	            				e2e_test.go:26
       	Error:      	Expected nil, but got: &errors.errorString{s:"timed out waiting for the condition"}
       	Test:       	namespacing filtering without namespace_selector all resources will be watched
       	Messages:   	checking number of routes
       
   
       /home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/ingress/namespace.go:177
   
       Full Stack Trace
       github.com/apache/apisix-ingress-controller/test/e2e/ingress.glob..func7.2.3()
       	/home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/ingress/namespace.go:177 +0x9cf
       github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync(0xc000091ec0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/leafnodes/runner.go:113 +0xa3
       github.com/onsi/ginkgo/internal/leafnodes.(*runner).run(0xc000091ec0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/leafnodes/runner.go:64 +0x15c
       github.com/onsi/ginkgo/internal/leafnodes.(*ItNode).Run(0xc0008671e0, 0x2106040, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/leafnodes/it_node.go:26 +0x87
       github.com/onsi/ginkgo/internal/spec.(*Spec).runSample(0xc000a28870, 0x0, 0x2106040, 0xc0001628c0)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/spec/spec.go:215 +0x72f
       github.com/onsi/ginkgo/internal/spec.(*Spec).Run(0xc000a28870, 0x2106040, 0xc0001628c0)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/spec/spec.go:138 +0xf2
       github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpec(0xc0002ce6e0, 0xc000a28870, 0x0)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/specrunner/spec_runner.go:200 +0x111
       github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpecs(0xc0002ce6e0, 0x1)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/specrunner/spec_runner.go:170 +0x147
       github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).Run(0xc0002ce6e0, 0xc0002ee570)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/specrunner/spec_runner.go:66 +0x117
       github.com/onsi/ginkgo/internal/suite.(*Suite).Run(0xc0001829a0, 0x7f945d065e80, 0xc0004b4d80, 0x1e52816, 0x1e, 0xc000057b00, 0x1, 0x1, 0x21468f8, 0xc0001628c0, ...)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/internal/suite/suite.go:79 +0x546
       github.com/onsi/ginkgo.runSpecsWithCustomReporters(0x2107d40, 0xc0004b4d80, 0x1e52816, 0x1e, 0xc000068f28, 0x1, 0x1, 0x0)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/ginkgo_dsl.go:238 +0x218
       github.com/onsi/ginkgo.RunSpecs(0x2107d40, 0xc0004b4d80, 0x1e52816, 0x1e, 0xac1ec19958)
       	/home/runner/go/pkg/mod/github.com/onsi/ginkgo@v1.16.4/ginkgo_dsl.go:213 +0xa7
       github.com/apache/apisix-ingress-controller/test/e2e.TestRunE2E(0xc0004b4d80)
       	/home/runner/work/apisix-ingress-controller/apisix-ingress-controller/test/e2e/e2e_test.go:26 +0x5c
       testing.tRunner(0xc0004b4d80, 0x1f209e0)
       	/opt/hostedtoolcache/go/1.16.14/x64/src/testing/testing.go:1203 +0xe5
       created by testing.(*T).Run
       	/opt/hostedtoolcache/go/1.16.14/x64/src/testing/testing.go:1248 +0x2b3
   ```


-- 
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 #919: fix: watch all namespaces by default

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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 [#919](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe509c8) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/75098d1e4b26136de3164a3aabd6ed018ffdcd6b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75098d1) will **decrease** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   > :exclamation: Current head fe509c8 differs from pull request most recent head 6ea634e. Consider uploading reports for the commit 6ea634e to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #919      +/-   ##
   ==========================================
   - Coverage   31.90%   31.89%   -0.01%     
   ==========================================
     Files          72       72              
     Lines        7899     7901       +2     
   ==========================================
     Hits         2520     2520              
   - Misses       5103     5105       +2     
     Partials      276      276              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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/compare.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/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-cGtnL2luZ3Jlc3MvY29tcGFyZS5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/namespace.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/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-cGtnL2luZ3Jlc3MvbmFtZXNwYWNlLmdv) | `0.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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/919?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 [11bd92b...6ea634e](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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 merged pull request #919: fix: watch all namespaces by default

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


   


-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1068629297


   ![image](https://user-images.githubusercontent.com/65757763/158497945-2da23e61-49ee-4cd7-ab35-9347afbd2c76.png)
   
   Can't reproduce the failed test cases in my environment. 
   


-- 
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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828652747



##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"

Review comment:
       I think there is no difference between two namespaces and three or more namespaces in this test cases.




-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1066553011


   > Can we add e2e test for this behavior?
   
   OK，I will try. Maybe need to take a litte more time.


-- 
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 change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r829023249



##########
File path: test/e2e/ingress/sanity.go
##########
@@ -27,7 +27,7 @@ import (
 )
 
 type ip struct {
-	IP string `json:"ip"`
+	IP string `json:"origin"`

Review comment:
       👍




-- 
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 change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828764322



##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"

Review comment:
       Yes, there is no difference.  Point it out just because the content of the comment below doesn't match the logic.




-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1073568510


   maybe need re-run again? 😂 @tao12345666333 


-- 
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 #919: fix: watch all namespaces by default

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


   let me check 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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828964016



##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBINWithNamespace(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in another namespace.
+			backendSvc, backendSvcPort = s.DefaultHTTPBackend()
+			route = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /headers
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, namespace), "creating second ApisixRoute")
+
+			// restart ingress-controller
+			pods, err := s.GetIngressPodDetails()
+			assert.Nil(ginkgo.GinkgoT(), err)
+			assert.Len(ginkgo.GinkgoT(), pods, 1)
+			ginkgo.GinkgoT().Logf("restart apisix-ingress-controller pod %s", pods[0].Name)
+			assert.Nil(ginkgo.GinkgoT(), s.KillPod(pods[0].Name))
+			time.Sleep(6 * time.Second)
+			// Two ApisixRoutes have been created at this time.
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(2), "checking number of upstreams")
+
+			body := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			var placeholder ip
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")
+			body = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")

Review comment:
       sorry for that. Now it has been fixed. PTAL.




-- 
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 change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828840444



##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBINWithNamespace(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in another namespace.
+			backendSvc, backendSvcPort = s.DefaultHTTPBackend()
+			route = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /headers
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, namespace), "creating second ApisixRoute")
+
+			// restart ingress-controller
+			pods, err := s.GetIngressPodDetails()
+			assert.Nil(ginkgo.GinkgoT(), err)
+			assert.Len(ginkgo.GinkgoT(), pods, 1)
+			ginkgo.GinkgoT().Logf("restart apisix-ingress-controller pod %s", pods[0].Name)
+			assert.Nil(ginkgo.GinkgoT(), s.KillPod(pods[0].Name))
+			time.Sleep(6 * time.Second)
+			// Two ApisixRoutes have been created at this time.
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(2), "checking number of upstreams")
+
+			body := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			var placeholder ip
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")
+			body = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")

Review comment:
       please see https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828601911  

##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBINWithNamespace(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in another namespace.
+			backendSvc, backendSvcPort = s.DefaultHTTPBackend()
+			route = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /headers
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, namespace), "creating second ApisixRoute")
+
+			// restart ingress-controller
+			pods, err := s.GetIngressPodDetails()
+			assert.Nil(ginkgo.GinkgoT(), err)
+			assert.Len(ginkgo.GinkgoT(), pods, 1)
+			ginkgo.GinkgoT().Logf("restart apisix-ingress-controller pod %s", pods[0].Name)
+			assert.Nil(ginkgo.GinkgoT(), s.KillPod(pods[0].Name))
+			time.Sleep(6 * time.Second)
+			// Two ApisixRoutes have been created at this time.
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(2), "checking number of upstreams")
+
+			body := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			var placeholder ip
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")
+			body = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")

Review comment:
       we will squash commits when merging. So there is no need to force push, which will cause the previous review records to be lost




-- 
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 change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r829023249



##########
File path: test/e2e/ingress/sanity.go
##########
@@ -27,7 +27,7 @@ import (
 )
 
 type ip struct {
-	IP string `json:"ip"`
+	IP string `json:"origin"`

Review comment:
       👍




-- 
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] gxthrj commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1073863977


   Re-runed E2E CI.


-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1066014934


   I didn't find any other documentation or other files related to this issue that need to be modified. 
   Correct me if I'm wrong. @tao12345666333 


-- 
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 edited a comment on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1066014881


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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 [#919](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d570739) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/75098d1e4b26136de3164a3aabd6ed018ffdcd6b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75098d1) will **decrease** coverage by `0.03%`.
   > The diff coverage is `0.00%`.
   
   > :exclamation: Current head d570739 differs from pull request most recent head ae028aa. Consider uploading reports for the commit ae028aa to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #919      +/-   ##
   ==========================================
   - Coverage   31.90%   31.86%   -0.04%     
   ==========================================
     Files          72       72              
     Lines        7899     7908       +9     
   ==========================================
     Hits         2520     2520              
   - Misses       5103     5112       +9     
     Partials      276      276              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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/compare.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/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-cGtnL2luZ3Jlc3MvY29tcGFyZS5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/ingress/namespace.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919/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-cGtnL2luZ3Jlc3MvbmFtZXNwYWNlLmdv) | `0.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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/919?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 [11bd92b...ae028aa](https://codecov.io/gh/apache/apisix-ingress-controller/pull/919?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 #919: fix: watch all namespaces by default

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


   let me re-run CI
   


-- 
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 change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828604659



##########
File path: test/e2e/scaffold/httpbin.go
##########
@@ -103,6 +103,15 @@ func (s *Scaffold) newHTTPBIN() (*corev1.Service, error) {
 	return svc, nil
 }
 
+func (s *Scaffold) NewHTTPBIN(namespace string) (*corev1.Service, error) {

Review comment:
       change its name to NewHTTPBINWithNamespace?

##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBIN(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in default namespace.

Review comment:
       The comments are inconsistent with the logic below

##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"

Review comment:
       Is it enough that we create in the current namespace and default namespace?

##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBIN(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in default namespace.
+			backendSvc, backendSvcPort = s.DefaultHTTPBackend()
+			route = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /headers
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, namespace), "creating second ApisixRoute")
+
+			// restart ingress-controller
+			pods, err := s.GetIngressPodDetails()
+			assert.Nil(ginkgo.GinkgoT(), err)
+			assert.Len(ginkgo.GinkgoT(), pods, 1)
+			ginkgo.GinkgoT().Logf("restart apisix-ingress-controller pod %s", pods[0].Name)
+			assert.Nil(ginkgo.GinkgoT(), s.KillPod(pods[0].Name))
+			time.Sleep(6 * time.Second)
+			// Two ApisixRoutes have been created at this time.
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(2), "checking number of upstreams")
+
+			body := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			var placeholder ip
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")

Review comment:
       Modifications are required here

##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -54,6 +53,7 @@ type Options struct {
 	APISIXAdminAPIKey     string
 	EnableWebhooks        bool
 	APISIXPublishAddress  string
+	disableNodeSelector   bool

Review comment:
       Its name should be related to namespace, right?

##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -64,7 +64,6 @@ type Scaffold struct {
 	nodes              []corev1.Node
 	etcdService        *corev1.Service
 	apisixService      *corev1.Service
-	httpbinDeployment  *appsv1.Deployment

Review comment:
       why remove 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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r829018538



##########
File path: test/e2e/ingress/sanity.go
##########
@@ -27,7 +27,7 @@ import (
 )
 
 type ip struct {
-	IP string `json:"ip"`
+	IP string `json:"origin"`

Review comment:
       The API response is like
   ```json
   {
     "origin": "xx.xx.xx.xx"
   }
   ```




-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1074976188


   Maybe there are other cases did not clean up the dirty data.  All cases ware passed after I delete 
   ```go
   assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
   ```


-- 
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 #919: fix: watch all namespaces by default

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


   Thanks, I'll check 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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828630113



##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -54,6 +53,7 @@ type Options struct {
 	APISIXAdminAPIKey     string
 	EnableWebhooks        bool
 	APISIXPublishAddress  string
+	disableNodeSelector   bool

Review comment:
       yes

##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"
+
+		// create another http-bin service in a new namespace.
+		ginkgo.BeforeEach(func() {
+			k8s.CreateNamespace(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig(),
+			}, namespace)
+			_, err := s.NewHTTPBIN(namespace)
+			assert.Nil(ginkgo.GinkgoT(), err, "create second httpbin service")
+		})
+
+		// clean this tmp namespace when test case is done.
+		ginkgo.AfterEach(func() {
+			err := k8s.DeleteNamespaceE(ginkgo.GinkgoT(), &k8s.KubectlOptions{
+				ConfigPath: scaffold.GetKubeconfig()}, namespace)
+			assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", namespace)
+		})
+
+		ginkgo.It("all resources will be watched", func() {
+			backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+			route := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+  name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /ip
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(route), "creating first ApisixRoute")
+			time.Sleep(3 * time.Second)
+
+			// Now create another ApisixRoute in default namespace.
+			backendSvc, backendSvcPort = s.DefaultHTTPBackend()
+			route = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2beta3
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+  http:
+  - name: rule1
+    match:
+      hosts:
+      - httpbin.com
+      paths:
+      - /headers
+    backends:
+    - serviceName: %s
+      servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, namespace), "creating second ApisixRoute")
+
+			// restart ingress-controller
+			pods, err := s.GetIngressPodDetails()
+			assert.Nil(ginkgo.GinkgoT(), err)
+			assert.Len(ginkgo.GinkgoT(), pods, 1)
+			ginkgo.GinkgoT().Logf("restart apisix-ingress-controller pod %s", pods[0].Name)
+			assert.Nil(ginkgo.GinkgoT(), s.KillPod(pods[0].Name))
+			time.Sleep(6 * time.Second)
+			// Two ApisixRoutes have been created at this time.
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
+			assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(2), "checking number of upstreams")
+
+			body := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			var placeholder ip
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusOK).Body().Raw()
+			err = json.Unmarshal([]byte(body), &placeholder)
+			assert.Nil(ginkgo.GinkgoT(), err, "unmarshalling IP")

Review comment:
       Good catch. Thanks

##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -64,7 +64,6 @@ type Scaffold struct {
 	nodes              []corev1.Node
 	etcdService        *corev1.Service
 	apisixService      *corev1.Service
-	httpbinDeployment  *appsv1.Deployment

Review comment:
       because it has never been used




-- 
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 #919: fix: watch all namespaces by default

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


   Thanks! I will finish the review today.
   
   re-run CI.


-- 
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 #919: fix: watch all namespaces by default

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


   Maybe it should be noted in the configuration file.
   This is an important change.
   I think multiple reminders are important。
   
   
   re-run CI


-- 
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] cmssczy edited a comment on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy edited a comment on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1068629297


   ![test-cases](https://user-images.githubusercontent.com/65757763/158497945-2da23e61-49ee-4cd7-ab35-9347afbd2c76.png)
   
   Can't reproduce the failed test cases in my environment. 
   


-- 
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 #919: fix: watch all namespaces by default

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


   let me re-run CI
   


-- 
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 #919: fix: watch all namespaces by default

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


   Let's move forward. Thanks!
   
   
   
   > Maybe there are other cases did not clean up the dirty data. All cases ware passed after I delete
   > 
   > ```go
   > assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2), "checking number of routes")
   > ```
   
   We can check this condition later


-- 
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] cmssczy commented on pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#issuecomment-1074076962


   I have ran the e2e-test for many times and all new test cases passed. The failed test cases can not reproduced in my local environment.


-- 
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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r828793586



##########
File path: test/e2e/ingress/namespace.go
##########
@@ -86,7 +88,94 @@ spec:
       servicePort: %d
 `, backendSvc, backendSvcPort[0])
 
-		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
-		_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+			assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromStringWithNamespace(route, "default"), "creating ApisixRoute")
+			_ = s.NewAPISIXClient().GET("/headers").WithHeader("Host", "httpbin.com").Expect().Status(http.StatusNotFound)
+		})
+	})
+
+	ginkgo.Context("without namespace_selector", func() {
+		// make namespace_selector empty
+		s.DisableNamespaceSelector()
+		namespace := "second-httpbin-service-namespace"

Review comment:
       I have updated the content  of comment. Maybe CI need re-run.




-- 
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] cmssczy commented on a change in pull request #919: fix: watch all namespaces by default

Posted by GitBox <gi...@apache.org>.
cmssczy commented on a change in pull request #919:
URL: https://github.com/apache/apisix-ingress-controller/pull/919#discussion_r829018538



##########
File path: test/e2e/ingress/sanity.go
##########
@@ -27,7 +27,7 @@ import (
 )
 
 type ip struct {
-	IP string `json:"ip"`
+	IP string `json:"origin"`

Review comment:
       The API response is like
   ```json
   {
     "origin": "xx.xx.xx.xx"
   }
   ```




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