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/12/13 03:29:59 UTC

[GitHub] [apisix-ingress-controller] Donghui0 opened a new pull request, #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

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

   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   - Related issues
   #578 
   #592 
   ___
   
   ___
   ### New feature or improvement
   make multiple controllers handle different ApisixRoute CRDs
   
   ___
   ### Backport patches
   - Why need to backport?
   
   - Source branch
   
   - Related commits and pull requests
   
   - Target branch
   


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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

   @Donghui0 Please see https://github.com/apache/apisix-ingress-controller/issues/592#issuecomment-1449264770 
   I have added more details.


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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


##########
go.mod:
##########
@@ -28,6 +28,11 @@ require (
 	sigs.k8s.io/yaml v1.3.0
 )
 
+require (
+	github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 // indirect
+	github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect
+)
+

Review Comment:
   you don't need to change this one. 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] github-actions[bot] commented on pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #593:
URL: https://github.com/apache/apisix-ingress-controller/pull/593#issuecomment-1320728942

   This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any 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 diff in pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs

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


##########
pkg/providers/apisix/apisix_route.go:
##########
@@ -534,6 +534,23 @@ func (c *apisixRouteController) handleSyncErr(obj interface{}, errOrigin error)
 	c.MetricsCollector.IncrSyncOperation("route", "failure")
 }
 
+func (c *apisixRouteController) isEffective(ar kube.ApisixRoute) bool {
+	var icn string
+
+	switch ar.GroupVersion() {
+	case config.ApisixV2beta3:
+		icn = ar.V2beta3().Spec.IngressClassName

Review Comment:
   ditto



##########
pkg/kube/apisix/apis/config/v2beta3/types.go:
##########
@@ -44,8 +44,9 @@ type ApisixStatus struct {
 
 // ApisixRouteSpec is the spec definition for ApisixRouteSpec.
 type ApisixRouteSpec struct {
-	HTTP   []ApisixRouteHTTP   `json:"http,omitempty" yaml:"http,omitempty"`
-	Stream []ApisixRouteStream `json:"stream,omitempty" yaml:"stream,omitempty"`
+	IngressClassName string              `json:"ingressClassName,omitempty" yaml:"ingressClassName,omitempty"`
+	HTTP             []ApisixRouteHTTP   `json:"http,omitempty" yaml:"http,omitempty"`
+	Stream           []ApisixRouteStream `json:"stream,omitempty" yaml:"stream,omitempty"`

Review Comment:
   We don't need to change the v2beta3 resource.



-- 
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] github-actions[bot] commented on pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #593:
URL: https://github.com/apache/apisix-ingress-controller/pull/593#issuecomment-1283252134

   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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

   CI fails, I think @AlinsRan  can help. 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] github-actions[bot] closed pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs #578
URL: https://github.com/apache/apisix-ingress-controller/pull/593


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

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

   Yesterday I had an online meeting with @Donghui0  and @Gallardot.
   After discussion, we will use this PR instead of #1523, and @Donghui0 will complete this PR. 
   
   I've just resolved the code merge conflict in this PR.
   
   For some basic technical solutions can refer to  https://github.com/apache/apisix-ingress-controller/pull/1523#issuecomment-1429344801
   
   I will also submit a more complete description at https://github.com/apache/apisix-ingress-controller/issues/592 later
   
   Thank you all!


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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


##########
go.mod:
##########
@@ -67,6 +67,8 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/onsi/ginkgo/v2 v2.9.0 // indirect
+	github.com/onsi/gomega v1.27.1 // indirect

Review Comment:
   These two dependencies are redundant and will only be used in e2e. Let me remove them.



-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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


##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -69,6 +69,8 @@ spec:
                 - required: ["http"]
                 - required: ["stream"]
               properties:
+                ingressClassName:
+                  type: string

Review Comment:
   no longer need this



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

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

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


[GitHub] [apisix-ingress-controller] Gallardot commented on pull request #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

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

   @Donghui0  Do you have time to work on this PR? Maybe I can help with that.
   
   


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs #578

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

    there are some conflicts, and I want the feature to just be added to the v2 version of the API
   
   https://github.com/apache/apisix-ingress-controller/issues/592#issuecomment-1221021549


-- 
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 #593: feat: make multiple controllers handle different ApisixRoute CRDs

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


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