Skip to content

Commit

Permalink
fix(multicluster)!: Link's probeSpec.period should be formatted as du…
Browse files Browse the repository at this point in the history
…ration (#13586)

Before the refactoring from #13420, `link mc link` would generate a Link resource with `probeSpec.period` formatted as duration. After that change, it was generated as an integer and the service mirror was refactored as well to consume that integer and convert it into a duration. If a Link with the old semantics is consumed by a service mirror controller containing that change (or vice-versa) it will error out with "could not parse probe period". This could happen for example when re-linking as part of a multicluster upgrade, when pending the rollout the old controller would consume the upgraded Link. Or for folks upgrading just by bumping the controller image tag and not re-linking (unwise as that may be).

This fix consists on:
- Updating the Link CRD, constraining `period` to be formatted as duration
- Having the `linkerd mc link` command produce a duration value for that period, being lenient to the gateway service annotation `mirror.linkerd.io/probe-period` either using an integer (which it does by default) or a duration
- Having the probe worker consume that value as a duration

BREAKING CHANGE: this realigns behavior with edge-24.11.8, but Links generated with edge-25.1.1 would be rejected.
  • Loading branch information
alpeb authored Jan 23, 2025
1 parent 31a5806 commit 23e02c7
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ spec:
type: string
period:
description: Interval in between probe requests
format: duration
type: string
port:
description: Port of remote gateway health endpoint
Expand Down
15 changes: 14 additions & 1 deletion multicluster/cmd/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"fmt"
"os"
"path"
"strconv"
"strings"
"time"

"github.com/linkerd/linkerd2/controller/gen/apis/link/v1alpha2"
"github.com/linkerd/linkerd2/multicluster/static"
Expand Down Expand Up @@ -599,10 +601,21 @@ func extractProbeSpec(gateway *corev1.Service) (v1alpha2.ProbeSpec, error) {
return v1alpha2.ProbeSpec{}, err
}

// the `mirror.linkerd.io/probe-period` annotation is initialized with a
// default value of "3", but we require a duration-formatted string. So we
// perform the conversion, if required.
period := gateway.Annotations[k8s.GatewayProbePeriod]
if secs, err := strconv.ParseInt(period, 10, 64); err == nil {
dur := time.Duration(secs) * time.Second
period = dur.String()
} else if _, err := time.ParseDuration(period); err != nil {
return v1alpha2.ProbeSpec{}, fmt.Errorf("could not parse probe period: %w", err)
}

return v1alpha2.ProbeSpec{
Path: path,
Port: fmt.Sprintf("%d", port),
Period: gateway.Annotations[k8s.GatewayProbePeriod],
Period: period,
}, nil
}

Expand Down
1 change: 1 addition & 0 deletions multicluster/cmd/testdata/install_default.golden

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions multicluster/cmd/testdata/install_ha.golden

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions multicluster/cmd/testdata/install_psp.golden

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions multicluster/service-mirror/probe_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,11 @@ func (pw *ProbeWorker) run() {
pw.log.Error("Probe spec is nil")
return
}
probeTickerPeriodSeconds, err := strconv.ParseInt(pw.probeSpec.Period, 10, 64)
probeTickerPeriod, err := time.ParseDuration(pw.probeSpec.Period)
if err != nil {
pw.log.Errorf("could not parse probe period: %s", err)
return
}
probeTickerPeriod := time.Duration(probeTickerPeriodSeconds) * time.Second
maxJitter := probeTickerPeriod / 10 // max jitter is 10% of period
probeTicker := NewTicker(probeTickerPeriod, maxJitter)
defer probeTicker.Stop()
Expand Down

0 comments on commit 23e02c7

Please sign in to comment.