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

Make linkerd mc link's output compatible with pre-edge-24.9.3 clusters #13161

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

alpeb
Copy link
Member

@alpeb alpeb commented Oct 9, 2024

Followup to #1306

Problem

if we run linkerd mc link using the latest CLI and attempted to apply the resulting manifests into a cluster with linkerd-multicluster on version edge-24.9.2 and earlier we would get the following error:

Error from server (BadRequest): error when creating "STDIN": Link in version "v1alpha1" cannot be handled as a Link: strict decoding error: unknown field "spec.probeSpec.failureThreshold", unknown field "spec.probeSpec.timeout"

Solution

This changes the linkerd mc link command so that it doesn't generate default entries for the new (optional) fields failureThreshold and timeout, so that the output remains compatible with previous versions of the Link CRD.

linkerd mc link will however still generate those fields for clusters that have an up-to-date multicluster extension. Those fields values continue to be retrieved from the gateway service's mirror.linkerd.io/probe-failure-threshold and mirror.linkerd.io/probe-timeout annotations.

We're also removing those fields from the linkerd-multicluster values.yaml file, and we're removing the corresponding annotations on the gateway workload. These values and annotations were added in edge-24.9.3.

This means that (at least for now) linkerd mc link will never generate those fields, and the way for users to tweak them is directly in the Link CR (once the user upgrades the linkerd-multicluster extension, the existing Link CRs will automatically receive the default values, as they are declared in the Link CRD definition).

Followup to #1306

## Problem

if we run `linkerd mc link` using the latest CLI and attempted to apply the resulting manifests into a cluster with linkerd-multicluster on version edge-24.9.2 and earlier we would get the following error:

```
Error from server (BadRequest): error when creating "STDIN": Link in version "v1alpha1" cannot be handled as a Link: strict decoding error: unknown field "spec.probeSpec.failureThreshold", unknown field "spec.probeSpec.timeout"
```

## Solution

This changes the `linkerd mc link` command so that it doesn't generate default entries for the new (optional) fields `failureThreshold` and `timeout`, so that the output remains compatible with previous versions of the Link CRD.

`linkerd mc link` will however still generate those fields for clusters that have an up-to-date multicluster extension. Those fields values continue to be retrieved from the gateway service's `mirror.linkerd.io/probe-failure-threshold` and `mirror.linkerd.io/probe-timeout` annotations.

Note that once the linkerd-multicluster extension gets upgraded in the cluster to match the CLI version, the existing Link CRs will automatically receive the default values for these fields.
@alpeb alpeb requested a review from a team as a code owner October 9, 2024 19:10
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.

Do we need to move this defaulting into the read side i.e. in ProbeWorker?

What happens when a ProbeWorker reads a Link with no Timeout or no FailureThresold?

@adleong
Copy link
Member

adleong commented Oct 9, 2024

Attempting to answer my own question.... when the mc extension is upgraded, the Link CRD is also upgraded, and Links will receive default values for the new fields from the CRD schema?

So a new ProbeWorker should always see these fields populated in a Link resource (either explicitly or from the Link CRD schema default). Is that correct?

@alpeb
Copy link
Member Author

alpeb commented Oct 9, 2024

That is correct. Given the manifests for the Link CRD and the service mirror workload are produced by the same command (linkerd mc link), I think it's safe to assume they agree on their version.

…rresponding annotations from the gateway workload
@alpeb
Copy link
Member Author

alpeb commented Oct 10, 2024

To still provide compatibility of the latest linkerd mc link's output produced from an up-to-date cluster, and applied to an old cluster, the latest commit stops outputting those fields completely and also removes them from the values file and the gateway annotations. Please check the updated PR description.

@alpeb alpeb merged commit b5aec5d into main Oct 10, 2024
40 checks passed
@alpeb alpeb deleted the alpeb/mc-link-bc branch October 10, 2024 18:51
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.

3 participants