Skip to content

Commit

Permalink
EndpointToBeReflected function fix
Browse files Browse the repository at this point in the history
  • Loading branch information
fra98 authored and adamjensenbot committed Sep 20, 2023
1 parent 1ceda8c commit 760021d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 15 deletions.
30 changes: 16 additions & 14 deletions pkg/virtualKubelet/forge/endpointslices.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,32 @@ func IsEndpointSliceManagedByReflection(obj metav1.Object) bool {
// EndpointToBeReflected filters out the endpoints targeting pods already running on the remote cluster.
func EndpointToBeReflected(endpoint *discoveryv1.Endpoint, localNodeClient corev1listers.NodeLister) bool {
if endpoint.NodeName == nil {
klog.Warning("Endpoint without nodeName")
return false
klog.Warning("Endpoint without nodeName. The endpoint is probably external to the cluster.")
// If the nodeName is not set, the endpoint is probably external to the cluster.
// We reflect it, as is it certainly not scheduled on the virtual node.
return true
}

// Get node associated with the endpoint.
epNode, err := localNodeClient.Get(*endpoint.NodeName)
if err != nil {
klog.Errorf("Unable to retrieve node %s: %s", *endpoint.NodeName, err.Error())
return false
}
vkNode, err := localNodeClient.Get(LiqoNodeName)
if err != nil {
klog.Errorf("Unable to retrieve node %s: %s", LiqoNodeName, err.Error())
return false
}
vkRemoteClusterID, err := getters.RetrieveRemoteClusterIDFromNode(epNode)
// Retrieve clusterIDs from the node labels.
epNodeClusterID, err := getters.RetrieveRemoteClusterIDFromNode(epNode)
if err != nil {
klog.Errorf("Unable to retrieve remote cluster ID from node %s: %s", epNode.GetName(), err.Error())
return false
}
nodeRemoteClusterID, err := getters.RetrieveRemoteClusterIDFromNode(vkNode)
if err != nil {
klog.Errorf("Unable to retrieve remote cluster ID from node %s: %s", vkNode.GetName(), err.Error())
return false
}
return !pointer.StringEqual(&nodeRemoteClusterID, &vkRemoteClusterID)

// We compare the clusterIDs to check whether the endpoint is scheduled on (any) virtual node
// associated to the same remote cluster managed by the current virtual kubelet (i.e. targeting pods
// already running on the remote cluster):
// - endpoints relative to the same remote cluster are not reflected, as the associated endpointslice is
// already handled on the remote cluster by Kubernetes, due to the presence of the remote pod.
// - endpoints relative to (1) local cluster, (2) different remote clusters, or (3) external are reflected.
return !pointer.StringEqual(&epNodeClusterID, &RemoteCluster.ClusterID)
}

// RemoteShadowEndpointSlice forges the remote shadowendpointslice, given the local endpointslice.
Expand Down
10 changes: 9 additions & 1 deletion pkg/virtualKubelet/forge/endpointslices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (fnl *FakeNodeLister) Get(name string) (*corev1.Node, error) {
Name: name,
Labels: map[string]string{},
}}
if name != LiqoNodeName {
if name == LiqoNodeName {
n.Labels[consts.RemoteClusterID] = RemoteClusterID
}
return n, nil
Expand Down Expand Up @@ -177,6 +177,14 @@ var _ = Describe("EndpointSlices Forging", func() {
It("should return no endpoints", func() { Expect(output).To(HaveLen(0)) })
})

When("translating an endpoint with empty NodeName (e.g., external endpoint)", func() {
BeforeEach(func() {
endpoint.NodeName = nil
input = []discoveryv1.Endpoint{endpoint}
})
It("should return the translated endpoint", func() { Expect(output).To(HaveLen(1)) })
})

When("translating multiple endpoints", func() {
BeforeEach(func() { input = []discoveryv1.Endpoint{endpoint, endpoint, endpoint} })
It("should return the correct number of endpoints", func() { Expect(output).To(HaveLen(3)) })
Expand Down

0 comments on commit 760021d

Please sign in to comment.