Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

Commit

Permalink
Add support for updating ExtraLabels
Browse files Browse the repository at this point in the history
  • Loading branch information
Dean-Coakley committed Mar 3, 2021
1 parent cce9ff7 commit 13a282e
Show file tree
Hide file tree
Showing 2 changed files with 271 additions and 4 deletions.
15 changes: 14 additions & 1 deletion pkg/controller/nginxingresscontroller/service.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package nginxingresscontroller

import (
"reflect"

k8sv1alpha1 "github.com/nginxinc/nginx-ingress-operator/pkg/apis/k8s/v1alpha1"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -47,10 +49,21 @@ func serviceForNginxIngressController(instance *k8sv1alpha1.NginxIngressControll
}

func hasServiceChanged(svc *corev1.Service, instance *k8sv1alpha1.NginxIngressController) bool {
return svc.Spec.Type != corev1.ServiceType(instance.Spec.ServiceType)
if svc.Spec.Type != corev1.ServiceType(instance.Spec.ServiceType) {
return true
}
if instance.Spec.Service != nil && !reflect.DeepEqual(svc.Labels, instance.Spec.Service.ExtraLabels) {
return true
}
return svc.Labels != nil && instance.Spec.Service == nil
}

func updateService(svc *corev1.Service, instance *k8sv1alpha1.NginxIngressController) *corev1.Service {
svc.Spec.Type = corev1.ServiceType(instance.Spec.ServiceType)
if instance.Spec.Service != nil {
svc.Labels = instance.Spec.Service.ExtraLabels
} else {
svc.Labels = nil
}
return svc
}
260 changes: 257 additions & 3 deletions pkg/controller/nginxingresscontroller/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
func TestServiceForNginxIngressController(t *testing.T) {
name := "my-service"
namespace := "my-nginx-ingress"
serviceType := "LoadBalancer"
extraLabels := map[string]string{"app": "my-nginx-ingress"}

instance := &k8sv1alpha1.NginxIngressController{
Expand All @@ -23,7 +22,7 @@ func TestServiceForNginxIngressController(t *testing.T) {
Labels: extraLabels,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: serviceType,
ServiceType: string(corev1.ServiceTypeLoadBalancer),
Service: &k8sv1alpha1.Service{
ExtraLabels: extraLabels,
},
Expand Down Expand Up @@ -57,7 +56,7 @@ func TestServiceForNginxIngressController(t *testing.T) {
},
},
Selector: map[string]string{"app": instance.Name},
Type: corev1.ServiceType(serviceType),
Type: corev1.ServiceTypeLoadBalancer,
},
}

Expand All @@ -66,3 +65,258 @@ func TestServiceForNginxIngressController(t *testing.T) {
t.Errorf("serviceForNginxIngressController() mismatch (-want +got):\n%s", diff)
}
}

func TestHasServiceChanged(t *testing.T) {
name := "my-service"
namespace := "my-nginx-ingress"
extraLabels := map[string]string{"app": "my-nginx-ingress"}

tests := []struct {
svc *corev1.Service
instance *k8sv1alpha1.NginxIngressController
expected bool
msg string
}{
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
},
},
false,
"no changes",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: "NodePort",
},
},
true,
"different service type",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: extraLabels,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
Service: &k8sv1alpha1.Service{
ExtraLabels: map[string]string{"new": "label"},
},
},
},
true,
"different label",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: extraLabels,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
Service: &k8sv1alpha1.Service{
ExtraLabels: nil,
},
},
},
true,
"remove label",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: extraLabels,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
},
},
true,
"remove service parameters",
},
}
for _, test := range tests {
result := hasServiceChanged(test.svc, test.instance)
if result != test.expected {
t.Errorf("hasServiceChanged() = %v, want %v for the case of %v", result, test.expected, test.msg)
}
}
}

func TestUpdateService(t *testing.T) {
name := "my-service"
namespace := "my-nginx-ingress"
extraLabels := map[string]string{"app": "my-nginx-ingress"}

tests := []struct {
svc *corev1.Service
instance *k8sv1alpha1.NginxIngressController
expected *corev1.Service
msg string
}{
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeNodePort),
},
},
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeNodePort,
},
},
"override service type",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: map[string]string{"my": "labelToBeOverridden"},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
Service: &k8sv1alpha1.Service{
ExtraLabels: extraLabels,
},
},
},
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: extraLabels,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
"override labels",
},
{
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: map[string]string{"my": "labelToBeRemoved"},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
&k8sv1alpha1.NginxIngressController{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: k8sv1alpha1.NginxIngressControllerSpec{
ServiceType: string(corev1.ServiceTypeLoadBalancer),
},
},
&corev1.Service{
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
},
"remove labels",
},
}
for _, test := range tests {
result := updateService(test.svc, test.instance)
if diff := cmp.Diff(test.expected, result); diff != "" {
t.Errorf("updateService() mismatch for the case of %v (-want +got):\n%s", test.msg, diff)
}
}
}

0 comments on commit 13a282e

Please sign in to comment.