Skip to content

Commit d78897f

Browse files
myasnikovdaniilmyasnikovdaniil
authored
fix(cluster-api): resync capi-operator every minute (#4939)
## What this PR does After an upgrade restarts capi-operator, a Cluster API provider can keep `Ready=False` for about 20 minutes while its controller is already running, and `capi-providers-core` times out waiting for the CoreProvider. The bundled operator v0.19.0 drops create events of provider Deployments it finds running after a restart (kubernetes-sigs/cluster-api-operator#1031), so it looks at them again only when one changes or on informer resync, and the first resync after a restart comes two sync periods later, about 20 minutes with the default 10m. This sets the operator `--sync-period` to 1m. On a test cluster a provider left `Ready=False` across an operator restart recovered 2m02s after the operator took its lease, the operator with the default period had not recovered after more than 4 minutes. A resync is served from the operator cache and writes nothing while providers are in sync: no provider object was updated over five resyncs, about 15 log lines a minute, 2m CPU and 26Mi memory. The vendored chart has no value for this flag, so a small package patch (reapplied by `make update`) adds one, and a test holds the rendered argument. The upstream fix is in cluster-api-operator v0.27.0, bumping to it is a bigger change and is left for a later release. Mitigates #4926. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note fix(cluster-api): resync capi-operator every minute so a provider left not ready across an operator restart recovers in about two minutes instead of about twenty ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * The Cluster API operator now refreshes its cached state every minute. After an operator restart, provider readiness status can recover sooner instead of remaining stale for a longer period. * Providers that are already in sync are not written back to the API during these cache refreshes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents d24b92f + c2dab12 commit d78897f

5 files changed

Lines changed: 51 additions & 0 deletions

File tree

‎hack/capi-operator_test.bats‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#!/usr/bin/env bats
2+
# The operator's resync period is what bounds the stale-Ready window after an
3+
# operator restart (kubernetes-sigs/cluster-api-operator#1031): about two
4+
# periods. It reaches the manager only through patches/sync-period.diff, which
5+
# `make update` has to reapply on every re-vendor, and through the value in
6+
# values.yaml. Losing either silently returns the window to about 20 minutes,
7+
# so this checks the argument the rendered Deployment actually carries.
8+
9+
load test_helper
10+
11+
REPO_ROOT="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")/.." && pwd)"
12+
OPERATOR="$REPO_ROOT/packages/system/capi-operator"
13+
14+
@test "the operator manager runs with a one-minute resync period" {
15+
local rendered args
16+
rendered="$BATS_TEST_TMPDIR/operator.yaml"
17+
helm template capi-operator "$OPERATOR" --namespace cozy-cluster-api > "$rendered"
18+
args="$(yq -N 'select(.kind == "Deployment") | .spec.template.spec.containers[] | select(.name == "manager") | .args[]' "$rendered")"
19+
[ -n "$args" ] || { echo "no manager container args rendered from $OPERATOR" >&2; return 1; }
20+
case "$(printf '%s\n' "$args" | grep -c '^--sync-period=')" in
21+
1) ;;
22+
*) echo "expected exactly one --sync-period in the manager args, got:" >&2; echo "$args" >&2; return 1 ;;
23+
esac
24+
printf '%s\n' "$args" | grep -qx -- '--sync-period=1m' || { echo "manager args: $args" >&2; return 1; }
25+
}

‎packages/system/capi-operator/Makefile‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,6 @@ include ../../../hack/package.mk
99

1010
update: clean capi-operator-update
1111
rm -rf charts/cluster-api-operator/charts/
12+
# The upstream chart has no value for --sync-period; values.yaml explains
13+
# why we set it.
14+
patch -p1 < patches/sync-period.diff

‎packages/system/capi-operator/charts/cluster-api-operator/templates/deployment.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ spec:
8686
- --leader-elect-retry-period={{ .retryPeriod }}
8787
{{- end }}
8888
{{- end }}
89+
{{- with .Values.syncPeriod }}
90+
- --sync-period={{ . }}
91+
{{- end }}
8992
command:
9093
- /manager
9194
{{- with .Values.image.manager }}

‎packages/system/capi-operator/patches/sync-period.diff‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--- a/charts/cluster-api-operator/templates/deployment.yaml
2+
+++ b/charts/cluster-api-operator/templates/deployment.yaml
3+
@@ -86,6 +86,9 @@
4+
- --leader-elect-retry-period={{ .retryPeriod }}
5+
{{- end }}
6+
{{- end }}
7+
+ {{- with .Values.syncPeriod }}
8+
+ - --sync-period={{ . }}
9+
+ {{- end }}
10+
command:
11+
- /manager
12+
{{- with .Values.image.manager }}

‎packages/system/capi-operator/values.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
cluster-api-operator:
2+
# A restarted v0.19 operator ignores the provider Deployments it finds
3+
# running (kubernetes-sigs/cluster-api-operator#1031), so a stale
4+
# Ready=False on a provider clears only at the first informer resync,
5+
# two periods after the restart: ~20 min at the 10m default, ~2 min at
6+
# 1m (both measured). A resync is served from the cache and changes
7+
# nothing when providers are in sync, so the short period costs no API
8+
# writes. Applied through patches/sync-period.diff.
9+
syncPeriod: 1m
210
resources:
311
manager:
412
limits:

0 commit comments

Comments
 (0)