Skip to content

Commit

Permalink
fix: avoid unnecessary headless endpoint mirrors cleanups during GC (l…
Browse files Browse the repository at this point in the history
…inkerd#12500)

Subject
Fixes a bug where headless endpoint mirrors get cleaned up during GC

Problem
When GC is triggered (which also happens at startup or when the link watch disconnects), the service mirror controller attempts to look for services that can be GC'ed. This is done by looping through the local mirrored services on the cluster, then extracting the name of the original service in the remote (by dropping the target name suffix).

However, this check doesn't account for the headless endpoint service mirrors (the per pod cluster IP services). For example, if you have nginx-svc in the west cluster and two replicas, the source cluster will end up with nginx-svc-west, nginx-set-0-west and nginx-set-1-west. The logic would then parse the resource name for the latter two services as nginx-set-0 and nginx-set-1 which won't exist on the remote and ends up deleting them as part of GC.

The next sync would recreate those mirrors but you end up with downtime.

Solution
For those cases, instead of parsing the remote resource from the local service name, retrieve the info from the `mirror.linkerd.io/headless-mirror-svc-name` label.

Validation
Unit tests

Fixes linkerd#12499

Signed-off-by: Marwan Ahmed <me@marwanad.com>
Signed-off-by: Mark S <the@wondersmith.dev>
  • Loading branch information
marwanad authored and the-wondersmith committed Apr 27, 2024
1 parent cf98619 commit a3e50bd
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
9 changes: 8 additions & 1 deletion multicluster/service-mirror/cluster_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,14 @@ func (rcsw *RemoteClusterServiceWatcher) cleanupOrphanedServices(ctx context.Con

var errors []error
for _, srv := range servicesOnLocalCluster {
_, err := rcsw.remoteAPIClient.Svc().Lister().Services(srv.Namespace).Get(rcsw.originalResourceName(srv.Name))
mirroredName := srv.Name
// For headless services with cluster IPs representing the backing pods, the mirrored service name
// is the root headless service in the source cluster
if remoteHeadlessSvcName, headlessMirror := srv.Labels[consts.MirroredHeadlessSvcNameLabel]; headlessMirror {
mirroredName = remoteHeadlessSvcName
}
remoteServiceName := rcsw.originalResourceName(mirroredName)
_, err := rcsw.remoteAPIClient.Svc().Lister().Services(srv.Namespace).Get(remoteServiceName)
if err != nil {
if kerrors.IsNotFound(err) {
// service does not exist anymore. Need to delete
Expand Down
4 changes: 4 additions & 0 deletions multicluster/service-mirror/cluster_watcher_mirroring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,14 @@ func TestGcOrphanedServicesMirroring(t *testing.T) {
environment: gcTriggered,
expectedLocalServices: []*corev1.Service{
mirrorService("test-service-1-remote", "test-namespace", "", nil),
headlessMirrorService("test-headless-service-remote", "test-namespace", "", nil),
endpointMirrorService("pod-0", "test-headless-service-remote", "test-namespace", "", nil),
},

expectedLocalEndpoints: []*corev1.Endpoints{
endpoints("test-service-1-remote", "test-namespace", "", "", nil),
headlessMirrorEndpoints("test-headless-service-remote", "test-namespace", "pod-0", "", "", nil),
endpointMirrorEndpoints("test-headless-service-remote", "test-namespace", "pod-0", "", "", nil),
},
},
} {
Expand Down
5 changes: 5 additions & 0 deletions multicluster/service-mirror/cluster_watcher_test_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,14 @@ var gcTriggered = &testEnvironment{
endpointsAsYaml("test-service-1-remote", "test-namespace", "", "", nil),
mirrorServiceAsYaml("test-service-2-remote", "test-namespace", "", nil),
endpointsAsYaml("test-service-2-remote", "test-namespace", "", "", nil),
headlessMirrorAsYaml("test-headless-service-remote", "test-namespace", "", nil),
endpointMirrorAsYaml("pod-0", "test-headless-service-remote", "test-namespace", "", nil),
headlessMirrorEndpointsAsYaml("test-headless-service-remote", "test-namespace", "pod-0", "", "", nil),
endpointMirrorEndpointsAsYaml("test-headless-service-remote", "test-namespace", "pod-0", "", "", nil),
},
remoteResources: []string{
remoteServiceAsYaml("test-service-1", "test-namespace", "", nil),
remoteHeadlessSvcAsYaml("test-headless-service", "test-namespace", "", nil),
},
link: multicluster.Link{
TargetClusterName: clusterName,
Expand Down

0 comments on commit a3e50bd

Please sign in to comment.