Skip to content

Commit

Permalink
Fix EndpointSlice duplication issue in finalize logic
Browse files Browse the repository at this point in the history
Signed-off-by: Andrei Kvapil <[email protected]>
  • Loading branch information
kvaps committed Dec 9, 2024
1 parent da9e0cf commit e26a55c
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/controller/kubevirteps/kubevirteps_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,11 @@ func (c *Controller) reconcileByAddressType(service *v1.Service, tenantSlices []
// Create the desired port configuration
var desiredPorts []discovery.EndpointPort

for _, port := range service.Spec.Ports {
for i := range service.Spec.Ports {
desiredPorts = append(desiredPorts, discovery.EndpointPort{
Port: &port.TargetPort.IntVal,
Protocol: &port.Protocol,
Name: &port.Name,
Port: &service.Spec.Ports[i].TargetPort.IntVal,
Protocol: &service.Spec.Ports[i].Protocol,
Name: &service.Spec.Ports[i].Name,
})
}

Expand Down
82 changes: 82 additions & 0 deletions pkg/controller/kubevirteps/kubevirteps_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
dfake "k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/testing"
Expand Down Expand Up @@ -642,5 +643,86 @@ var _ = g.Describe("KubevirtEPSController", g.Ordered, func() {
}).Should(BeTrue(), "Expect call to the infraDynamic.Get to return the VMI")

})

g.It("Should correctly handle multiple unique ports in EndpointSlice", func() {
// Create a VMI in the infra cluster
createAndAssertVMI("worker-0-test", "ip-10-32-5-13", "123.45.67.89")

// Create an EndpointSlice in the tenant cluster
createAndAssertTenantSlice("test-epslice", "tenant-service-name", discoveryv1.AddressTypeIPv4,
*createPort("http", 80, v1.ProtocolTCP),
[]discoveryv1.Endpoint{*createEndpoint("123.45.67.89", "worker-0-test", true, true, false)})

// Define several unique ports for the Service
servicePorts := []v1.ServicePort{
{
Name: "client",
Protocol: v1.ProtocolTCP,
Port: 10001,
TargetPort: intstr.FromInt(30396),
NodePort: 30396,
AppProtocol: nil,
},
{
Name: "dashboard",
Protocol: v1.ProtocolTCP,
Port: 8265,
TargetPort: intstr.FromInt(31003),
NodePort: 31003,
AppProtocol: nil,
},
{
Name: "metrics",
Protocol: v1.ProtocolTCP,
Port: 8080,
TargetPort: intstr.FromInt(30452),
NodePort: 30452,
AppProtocol: nil,
},
}

// Create a Service with the first port
createAndAssertInfraServiceLB("infra-multiport-service", "tenant-service-name", "test-cluster",
servicePorts[0],
v1.ServiceExternalTrafficPolicyLocal)

// Update the Service by adding the remaining ports
svc, err := testVals.infraClient.CoreV1().Services(infraNamespace).Get(context.TODO(), "infra-multiport-service", metav1.GetOptions{})
Expect(err).To(BeNil())

svc.Spec.Ports = servicePorts

_, err = testVals.infraClient.CoreV1().Services(infraNamespace).Update(context.TODO(), svc, metav1.UpdateOptions{})
Expect(err).To(BeNil())

var epsListMultiPort *discoveryv1.EndpointSliceList

// Verify that the EndpointSlice is created with correct unique ports
Eventually(func() (bool, error) {
epsListMultiPort, err = testVals.infraClient.DiscoveryV1().EndpointSlices(infraNamespace).List(context.TODO(), metav1.ListOptions{})
if len(epsListMultiPort.Items) != 1 {
return false, err
}

createdSlice := epsListMultiPort.Items[0]
expectedPortNames := []string{"client", "dashboard", "metrics"}
foundPortNames := []string{}

for _, port := range createdSlice.Ports {
if port.Name != nil {
foundPortNames = append(foundPortNames, *port.Name)
}
}

// Verify that all expected ports are present and without duplicates
if len(foundPortNames) != len(expectedPortNames) {
return false, err
}

portSet := sets.NewString(foundPortNames...)
expectedPortSet := sets.NewString(expectedPortNames...)
return portSet.Equal(expectedPortSet), err
}).Should(BeTrue(), "EndpointSlice should contain all unique ports from the Service without duplicates")
})
})
})

0 comments on commit e26a55c

Please sign in to comment.