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 2021/02/04 10:22:13 UTC

[GitHub] [apisix-ingress-controller] tokers opened a new pull request #236: chore: new apisix upstream controller

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


   * `pkg/crd` contains translators to translate resources from declarative configurations to the APISIX resources.
   * refactor the data structure of ApisixUpstream and Upstream.
   * refactor the ApisixUpstream controller.


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

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

[GitHub] [apisix-ingress-controller] huahu0213 commented on pull request #236: chore: new apisix upstream controller

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


   > ci failed :(
   
   So sorry ... I will figure it out tomorrow 


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

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

[GitHub] [apisix-ingress-controller] codecov-io edited a comment on pull request #236: chore: new apisix upstream controller

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=h1) Report
   > Merging [#236](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=desc) (31434f1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/0b15c20d15ded6c8c9c149ef5f9ba2bb7e3b961b?el=desc) (0b15c20) will **decrease** coverage by `0.54%`.
   > The diff coverage is `54.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #236      +/-   ##
   ==========================================
   - Coverage   45.81%   45.27%   -0.55%     
   ==========================================
     Files          33       34       +1     
     Lines        1997     2052      +55     
   ==========================================
   + Hits          915      929      +14     
   - Misses        939      984      +45     
   + Partials      143      139       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/ingress/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3JvdXRlLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/types/event.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL3R5cGVzL2V2ZW50Lmdv) | `100.00% <ø> (ø)` | |
   | [pkg/kube/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvdHJhbnNsYXRvci5nbw==) | `58.42% <58.42%> (ø)` | |
   | [pkg/apisix/resource.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yZXNvdXJjZS5nbw==) | `80.89% <100.00%> (ø)` | |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-dGVzdC9lMmUvZTJlLmdv) | | |
   | [pkg/kube/init.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5pdC5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/infra.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5mcmEuZ28=) | `0.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=footer). Last update [0b15c20...31434f1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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

[GitHub] [apisix-ingress-controller] gxthrj commented on pull request #236: chore: new apisix upstream controller

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


   CI failed
   


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

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

[GitHub] [apisix-ingress-controller] codecov-io commented on pull request #236: chore: new apisix upstream controller

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=h1) Report
   > Merging [#236](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=desc) (5624a68) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/0b15c20d15ded6c8c9c149ef5f9ba2bb7e3b961b?el=desc) (0b15c20) will **increase** coverage by `0.23%`.
   > The diff coverage is `54.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #236      +/-   ##
   ==========================================
   + Coverage   45.81%   46.05%   +0.23%     
   ==========================================
     Files          33       34       +1     
     Lines        1997     2052      +55     
   ==========================================
   + Hits          915      945      +30     
   - Misses        939      971      +32     
   + Partials      143      136       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/ingress/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3JvdXRlLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/types/event.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL3R5cGVzL2V2ZW50Lmdv) | `100.00% <ø> (ø)` | |
   | [pkg/kube/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvdHJhbnNsYXRvci5nbw==) | `58.42% <58.42%> (ø)` | |
   | [pkg/apisix/resource.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yZXNvdXJjZS5nbw==) | `80.89% <100.00%> (ø)` | |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-dGVzdC9lMmUvZTJlLmdv) | | |
   | [pkg/kube/infra.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5mcmEuZ28=) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/init.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5pdC5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `41.72% <0.00%> (+1.98%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `35.07% <0.00%> (+6.16%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=footer). Last update [0b15c20...5624a68](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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

[GitHub] [apisix-ingress-controller] membphis commented on pull request #236: chore: new apisix upstream controller

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


   ci failed  :(


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

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

[GitHub] [apisix-ingress-controller] tokers commented on pull request #236: chore: new apisix upstream controller

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






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

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

[GitHub] [apisix-ingress-controller] tokers merged pull request #236: chore: new apisix upstream controller

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


   


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

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

[GitHub] [apisix-ingress-controller] membphis commented on pull request #236: chore: new apisix upstream controller

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


   > Also, the reason that the coverage ratio reduces is we deleted some files and its test cases.
   
   nice


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

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

[GitHub] [apisix-ingress-controller] huahu0213 removed a comment on pull request #236: chore: new apisix upstream controller

Posted by GitBox <gi...@apache.org>.
huahu0213 removed a comment on pull request #236:
URL: https://github.com/apache/apisix-ingress-controller/pull/236#issuecomment-773428221


   > ci failed :(
   
   So sorry ... I will figure it out tomorrow 


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

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

[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #236: chore: new apisix upstream controller

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



##########
File path: pkg/ingress/controller/apisix_upstream.go
##########
@@ -15,202 +15,244 @@
 package controller
 
 import (
-	"fmt"
-	"time"
-
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
+	"context"
+
+	"go.uber.org/zap"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/apisix"
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/endpoint"
+	apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
 	configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
-	clientset "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned"
-	apisixscheme "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme"
-	informersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v1"
-	listersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v1"
 	"github.com/apache/apisix-ingress-controller/pkg/log"
-	"github.com/apache/apisix-ingress-controller/pkg/seven/state"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
 )
 
-type ApisixUpstreamController struct {
-	controller           *Controller
-	kubeclientset        kubernetes.Interface
-	apisixClientset      clientset.Interface
-	apisixUpstreamList   listersv1.ApisixUpstreamLister
-	apisixUpstreamSynced cache.InformerSynced
-	workqueue            workqueue.RateLimitingInterface
+type apisixUpstreamController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
 }
 
-func BuildApisixUpstreamController(
-	kubeclientset kubernetes.Interface,
-	apisixUpstreamClientset clientset.Interface,
-	apisixUpstreamInformer informersv1.ApisixUpstreamInformer,
-	root *Controller) *ApisixUpstreamController {
-
-	runtime.Must(apisixscheme.AddToScheme(scheme.Scheme))
-	controller := &ApisixUpstreamController{
-		controller:           root,
-		kubeclientset:        kubeclientset,
-		apisixClientset:      apisixUpstreamClientset,
-		apisixUpstreamList:   apisixUpstreamInformer.Lister(),
-		apisixUpstreamSynced: apisixUpstreamInformer.Informer().HasSynced,
-		workqueue:            workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "ApisixUpstreams"),
-	}
-	apisixUpstreamInformer.Informer().AddEventHandler(
+func (c *Controller) newApisixUpstreamController() *apisixUpstreamController {
+	ctl := &apisixUpstreamController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ApisixUpstream"),
+		workers:    1,

Review comment:
       OK, I'll.




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

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

[GitHub] [apisix-ingress-controller] tokers commented on pull request #236: chore: new apisix upstream controller

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


   @membphis It's somewhat difficult to test the controller loop in unit test cases, we test it already in the e2e test suites.
   
   Also, the reason that the coverage ratio reduces is we deleted some files and its 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.

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

[GitHub] [apisix-ingress-controller] huahu0213 commented on pull request #236: chore: new apisix upstream controller

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


   > ci failed :(
   
   So sorry ... I will figure it out tomorrow 


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

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

[GitHub] [apisix-ingress-controller] tokers commented on pull request #236: chore: new apisix upstream controller

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


   > ci failed :(
   
   Fixed.


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

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

[GitHub] [apisix-ingress-controller] membphis commented on pull request #236: chore: new apisix upstream controller

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


   ci failed  :(


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

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

[GitHub] [apisix-ingress-controller] membphis commented on pull request #236: chore: new apisix upstream controller

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


   I think we need more test cases
   
   ![image](https://user-images.githubusercontent.com/6814606/107121509-c35a6e00-68cd-11eb-9186-1d9dc193eede.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.

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

[GitHub] [apisix-ingress-controller] tokers commented on pull request #236: chore: new apisix upstream controller

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


   @gxthrj @membphis Please take a look when you have 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.

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

[GitHub] [apisix-ingress-controller] huahu0213 removed a comment on pull request #236: chore: new apisix upstream controller

Posted by GitBox <gi...@apache.org>.
huahu0213 removed a comment on pull request #236:
URL: https://github.com/apache/apisix-ingress-controller/pull/236#issuecomment-773428221


   > ci failed :(
   
   So sorry ... I will figure it out tomorrow 


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

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

[GitHub] [apisix-ingress-controller] gxthrj commented on a change in pull request #236: chore: new apisix upstream controller

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



##########
File path: pkg/ingress/controller/apisix_upstream.go
##########
@@ -15,202 +15,244 @@
 package controller
 
 import (
-	"fmt"
-	"time"
-
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
+	"context"
+
+	"go.uber.org/zap"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/apisix"
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/endpoint"
+	apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
 	configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
-	clientset "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned"
-	apisixscheme "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme"
-	informersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v1"
-	listersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v1"
 	"github.com/apache/apisix-ingress-controller/pkg/log"
-	"github.com/apache/apisix-ingress-controller/pkg/seven/state"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
 )
 
-type ApisixUpstreamController struct {
-	controller           *Controller
-	kubeclientset        kubernetes.Interface
-	apisixClientset      clientset.Interface
-	apisixUpstreamList   listersv1.ApisixUpstreamLister
-	apisixUpstreamSynced cache.InformerSynced
-	workqueue            workqueue.RateLimitingInterface
+type apisixUpstreamController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
 }
 
-func BuildApisixUpstreamController(
-	kubeclientset kubernetes.Interface,
-	apisixUpstreamClientset clientset.Interface,
-	apisixUpstreamInformer informersv1.ApisixUpstreamInformer,
-	root *Controller) *ApisixUpstreamController {
-
-	runtime.Must(apisixscheme.AddToScheme(scheme.Scheme))
-	controller := &ApisixUpstreamController{
-		controller:           root,
-		kubeclientset:        kubeclientset,
-		apisixClientset:      apisixUpstreamClientset,
-		apisixUpstreamList:   apisixUpstreamInformer.Lister(),
-		apisixUpstreamSynced: apisixUpstreamInformer.Informer().HasSynced,
-		workqueue:            workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "ApisixUpstreams"),
-	}
-	apisixUpstreamInformer.Informer().AddEventHandler(
+func (c *Controller) newApisixUpstreamController() *apisixUpstreamController {
+	ctl := &apisixUpstreamController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ApisixUpstream"),
+		workers:    1,

Review comment:
       OK, please add a good first issue~




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

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

[GitHub] [apisix-ingress-controller] gxthrj commented on a change in pull request #236: chore: new apisix upstream controller

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



##########
File path: pkg/ingress/controller/apisix_upstream.go
##########
@@ -15,202 +15,244 @@
 package controller
 
 import (
-	"fmt"
-	"time"
-
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
+	"context"
+
+	"go.uber.org/zap"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/apisix"
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/endpoint"
+	apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
 	configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
-	clientset "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned"
-	apisixscheme "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme"
-	informersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v1"
-	listersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v1"
 	"github.com/apache/apisix-ingress-controller/pkg/log"
-	"github.com/apache/apisix-ingress-controller/pkg/seven/state"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
 )
 
-type ApisixUpstreamController struct {
-	controller           *Controller
-	kubeclientset        kubernetes.Interface
-	apisixClientset      clientset.Interface
-	apisixUpstreamList   listersv1.ApisixUpstreamLister
-	apisixUpstreamSynced cache.InformerSynced
-	workqueue            workqueue.RateLimitingInterface
+type apisixUpstreamController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
 }
 
-func BuildApisixUpstreamController(
-	kubeclientset kubernetes.Interface,
-	apisixUpstreamClientset clientset.Interface,
-	apisixUpstreamInformer informersv1.ApisixUpstreamInformer,
-	root *Controller) *ApisixUpstreamController {
-
-	runtime.Must(apisixscheme.AddToScheme(scheme.Scheme))
-	controller := &ApisixUpstreamController{
-		controller:           root,
-		kubeclientset:        kubeclientset,
-		apisixClientset:      apisixUpstreamClientset,
-		apisixUpstreamList:   apisixUpstreamInformer.Lister(),
-		apisixUpstreamSynced: apisixUpstreamInformer.Informer().HasSynced,
-		workqueue:            workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "ApisixUpstreams"),
-	}
-	apisixUpstreamInformer.Informer().AddEventHandler(
+func (c *Controller) newApisixUpstreamController() *apisixUpstreamController {
+	ctl := &apisixUpstreamController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ApisixUpstream"),
+		workers:    1,

Review comment:
       If we add workers here, had better config it in yaml.




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

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

[GitHub] [apisix-ingress-controller] codecov-io commented on pull request #236: chore: new apisix upstream controller

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=h1) Report
   > Merging [#236](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=desc) (5624a68) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/0b15c20d15ded6c8c9c149ef5f9ba2bb7e3b961b?el=desc) (0b15c20) will **increase** coverage by `0.23%`.
   > The diff coverage is `54.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #236      +/-   ##
   ==========================================
   + Coverage   45.81%   46.05%   +0.23%     
   ==========================================
     Files          33       34       +1     
     Lines        1997     2052      +55     
   ==========================================
   + Hits          915      945      +30     
   - Misses        939      971      +32     
   + Partials      143      136       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [pkg/ingress/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3JvdXRlLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/types/event.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL3R5cGVzL2V2ZW50Lmdv) | `100.00% <ø> (ø)` | |
   | [pkg/kube/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvdHJhbnNsYXRvci5nbw==) | `58.42% <58.42%> (ø)` | |
   | [pkg/apisix/resource.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yZXNvdXJjZS5nbw==) | `80.89% <100.00%> (ø)` | |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-dGVzdC9lMmUvZTJlLmdv) | | |
   | [pkg/kube/infra.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5mcmEuZ28=) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/init.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2t1YmUvaW5pdC5nbw==) | `0.00% <0.00%> (ø)` | |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `41.72% <0.00%> (+1.98%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `35.07% <0.00%> (+6.16%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=footer). Last update [0b15c20...5624a68](https://codecov.io/gh/apache/apisix-ingress-controller/pull/236?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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

[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #236: chore: new apisix upstream controller

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



##########
File path: pkg/ingress/controller/apisix_upstream.go
##########
@@ -15,202 +15,244 @@
 package controller
 
 import (
-	"fmt"
-	"time"
-
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
+	"context"
+
+	"go.uber.org/zap"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/apisix"
-	"github.com/apache/apisix-ingress-controller/pkg/ingress/endpoint"
+	apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
 	configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
-	clientset "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned"
-	apisixscheme "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/scheme"
-	informersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions/config/v1"
-	listersv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/listers/config/v1"
 	"github.com/apache/apisix-ingress-controller/pkg/log"
-	"github.com/apache/apisix-ingress-controller/pkg/seven/state"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
 )
 
-type ApisixUpstreamController struct {
-	controller           *Controller
-	kubeclientset        kubernetes.Interface
-	apisixClientset      clientset.Interface
-	apisixUpstreamList   listersv1.ApisixUpstreamLister
-	apisixUpstreamSynced cache.InformerSynced
-	workqueue            workqueue.RateLimitingInterface
+type apisixUpstreamController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
 }
 
-func BuildApisixUpstreamController(
-	kubeclientset kubernetes.Interface,
-	apisixUpstreamClientset clientset.Interface,
-	apisixUpstreamInformer informersv1.ApisixUpstreamInformer,
-	root *Controller) *ApisixUpstreamController {
-
-	runtime.Must(apisixscheme.AddToScheme(scheme.Scheme))
-	controller := &ApisixUpstreamController{
-		controller:           root,
-		kubeclientset:        kubeclientset,
-		apisixClientset:      apisixUpstreamClientset,
-		apisixUpstreamList:   apisixUpstreamInformer.Lister(),
-		apisixUpstreamSynced: apisixUpstreamInformer.Informer().HasSynced,
-		workqueue:            workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "ApisixUpstreams"),
-	}
-	apisixUpstreamInformer.Informer().AddEventHandler(
+func (c *Controller) newApisixUpstreamController() *apisixUpstreamController {
+	ctl := &apisixUpstreamController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ApisixUpstream"),
+		workers:    1,

Review comment:
       It's an extention, we can add good first issue, let others do 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.

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

[GitHub] [apisix-ingress-controller] tokers commented on pull request #236: chore: new apisix upstream controller

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


   > > Also, the reason that the coverage ratio reduces is we deleted some files and its test cases.
   > 
   > nice
   
   And I created an issue #245 , will add more cases in the future.


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

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