diff --git a/lib/resourceapply/core.go b/lib/resourceapply/core.go index b5acd27f5..e27469449 100644 --- a/lib/resourceapply/core.go +++ b/lib/resourceapply/core.go @@ -55,8 +55,10 @@ func ApplyService(client coreclientv1.ServicesGetter, required *corev1.Service) modified := pointer.BoolPtr(false) resourcemerge.EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) + resourcemerge.EnsureServicePorts(modified, &existing.Spec.Ports, required.Spec.Ports) selectorSame := equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) typeSame := equality.Semantic.DeepEqual(existing.Spec.Type, required.Spec.Type) + if selectorSame && typeSame && !*modified { return nil, false, nil } diff --git a/lib/resourcemerge/core.go b/lib/resourcemerge/core.go index b75d00131..667a64716 100644 --- a/lib/resourcemerge/core.go +++ b/lib/resourcemerge/core.go @@ -202,6 +202,51 @@ func ensureContainerPort(modified *bool, existing *corev1.ContainerPort, require } } +func EnsureServicePorts(modified *bool, existing *[]corev1.ServicePort, required []corev1.ServicePort) { + for i, existingServicePort := range *existing { + var existingCurr *corev1.ServicePort + for _, requiredServicePort := range required { + if existingServicePort.Name == requiredServicePort.Name { + existingCurr = &(*existing)[i] + ensureServicePort(modified, existingCurr, requiredServicePort) + break + } + } + if existingCurr == nil { + *modified = true + *existing = append((*existing)[:i], (*existing)[i+1:]...) + } + } + + for _, requiredServicePort := range required { + match := false + for _, existingServicePort := range *existing { + if existingServicePort.Name == requiredServicePort.Name { + match = true + break + } + } + if !match { + *modified = true + *existing = append(*existing, requiredServicePort) + } + } +} + +func ensureServicePort(modified *bool, existing *corev1.ServicePort, required corev1.ServicePort) { + ensureServicePortDefaults(&required) + if !equality.Semantic.DeepEqual(required, *existing) { + *modified = true + *existing = required + } +} + +func ensureServicePortDefaults(servicePort *corev1.ServicePort) { + if servicePort.Protocol == "" { + servicePort.Protocol = corev1.ProtocolTCP + } +} + func ensureVolumeMount(modified *bool, existing *corev1.VolumeMount, required corev1.VolumeMount) { if !equality.Semantic.DeepEqual(required, *existing) { *modified = true diff --git a/lib/resourcemerge/core_test.go b/lib/resourcemerge/core_test.go index 4978302af..c2afc9b42 100644 --- a/lib/resourcemerge/core_test.go +++ b/lib/resourcemerge/core_test.go @@ -1,6 +1,7 @@ package resourcemerge import ( + "k8s.io/apimachinery/pkg/util/intstr" "testing" corev1 "k8s.io/api/core/v1" @@ -262,3 +263,391 @@ func TestEnsurePodSpec(t *testing.T) { }) } } + +func TestEnsureServicePorts(t *testing.T) { + tests := []struct { + name string + existing corev1.Service + input corev1.Service + + expectedModified bool + expected corev1.Service + }{ + { + name: "empty inputs", + existing: corev1.Service{}, + input: corev1.Service{}, + expectedModified: false, + expected: corev1.Service{}, + }, + { + name: "add port (no name)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + }, + { + name: "add port (with name)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + }, + { + name: "remove port (no name)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Port: 8282, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + }, + { + name: "remove port (with name)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "bar", + Port: 8282, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{}, + }, + }, + { + name: "replace port (same port name, different port)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "test", + Protocol: corev1.ProtocolUDP, + Port: 8080, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "test", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "test", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + }, + { + name: "replace port (no name, different port)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Port: 8080, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + }, + }, + }, + }, + { + name: "replace multiple ports", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Port: 8080, + }, + { + Name: "bar", + Port: 8081, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + { + Name: "bar", + Protocol: corev1.ProtocolUDP, + Port: 8283, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8282, + }, + { + Name: "bar", + Protocol: corev1.ProtocolUDP, + Port: 8283, + }, + }, + }, + }, + }, + { + name: "equal (same ports different order)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8080, + }, + { + Name: "bar", + Protocol: corev1.ProtocolUDP, + Port: 8081, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "bar", + Protocol: corev1.ProtocolUDP, + Port: 8081, + }, + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8080, + }, + }, + }, + }, + expectedModified: false, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolUDP, + Port: 8080, + }, + { + Name: "bar", + Protocol: corev1.ProtocolUDP, + Port: 8081, + }, + }, + }, + }, + }, + { + name: "equal (Protocol defaults to ProtocolTCP)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Port: 8080, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "bar", + Protocol: corev1.ProtocolTCP, + Port: 8081, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Port: 8080, + }, + { + Name: "bar", + Port: 8081, + }, + }, + }, + }, + expectedModified: false, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolTCP, + Port: 8080, + }, + { + Name: "bar", + Protocol: corev1.ProtocolTCP, + Port: 8081, + }, + }, + }, + }, + }, + { + name: "replace nodePort and targetPort, defaults to ProtocolTCP)", + existing: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Port: 8080, + TargetPort: intstr.FromInt(8081), + NodePort: 8081, + Protocol: corev1.ProtocolTCP, + }, + }, + }, + }, + input: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Port: 8080, + }, + }, + }, + }, + expectedModified: true, + expected: corev1.Service{ + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "foo", + Protocol: corev1.ProtocolTCP, + Port: 8080, + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + modified := pointer.BoolPtr(false) + EnsureServicePorts(modified, &test.existing.Spec.Ports, test.input.Spec.Ports) + if *modified != test.expectedModified { + t.Errorf("mismatch modified got: %v want: %v", *modified, test.expectedModified) + } + + if !equality.Semantic.DeepEqual(test.existing, test.expected) { + t.Errorf("unexpected: %s", diff.ObjectReflectDiff(test.expected, test.existing)) + } + }) + } +}