From 7e44ed979f4a459778139a775acc6dc57c629715 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 15 Jun 2020 16:08:36 -0700 Subject: [PATCH 01/17] Support Class Field in VS/VSR --- deployments/common/vs-definition.yaml | 2 + deployments/common/vsr-definition.yaml | 2 + .../templates/controller-vs-definition.yaml | 2 + .../templates/controller-vsr-definition.yaml | 2 + ...server-and-virtualserverroute-resources.md | 8 ++ .../running-multiple-ingress-controllers.md | 1 + internal/k8s/controller.go | 22 +++- internal/k8s/controller_test.go | 111 ++++++++++++++++++ internal/k8s/handlers.go | 24 ++++ pkg/apis/configuration/v1/types.go | 16 +-- 10 files changed, 177 insertions(+), 13 deletions(-) diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml index 593767d84b..537781b497 100644 --- a/deployments/common/vs-definition.yaml +++ b/deployments/common/vs-definition.yaml @@ -57,6 +57,8 @@ spec: description: VirtualServerSpec is the spec of the VirtualServer resource. type: object properties: + class: + type: string host: type: string routes: diff --git a/deployments/common/vsr-definition.yaml b/deployments/common/vsr-definition.yaml index e960db4363..8237f7531f 100644 --- a/deployments/common/vsr-definition.yaml +++ b/deployments/common/vsr-definition.yaml @@ -55,6 +55,8 @@ spec: spec: type: object properties: + class: + type: string host: type: string subroutes: diff --git a/deployments/helm-chart/templates/controller-vs-definition.yaml b/deployments/helm-chart/templates/controller-vs-definition.yaml index 8936296b76..3647df425f 100644 --- a/deployments/helm-chart/templates/controller-vs-definition.yaml +++ b/deployments/helm-chart/templates/controller-vs-definition.yaml @@ -60,6 +60,8 @@ spec: description: VirtualServerSpec is the spec of the VirtualServer resource. type: object properties: + class: + type: string host: type: string routes: diff --git a/deployments/helm-chart/templates/controller-vsr-definition.yaml b/deployments/helm-chart/templates/controller-vsr-definition.yaml index 3da6d71648..ac2eac9839 100644 --- a/deployments/helm-chart/templates/controller-vsr-definition.yaml +++ b/deployments/helm-chart/templates/controller-vsr-definition.yaml @@ -58,6 +58,8 @@ spec: spec: type: object properties: + class: + type: string host: type: string subroutes: diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 385c4ffb91..916a322423 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -95,6 +95,10 @@ spec: - A list of routes. - `[]route <#virtualserver-route>`_ - No + * - ``class`` + - Specifies which Ingress controller must handle the Virtual Server resource. Set to `nginx` to make NGINX Ingress controller handle it. + - ``string`` + - No ``` ### VirtualServer.TLS @@ -273,6 +277,10 @@ Note that each subroute must have a `path` that starts with the same prefix (her - A list of subroutes. - `[]subroute <#virtualserverroute-subroute>`_ - No + * - ``class`` + - Specifies which Ingress controller must handle the Virtual Server Route resource. Set to `nginx` to make NGINX Ingress controller handle it. + - ``string``_ + - No ``` ### VirtualServerRoute.Subroute diff --git a/docs-web/installation/running-multiple-ingress-controllers.md b/docs-web/installation/running-multiple-ingress-controllers.md index 4fe99dacaf..65029b15a6 100644 --- a/docs-web/installation/running-multiple-ingress-controllers.md +++ b/docs-web/installation/running-multiple-ingress-controllers.md @@ -10,6 +10,7 @@ This document explains the following topics: The smooth coexistence of multiple Ingress Controllers in one cluster is provided by the Ingress class concept, which mandates the following: * Every Ingress Controller must only handle Ingress resources for its particular class. * Ingress resources should be annotated with the `kubernetes.io/ingress.class` annotation set to the value, which corresponds to the class of the Ingress Controller the user wants to use. +* Virtual Server and Virtual Server Route resources should have the `class` field set to the value, which corresponds to the class of the Ingress Controller the user wants to use. ### Configuring Ingress Class diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 194b325802..8d52eca435 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2372,14 +2372,24 @@ func (lbc *LoadBalancerController) getServiceForIngressBackend(backend *extensio // IsNginxIngress checks if resource ingress class annotation (if exists) is matching with ingress controller class // If annotation is absent and use-ingress-class-only enabled - ingress resource would ignore -func (lbc *LoadBalancerController) IsNginxIngress(ing *extensions.Ingress) bool { - if class, exists := ing.Annotations[ingressClassKey]; exists { - if lbc.useIngressClassOnly { - return class == lbc.ingressClass +func (lbc *LoadBalancerController) IsNginxIngress(obj interface{}) bool { + var class string + switch obj.(type) { + case string: + class = obj.(string) + case *extensions.Ingress: + ing := obj.(*extensions.Ingress) + if ingClass, exists := ing.Annotations[ingressClassKey]; exists { + class = ingClass } - return class == lbc.ingressClass || class == "" + default: + return false } - return !lbc.useIngressClassOnly + + if lbc.useIngressClassOnly { + return class == lbc.ingressClass + } + return class == lbc.ingressClass || class == "" } // isHealthCheckEnabled checks if health checks are enabled so we can only query pods if enabled. diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index feda7a6181..e720dd3b99 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -85,6 +85,55 @@ func TestIsNginxIngress(t *testing.T) { }, } + var testsWithoutIngressClassOnlyVS = []struct { + lbc *LoadBalancerController + ing *conf_v1.VirtualServerSpec + expected bool + }{ + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: false, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: "", + }, + true, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: false, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: "gce", + }, + false, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: false, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: ingressClass, + }, + true, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: false, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{}, + true, + }, + } + var testsWithIngressClassOnly = []struct { lbc *LoadBalancerController ing *extensions.Ingress @@ -144,6 +193,55 @@ func TestIsNginxIngress(t *testing.T) { }, } + var testsWithIngressClassOnlyVS = []struct { + lbc *LoadBalancerController + ing *conf_v1.VirtualServerSpec + expected bool + }{ + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: true, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: "", + }, + false, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: true, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: "gce", + }, + false, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: true, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{ + IngressClass: ingressClass, + }, + true, + }, + { + &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: true, + metricsCollector: collectors.NewControllerFakeCollector(), + }, + &conf_v1.VirtualServerSpec{}, + false, + }, + } + for _, test := range testsWithoutIngressClassOnly { if result := test.lbc.IsNginxIngress(test.ing); result != test.expected { classAnnotation := "N/A" @@ -154,6 +252,12 @@ func TestIsNginxIngress(t *testing.T) { test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, classAnnotation, result, test.expected) } } + for _, test := range testsWithoutIngressClassOnlyVS { + if result := test.lbc.IsNginxIngress(test.ing.IngressClass); result != test.expected { + t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) + } + } for _, test := range testsWithIngressClassOnly { if result := test.lbc.IsNginxIngress(test.ing); result != test.expected { @@ -166,6 +270,13 @@ func TestIsNginxIngress(t *testing.T) { } } + for _, test := range testsWithIngressClassOnlyVS { + if result := test.lbc.IsNginxIngress(test.ing.IngressClass); result != test.expected { + t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) + } + } + } func TestCreateMergableIngresses(t *testing.T) { diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 6b62d9f57f..709ac404c0 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -316,6 +316,10 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vs := obj.(*conf_v1.VirtualServer) + if !lbc.IsNginxIngress(vs.Spec.IngressClass) { + glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) + return + } glog.V(3).Infof("Adding VirtualServer: %v", vs.Name) lbc.AddSyncQueue(vs) }, @@ -333,12 +337,20 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return } } + if !lbc.IsNginxIngress(vs.Spec.IngressClass) { + glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) + return + } glog.V(3).Infof("Removing VirtualServer: %v", vs.Name) lbc.AddSyncQueue(vs) }, UpdateFunc: func(old, cur interface{}) { curVs := cur.(*conf_v1.VirtualServer) oldVs := old.(*conf_v1.VirtualServer) + if !lbc.IsNginxIngress(curVs.Spec.IngressClass) { + glog.Infof("Ignoring VS %v based on class %v", curVs.Name, curVs.Spec.IngressClass) + return + } if !reflect.DeepEqual(oldVs.Spec, curVs.Spec) { glog.V(3).Infof("VirtualServer %v changed, syncing", curVs.Name) lbc.AddSyncQueue(curVs) @@ -351,6 +363,10 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vsr := obj.(*conf_v1.VirtualServerRoute) + if !lbc.IsNginxIngress(vsr.Spec.IngressClass) { + glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + return + } glog.V(3).Infof("Adding VirtualServerRoute: %v", vsr.Name) lbc.AddSyncQueue(vsr) }, @@ -368,12 +384,20 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return } } + if !lbc.IsNginxIngress(vsr.Spec.IngressClass) { + glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + return + } glog.V(3).Infof("Removing VirtualServerRoute: %v", vsr.Name) lbc.AddSyncQueue(vsr) }, UpdateFunc: func(old, cur interface{}) { curVsr := cur.(*conf_v1.VirtualServerRoute) oldVsr := old.(*conf_v1.VirtualServerRoute) + if !lbc.IsNginxIngress(curVsr.Spec.IngressClass) { + glog.Infof("Ignoring VSR %v based on class %v", curVsr.Name, curVsr.Spec.IngressClass) + return + } if !reflect.DeepEqual(oldVsr.Spec, curVsr.Spec) { glog.V(3).Infof("VirtualServerRoute %v changed, syncing", curVsr.Name) lbc.AddSyncQueue(curVsr) diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 9ee54e19fd..f59c3158a0 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -28,10 +28,11 @@ type VirtualServer struct { // VirtualServerSpec is the spec of the VirtualServer resource. type VirtualServerSpec struct { - Host string `json:"host"` - TLS *TLS `json:"tls"` - Upstreams []Upstream `json:"upstreams"` - Routes []Route `json:"routes"` + IngressClass string `json:"class"` + Host string `json:"host"` + TLS *TLS `json:"tls"` + Upstreams []Upstream `json:"upstreams"` + Routes []Route `json:"routes"` } // Upstream defines an upstream. @@ -226,9 +227,10 @@ type VirtualServerRoute struct { } type VirtualServerRouteSpec struct { - Host string `json:"host"` - Upstreams []Upstream `json:"upstreams"` - Subroutes []Route `json:"subroutes"` + IngressClass string `json:"class"` + Host string `json:"host"` + Upstreams []Upstream `json:"upstreams"` + Subroutes []Route `json:"subroutes"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object From 170abf5e074cef12368c31d9f46cfb8b8883a437 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 16 Jun 2020 10:53:10 -0700 Subject: [PATCH 02/17] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raúl --- .../virtualserver-and-virtualserverroute-resources.md | 4 ++-- docs-web/installation/running-multiple-ingress-controllers.md | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 916a322423..0b392e088e 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -96,7 +96,7 @@ spec: - `[]route <#virtualserver-route>`_ - No * - ``class`` - - Specifies which Ingress controller must handle the Virtual Server resource. Set to `nginx` to make NGINX Ingress controller handle it. + - Specifies which Ingress controller must handle the VirtualServer resource. Set to `nginx` to make NGINX Ingress controller handle it. - ``string`` - No ``` @@ -278,7 +278,7 @@ Note that each subroute must have a `path` that starts with the same prefix (her - `[]subroute <#virtualserverroute-subroute>`_ - No * - ``class`` - - Specifies which Ingress controller must handle the Virtual Server Route resource. Set to `nginx` to make NGINX Ingress controller handle it. + - Specifies which Ingress controller must handle the VirtualServerRoute resource. Set to `nginx` to make NGINX Ingress controller handle it. - ``string``_ - No ``` diff --git a/docs-web/installation/running-multiple-ingress-controllers.md b/docs-web/installation/running-multiple-ingress-controllers.md index 65029b15a6..7f5d7bf57c 100644 --- a/docs-web/installation/running-multiple-ingress-controllers.md +++ b/docs-web/installation/running-multiple-ingress-controllers.md @@ -10,7 +10,7 @@ This document explains the following topics: The smooth coexistence of multiple Ingress Controllers in one cluster is provided by the Ingress class concept, which mandates the following: * Every Ingress Controller must only handle Ingress resources for its particular class. * Ingress resources should be annotated with the `kubernetes.io/ingress.class` annotation set to the value, which corresponds to the class of the Ingress Controller the user wants to use. -* Virtual Server and Virtual Server Route resources should have the `class` field set to the value, which corresponds to the class of the Ingress Controller the user wants to use. +* VirtualServer and VirtualServerRoute resources should have the `class` field set to the value, which corresponds to the class of the Ingress Controller the user wants to use. ### Configuring Ingress Class @@ -39,4 +39,3 @@ Considering the options above, you can run multiple NGINX Ingress Controllers, e * [Command-line arguments](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments) **Note**: all mentioned command-line arguments are also available as the parameters in the [Helm chart](/nginx-ingress-controller/installation/installation-with-helm). - From 5535e1851d1f35dbf482aea1ab953b6fcef86d76 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 18 Jun 2020 15:52:23 -0700 Subject: [PATCH 03/17] Apply suggestions from code review Co-authored-by: Michael Pleshakov --- .../virtualserver-and-virtualserverroute-resources.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 0b392e088e..8d0871a162 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -96,7 +96,7 @@ spec: - `[]route <#virtualserver-route>`_ - No * - ``class`` - - Specifies which Ingress controller must handle the VirtualServer resource. Set to `nginx` to make NGINX Ingress controller handle it. + - Specifies which Ingress controller must handle the VirtualServer resource. - ``string`` - No ``` @@ -278,7 +278,7 @@ Note that each subroute must have a `path` that starts with the same prefix (her - `[]subroute <#virtualserverroute-subroute>`_ - No * - ``class`` - - Specifies which Ingress controller must handle the VirtualServerRoute resource. Set to `nginx` to make NGINX Ingress controller handle it. + - Specifies which Ingress controller must handle the VirtualServerRoute resource. Must be the same as the ``class`` of the VirtualServer that references this resource. - ``string``_ - No ``` From 0c378af4ab4a4a389742de47013859846e3eae71 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 22 Jun 2020 16:27:02 -0700 Subject: [PATCH 04/17] Replace IsNginxIngress with HasCorrectIngressClass --- internal/k8s/controller.go | 16 ++-- internal/k8s/controller_test.go | 146 ++++++++++++++++---------------- internal/k8s/handlers.go | 18 ++-- 3 files changed, 92 insertions(+), 88 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 8d52eca435..df89f00ec4 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -442,7 +442,7 @@ func (lbc *LoadBalancerController) syncEndpoint(task task) { var mergableIngressesSlice []*configs.MergeableIngresses for i := range ings { - if !lbc.IsNginxIngress(&ings[i]) { + if !lbc.HasCorrectIngressClass(&ings[i]) { continue } if isMinion(&ings[i]) { @@ -647,7 +647,7 @@ func (lbc *LoadBalancerController) GetManagedIngresses() ([]extensions.Ingress, ings, _ := lbc.ingressLister.List() for i := range ings.Items { ing := ings.Items[i] - if !lbc.IsNginxIngress(&ing) { + if !lbc.HasCorrectIngressClass(&ing) { continue } if isMinion(&ing) { @@ -1459,7 +1459,7 @@ items: continue } - if !lbc.IsNginxIngress(&ing) { + if !lbc.HasCorrectIngressClass(&ing) { continue } @@ -1511,7 +1511,7 @@ items: func (lbc *LoadBalancerController) EnqueueIngressForService(svc *api_v1.Service) { ings := lbc.getIngressesForService(svc) for _, ing := range ings { - if !lbc.IsNginxIngress(&ing) { + if !lbc.HasCorrectIngressClass(&ing) { continue } if isMinion(&ing) { @@ -2370,9 +2370,9 @@ func (lbc *LoadBalancerController) getServiceForIngressBackend(backend *extensio return nil, fmt.Errorf("service %s doesn't exist", svcKey) } -// IsNginxIngress checks if resource ingress class annotation (if exists) is matching with ingress controller class +// HasCorrectIngressClass checks if resource ingress class annotation (if exists) is matching with ingress controller class // If annotation is absent and use-ingress-class-only enabled - ingress resource would ignore -func (lbc *LoadBalancerController) IsNginxIngress(obj interface{}) bool { +func (lbc *LoadBalancerController) HasCorrectIngressClass(obj interface{}) bool { var class string switch obj.(type) { case string: @@ -2436,7 +2436,7 @@ func (lbc *LoadBalancerController) getMinionsForMaster(master *configs.IngressEx var minionPaths = make(map[string]*extensions.Ingress) for i := range ings.Items { - if !lbc.IsNginxIngress(&ings.Items[i]) { + if !lbc.HasCorrectIngressClass(&ings.Items[i]) { continue } if !isMinion(&ings.Items[i]) { @@ -2490,7 +2490,7 @@ func (lbc *LoadBalancerController) FindMasterForMinion(minion *extensions.Ingres } for i := range ings.Items { - if !lbc.IsNginxIngress(&ings.Items[i]) { + if !lbc.HasCorrectIngressClass(&ings.Items[i]) { continue } if !lbc.configurator.HasIngress(&ings.Items[i]) { diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index e720dd3b99..a722e62f31 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -23,7 +23,7 @@ import ( "k8s.io/client-go/tools/cache" ) -func TestIsNginxIngress(t *testing.T) { +func TestHasCorrectIngressClass(t *testing.T) { ingressClass := "ing-ctrl" var testsWithoutIngressClassOnly = []struct { @@ -85,58 +85,95 @@ func TestIsNginxIngress(t *testing.T) { }, } - var testsWithoutIngressClassOnlyVS = []struct { + var testsWithIngressClassOnly = []struct { lbc *LoadBalancerController - ing *conf_v1.VirtualServerSpec + ing *extensions.Ingress expected bool }{ { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: false, + useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &conf_v1.VirtualServerSpec{ - IngressClass: "", + &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ingressClassKey: ""}, + }, }, - true, + false, }, { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: false, + useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &conf_v1.VirtualServerSpec{ - IngressClass: "gce", + &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ingressClassKey: "gce"}, + }, }, false, }, { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: false, + useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &conf_v1.VirtualServerSpec{ - IngressClass: ingressClass, + &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ingressClassKey: ingressClass}, + }, }, true, }, { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: false, + useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &conf_v1.VirtualServerSpec{}, - true, + &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + false, }, } - var testsWithIngressClassOnly = []struct { + for _, test := range testsWithoutIngressClassOnly { + if result := test.lbc.HasCorrectIngressClass(test.ing); result != test.expected { + classAnnotation := "N/A" + if class, exists := test.ing.Annotations[ingressClassKey]; exists { + classAnnotation = class + } + t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ing.Annotations['%v']=%v; got %v, expected %v", + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, classAnnotation, result, test.expected) + } + } + + for _, test := range testsWithIngressClassOnly { + if result := test.lbc.HasCorrectIngressClass(test.ing); result != test.expected { + classAnnotation := "N/A" + if class, exists := test.ing.Annotations[ingressClassKey]; exists { + classAnnotation = class + } + t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ing.Annotations['%v']=%v; got %v, expected %v", + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, classAnnotation, result, test.expected) + } + } + +} + +func TestHasCorrectIngressClassVS(t *testing.T) { + ingressClass := "ing-ctrl" + + var testsWithIngressClassOnlyVS = []struct { lbc *LoadBalancerController - ing *extensions.Ingress + ing *conf_v1.VirtualServerSpec expected bool }{ { @@ -145,10 +182,8 @@ func TestIsNginxIngress(t *testing.T) { useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &extensions.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Annotations: map[string]string{ingressClassKey: ""}, - }, + &conf_v1.VirtualServerSpec{ + IngressClass: "", }, false, }, @@ -158,10 +193,8 @@ func TestIsNginxIngress(t *testing.T) { useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &extensions.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Annotations: map[string]string{ingressClassKey: "gce"}, - }, + &conf_v1.VirtualServerSpec{ + IngressClass: "gce", }, false, }, @@ -171,10 +204,8 @@ func TestIsNginxIngress(t *testing.T) { useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &extensions.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Annotations: map[string]string{ingressClassKey: ingressClass}, - }, + &conf_v1.VirtualServerSpec{ + IngressClass: ingressClass, }, true, }, @@ -184,16 +215,11 @@ func TestIsNginxIngress(t *testing.T) { useIngressClassOnly: true, metricsCollector: collectors.NewControllerFakeCollector(), }, - &extensions.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Annotations: map[string]string{}, - }, - }, + &conf_v1.VirtualServerSpec{}, false, }, } - - var testsWithIngressClassOnlyVS = []struct { + var testsWithoutIngressClassOnlyVS = []struct { lbc *LoadBalancerController ing *conf_v1.VirtualServerSpec expected bool @@ -201,18 +227,18 @@ func TestIsNginxIngress(t *testing.T) { { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: true, + useIngressClassOnly: false, metricsCollector: collectors.NewControllerFakeCollector(), }, &conf_v1.VirtualServerSpec{ IngressClass: "", }, - false, + true, }, { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: true, + useIngressClassOnly: false, metricsCollector: collectors.NewControllerFakeCollector(), }, &conf_v1.VirtualServerSpec{ @@ -223,7 +249,7 @@ func TestIsNginxIngress(t *testing.T) { { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: true, + useIngressClassOnly: false, metricsCollector: collectors.NewControllerFakeCollector(), }, &conf_v1.VirtualServerSpec{ @@ -234,49 +260,27 @@ func TestIsNginxIngress(t *testing.T) { { &LoadBalancerController{ ingressClass: ingressClass, - useIngressClassOnly: true, + useIngressClassOnly: false, metricsCollector: collectors.NewControllerFakeCollector(), }, &conf_v1.VirtualServerSpec{}, - false, + true, }, } - for _, test := range testsWithoutIngressClassOnly { - if result := test.lbc.IsNginxIngress(test.ing); result != test.expected { - classAnnotation := "N/A" - if class, exists := test.ing.Annotations[ingressClassKey]; exists { - classAnnotation = class - } - t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ing.Annotations['%v']=%v; got %v, expected %v", - test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, classAnnotation, result, test.expected) - } - } - for _, test := range testsWithoutIngressClassOnlyVS { - if result := test.lbc.IsNginxIngress(test.ing.IngressClass); result != test.expected { - t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", + for _, test := range testsWithIngressClassOnlyVS { + if result := test.lbc.HasCorrectIngressClass(test.ing.IngressClass); result != test.expected { + t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) } } - for _, test := range testsWithIngressClassOnly { - if result := test.lbc.IsNginxIngress(test.ing); result != test.expected { - classAnnotation := "N/A" - if class, exists := test.ing.Annotations[ingressClassKey]; exists { - classAnnotation = class - } - t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ing.Annotations['%v']=%v; got %v, expected %v", - test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, classAnnotation, result, test.expected) - } - } - - for _, test := range testsWithIngressClassOnlyVS { - if result := test.lbc.IsNginxIngress(test.ing.IngressClass); result != test.expected { - t.Errorf("lbc.IsNginxIngress(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", + for _, test := range testsWithoutIngressClassOnlyVS { + if result := test.lbc.HasCorrectIngressClass(test.ing.IngressClass); result != test.expected { + t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) } } - } func TestCreateMergableIngresses(t *testing.T) { diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 709ac404c0..13ac366766 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -93,7 +93,7 @@ func createIngressHandlers(lbc *LoadBalancerController) cache.ResourceEventHandl return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { ingress := obj.(*v1beta1.Ingress) - if !lbc.IsNginxIngress(ingress) { + if !lbc.HasCorrectIngressClass(ingress) { glog.Infof("Ignoring Ingress %v based on Annotation %v", ingress.Name, ingressClassKey) return } @@ -114,7 +114,7 @@ func createIngressHandlers(lbc *LoadBalancerController) cache.ResourceEventHandl return } } - if !lbc.IsNginxIngress(ingress) { + if !lbc.HasCorrectIngressClass(ingress) { return } if isMinion(ingress) { @@ -133,7 +133,7 @@ func createIngressHandlers(lbc *LoadBalancerController) cache.ResourceEventHandl UpdateFunc: func(old, current interface{}) { c := current.(*v1beta1.Ingress) o := old.(*v1beta1.Ingress) - if !lbc.IsNginxIngress(c) { + if !lbc.HasCorrectIngressClass(c) { return } if hasChanges(o, c) { @@ -316,7 +316,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vs := obj.(*conf_v1.VirtualServer) - if !lbc.IsNginxIngress(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) return } @@ -337,7 +337,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return } } - if !lbc.IsNginxIngress(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) return } @@ -347,7 +347,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven UpdateFunc: func(old, cur interface{}) { curVs := cur.(*conf_v1.VirtualServer) oldVs := old.(*conf_v1.VirtualServer) - if !lbc.IsNginxIngress(curVs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(curVs.Spec.IngressClass) { glog.Infof("Ignoring VS %v based on class %v", curVs.Name, curVs.Spec.IngressClass) return } @@ -363,7 +363,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vsr := obj.(*conf_v1.VirtualServerRoute) - if !lbc.IsNginxIngress(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } @@ -384,7 +384,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return } } - if !lbc.IsNginxIngress(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } @@ -394,7 +394,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc UpdateFunc: func(old, cur interface{}) { curVsr := cur.(*conf_v1.VirtualServerRoute) oldVsr := old.(*conf_v1.VirtualServerRoute) - if !lbc.IsNginxIngress(curVsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(curVsr.Spec.IngressClass) { glog.Infof("Ignoring VSR %v based on class %v", curVsr.Name, curVsr.Spec.IngressClass) return } From e797941ede47ffa5e3cf7bb4eb1e4ed8af93e136 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 22 Jun 2020 17:23:01 -0700 Subject: [PATCH 05/17] fix comment --- internal/k8s/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index df89f00ec4..16b4b9aa04 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2370,7 +2370,7 @@ func (lbc *LoadBalancerController) getServiceForIngressBackend(backend *extensio return nil, fmt.Errorf("service %s doesn't exist", svcKey) } -// HasCorrectIngressClass checks if resource ingress class annotation (if exists) is matching with ingress controller class +// HasCorrectIngressClass checks if resource ingress class annotation (if exists) or ingressClass string for VS/VSR is matching with ingress controller class // If annotation is absent and use-ingress-class-only enabled - ingress resource would ignore func (lbc *LoadBalancerController) HasCorrectIngressClass(obj interface{}) bool { var class string From 3f39a8fc43b0306dcb0ff7760e524b7d7af8adca Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 22 Jun 2020 17:29:20 -0700 Subject: [PATCH 06/17] rename to ingressClassName --- deployments/common/vs-definition.yaml | 2 +- deployments/common/vsr-definition.yaml | 2 +- .../helm-chart/templates/controller-vs-definition.yaml | 2 +- .../helm-chart/templates/controller-vsr-definition.yaml | 2 +- .../virtualserver-and-virtualserverroute-resources.md | 6 +++--- pkg/apis/configuration/v1/types.go | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml index 537781b497..d4ca2c374d 100644 --- a/deployments/common/vs-definition.yaml +++ b/deployments/common/vs-definition.yaml @@ -57,7 +57,7 @@ spec: description: VirtualServerSpec is the spec of the VirtualServer resource. type: object properties: - class: + ingressClassName: type: string host: type: string diff --git a/deployments/common/vsr-definition.yaml b/deployments/common/vsr-definition.yaml index 8237f7531f..d28bc215d6 100644 --- a/deployments/common/vsr-definition.yaml +++ b/deployments/common/vsr-definition.yaml @@ -55,7 +55,7 @@ spec: spec: type: object properties: - class: + ingressClassName: type: string host: type: string diff --git a/deployments/helm-chart/templates/controller-vs-definition.yaml b/deployments/helm-chart/templates/controller-vs-definition.yaml index 3647df425f..de041cad3e 100644 --- a/deployments/helm-chart/templates/controller-vs-definition.yaml +++ b/deployments/helm-chart/templates/controller-vs-definition.yaml @@ -60,7 +60,7 @@ spec: description: VirtualServerSpec is the spec of the VirtualServer resource. type: object properties: - class: + ingressClassName: type: string host: type: string diff --git a/deployments/helm-chart/templates/controller-vsr-definition.yaml b/deployments/helm-chart/templates/controller-vsr-definition.yaml index ac2eac9839..2de1e3c4c2 100644 --- a/deployments/helm-chart/templates/controller-vsr-definition.yaml +++ b/deployments/helm-chart/templates/controller-vsr-definition.yaml @@ -58,7 +58,7 @@ spec: spec: type: object properties: - class: + ingressClassName: type: string host: type: string diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 8d0871a162..c8d1d15727 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -95,7 +95,7 @@ spec: - A list of routes. - `[]route <#virtualserver-route>`_ - No - * - ``class`` + * - ``ingressClassName`` - Specifies which Ingress controller must handle the VirtualServer resource. - ``string`` - No @@ -277,8 +277,8 @@ Note that each subroute must have a `path` that starts with the same prefix (her - A list of subroutes. - `[]subroute <#virtualserverroute-subroute>`_ - No - * - ``class`` - - Specifies which Ingress controller must handle the VirtualServerRoute resource. Must be the same as the ``class`` of the VirtualServer that references this resource. + * - ``ingressClassName`` + - Specifies which Ingress controller must handle the VirtualServerRoute resource. Must be the same as the ``ingressClassName`` of the VirtualServer that references this resource. - ``string``_ - No ``` diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index f59c3158a0..6ce20dbfbe 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -28,7 +28,7 @@ type VirtualServer struct { // VirtualServerSpec is the spec of the VirtualServer resource. type VirtualServerSpec struct { - IngressClass string `json:"class"` + IngressClass string `json:"ingressClassName"` Host string `json:"host"` TLS *TLS `json:"tls"` Upstreams []Upstream `json:"upstreams"` @@ -227,7 +227,7 @@ type VirtualServerRoute struct { } type VirtualServerRouteSpec struct { - IngressClass string `json:"class"` + IngressClass string `json:"ingressClassName"` Host string `json:"host"` Upstreams []Upstream `json:"upstreams"` Subroutes []Route `json:"subroutes"` From c835df2baa0c37acdc67ec7f4d76422f89f8c734 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 22 Jun 2020 18:04:16 -0700 Subject: [PATCH 07/17] Add more checks --- internal/k8s/controller.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 16b4b9aa04..e7ea3094de 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -886,6 +886,11 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vs := obj.(*conf_v1.VirtualServer) + if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + glog.V(2).Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) + return + } + validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) if validationErr != nil { err := lbc.configurator.DeleteVirtualServer(key) @@ -1000,6 +1005,11 @@ func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { vsr := obj.(*conf_v1.VirtualServerRoute) + if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + glog.V(2).Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + return + } + validationErr := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus) if validationErr != nil { reason := "Rejected" @@ -1739,6 +1749,11 @@ func (lbc *LoadBalancerController) getVirtualServers() []*conf_v1.VirtualServer for _, obj := range lbc.virtualServerLister.List() { vs := obj.(*conf_v1.VirtualServer) + if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + glog.V(3).Infof("Ignoring VSR %v based on class %v", vs.Name, vs.Spec.IngressClass) + continue + } + err := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) if err != nil { glog.V(3).Infof("Skipping invalid VirtualServer %s/%s: %v", vs.Namespace, vs.Name, err) @@ -1757,6 +1772,11 @@ func (lbc *LoadBalancerController) getVirtualServerRoutes() []*conf_v1.VirtualSe for _, obj := range lbc.virtualServerRouteLister.List() { vsr := obj.(*conf_v1.VirtualServerRoute) + if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + glog.V(3).Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + continue + } + err := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus) if err != nil { glog.V(3).Infof("Skipping invalid VirtualServerRoute %s/%s: %v", vsr.Namespace, vsr.Name, err) From 7ac9501c7a1fe357c3454f69355a6e4c0e9a1854 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 22 Jun 2020 18:12:38 -0700 Subject: [PATCH 08/17] Update docs --- .../running-multiple-ingress-controllers.md | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs-web/installation/running-multiple-ingress-controllers.md b/docs-web/installation/running-multiple-ingress-controllers.md index 7f5d7bf57c..f6a84c3329 100644 --- a/docs-web/installation/running-multiple-ingress-controllers.md +++ b/docs-web/installation/running-multiple-ingress-controllers.md @@ -5,34 +5,36 @@ This document explains the following topics: * How to run NGINX Ingress Controller in the same cluster with another Ingress Controller, such as an Ingress Controller for a cloud HTTP load balancer, and prevent any conflicts between the Ingress Controllers. * How to run multiple NGINX Ingress Controllers. +**Note**: In this document we refer to Ingress, VirtualServer and VirtualServerRoute resources as configuration resources. + ## Ingress Class The smooth coexistence of multiple Ingress Controllers in one cluster is provided by the Ingress class concept, which mandates the following: * Every Ingress Controller must only handle Ingress resources for its particular class. * Ingress resources should be annotated with the `kubernetes.io/ingress.class` annotation set to the value, which corresponds to the class of the Ingress Controller the user wants to use. -* VirtualServer and VirtualServerRoute resources should have the `class` field set to the value, which corresponds to the class of the Ingress Controller the user wants to use. +* VirtualServer and VirtualServerRoute resources should have the `ingressClassName` field set to the value, which corresponds to the class of the Ingress Controller the user wants to use. ### Configuring Ingress Class -The default Ingress class of NGINX Ingress Controller is `nginx`, which means that it only handles Ingress resources with the `kubernetes.io/ingress.class` annotation set to `nginx`. You can customize the class through the `-ingress-class` command-line argument. +The default Ingress class of NGINX Ingress Controller is `nginx`, which means that it only handles configuration resources with the `class` set to `nginx`. You can customize the class through the `-ingress-class` command-line argument. -**Note**: By default, if the `kubernetes.io/ingress.class` annotation is not set in an Ingress resource, the Ingress Controller will handle the resource. This is controlled via the `-use-ingress-class-only` argument. +**Note**: By default, if the `class` is not set in a configuration resource, the Ingress Controller will handle the resource. This is controlled via the `-use-ingress-class-only` argument. ## Running NGINX Ingress Controller and Another Ingress Controller It is possible to run NGINX Ingress Controller and an Ingress Controller for another load balancer in the same cluster. This is often the case if you create your cluster through a cloud provider managed Kubernetes service that by default might include the Ingress Controller for the HTTP load balancer of the cloud provider, and you want to use NGINX Ingress Controller. -To make sure that NGINX Ingress Controller handles particular Ingress resources, annotate those Ingress resources with the `kubernetes.io/ingress.class` set to `nginx` or the value that you configured. +To make sure that NGINX Ingress Controller handles particular configuration resources, update those resources with the `class` set to `nginx` or the value that you configured. ## Running Multiple NGINX Ingress Controllers -When running NGINX Ingress Controller, you have the following options with regards to which Ingress resources it handles: -* **Cluster-wide Ingress Controller (default)**. The Ingress Controller handles Ingress resources created in any namespace of the cluster. As NGINX is a high-performance load balancer capable of serving many applications at the same time, this option is used by default in our installation manifests and Helm chart. -* **Single-namespace Ingress Controller**. You can configure the Ingress Controller to handle Ingress resources only from a particular namespace, which is controlled through the `-watch-namespace` command-line argument. This can be useful if you want to use different NGINX Ingress Controllers for different applications, both in terms of isolation and/or operation. -* **Ingress Controller for Specific Ingress Class**. This option works in conjunction with either of the options above. You can further customize which Ingress resources are handled by the Ingress Controller by configuring the class of the Ingress Controller and using that class in your Ingress resources. See the section [Configuring Ingress Class](#configuring-ingress-class). +When running NGINX Ingress Controller, you have the following options with regards to which configuration resources it handles: +* **Cluster-wide Ingress Controller (default)**. The Ingress Controller handles configuration resources created in any namespace of the cluster. As NGINX is a high-performance load balancer capable of serving many applications at the same time, this option is used by default in our installation manifests and Helm chart. +* **Single-namespace Ingress Controller**. You can configure the Ingress Controller to handle configuration resources only from a particular namespace, which is controlled through the `-watch-namespace` command-line argument. This can be useful if you want to use different NGINX Ingress Controllers for different applications, both in terms of isolation and/or operation. +* **Ingress Controller for Specific Ingress Class**. This option works in conjunction with either of the options above. You can further customize which configuration resources are handled by the Ingress Controller by configuring the class of the Ingress Controller and using that class in your configuration resources. See the section [Configuring Ingress Class](#configuring-ingress-class). -Considering the options above, you can run multiple NGINX Ingress Controllers, each handling a different set of Ingress resources. +Considering the options above, you can run multiple NGINX Ingress Controllers, each handling a different set of configuration resources. ## See Also From 12154c376f6e2c94c546fae6dc65b0efb2b82bb7 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 23 Jun 2020 10:04:21 -0700 Subject: [PATCH 09/17] Update crds --- deployments/common/vs-definition.yaml | 4 ++-- deployments/common/vsr-definition.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml index d4ca2c374d..a43161dc73 100644 --- a/deployments/common/vs-definition.yaml +++ b/deployments/common/vs-definition.yaml @@ -57,10 +57,10 @@ spec: description: VirtualServerSpec is the spec of the VirtualServer resource. type: object properties: - ingressClassName: - type: string host: type: string + ingressClassName: + type: string routes: type: array items: diff --git a/deployments/common/vsr-definition.yaml b/deployments/common/vsr-definition.yaml index d28bc215d6..3a6ba1ac78 100644 --- a/deployments/common/vsr-definition.yaml +++ b/deployments/common/vsr-definition.yaml @@ -55,10 +55,10 @@ spec: spec: type: object properties: - ingressClassName: - type: string host: type: string + ingressClassName: + type: string subroutes: type: array items: From e6fce1a7390b88b8f82d9d28fe910ddfdd938fcb Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 23 Jun 2020 15:22:27 -0700 Subject: [PATCH 10/17] Apply suggestions from code review Co-authored-by: Michael Pleshakov --- internal/k8s/controller.go | 4 ++-- internal/k8s/handlers.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index e7ea3094de..9d19758973 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1750,7 +1750,7 @@ func (lbc *LoadBalancerController) getVirtualServers() []*conf_v1.VirtualServer vs := obj.(*conf_v1.VirtualServer) if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { - glog.V(3).Infof("Ignoring VSR %v based on class %v", vs.Name, vs.Spec.IngressClass) + glog.V(3).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) continue } @@ -1773,7 +1773,7 @@ func (lbc *LoadBalancerController) getVirtualServerRoutes() []*conf_v1.VirtualSe vsr := obj.(*conf_v1.VirtualServerRoute) if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { - glog.V(3).Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) continue } diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 13ac366766..9b9d913849 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -317,7 +317,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven AddFunc: func(obj interface{}) { vs := obj.(*conf_v1.VirtualServer) if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { - glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) + glog.Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) return } glog.V(3).Infof("Adding VirtualServer: %v", vs.Name) From 90bdc818737bd7aabe968cceda3a674815707de8 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Wed, 24 Jun 2020 15:10:40 -0700 Subject: [PATCH 11/17] Apply suggestions from code review --- internal/k8s/controller.go | 21 +++--- internal/k8s/controller_test.go | 111 +++++++++++++++----------------- internal/k8s/handlers.go | 12 ++-- 3 files changed, 68 insertions(+), 76 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 9d19758973..ce057fc476 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -886,7 +886,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vs := obj.(*conf_v1.VirtualServer) - if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs) { glog.V(2).Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) return } @@ -1005,7 +1005,7 @@ func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { vsr := obj.(*conf_v1.VirtualServerRoute) - if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr) { glog.V(2).Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } @@ -1749,7 +1749,7 @@ func (lbc *LoadBalancerController) getVirtualServers() []*conf_v1.VirtualServer for _, obj := range lbc.virtualServerLister.List() { vs := obj.(*conf_v1.VirtualServer) - if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs) { glog.V(3).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) continue } @@ -1772,7 +1772,7 @@ func (lbc *LoadBalancerController) getVirtualServerRoutes() []*conf_v1.VirtualSe for _, obj := range lbc.virtualServerRouteLister.List() { vsr := obj.(*conf_v1.VirtualServerRoute) - if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr) { glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) continue } @@ -2391,17 +2391,18 @@ func (lbc *LoadBalancerController) getServiceForIngressBackend(backend *extensio } // HasCorrectIngressClass checks if resource ingress class annotation (if exists) or ingressClass string for VS/VSR is matching with ingress controller class -// If annotation is absent and use-ingress-class-only enabled - ingress resource would ignore func (lbc *LoadBalancerController) HasCorrectIngressClass(obj interface{}) bool { var class string switch obj.(type) { - case string: - class = obj.(string) + case *conf_v1.VirtualServer: + vs := obj.(*conf_v1.VirtualServer) + class = vs.Spec.IngressClass + case *conf_v1.VirtualServerRoute: + vsr := obj.(*conf_v1.VirtualServerRoute) + class = vsr.Spec.IngressClass case *extensions.Ingress: ing := obj.(*extensions.Ingress) - if ingClass, exists := ing.Annotations[ingressClassKey]; exists { - class = ingClass - } + class = ing.Annotations[ingressClassKey] default: return false } diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index a722e62f31..f60fd6b8fb 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -170,115 +170,106 @@ func TestHasCorrectIngressClass(t *testing.T) { func TestHasCorrectIngressClassVS(t *testing.T) { ingressClass := "ing-ctrl" + lbcIngOnlyTrue := &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: true, + metricsCollector: collectors.NewControllerFakeCollector(), + } var testsWithIngressClassOnlyVS = []struct { lbc *LoadBalancerController - ing *conf_v1.VirtualServerSpec + ing *conf_v1.VirtualServer expected bool }{ { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: true, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: "", + lbcIngOnlyTrue, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: "", + }, }, false, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: true, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: "gce", + lbcIngOnlyTrue, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: "gce", + }, }, false, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: true, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: ingressClass, + lbcIngOnlyTrue, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: ingressClass, + }, }, true, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: true, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{}, + lbcIngOnlyTrue, + &conf_v1.VirtualServer{}, false, }, } + + lbcIngOnlyFalse := &LoadBalancerController{ + ingressClass: ingressClass, + useIngressClassOnly: false, + metricsCollector: collectors.NewControllerFakeCollector(), + } var testsWithoutIngressClassOnlyVS = []struct { lbc *LoadBalancerController - ing *conf_v1.VirtualServerSpec + ing *conf_v1.VirtualServer expected bool }{ { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: false, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: "", + lbcIngOnlyFalse, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: "", + }, }, true, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: false, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: "gce", + lbcIngOnlyFalse, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: "gce", + }, }, false, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: false, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{ - IngressClass: ingressClass, + lbcIngOnlyFalse, + &conf_v1.VirtualServer{ + Spec: conf_v1.VirtualServerSpec{ + IngressClass: ingressClass, + }, }, true, }, { - &LoadBalancerController{ - ingressClass: ingressClass, - useIngressClassOnly: false, - metricsCollector: collectors.NewControllerFakeCollector(), - }, - &conf_v1.VirtualServerSpec{}, + lbcIngOnlyFalse, + &conf_v1.VirtualServer{}, true, }, } for _, test := range testsWithIngressClassOnlyVS { - if result := test.lbc.HasCorrectIngressClass(test.ing.IngressClass); result != test.expected { + if result := test.lbc.HasCorrectIngressClass(test.ing); result != test.expected { t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", - test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.Spec.IngressClass, result, test.expected) } } for _, test := range testsWithoutIngressClassOnlyVS { - if result := test.lbc.HasCorrectIngressClass(test.ing.IngressClass); result != test.expected { + if result := test.lbc.HasCorrectIngressClass(test.ing); result != test.expected { t.Errorf("lbc.HasCorrectIngressClass(ing), lbc.ingressClass=%v, lbc.useIngressClassOnly=%v, ingressClassKey=%v, ing.IngressClass=%v; got %v, expected %v", - test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.IngressClass, result, test.expected) + test.lbc.ingressClass, test.lbc.useIngressClassOnly, ingressClassKey, test.ing.Spec.IngressClass, result, test.expected) } } } diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index 9b9d913849..a5c5f192d7 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -316,7 +316,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vs := obj.(*conf_v1.VirtualServer) - if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs) { glog.Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) return } @@ -337,7 +337,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven return } } - if !lbc.HasCorrectIngressClass(vs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vs) { glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) return } @@ -347,7 +347,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven UpdateFunc: func(old, cur interface{}) { curVs := cur.(*conf_v1.VirtualServer) oldVs := old.(*conf_v1.VirtualServer) - if !lbc.HasCorrectIngressClass(curVs.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(curVs) { glog.Infof("Ignoring VS %v based on class %v", curVs.Name, curVs.Spec.IngressClass) return } @@ -363,7 +363,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { vsr := obj.(*conf_v1.VirtualServerRoute) - if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr) { glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } @@ -384,7 +384,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc return } } - if !lbc.HasCorrectIngressClass(vsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(vsr) { glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } @@ -394,7 +394,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc UpdateFunc: func(old, cur interface{}) { curVsr := cur.(*conf_v1.VirtualServerRoute) oldVsr := old.(*conf_v1.VirtualServerRoute) - if !lbc.HasCorrectIngressClass(curVsr.Spec.IngressClass) { + if !lbc.HasCorrectIngressClass(curVsr) { glog.Infof("Ignoring VSR %v based on class %v", curVsr.Name, curVsr.Spec.IngressClass) return } From 56e550a36647be3ddb81400950b000c00e2c8fdd Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Wed, 24 Jun 2020 16:10:03 -0700 Subject: [PATCH 12/17] Add check in createVirtualServer --- internal/k8s/controller.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index ce057fc476..878dddb7bd 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -886,11 +886,6 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vs := obj.(*conf_v1.VirtualServer) - if !lbc.HasCorrectIngressClass(vs) { - glog.V(2).Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) - return - } - validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) if validationErr != nil { err := lbc.configurator.DeleteVirtualServer(key) @@ -1005,11 +1000,6 @@ func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { vsr := obj.(*conf_v1.VirtualServerRoute) - if !lbc.HasCorrectIngressClass(vsr) { - glog.V(2).Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) - return - } - validationErr := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus) if validationErr != nil { reason := "Rejected" @@ -2001,6 +1991,10 @@ func newVirtualServerRouteErrorFromVSR(virtualServerRoute *conf_v1.VirtualServer } func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.VirtualServer) (*configs.VirtualServerEx, []virtualServerRouteError) { + if !lbc.HasCorrectIngressClass(virtualServer) { + glog.V(2).Infof("Ignoring VirtualServer %v based on class %v", virtualServer.Name, virtualServer.Spec.IngressClass) + return &configs.VirtualServerEx{}, nil + } virtualServerEx := configs.VirtualServerEx{ VirtualServer: virtualServer, } From 3be93b1be39baaa48936ebab93a03156f6f6b15c Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 25 Jun 2020 16:51:40 -0700 Subject: [PATCH 13/17] Apply suggestions from code review --- internal/k8s/controller.go | 14 ++++++++++---- internal/k8s/handlers.go | 10 +++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index cbbba26003..e61332b996 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1418,6 +1418,11 @@ func (lbc *LoadBalancerController) updateVirtualServersStatusFromEvents() error for _, obj := range lbc.virtualServerLister.List() { vs := obj.(*conf_v1.VirtualServer) + if !lbc.HasCorrectIngressClass(vs) { + glog.V(2).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) + continue + } + events, err := lbc.client.CoreV1().Events(vs.Namespace).List(context.TODO(), meta_v1.ListOptions{FieldSelector: fmt.Sprintf("involvedObject.name=%v,involvedObject.uid=%v", vs.Name, vs.UID)}) if err != nil { @@ -2073,10 +2078,6 @@ func newVirtualServerRouteErrorFromVSR(virtualServerRoute *conf_v1.VirtualServer } func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.VirtualServer) (*configs.VirtualServerEx, []virtualServerRouteError) { - if !lbc.HasCorrectIngressClass(virtualServer) { - glog.V(2).Infof("Ignoring VirtualServer %v based on class %v", virtualServer.Name, virtualServer.Spec.IngressClass) - return &configs.VirtualServerEx{}, nil - } virtualServerEx := configs.VirtualServerEx{ VirtualServer: virtualServer, } @@ -2149,6 +2150,11 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi vsr := obj.(*conf_v1.VirtualServerRoute) + if !lbc.HasCorrectIngressClass(vsr) { + glog.V(2).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + continue + } + err = validation.ValidateVirtualServerRouteForVirtualServer(vsr, virtualServer.Spec.Host, r.Path, lbc.isNginxPlus) if err != nil { glog.Warningf("VirtualServer %s/%s references invalid VirtualServerRoute %s: %v", virtualServer.Name, virtualServer.Namespace, vsrKey, err) diff --git a/internal/k8s/handlers.go b/internal/k8s/handlers.go index a5c5f192d7..d722583635 100644 --- a/internal/k8s/handlers.go +++ b/internal/k8s/handlers.go @@ -338,7 +338,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven } } if !lbc.HasCorrectIngressClass(vs) { - glog.Infof("Ignoring VS %v based on class %v", vs.Name, vs.Spec.IngressClass) + glog.Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) return } glog.V(3).Infof("Removing VirtualServer: %v", vs.Name) @@ -348,7 +348,7 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven curVs := cur.(*conf_v1.VirtualServer) oldVs := old.(*conf_v1.VirtualServer) if !lbc.HasCorrectIngressClass(curVs) { - glog.Infof("Ignoring VS %v based on class %v", curVs.Name, curVs.Spec.IngressClass) + glog.Infof("Ignoring VirtualServer %v based on class %v", curVs.Name, curVs.Spec.IngressClass) return } if !reflect.DeepEqual(oldVs.Spec, curVs.Spec) { @@ -364,7 +364,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc AddFunc: func(obj interface{}) { vsr := obj.(*conf_v1.VirtualServerRoute) if !lbc.HasCorrectIngressClass(vsr) { - glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + glog.Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } glog.V(3).Infof("Adding VirtualServerRoute: %v", vsr.Name) @@ -385,7 +385,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc } } if !lbc.HasCorrectIngressClass(vsr) { - glog.Infof("Ignoring VSR %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + glog.Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) return } glog.V(3).Infof("Removing VirtualServerRoute: %v", vsr.Name) @@ -395,7 +395,7 @@ func createVirtualServerRouteHandlers(lbc *LoadBalancerController) cache.Resourc curVsr := cur.(*conf_v1.VirtualServerRoute) oldVsr := old.(*conf_v1.VirtualServerRoute) if !lbc.HasCorrectIngressClass(curVsr) { - glog.Infof("Ignoring VSR %v based on class %v", curVsr.Name, curVsr.Spec.IngressClass) + glog.Infof("Ignoring VirtualServerRoute %v based on class %v", curVsr.Name, curVsr.Spec.IngressClass) return } if !reflect.DeepEqual(oldVsr.Spec, curVsr.Spec) { From 864f0fae90e14f2dbd62be1901a31734fda560e1 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 25 Jun 2020 16:56:42 -0700 Subject: [PATCH 14/17] last one --- internal/k8s/controller.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index e61332b996..d7feccff31 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1419,7 +1419,7 @@ func (lbc *LoadBalancerController) updateVirtualServersStatusFromEvents() error vs := obj.(*conf_v1.VirtualServer) if !lbc.HasCorrectIngressClass(vs) { - glog.V(2).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) + glog.V(3).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass) continue } @@ -1455,6 +1455,11 @@ func (lbc *LoadBalancerController) updateVirtualServerRoutesStatusFromEvents() e for _, obj := range lbc.virtualServerRouteLister.List() { vsr := obj.(*conf_v1.VirtualServerRoute) + if !lbc.HasCorrectIngressClass(vsr) { + glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + continue + } + events, err := lbc.client.CoreV1().Events(vsr.Namespace).List(context.TODO(), meta_v1.ListOptions{FieldSelector: fmt.Sprintf("involvedObject.name=%v,involvedObject.uid=%v", vsr.Name, vsr.UID)}) if err != nil { @@ -2151,7 +2156,7 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi vsr := obj.(*conf_v1.VirtualServerRoute) if !lbc.HasCorrectIngressClass(vsr) { - glog.V(2).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) continue } From 7eb729cd9574b499405585679ddf41a2e083a7e4 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 25 Jun 2020 17:18:11 -0700 Subject: [PATCH 15/17] Make it a warning and report the error --- cmd/nginx-ingress/main.go | 2 +- internal/k8s/controller.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 41b5cb796b..81a22c9c19 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -69,7 +69,7 @@ var ( which can be disabled by setting the "-use-ingress-class-only" flag`) useIngressClassOnly = flag.Bool("use-ingress-class-only", false, - `Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation`) + `Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation or the "ingressClassName" field in VirtualServer/VirtualServerRoute`) defaultServerSecret = flag.String("default-server-tls-secret", "", `A Secret with a TLS certificate and key for TLS termination of the default server. Format: /. diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index d7feccff31..ccea911bb1 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2156,7 +2156,8 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi vsr := obj.(*conf_v1.VirtualServerRoute) if !lbc.HasCorrectIngressClass(vsr) { - glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + glog.Warningf("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass) + virtualServerRouteErrors = append(virtualServerRouteErrors, newVirtualServerRouteErrorFromVSR(vsr, errors.New("VirtualServerRoute with incorrect class name"))) continue } From fa14fad8ba3ffe573e34f247de1d5279d0822f43 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 25 Jun 2020 17:28:13 -0700 Subject: [PATCH 16/17] docs --- cmd/nginx-ingress/main.go | 2 +- deployments/helm-chart/README.md | 4 ++-- deployments/helm-chart/values.yaml | 4 ++-- .../global-configuration/command-line-arguments.md | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 81a22c9c19..ab76228686 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -64,7 +64,7 @@ var ( ingressClass = flag.String("ingress-class", "nginx", `A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - - i.e. have the annotation "kubernetes.io/ingress.class" equal to the class. Additionally, + - i.e. have the annotation "kubernetes.io/ingress.class" or the "ingressClassName" field in VirtualServer/VirtualServerRoute equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation, which can be disabled by setting the "-use-ingress-class-only" flag`) diff --git a/deployments/helm-chart/README.md b/deployments/helm-chart/README.md index c53ab31000..cf5a4d1712 100644 --- a/deployments/helm-chart/README.md +++ b/deployments/helm-chart/README.md @@ -142,8 +142,8 @@ Parameter | Description | Default `controller.volumeMounts` | The volumeMounts of the Ingress controller pods. | [] `controller.resources` | The resources of the Ingress controller pods. | {} `controller.replicaCount` | The number of replicas of the Ingress controller deployment. | 1 -`controller.ingressClass` | A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation `"kubernetes.io/ingress.class"` equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. | nginx -`controller.useIngressClassOnly` | Ignore Ingress resources without the `"kubernetes.io/ingress.class"` annotation. | false +`controller.ingressClass` | A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation `"kubernetes.io/ingress.class"` or the "ingressClassName" field in VirtualServer/VirtualServerRoute equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. | nginx +`controller.useIngressClassOnly` | Ignore Ingress resources without the `"kubernetes.io/ingress.class"` annotation or the "ingressClassName" field in VirtualServer/VirtualServerRoute. | false `controller.watchNamespace` | Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces. | "" `controller.enableCustomResources` | Enable the custom resources. | true `controller.enableTLSPassthrough` | Enable TLS Passthrough on port 443. Requires `controller.enableCustomResources`. | false diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index 15b16e5a68..d87f435485 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -105,11 +105,11 @@ controller: replicaCount: 1 ## A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - ## i.e. have the annotation "kubernetes.io/ingress.class" equal to the class. + ## i.e. have the annotation "kubernetes.io/ingress.class" or the "ingressClassName" field in VirtualServer/VirtualServerRoute equal to the class. ## Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. ingressClass: nginx - ## Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation. + ## Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation or the "ingressClassName" field in VirtualServer/VirtualServerRoute. useIngressClassOnly: false ## Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces. diff --git a/docs-web/configuration/global-configuration/command-line-arguments.md b/docs-web/configuration/global-configuration/command-line-arguments.md index 6c244e105d..7b9b688c91 100644 --- a/docs-web/configuration/global-configuration/command-line-arguments.md +++ b/docs-web/configuration/global-configuration/command-line-arguments.md @@ -62,7 +62,7 @@ Below we describe the available command-line arguments: .. option:: -ingress-class - A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class (in other words, have the annotation "kubernetes.io/ingress.class"). + A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class (i.e. have the annotation "kubernetes.io/ingress.class" or the "ingressClassName" field in VirtualServer/VirtualServerRoute"). Additionally, the Ingress controller processes Ingress resources that do not have that annotation, which can be disabled by setting the :option:`-use-ingress-class-only` flag (default "nginx"). .. option:: -ingress-template-path @@ -132,7 +132,7 @@ Below we describe the available command-line arguments: .. option:: -use-ingress-class-only - Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation. + Ignore Ingress resources without the "kubernetes.io/ingress.class" annotation or the "ingressClassName" field in VirtualServer/VirtualServerRoute. .. option:: -v From ae76bbe0963b2c8fa3eddb7a587cf798012dacb3 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Thu, 25 Jun 2020 17:33:59 -0700 Subject: [PATCH 17/17] more docs --- deployments/helm-chart/README.md | 4 ++-- docs-web/installation/installation-with-helm.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deployments/helm-chart/README.md b/deployments/helm-chart/README.md index cf5a4d1712..ae7f17b279 100644 --- a/deployments/helm-chart/README.md +++ b/deployments/helm-chart/README.md @@ -142,8 +142,8 @@ Parameter | Description | Default `controller.volumeMounts` | The volumeMounts of the Ingress controller pods. | [] `controller.resources` | The resources of the Ingress controller pods. | {} `controller.replicaCount` | The number of replicas of the Ingress controller deployment. | 1 -`controller.ingressClass` | A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation `"kubernetes.io/ingress.class"` or the "ingressClassName" field in VirtualServer/VirtualServerRoute equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. | nginx -`controller.useIngressClassOnly` | Ignore Ingress resources without the `"kubernetes.io/ingress.class"` annotation or the "ingressClassName" field in VirtualServer/VirtualServerRoute. | false +`controller.ingressClass` | A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation `"kubernetes.io/ingress.class"` or the `"ingressClassName"` field in VirtualServer/VirtualServerRoute equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. | nginx +`controller.useIngressClassOnly` | Ignore Ingress resources without the `"kubernetes.io/ingress.class"` annotation or the `"ingressClassName"` field in VirtualServer/VirtualServerRoute. | false `controller.watchNamespace` | Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces. | "" `controller.enableCustomResources` | Enable the custom resources. | true `controller.enableTLSPassthrough` | Enable TLS Passthrough on port 443. Requires `controller.enableCustomResources`. | false diff --git a/docs-web/installation/installation-with-helm.md b/docs-web/installation/installation-with-helm.md index f0cbf6d6d9..154f9672a8 100644 --- a/docs-web/installation/installation-with-helm.md +++ b/docs-web/installation/installation-with-helm.md @@ -197,10 +197,10 @@ The following tables lists the configurable parameters of the NGINX Ingress cont - The number of replicas of the Ingress controller deployment. - 1 * - ``controller.ingressClass`` - - A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation ``"kubernetes.io/ingress.class"`` equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. + - A class of the Ingress controller. The Ingress controller only processes Ingress resources that belong to its class - i.e. have the annotation ``"kubernetes.io/ingress.class"`` or the ``"ingressClassName"`` field in VirtualServer/VirtualServerRoute equal to the class. Additionally, the Ingress controller processes Ingress resources that do not have that annotation which can be disabled by setting the "-use-ingress-class-only" flag. - nginx * - ``controller.useIngressClassOnly`` - - Ignore Ingress resources without the ``"kubernetes.io/ingress.class"`` annotation. + - Ignore Ingress resources without the ``"kubernetes.io/ingress.class"`` annotation or the ``"ingressClassName"`` field in VirtualServer/VirtualServerRoute. - false * - ``controller.watchNamespace`` - Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces.