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

Commit 3815243

Browse files
Dean-CoakleyDean-Coakley
committed
Add support for updating ExtraLabels
1 parent ad81ccf commit 3815243

File tree

2 files changed

+269
-4
lines changed

2 files changed

+269
-4
lines changed

‎pkg/controller/nginxingresscontroller/service.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package nginxingresscontroller
22

33
import (
4+
"reflect"
5+
46
k8sv1alpha1 "github.com/nginxinc/nginx-ingress-operator/pkg/apis/k8s/v1alpha1"
57
corev1 "k8s.io/api/core/v1"
68
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -47,10 +49,21 @@ func serviceForNginxIngressController(instance *k8sv1alpha1.NginxIngressControll
4749
}
4850

4951
func hasServiceChanged(svc *corev1.Service, instance *k8sv1alpha1.NginxIngressController) bool {
50-
return svc.Spec.Type != corev1.ServiceType(instance.Spec.ServiceType)
52+
if svc.Spec.Type != corev1.ServiceType(instance.Spec.ServiceType) {
53+
return true
54+
}
55+
if instance.Spec.Service != nil && !reflect.DeepEqual(svc.Labels, instance.Spec.Service.ExtraLabels) {
56+
return true
57+
}
58+
return svc.Labels != nil && instance.Spec.Service == nil
5159
}
5260

5361
func updateService(svc *corev1.Service, instance *k8sv1alpha1.NginxIngressController) *corev1.Service {
5462
svc.Spec.Type = corev1.ServiceType(instance.Spec.ServiceType)
63+
if instance.Spec.Service != nil {
64+
svc.Labels = instance.Spec.Service.ExtraLabels
65+
} else {
66+
svc.Labels = nil
67+
}
5568
return svc
5669
}

‎pkg/controller/nginxingresscontroller/service_test.go‎

Lines changed: 255 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
func TestServiceForNginxIngressController(t *testing.T) {
1414
name := "my-service"
1515
namespace := "my-nginx-ingress"
16-
serviceType := "LoadBalancer"
1716
extraLabels := map[string]string{"app": "my-nginx-ingress"}
1817

1918
instance := &k8sv1alpha1.NginxIngressController{
@@ -23,7 +22,7 @@ func TestServiceForNginxIngressController(t *testing.T) {
2322
Labels: extraLabels,
2423
},
2524
Spec: k8sv1alpha1.NginxIngressControllerSpec{
26-
ServiceType: serviceType,
25+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
2726
Service: &k8sv1alpha1.Service{
2827
ExtraLabels: extraLabels,
2928
},
@@ -57,7 +56,7 @@ func TestServiceForNginxIngressController(t *testing.T) {
5756
},
5857
},
5958
Selector: map[string]string{"app": instance.Name},
60-
Type: corev1.ServiceType(serviceType),
59+
Type: corev1.ServiceTypeLoadBalancer,
6160
},
6261
}
6362

@@ -66,3 +65,256 @@ func TestServiceForNginxIngressController(t *testing.T) {
6665
t.Errorf("serviceForNginxIngressController() mismatch (-want +got):\n%s", diff)
6766
}
6867
}
68+
69+
func TestHasServiceChanged(t *testing.T) {
70+
name := "my-service"
71+
namespace := "my-nginx-ingress"
72+
extraLabels := map[string]string{"app": "my-nginx-ingress"}
73+
74+
tests := []struct {
75+
svc *corev1.Service
76+
instance *k8sv1alpha1.NginxIngressController
77+
expected bool
78+
msg string
79+
}{
80+
{
81+
&corev1.Service{
82+
ObjectMeta: v1.ObjectMeta{
83+
Name: name,
84+
Namespace: namespace,
85+
},
86+
Spec: corev1.ServiceSpec{
87+
Type: corev1.ServiceTypeLoadBalancer,
88+
},
89+
},
90+
&k8sv1alpha1.NginxIngressController{
91+
ObjectMeta: v1.ObjectMeta{
92+
Name: name,
93+
Namespace: namespace,
94+
},
95+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
96+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
97+
},
98+
},
99+
false,
100+
"no changes",
101+
},
102+
{
103+
&corev1.Service{
104+
ObjectMeta: v1.ObjectMeta{
105+
Name: name,
106+
Namespace: namespace,
107+
},
108+
Spec: corev1.ServiceSpec{
109+
Type: corev1.ServiceTypeLoadBalancer,
110+
},
111+
},
112+
&k8sv1alpha1.NginxIngressController{
113+
ObjectMeta: v1.ObjectMeta{
114+
Name: name,
115+
Namespace: namespace,
116+
},
117+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
118+
ServiceType: "NodePort",
119+
},
120+
},
121+
true,
122+
"different service type",
123+
},
124+
{
125+
&corev1.Service{
126+
ObjectMeta: v1.ObjectMeta{
127+
Name: name,
128+
Namespace: namespace,
129+
Labels: extraLabels,
130+
},
131+
Spec: corev1.ServiceSpec{
132+
Type: corev1.ServiceTypeLoadBalancer,
133+
},
134+
},
135+
&k8sv1alpha1.NginxIngressController{
136+
ObjectMeta: v1.ObjectMeta{
137+
Name: name,
138+
Namespace: namespace,
139+
},
140+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
141+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
142+
Service: &k8sv1alpha1.Service{
143+
ExtraLabels: map[string]string{"new": "label"},
144+
},
145+
},
146+
},
147+
true,
148+
"different label",
149+
},
150+
{
151+
&corev1.Service{
152+
ObjectMeta: v1.ObjectMeta{
153+
Name: name,
154+
Namespace: namespace,
155+
Labels: extraLabels,
156+
},
157+
Spec: corev1.ServiceSpec{
158+
Type: corev1.ServiceTypeLoadBalancer,
159+
},
160+
},
161+
&k8sv1alpha1.NginxIngressController{
162+
ObjectMeta: v1.ObjectMeta{
163+
Name: name,
164+
Namespace: namespace,
165+
},
166+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
167+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
168+
Service: &k8sv1alpha1.Service{
169+
ExtraLabels: nil,
170+
},
171+
},
172+
},
173+
true,
174+
"remove label",
175+
},
176+
{
177+
&corev1.Service{
178+
ObjectMeta: v1.ObjectMeta{
179+
Name: name,
180+
Namespace: namespace,
181+
Labels: extraLabels,
182+
},
183+
Spec: corev1.ServiceSpec{
184+
Type: corev1.ServiceTypeLoadBalancer,
185+
},
186+
},
187+
&k8sv1alpha1.NginxIngressController{
188+
ObjectMeta: v1.ObjectMeta{
189+
Name: name,
190+
Namespace: namespace,
191+
},
192+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
193+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
194+
},
195+
},
196+
true,
197+
"remove service parameters",
198+
},
199+
}
200+
for _, test := range tests {
201+
result := hasServiceChanged(test.svc, test.instance)
202+
if result != test.expected {
203+
t.Errorf("hasServiceChanged() = %v, want %v for the case of %v", result, test.expected, test.msg)
204+
}
205+
}
206+
}
207+
208+
func TestUpdateService(t *testing.T) {
209+
name := "my-service"
210+
namespace := "my-nginx-ingress"
211+
extraLabels := map[string]string{"app": "my-nginx-ingress"}
212+
213+
tests := []struct {
214+
svc *corev1.Service
215+
instance *k8sv1alpha1.NginxIngressController
216+
expected *corev1.Service
217+
msg string
218+
}{
219+
{
220+
&corev1.Service{
221+
ObjectMeta: v1.ObjectMeta{
222+
Name: name,
223+
Namespace: namespace,
224+
},
225+
Spec: corev1.ServiceSpec{
226+
Type: corev1.ServiceTypeLoadBalancer,
227+
},
228+
},
229+
&k8sv1alpha1.NginxIngressController{
230+
ObjectMeta: v1.ObjectMeta{
231+
Name: name,
232+
Namespace: namespace,
233+
},
234+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
235+
ServiceType: string(corev1.ServiceTypeNodePort),
236+
},
237+
},
238+
&corev1.Service{
239+
ObjectMeta: v1.ObjectMeta{
240+
Name: name,
241+
Namespace: namespace,
242+
},
243+
Spec: corev1.ServiceSpec{
244+
Type: corev1.ServiceTypeNodePort,
245+
},
246+
},
247+
"override service type",
248+
},
249+
{
250+
&corev1.Service{
251+
ObjectMeta: v1.ObjectMeta{
252+
Name: name,
253+
Namespace: namespace,
254+
},
255+
Spec: corev1.ServiceSpec{
256+
Type: corev1.ServiceTypeLoadBalancer,
257+
},
258+
},
259+
&k8sv1alpha1.NginxIngressController{
260+
ObjectMeta: v1.ObjectMeta{
261+
Name: name,
262+
Namespace: namespace,
263+
},
264+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
265+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
266+
Service: &k8sv1alpha1.Service{
267+
ExtraLabels: extraLabels,
268+
},
269+
},
270+
},
271+
&corev1.Service{
272+
ObjectMeta: v1.ObjectMeta{
273+
Name: name,
274+
Namespace: namespace,
275+
Labels: extraLabels,
276+
},
277+
Spec: corev1.ServiceSpec{
278+
Type: corev1.ServiceTypeLoadBalancer,
279+
},
280+
},
281+
"override labels",
282+
},
283+
{
284+
&corev1.Service{
285+
ObjectMeta: v1.ObjectMeta{
286+
Name: name,
287+
Namespace: namespace,
288+
},
289+
Spec: corev1.ServiceSpec{
290+
Type: corev1.ServiceTypeLoadBalancer,
291+
},
292+
},
293+
&k8sv1alpha1.NginxIngressController{
294+
ObjectMeta: v1.ObjectMeta{
295+
Name: name,
296+
Namespace: namespace,
297+
},
298+
Spec: k8sv1alpha1.NginxIngressControllerSpec{
299+
ServiceType: string(corev1.ServiceTypeLoadBalancer),
300+
},
301+
},
302+
&corev1.Service{
303+
ObjectMeta: v1.ObjectMeta{
304+
Name: name,
305+
Namespace: namespace,
306+
},
307+
Spec: corev1.ServiceSpec{
308+
Type: corev1.ServiceTypeLoadBalancer,
309+
},
310+
},
311+
"remove labels",
312+
},
313+
}
314+
for _, test := range tests {
315+
result := updateService(test.svc, test.instance)
316+
if diff := cmp.Diff(test.expected, result); diff != "" {
317+
t.Errorf("updateService() mismatch for the case of %v (-want +got):\n%s", test.msg, diff)
318+
}
319+
}
320+
}

0 commit comments

Comments
 (0)