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/07/24 12:00:58 UTC

[GitHub] [apisix-ingress-controller] fhuzero opened a new pull request #606: feat: add support for Knative

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


   - Why submit this pull request?
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   - Related issues
   #533 
   
   ___
   ### New feature or improvement
   This PR provides support for Knative so that APISIX Ingress Controller is an alternative to KIngress implementation. It can replace Istio, Gloo, Kourier, or Kong to serve as the network layer of Knative Serving. APISIX Ingress Controller is now aware of status change of Knative ingress, able to parse it, and can translate it to APISIX custom resource like routes and upstreams.
   
   The current supported API version of Knative ingress is _networking/v1alpha1_.
   
   Some key changes are listed as follows.
   
   - Add a general KnativeIngress interface to encapsulate different versions
   - Add a KnativeIngressVersion to support different Knative ingress version
   - Add KnativeClient to operate Knative builtin resources
   - Add KnativeIngressInformer and KnativeIngressLister
   - Add knativeIngressController and its related methods such as event hander
   - Add function (TranslateKnativeIngress) to translate KnativeIngress to APISIX routes and upstreams
   


-- 
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] tokers commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)

Review comment:
       But it's so mysterious if we select one of them by the percent, I think just pick the first one is OK.




-- 
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 #606: feat: add support for Knative

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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 [#606](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (85681b1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/7291212964a7f3505fb1bb624fb79df0c9eb0678?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7291212) will **decrease** coverage by `1.67%`.
   > The diff coverage is `4.26%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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/606?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     #606      +/-   ##
   ==========================================
   - Coverage   34.40%   32.72%   -1.68%     
   ==========================================
     Files          55       57       +2     
     Lines        5468     5763     +295     
   ==========================================
   + Hits         1881     1886       +5     
   - Misses       3362     3652     +290     
     Partials      225      225              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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/controller.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvY29udHJvbGxlci5nbw==) | `1.03% <0.00%> (-0.05%)` | :arrow_down: |
   | [pkg/ingress/knative\_ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2luZ3Jlc3Mva25hdGl2ZV9pbmdyZXNzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/knative\_ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2t1YmUvdHJhbnNsYXRpb24va25hdGl2ZV9pbmdyZXNzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | `44.33% <0.00%> (-2.67%)` | :arrow_down: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `83.87% <85.71%> (-1.85%)` | :arrow_down: |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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/606?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 [7291212...85681b1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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] Yiyiyimu edited a comment on pull request #606: feat: add support for Knative

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


   Hi @fhuzero ~I think it would be nice if you could also [include the documentations](https://github.com/apache/apisix-ingress-controller/commit/979531b274751a97a7e39892fe0550dbb256281f) in this PR~
   
   I just noticed something wrong with knative image compiling. We could add the docs when it succeeds


-- 
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] fhuzero commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)
+			servicePort := knativeBackend.ServicePort
+			serviceName := fmt.Sprintf("%s.%s.%s", knativeBackend.ServiceNamespace, knativeBackend.ServiceName,

Review comment:
       Thanks for pointing out the problem! `serviceName` should only consists of `knativeBackend.ServiceName`. This was fixed in a later commit but I failed to identify it when I splited the huge PR. I'll fix it soon.




-- 
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] tokers commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/ingress/knative_ingress.go
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"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/kube"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+)
+
+//const knativeIngressClassKey = "networking.knative.dev/ingress.class"
+
+type knativeIngressController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
+}
+
+func (c *Controller) newKnativeIngressController() *knativeIngressController {
+	ctl := &knativeIngressController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "KnativeIngress"),
+		workers:    1,
+	}
+
+	c.knativeIngressInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctl.onAdd,
+		UpdateFunc: ctl.onUpdate,
+		DeleteFunc: ctl.OnDelete,
+	})
+	return ctl
+}
+
+func (c *knativeIngressController) run(ctx context.Context) {
+	log.Info("knative ingress controller started")
+	defer log.Infof("knative ingress controller exited")
+	defer c.workqueue.ShutDown()
+
+	if !cache.WaitForCacheSync(ctx.Done(), c.controller.knativeIngressInformer.HasSynced) {
+		log.Errorf("cache sync failed")
+		return
+	}
+	for i := 0; i < c.workers; i++ {
+		go c.runWorker(ctx)
+	}
+	<-ctx.Done()
+}
+
+func (c *knativeIngressController) runWorker(ctx context.Context) {
+	for {
+		obj, quit := c.workqueue.Get()
+		if quit {
+			return
+		}
+		err := c.sync(ctx, obj.(*types.Event))
+		c.workqueue.Done(obj)
+		c.handleSyncErr(obj, err)
+	}
+}
+
+func (c *knativeIngressController) sync(ctx context.Context, ev *types.Event) error {
+	ingEv := ev.Object.(kube.KnativeIngressEvent)
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingEv.Key)
+	if err != nil {
+		log.Errorf("found knative ingress resource with invalid meta namespace key %s: %s", ingEv.Key, err)
+		return err
+	}
+
+	var ing kube.KnativeIngress
+	switch ingEv.GroupVersion {

Review comment:
       Intercept unexpected resources in an earlier phase is better.




-- 
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] fhuzero commented on pull request #606: feat: add support for Knative

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


   > @fhuzero Some conflicts need to be dealt with.
   
   Thanks for your suggestion! The main conflict is package version difference between this PR and knative/pkg used in knative conformance test. I have tried to align the version but failed. Once knative/pkg upgrades their dependency, I'll solve conflicts between this PR and main 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] codecov-commenter commented on pull request #606: feat: add support for Knative

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


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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 [#606](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (38dd004) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/7291212964a7f3505fb1bb624fb79df0c9eb0678?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7291212) will **decrease** coverage by `1.56%`.
   > The diff coverage is `4.18%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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/606?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     #606      +/-   ##
   ==========================================
   - Coverage   34.40%   32.83%   -1.57%     
   ==========================================
     Files          55       57       +2     
     Lines        5468     5769     +301     
   ==========================================
   + Hits         1881     1894      +13     
   - Misses       3362     3653     +291     
   + Partials      225      222       -3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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/controller.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2luZ3Jlc3MvY29udHJvbGxlci5nbw==) | `1.03% <0.00%> (-0.05%)` | :arrow_down: |
   | [pkg/ingress/knative\_ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2luZ3Jlc3Mva25hdGl2ZV9pbmdyZXNzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/knative\_ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2t1YmUvdHJhbnNsYXRpb24va25hdGl2ZV9pbmdyZXNzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/kube/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | `44.33% <0.00%> (-2.67%)` | :arrow_down: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `83.87% <85.71%> (-1.85%)` | :arrow_down: |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `26.94% <0.00%> (+1.62%)` | :arrow_up: |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606/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-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `37.76% <0.00%> (+2.09%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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/606?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 [7291212...38dd004](https://codecov.io/gh/apache/apisix-ingress-controller/pull/606?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] fhuzero commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)

Review comment:
       OK, I'll just pick the first 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] fhuzero commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/ingress/knative_ingress.go
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"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/kube"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+)
+
+//const knativeIngressClassKey = "networking.knative.dev/ingress.class"
+
+type knativeIngressController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
+}
+
+func (c *Controller) newKnativeIngressController() *knativeIngressController {
+	ctl := &knativeIngressController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "KnativeIngress"),
+		workers:    1,
+	}
+
+	c.knativeIngressInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctl.onAdd,
+		UpdateFunc: ctl.onUpdate,
+		DeleteFunc: ctl.OnDelete,
+	})
+	return ctl
+}
+
+func (c *knativeIngressController) run(ctx context.Context) {
+	log.Info("knative ingress controller started")
+	defer log.Infof("knative ingress controller exited")
+	defer c.workqueue.ShutDown()
+
+	if !cache.WaitForCacheSync(ctx.Done(), c.controller.knativeIngressInformer.HasSynced) {
+		log.Errorf("cache sync failed")
+		return
+	}
+	for i := 0; i < c.workers; i++ {
+		go c.runWorker(ctx)
+	}
+	<-ctx.Done()
+}
+
+func (c *knativeIngressController) runWorker(ctx context.Context) {
+	for {
+		obj, quit := c.workqueue.Get()
+		if quit {
+			return
+		}
+		err := c.sync(ctx, obj.(*types.Event))
+		c.workqueue.Done(obj)
+		c.handleSyncErr(obj, err)
+	}
+}
+
+func (c *knativeIngressController) sync(ctx context.Context, ev *types.Event) error {
+	ingEv := ev.Object.(kube.KnativeIngressEvent)
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingEv.Key)
+	if err != nil {
+		log.Errorf("found knative ingress resource with invalid meta namespace key %s: %s", ingEv.Key, err)
+		return err
+	}
+
+	var ing kube.KnativeIngress
+	switch ingEv.GroupVersion {

Review comment:
       Got it! I'll put version judgement in `onAdd`, `onUpdate`, and `onDelete`.




-- 
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] fhuzero commented on pull request #606: feat: add support for Knative

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


   > @fhuzero Some conflicts need to be dealt with.
   
   Thanks for your suggestion! The main conflict is package version difference between this PR and knative/pkg used in knative conformance test. I have tried to align the version but failed. Once knative/pkg upgrades their dependency, I'll solve conflicts between this PR and main 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] Yiyiyimu commented on pull request #606: feat: add support for Knative

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


   Hi @fhuzero I think it would be nice if you could also [include the documentations](https://github.com/apache/apisix-ingress-controller/commit/979531b274751a97a7e39892fe0550dbb256281f) in this PR


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

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

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



[GitHub] [apisix-ingress-controller] fhuzero commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)

Review comment:
       This is an initial version of Knative support, and `knativeSelectSplit` does not implement traffic split logic of KIngress right now. I plan to add KIngress features like traffic-split, append-headers, etc. in later PR. I can also add traffic-split feature in this PR.




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

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

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



[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/ingress/knative_ingress.go
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"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/kube"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+)
+
+//const knativeIngressClassKey = "networking.knative.dev/ingress.class"
+
+type knativeIngressController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
+}
+
+func (c *Controller) newKnativeIngressController() *knativeIngressController {
+	ctl := &knativeIngressController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "KnativeIngress"),
+		workers:    1,
+	}
+
+	c.knativeIngressInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctl.onAdd,
+		UpdateFunc: ctl.onUpdate,
+		DeleteFunc: ctl.OnDelete,
+	})
+	return ctl
+}
+
+func (c *knativeIngressController) run(ctx context.Context) {
+	log.Info("knative ingress controller started")
+	defer log.Infof("knative ingress controller exited")
+	defer c.workqueue.ShutDown()
+
+	if !cache.WaitForCacheSync(ctx.Done(), c.controller.knativeIngressInformer.HasSynced) {
+		log.Errorf("cache sync failed")
+		return
+	}
+	for i := 0; i < c.workers; i++ {
+		go c.runWorker(ctx)
+	}
+	<-ctx.Done()
+}
+
+func (c *knativeIngressController) runWorker(ctx context.Context) {
+	for {
+		obj, quit := c.workqueue.Get()
+		if quit {
+			return
+		}
+		err := c.sync(ctx, obj.(*types.Event))
+		c.workqueue.Done(obj)
+		c.handleSyncErr(obj, err)
+	}
+}
+
+func (c *knativeIngressController) sync(ctx context.Context, ev *types.Event) error {
+	ingEv := ev.Object.(kube.KnativeIngressEvent)
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingEv.Key)
+	if err != nil {
+		log.Errorf("found knative ingress resource with invalid meta namespace key %s: %s", ingEv.Key, err)
+		return err
+	}
+
+	var ing kube.KnativeIngress
+	switch ingEv.GroupVersion {

Review comment:
       Version judgement can be made in the `onAdd`, `onUpdate` and `onDelete` callbacks.

##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)
+			servicePort := knativeBackend.ServicePort
+			serviceName := fmt.Sprintf("%s.%s.%s", knativeBackend.ServiceNamespace, knativeBackend.ServiceName,

Review comment:
       I am confused by the service name. Why here we should concatenate the namespace and name and port, but it was used just as the Kubernetes Service Name. I was wondering whether it really works.

##########
File path: pkg/kube/translation/knative_ingress.go
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package translation
+
+import (
+	"fmt"
+	"strings"
+
+	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	knativev1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
+
+	"github.com/apache/apisix-ingress-controller/pkg/id"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func (t *translator) translateKnativeIngressV1alpha1(ing *knativev1alpha1.Ingress) (*TranslateContext, error) {
+	ctx := &TranslateContext{
+		upstreamMap: make(map[string]struct{}),
+	}
+	plugins := t.translateAnnotations(ing.Annotations)
+
+	for i, rule := range ing.Spec.Rules {
+		hosts := rule.Hosts
+		if rule.HTTP == nil {
+			continue
+		}
+		for j, httpPath := range rule.HTTP.Paths {
+			var (
+				ups *apisixv1.Upstream
+				err error
+			)
+			knativeBackend := knativeSelectSplit(httpPath.Splits)

Review comment:
       Why just select one? Maybe others should be put to the traffic-split plugin.




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

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

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



[GitHub] [apisix-ingress-controller] gxthrj commented on pull request #606: feat: add support for Knative

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


   @fhuzero  Some conflicts need to be dealt with.


-- 
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] fhuzero commented on a change in pull request #606: feat: add support for Knative

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



##########
File path: pkg/ingress/knative_ingress.go
##########
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package ingress
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"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/kube"
+	"github.com/apache/apisix-ingress-controller/pkg/log"
+	"github.com/apache/apisix-ingress-controller/pkg/types"
+)
+
+//const knativeIngressClassKey = "networking.knative.dev/ingress.class"
+
+type knativeIngressController struct {
+	controller *Controller
+	workqueue  workqueue.RateLimitingInterface
+	workers    int
+}
+
+func (c *Controller) newKnativeIngressController() *knativeIngressController {
+	ctl := &knativeIngressController{
+		controller: c,
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.NewItemFastSlowRateLimiter(1*time.Second, 60*time.Second, 5), "KnativeIngress"),
+		workers:    1,
+	}
+
+	c.knativeIngressInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctl.onAdd,
+		UpdateFunc: ctl.onUpdate,
+		DeleteFunc: ctl.OnDelete,
+	})
+	return ctl
+}
+
+func (c *knativeIngressController) run(ctx context.Context) {
+	log.Info("knative ingress controller started")
+	defer log.Infof("knative ingress controller exited")
+	defer c.workqueue.ShutDown()
+
+	if !cache.WaitForCacheSync(ctx.Done(), c.controller.knativeIngressInformer.HasSynced) {
+		log.Errorf("cache sync failed")
+		return
+	}
+	for i := 0; i < c.workers; i++ {
+		go c.runWorker(ctx)
+	}
+	<-ctx.Done()
+}
+
+func (c *knativeIngressController) runWorker(ctx context.Context) {
+	for {
+		obj, quit := c.workqueue.Get()
+		if quit {
+			return
+		}
+		err := c.sync(ctx, obj.(*types.Event))
+		c.workqueue.Done(obj)
+		c.handleSyncErr(obj, err)
+	}
+}
+
+func (c *knativeIngressController) sync(ctx context.Context, ev *types.Event) error {
+	ingEv := ev.Object.(kube.KnativeIngressEvent)
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingEv.Key)
+	if err != nil {
+		log.Errorf("found knative ingress resource with invalid meta namespace key %s: %s", ingEv.Key, err)
+		return err
+	}
+
+	var ing kube.KnativeIngress
+	switch ingEv.GroupVersion {

Review comment:
       I followed version judgement in pkg/ingress/ingress.go [here](https://github.com/apache/apisix-ingress-controller/blob/master/pkg/ingress/ingress.go#L93), which judges ingress version in `sync`, instead of `onAdd`, `onUpdate`, and `onDelete`. Could you please tell me differences between 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] gxthrj commented on pull request #606: feat: add support for Knative

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


   @fhuzero  Some conflicts need to be dealt with.


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