Skip to content

Commit e03bd44

Browse files
committed
Fix EndpointSlice duplication issue in finalize logic
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 443a1fe commit e03bd44

File tree

2 files changed

+87
-4
lines changed

2 files changed

+87
-4
lines changed

pkg/controller/kubevirteps/kubevirteps_controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -474,11 +474,11 @@ func (c *Controller) reconcileByAddressType(service *v1.Service, tenantSlices []
474474
// Create the desired port configuration
475475
var desiredPorts []discovery.EndpointPort
476476

477-
for _, port := range service.Spec.Ports {
477+
for i := range service.Spec.Ports {
478478
desiredPorts = append(desiredPorts, discovery.EndpointPort{
479-
Port: &port.TargetPort.IntVal,
480-
Protocol: &port.Protocol,
481-
Name: &port.Name,
479+
Port: &service.Spec.Ports[i].TargetPort.IntVal,
480+
Protocol: &service.Spec.Ports[i].Protocol,
481+
Name: &service.Spec.Ports[i].Name,
482482
})
483483
}
484484

pkg/controller/kubevirteps/kubevirteps_controller_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"k8s.io/apimachinery/pkg/runtime"
1414
"k8s.io/apimachinery/pkg/runtime/schema"
1515
"k8s.io/apimachinery/pkg/util/intstr"
16+
"k8s.io/apimachinery/pkg/util/sets"
1617
dfake "k8s.io/client-go/dynamic/fake"
1718
"k8s.io/client-go/kubernetes/fake"
1819
"k8s.io/client-go/testing"
@@ -686,5 +687,87 @@ var _ = g.Describe("KubevirtEPSController", g.Ordered, func() {
686687
return false, err
687688
}).Should(BeTrue(), "EndpointSlice in infra cluster should be recreated by the controller after deletion")
688689
})
690+
691+
g.It("Should correctly handle multiple unique ports in EndpointSlice", func() {
692+
// Create a VMI in the infra cluster
693+
createAndAssertVMI("worker-0-test", "ip-10-32-5-13", "123.45.67.89")
694+
695+
// Create an EndpointSlice in the tenant cluster
696+
createAndAssertTenantSlice("test-epslice", "tenant-service-name", discoveryv1.AddressTypeIPv4,
697+
*createPort("http", 80, v1.ProtocolTCP),
698+
[]discoveryv1.Endpoint{*createEndpoint("123.45.67.89", "worker-0-test", true, true, false)})
699+
700+
// Define several unique ports for the Service
701+
servicePorts := []v1.ServicePort{
702+
{
703+
Name: "client",
704+
Protocol: v1.ProtocolTCP,
705+
Port: 10001,
706+
TargetPort: intstr.FromInt(30396),
707+
NodePort: 30396,
708+
AppProtocol: nil,
709+
},
710+
{
711+
Name: "dashboard",
712+
Protocol: v1.ProtocolTCP,
713+
Port: 8265,
714+
TargetPort: intstr.FromInt(31003),
715+
NodePort: 31003,
716+
AppProtocol: nil,
717+
},
718+
{
719+
Name: "metrics",
720+
Protocol: v1.ProtocolTCP,
721+
Port: 8080,
722+
TargetPort: intstr.FromInt(30452),
723+
NodePort: 30452,
724+
AppProtocol: nil,
725+
},
726+
}
727+
728+
// Create a Service with the first port
729+
createAndAssertInfraServiceLB("infra-multiport-service", "tenant-service-name", "test-cluster",
730+
servicePorts[0],
731+
v1.ServiceExternalTrafficPolicyLocal)
732+
733+
// Update the Service by adding the remaining ports
734+
svc, err := testVals.infraClient.CoreV1().Services(infraNamespace).Get(context.TODO(), "infra-multiport-service", metav1.GetOptions{})
735+
Expect(err).To(BeNil())
736+
737+
svc.Spec.Ports = servicePorts
738+
739+
_, err = testVals.infraClient.CoreV1().Services(infraNamespace).Update(context.TODO(), svc, metav1.UpdateOptions{})
740+
Expect(err).To(BeNil())
741+
742+
var epsListMultiPort *discoveryv1.EndpointSliceList
743+
744+
// Verify that the EndpointSlice is created with correct unique ports
745+
Eventually(func() (bool, error) {
746+
epsListMultiPort, err = testVals.infraClient.DiscoveryV1().EndpointSlices(infraNamespace).List(context.TODO(), metav1.ListOptions{})
747+
if len(epsListMultiPort.Items) != 1 {
748+
return false, err
749+
}
750+
751+
createdSlice := epsListMultiPort.Items[0]
752+
expectedPortNames := []string{"client", "dashboard", "metrics"}
753+
foundPortNames := []string{}
754+
755+
for _, port := range createdSlice.Ports {
756+
if port.Name != nil {
757+
foundPortNames = append(foundPortNames, *port.Name)
758+
}
759+
}
760+
761+
// Verify that all expected ports are present and without duplicates
762+
if len(foundPortNames) != len(expectedPortNames) {
763+
return false, err
764+
}
765+
766+
portSet := sets.NewString(foundPortNames...)
767+
expectedPortSet := sets.NewString(expectedPortNames...)
768+
return portSet.Equal(expectedPortSet), err
769+
}).Should(BeTrue(), "EndpointSlice should contain all unique ports from the Service without duplicates")
770+
})
771+
689772
})
690773
})

0 commit comments

Comments
 (0)