Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug endpoints not reflected #2032

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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