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: avoid unnecessary headless endpoint mirrors cleanups during GC #12500

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

marwanad
Copy link
Contributor

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 #12499

@marwanad marwanad requested a review from a team as a code owner April 25, 2024 00:33
@marwanad marwanad force-pushed the avoid-headless-service-cleanup branch from aa28959 to 635cb00 Compare April 25, 2024 00:35
Signed-off-by: Marwan Ahmed <me@marwanad.com>
Signed-off-by: Marwan Ahmed <me@marwanad.com>
@marwanad marwanad force-pushed the avoid-headless-service-cleanup branch from 635cb00 to 73abc21 Compare April 25, 2024 00:36
Copy link

@abatilo abatilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incredible work

Copy link
Member

@adleong adleong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice find and really nice fix!

@adleong adleong merged commit 5acac25 into linkerd:main Apr 26, 2024
37 checks passed
the-wondersmith pushed a commit to the-wondersmith/linkerd2 that referenced this pull request Apr 27, 2024
…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>
the-wondersmith pushed a commit to the-wondersmith/linkerd2 that referenced this pull request Apr 29, 2024
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Headless endpoint mirrors are incorrectly cleaned up as part of GC
4 participants