Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Class Field in VS/VSR #994

Merged
merged 18 commits into from
Jun 26, 2020
Merged

Support Class Field in VS/VSR #994

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7e44ed9
Support Class Field in VS/VSR
lucacome Jun 15, 2020
170abf5
Apply suggestions from code review
lucacome Jun 16, 2020
5535e18
Apply suggestions from code review
lucacome Jun 18, 2020
0c378af
Replace IsNginxIngress with HasCorrectIngressClass
lucacome Jun 22, 2020
e797941
fix comment
lucacome Jun 23, 2020
3f39a8f
rename to ingressClassName
lucacome Jun 23, 2020
c835df2
Add more checks
lucacome Jun 23, 2020
7ac9501
Update docs
lucacome Jun 23, 2020
12154c3
Update crds
lucacome Jun 23, 2020
e6fce1a
Apply suggestions from code review
lucacome Jun 23, 2020
90bdc81
Apply suggestions from code review
lucacome Jun 24, 2020
56e550a
Add check in createVirtualServer
lucacome Jun 24, 2020
4802adb
Merge branch 'master' into class-field
lucacome Jun 24, 2020
3be93b1
Apply suggestions from code review
lucacome Jun 25, 2020
864f0fa
last one
lucacome Jun 25, 2020
7eb729c
Make it a warning and report the error
lucacome Jun 26, 2020
fa14fad
docs
lucacome Jun 26, 2020
ae76bbe
more docs
lucacome Jun 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions internal/k8s/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,11 +886,6 @@ 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)
Expand Down Expand Up @@ -1005,11 +1000,6 @@ 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"
Expand Down Expand Up @@ -1831,8 +1821,8 @@ 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)
if !lbc.HasCorrectIngressClass(vs) {
glog.V(3).Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass)
continue
}

Expand All @@ -1854,8 +1844,8 @@ 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)
if !lbc.HasCorrectIngressClass(vsr) {
glog.V(3).Infof("Ignoring VirtualServerRoute %v based on class %v", vsr.Name, vsr.Spec.IngressClass)
continue
}

Expand Down Expand Up @@ -2083,6 +2073,10 @@ func newVirtualServerRouteErrorFromVSR(virtualServerRoute *conf_v1.VirtualServer
}

func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.VirtualServer) (*configs.VirtualServerEx, []virtualServerRouteError) {
if !lbc.HasCorrectIngressClass(virtualServer) {
lucacome marked this conversation as resolved.
Show resolved Hide resolved
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,
}
Expand Down Expand Up @@ -2473,17 +2467,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
}
Expand Down
111 changes: 51 additions & 60 deletions internal/k8s/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions internal/k8s/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ func createVirtualServerHandlers(lbc *LoadBalancerController) cache.ResourceEven
return cache.ResourceEventHandlerFuncs{
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)
if !lbc.HasCorrectIngressClass(vs) {
glog.Infof("Ignoring VirtualServer %v based on class %v", vs.Name, vs.Spec.IngressClass)
lucacome marked this conversation as resolved.
Show resolved Hide resolved
return
}
glog.V(3).Infof("Adding VirtualServer: %v", vs.Name)
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down