feat: incorporate johnduhart fork customizations - #2
Merged
Conversation
…ption ApplyPodTemplateSpecOverrides and the JSONPatch path of ApplyDeploymentOverrides unmarshaled the patched JSON back into the still-populated target struct. encoding/json merges JSON arrays element-wise into existing slice elements, so patched list element i inherited leftover fields from the old element i: an env var added via a strategic merge override ended up with both value and valueFrom, and an added secret volume kept the configMap source of the volume it displaced -- both rejected by the API server. The patch output itself is correct; only the decode step corrupted it. Decode into a zeroed value and assign it to the target instead. The regression was introduced in alexandrevilain#720. Fixes alexandrevilain#793.
…mitting JSON nulls
The defaulting webhook re-serializes the whole TemporalCluster to compute
its admission patch. Optional pointer, slice and map fields whose json tag
lacked omitempty were serialized as JSON null when left unset (e.g. omitting
spec.authorization.jwtKeyProvider produced
"jwtKeyProvider":{"keySourceURIs":null,"refreshInterval":null}). The
api-server validates the webhook's own output against the CRD structural
schema, which rejects null for non-nullable fields, so an otherwise valid
TemporalCluster could not be applied.
Add omitempty to every +optional pointer, slice and map field in
api/v1beta1 whose tag was missing it. Nil values are now omitted from the
serialized object instead of rendered as null, and CRD-level defaults
(e.g. jobTtlSecondsAfterFinished) can apply as intended.
Value-typed fields (string/bool/int/struct) are deliberately left
untouched: per alexandrevilain#514, omitempty on
defaulter-set value fields (e.g. datastore skipCreate) makes every
serialization drop the field and causes useless mutating webhook patches.
json tags do not affect generated CRDs: make generate and make manifests
produce zero diffs under config/crd/bases.
The chart's bundled CRD aggregate (charts/temporal-operator/crds/ temporal-operator.crds.yaml) is only regenerated by the release-chart workflow at release time and the result is never committed back, so the committed copy drifted from config/crd/bases. It predates the overrides jsonPatch field introduced in alexandrevilain#875 and also lacks customSearchAttributes, allowSearchAttributeDeletion and permissiveMetrics, so installing CRDs from the committed chart rejects or prunes documented fields. Regenerated with the existing mechanism used by make artifacts/helm: kustomize build config/crd (kustomize v4.5.7). config/crd/bases itself was already in sync with the Go types (make manifests is a no-op).
…d mTLS The operator's own cluster client (used to reconcile TemporalNamespace, TemporalSchedule, etc.) relied on GetPublicClientAddress, which always returns the public frontend address when frontend mTLS is enabled — even when the internal frontend is enabled. On clusters combining frontend mTLS, authorization and the internal frontend, the operator therefore dialed the authorized public frontend and required a user-side grant for its identity, while temporal hard-wires the internal frontend to noop claim mapper/authorizer precisely for internal callers. Root cause: the internal frontend is served using the internode mTLS settings, not the frontend ones (its gRPC server uses the internode TLS group, whose client CA pool only contains the internode intermediate CA), so the frontend client certificate can't be used against it — which is why alexandrevilain#961 added the frontend mTLS carve-out to GetPublicClientAddress instead of switching certificates. Add GetOperatorClientAddress, which always prefers the internal frontend when enabled and delegates to GetPublicClientAddress otherwise, and make buildClusterClientOptions use it with a matching TLS selection: when the internal frontend is enabled, authenticate with the internode certificate when internode mTLS is enabled (the same certificate the server's own system worker presents to the internal frontend) and connect without TLS otherwise, since the internal frontend serves plaintext when internode mTLS is disabled. GetPublicClientAddress and the rendered server configuration are left untouched, preserving alexandrevilain#961 behavior for every cluster shape without internal frontend, and for the rendered publicClient stanza in all shapes. Closes alexandrevilain#957
The ReconcileError condition was only ever set to True by the error
handlers and never reset afterwards. A transient failure (e.g. an
optimistic-lock conflict updating a deployment) therefore stayed
reported as ReconcileError=True forever, even while every subsequent
reconcile succeeded and reported Ready=True and ReconcileSuccess=True,
which is misleading for anything monitoring status conditions.
Following the convention for abnormal-true conditions, the
SetTemporal{Cluster,Namespace,Schedule}ReconcileSuccess helpers now
keep the ReconcileError condition present and flip it to False (with
the success reason and an empty message) whenever a reconcile cycle
succeeds.
The chart's CRD aggregate (charts/temporal-operator/crds/ temporal-operator.crds.yaml) is only regenerated by the release-time make helm/artifacts targets, so a change under config/crd that forgets to refresh the aggregate lands silently and the committed chart drifts from the generated manifests -- exactly the drift fixed in 12d0379. Add a verify-chart-crds Makefile target that installs the pinned kustomize (v4.5.7) via the existing installer, builds config/crd and diffs the result against the committed aggregate, failing with the regeneration instruction on mismatch. Wire it into the tests workflow as a chart-crds job running on pull_request and push to main.
Adds operator support for Temporal Server v1.29, v1.30 and v1.31, extending the
supported version range to `>= 1.14.0 < 1.32.0` (default version 1.31.1, default
UI 2.49.1). All >= 1.30 behaviour is version-gated (version.V1_30_0 / V1_31_0);
clusters < 1.30 keep the previous dockerize/curl paths unchanged, so the operator
stays backward compatible across the whole supported range.
v1.29
- Version range/default bumps only. v1.29 is a dynamic-config-only release
(task-queue fairness, task-queue config API), already covered by the cluster
dynamicConfig field.
v1.30
- dockerize/auto-setup were removed from the temporalio/server image and config
templating moved into the server binary (embedded sprig engine). For clusters
>= 1.30 the operator now emits config templates with the `# enable-template`
header and sprig `{{ env "X" }}` placeholders (instead of dockerize
`{{ .Env.X }}`), sets TEMPORAL_SERVER_CONFIG_FILE_PATH, and selects the service
via the new TEMPORAL_SERVICES env var (legacy SERVICES kept for compatibility).
- curl and jq were removed from the temporalio/admin-tools image, which broke the
operator's Elasticsearch visibility setup scripts. For clusters >= 1.30 the
operator now drives ES visibility through the temporal-elasticsearch-tool
shipped in the image (setup-schema, create-index, update-schema), analogous to
temporal-sql-tool. Its embedded index template applies all built-in search
attributes automatically. The MTLS sidecar-shutdown step uses wget instead of
curl on >= 1.30.
- v1.30.0 has no published GitHub release upstream (silently skipped) and is now
rejected as a broken release; use v1.30.1+.
v1.31
- New sql.passwordCommand datastore field: resolves the datastore password by
running an external command (e.g. to generate a short-lived cloud IAM auth
token for AWS RDS / GCP Cloud SQL). Wired into both the rendered server config
and the persistence schema-setup jobs (temporal-sql-tool via a shell command
substitution). Mutually exclusive with passwordSecretRef and validated by the
webhook (rejected on clusters < 1.31 and when combined with a password secret).
Build / dependencies
- go directive bumped to 1.26.4 with the Dockerfile builder image updated to
match.
- go.temporal.io/server v1.31.1, go.temporal.io/api v1.62.8,
go.temporal.io/sdk v1.41.1, plus the associated Kubernetes dependency bumps.
Testing
- make test (unit + envtest) is green, including new unit tests that load the
generated 1.30 config through the real go.temporal.io/server config loader and
cover the passwordCommand and ES-tool script rendering.
- Validated end-to-end on a real RKE2 cluster (k8s v1.35): 1.29.7, 1.30.5 and
1.31.1 clusters each reach Ready=True with a working namespace-create +
workflow round-trip; the 1.30 sprig/entrypoint contract, passwordCommand auth
(including webhook rejection paths) and the temporal-elasticsearch-tool ES
visibility setup (index + v10-v13 built-in search attributes) were exercised
against real images.
Signed-off-by: Ivan Milchev <ivan@mondoo.com>
PR alexandrevilain#987 extended defaultUpgradePath with 1.29.7/1.30.5/1.31.1, which is correct, but also moved newDatastoreVersion from 1.24.3 to 1.31.1. Those two variables play opposite roles: newDatastoreVersion is the version the cluster is CREATED at (persistence_test.go:110,157,262,307), and defaultUpgradePath is the sequence it is then upgraded THROUGH. With both at 1.31.1 the first upgrade step asks for 1.25.2, a six-minor downgrade that ValidateUpdate rejects via UpgradeConstraint. Restoring 1.24.3 makes the walk 1.24.3 -> 1.25.2 -> ... -> 1.31.1 again, so each step is the single-minor increment the constraint allows.
PR alexandrevilain#987 added v1.30.0 to ForbiddenBrokenReleases but missed the other two retracted releases in the supported range. Upstream's own go.mod carries the authoritative list: retract ( v1.30.0 v1.26.1 // Contains retractions only. v1.26.0 // Published accidentally. ) Confirmed independently: none of the three has a published GitHub release, while v1.26.2 and v1.30.1 do. Without this, the webhook would accept a spec.version that has no corresponding container image and the cluster would sit in ImagePullBackOff. Also rewords the v1.30.0 comment to cite the retraction rather than the absent release page, since the retract block is the primary source.
PR alexandrevilain#987 moves the module to go 1.26.4, which the pinned golangci-lint v1.64.8 cannot lint: the prebuilt binary is built with go1.24.1 and exits 1 with 'package requires newer Go version go1.26 (application built with go1.24) (typecheck)'. v1.64.8 is the last v1 release, so there is no v1 version to move to - v2 is the only way forward. - .github/workflows/tests.yaml: GOLANG_CI_VERSION v1.64.8 -> v2.12.2, and golangci-lint-action v6 -> v9 (v6 cannot drive a v2 binary). v2.12.2 is built with go1.26.2, the same language version as our module. - Makefile: same version bump, plus the /v2 module suffix that the v2 install path requires. - .golangci.yaml: converted by 'golangci-lint migrate'. v2 merges gosimple, stylecheck and typecheck into staticcheck, and moves gofmt and goimports into a formatters section. The migration is deliberately signal-neutral - it should not smuggle in unrelated refactors or suppressions. Two settings restore the v1 scope: - staticcheck: exclude QF*. v1 applied 'all' to stylecheck, where it meant the ST* checks; under the merged linter 'all' also pulls in the QF* quickfix suggestions, which v1 never ran (18 findings, all pre-existing). - goconst: ignore-tests. Table-driven tests repeat short literals by nature and v1 did not report them (~69 findings, all pre-existing). Dropped the stale run.go: "1.22" pin so the language version derives from go.mod rather than silently holding linters to older semantics. Genuine findings are fixed rather than suppressed: - pkg/version/version.go: //nolint:stylecheck -> //nolint:staticcheck. The old directive silently stopped matching after the merge, which un-suppressed ST1003 on all nine V1_x_x constants. - govet: disable the inline analyzer. It reports 'cannot inline: type parameter inference is not yet supported' on generic calls such as slices.Contains - the analyzer describing its own limitation, and it only started firing at go 1.26. - prealloc (4) and goconst (1): preallocate the schema-job slices and the e2e feature table, and hoist the repeated 0.0.0.0 bind address to a constant. Verified 0 issues both via the prebuilt binary CI uses and via make lint, which installs from source.
PR alexandrevilain#987 declared Timeout as metav1.Duration - a struct - with omitempty. omitempty has no effect on structs, so the field always serializes, and the mutating webhook would emit "timeout":"0s" on every cluster that uses passwordCommand without setting a timeout. That is the same class of useless webhook patch churn our 8393ba1 removed from 21 other fields. Making it *metav1.Duration lets omitempty work, and the consumer in pkg/temporal/persistence/config.go now only sets the server-side Timeout when the user actually specified one, so the server applies its own default rather than receiving a hard 0s. Regenerating required bumping controller-gen v0.16.3 -> v0.21.0: the old version cannot build under the go 1.26.4 toolchain alexandrevilain#987 introduces (golang.org/x/tools v0.24.0 fails to compile). The regenerated output is almost identical, with one substantive and welcome difference: cassandra consistency / serialConsistency: type: integer -> type: string gocql.Consistency is uint16 underneath but implements MarshalText, so it serializes as a string, and the field already carried string enums (ANY, ONE, LOCAL_QUORUM, ...). v0.16.3 typed it from the underlying kind and produced a schema where those enum values could never validate. v0.21.0 honours the TextMarshaler and emits the correct type. This fixes a latent bug for Cassandra users; we run postgres12 so we are unaffected. Chart CRDs regenerated to match; make verify-chart-crds passes.
Decided on platform-temporal#26: alexandrevilain#987 forces client-go, api and apimachinery to 0.35.1 because go.temporal.io/server v1.31.1 requires them, but left controller-runtime at v0.21.0, which targets client-go 0.33.0. Running the layer that drives every reconcile two minors ahead of its tested client-go is the kind of skew that surfaces as subtle informer/cache/watch misbehaviour rather than a build error, so we take the matched pair instead. v0.23.x is the release paired with client-go 0.35 (v0.24.x pairs with 0.36). The bump also pulled apiextensions-apiserver and component-base from 0.33.3 to 0.35.0, closing the rest of the skew alexandrevilain#987 left behind. Two API changes needed handling: - The webhook builder is now generic: NewWebhookManagedBy takes the object and .For() is gone. Migrated to the typed builder rather than the deprecated CustomDefaulter/CustomValidator aliases, which let the four webhook methods take *v1beta1.TemporalCluster directly and made getClusterFromRequest and its five call sites redundant. No coverage is lost - no test exercised the wrong-type path it guarded. - mgr.GetEventRecorderFor is deprecated in favour of GetEventRecorder, but the replacement returns events.EventRecorder rather than record.EventRecorder. Those are different interfaces, and switching moves event emission from the core v1 API group to events.k8s.io/v1 - an observable behaviour change that does not belong in a dependency bump. Kept with a documented nolint; there is a single Event call site (controllers/temporalcluster_controller.go:268) so the migration is cheap whenever we choose to do it. Verified with a cold golangci-lint cache: a warm cache reported both a false 0-issues result and a false 'unused nolint directive'.
…cripts Three defects in the temporal-elasticsearch-tool path added for >= 1.30, the worst of which reports success while doing nothing. An empty Elasticsearch username rendered a bare "--user". argsMapToString renders an empty value as a flag with no value, and the tool's parser (urfave/cli v1 over the stdlib flag package) takes the *following* token as a string flag's value. For the setup script that token is the "setup-schema" subcommand itself, so the tool ran no command at all, printed its help and exited 0 -- the schema job was recorded successful while neither the index template nor the visibility index was ever created, and the failure only surfaced later as query errors from the frontend. Verified directly against go.temporal.io/server v1.31.1's BuildCLIOptions: with a bare --user the "user" flag comes back as "setup-schema" and no command action fires. An empty username is what an auth-less Elasticsearch needs and the CRD permits it (username is required but has no minimum length), so this was reachable. The same rendering hazard existed for the SQL and Cassandra --user flags; those fail loudly rather than silently, but they are guarded here too. "set -eu" in both new templates could skip the shared "scripts" footer. Every other template deliberately omits set -e so that footer always runs. With ES visibility plus a linkerd or istio provider, a failing tool invocation exited immediately and the sidecar was never told to shut down, leaving the Job pod Running indefinitely instead of failing and retrying -- and persistence reconciliation blocked on that job forever. The steps are now chained with &&, which keeps fail-fast behaviour while leaving $? for the footer to propagate. Chaining is safe because create-index is idempotent: the tool treats resource_already_exists_exception as success. The wget shutdown branches appended "|| true" while the curl branches did not. Since $x is captured before the shutdown call, that could only ever matter under set -e; with set -e gone it just hid an unreachable proxy, so both providers now behave identically. Checked the image rather than assuming: temporalio/admin-tools:1.31.1 ships BusyBox v1.37.0 wget (which supports --post-data) and no curl at all. Finally, getStoreTool no longer returns "temporal-elasticsearch-tool" unconditionally. That binary does not exist in admin-tools <= 1.29, and the >= 1.30 gate was duplicated in two callers, so any third caller would have emitted a command that cannot run. The gate now lives in one helper and the empty sentinel is restored for older versions.
…sking for it
Merging the existing pod template metadata first, as the preserve-annotations
change did, made every operator-managed key permanent. The feature helpers
return an *empty* map when their feature is disabled rather than a removal
signal, so nothing they had previously set could ever be deleted.
Concretely: create a cluster with spec.mTLS.provider istio, then remove the
mTLS block. istio.GetLabels/GetAnnotations return {}, so
`sidecar.istio.io/inject: "true"` and `proxy.istio.io/config` survived from the
existing template and istio kept injecting sidecars indefinitely. The same held
for `linkerd.io/inject` after a provider switch, and for the `prometheus.io/*`
scrape annotations after setting spec.metrics.enabled to false.
Keys under the metadata namespaces this operator computes are now dropped from
the existing template before the freshly computed set is overlaid. Everything
else is still preserved untouched, which is the whole point of merging:
`kubectl.kubernetes.io/restartedAt` and other externally-written annotations
must survive reconciliation.
Prefixes are used rather than an exact key list so that a helper gaining a new
key (a fifth prometheus.io annotation, say) does not silently reintroduce the
bug.
The existing tests only covered the additive direction; tests are added for
removal, for the enabled case still working, and for a stale version label.
… integers normalizeJSONNumbers had two problems, the second reintroducing the very bug it exists to prevent. It narrowed to int unconditionally, so on a 32-bit build a value such as 5368709120 (a 5 GiB blob-size limit) wrapped to a different number, and that number was written into the dynamic_config.yaml ConfigMap. Values that do not fit now stay int64; values that do are still int, which is what yaml.v3 produces when it unmarshals the config back and what keeps the reconciliation deep-equal comparison stable. More importantly, a value written in exponent form -- `limit.blobSize.error: 1e9`, valid JSON -- is not parseable by json.Number.Int64, so it fell through to Float64 and yaml.v3 wrote it back as "1e+09". Temporal's file-based dynamic config client rejects scientific notation for a setting that expects an integer, which is exactly the failure this normalisation was added to avoid. Integral values are now converted back to an integer type. Note that returning the number's original text instead would not help: that emits a quoted YAML string, which Temporal rejects just the same. Text is kept only for values too precise for float64, where the alternative is silently dropping digits.
The webhook's rejection message for a broken release suggested IncPatch(), but broken releases can be consecutive: v1.26.0 and v1.26.1 are both retracted upstream, as are v1.21.0 and v1.21.1. A user applying 1.26.0 was told to move to 1.26.1 and was then rejected again, with no hint that 1.26.2 is the real target. NextNonBrokenPatch skips any candidate that is itself forbidden. Also memoize the compiled semver constraints behind GreaterOrEqual and LessThan. Both formatted a constraint string and recompiled it on every call, and they are called repeatedly within a single reconcile -- once per datastore, once per deployment builder, once per config section -- nearly always against the same handful of package-level version constants. The previous code also discarded the parse error, which would have made Check dereference a nil *Constraints had the format string ever changed.
…asswordCommand The existing test asserted the buggy suggestion (1.21.0 -> 1.21.1, itself a forbidden release); it now asserts 1.21.2. Also warn when sql.passwordCommand is set. The field works for the server pods, which resolve the password natively, but the persistence schema jobs run the same command inside the admin-tools image and that image cannot be extended: SchemaJobBuilder.Build hardcodes the pod's volumes to the scripts ConfigMap plus datastore TLS, and exposes only JobInitContainers/JobResources/ JobTTLSecondsAfterFinished -- an init container has no shared writable volume through which to hand a binary over. So for the documented use case, an RDS or Cloud SQL IAM token helper, the command is not found, the substitution yields an empty string, and the first create-database job fails with a password-authentication error that is hard to attribute. Users get told this up front instead of discovering it there. Giving the schema jobs pod-level overrides is a feature in its own right and is tracked separately.
The path /etc/temporal/config/config_template.yaml was written out three times -- as the TEMPORAL_SERVER_CONFIG_FILE_PATH value, as the config volumeMount's MountPath, and as its SubPath -- plus a fourth time as the ConfigMap key in the config builder. Changing the mount path or the key in one place would leave the others pointing at a file that does not exist, and on Temporal >= 1.30 (which has no fixed built-in location and relies on the env var) the server would exit at startup with "could not read config file". Nothing links the four at compile time and no test covers the pairing.
The 1.30 and 1.31 entries each stated a default Temporal/UI version and a supported range, so the Unreleased section named two different sets of defaults in consecutive bullets. Only the 1.31 values match temporalcluster_defaults.go; the 1.30 entry now describes the mechanism change without restating a superseded default. Document the CRD schema change that came in with the controller-gen v0.16.3 -> v0.21.0 bump: cassandra.consistency and cassandra.serialConsistency move from type: integer to type: string. This is a fix -- the old schema declared type: integer alongside string enum values, so no value could ever validate, and gocql.Consistency has always marshalled as text -- but it is a schema change to pre-existing user-facing fields, and it was bundled inside a tooling bump in a PR about 1.29/1.30/1.31 support. It belongs in the changelog. Also record the known limitation of sql.passwordCommand in the schema jobs, and add a Fixes section for the pod-metadata, broken-release-suggestion and dynamic-config-integer defects. Finally, replace the assertion in the goconst comment with the evidence for it. The claim that v1 did not report these findings is testable: the v1 config carried no test exclusions at all (only zz_generated), the lint job was green on upstream main 1398896 with v1.64.8, and the files this setting affects already existed at that commit. So this restores v1's signal rather than suppressing it.
The lint job restores a golangci-lint cache whose contents change the verdict. Two runs of this workflow over a byte-identical main.go disagreed: the run that populated the cache reported no issues, and the next run, which restored it, reported both `//nolint:staticcheck // SA1019` directives in main.go as unused. The cached verdict is the wrong one. Deleting those directives and running cold shows SA1019 firing at main.go:116 and main.go:136 (`mgr.GetEventRecorderFor` is deprecated in controller-runtime v0.23), so the directives are used and necessary. The cache fails in the other direction too: a warm local cache reported "0 issues" on a tree that genuinely had one, which is the more dangerous failure since it is silent. setup-go already runs with cache: false in this job. The cached run still took 130s, so the cache buys roughly nothing while making lint results depend on which commit last populated it.
…rgets TestPersistence is about to go from 8 version-steps to 35 as the skip filter that hid five of its six cases is removed. Measured against the current suite, one case (cassandra, 8 steps) takes 308-412s depending on runner, and the whole e2e step takes ~18m; the extra cases push the package well past the old 60m budget's comfortable margin. E2E_TIMEOUT defaults to 90m and is overridable. Also introduce CONTAINER_TOOL for the local-development targets. It prefers podman when installed and falls back to docker, so a podman-only machine works without extra flags while CI, which has docker, is unaffected.
TestPersistence has skipped all but "cassandra persistence" since 624c28f (2024-12-01). That left the pure-SQL upgrade paths -- the ones most deployments actually use -- completely unexercised. The cassandra case does use postgres12 for its *visibility* store, so SQL visibility migrations had incidental coverage, but nothing verified a SQL *default* store surviving an upgrade, which is the shape this operator is most often deployed in. Removing the filter turns on five more cases and takes the suite from 8 version-steps to 35. One of those five could never have passed. "postgres persistence with ES advanced visibility" set spec.persistence.advancedVisibilityStore while creating the cluster at 1.24.3, and the webhook has forbidden that field for clusters >= 1.24 since 84722d5 (2024-09-26) -- Temporal 1.24 folded "advanced visibility" into plain visibility. Admission would have rejected it. The skip filter landed later and hid it, so the case has been both dead and invalid. It is revived in the supported shape, Elasticsearch as the visibility store, and moved to defaultVersion (>= 1.30) rather than 1.24.3. That is where the ES code actually needs coverage: admin-tools >= 1.30 dropped curl and jq, so the operator drives Elasticsearch through temporal-elasticsearch-tool, and that path had no end-to-end coverage at all despite being new. Nothing is lost by not exercising the older curl path here, since this case has not run since 2024. All six cases were checked against the real validating webhook at their creation version and at every rung of their upgrade path before this change: zero rejections. Also drop a dead branch in deployAndWaitForTemporalWithPostgres whose two arms assigned the same plugin name, and preallocate featureTable now that the loop appends unconditionally.
The table stopped at 1.26, so nothing pinned the tag scheme for the versions this fork just added support for. 1.30 is the interesting one: it is where the admin-tools image was stripped to bare alpine and gained temporal-elasticsearch-tool, but the tag naming is unchanged, so the major.minor rule still applies and should stay asserted.
The first run of the full persistence matrix surfaced two failures. Neither is
caused by the operator changes in this branch; both are exposed by running 35
upgrade steps where only 8 ran before.
AssertClusterCanHandleWorkflows port-forwards to the frontend and runs a
workflow with no retry. A cluster reporting Ready does not guarantee the
frontend Service has stopped routing to a pod still terminating from the
rolling update, so this can fail transiently. It did, once, on one of four
Kubernetes versions, at the very first rung of the legacy postgres path. At 8
upgrade steps a per-step failure rate that small goes unnoticed; at 35 it is a
regularly red build. The connect-worker-workflow cycle now returns an error
instead of failing the test, and is retried for up to a minute.
mysql8 is a real failure, so it is documented and bounded rather than papered
over. Upgrading a mysql8 cluster to 1.29.7 leaves it permanently un-Ready: the
operator updates every Deployment successfully and the pods then never reach
Ready, timing out the 600s wait. It reproduced on all four Kubernetes versions.
It is not flakiness or resource exhaustion -- in two of the four jobs mysql8 ran
second, on a barely loaded node, and still failed at exactly that rung, while
postgres ran last in those same jobs and passed.
Three candidate causes were ruled out directly rather than by argument:
- the schema migration. Running the real 1.17 -> 1.18 mysql8 update
(v1.18/tasks_v2.sql) with temporal-sql-tool from admin-tools:1.29 against
MySQL 8.4.11 succeeds cleanly.
- the server. temporalio/auto-setup:1.29.7 with DB=mysql8 starts and serves
against that same MySQL.
- the migration content. mysql8 and postgresql12 receive the same
1.17 -> 1.18 migration, and postgres12 walks the entire path fine.
Root-causing it needs the failing pods' logs, which the e2e artifacts do not
capture: kind exports logs after the test namespace is torn down, so the
namespace is already gone. Rather than block the SQL coverage this change
exists to provide, mysql8 is held at the last version it is known to reach.
Restore defaultUpgradePath there once the 1.29 failure is understood.
aalbertengo-mdsol
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR brings in commits from the johnduhart/temporal-operator fork that are ahead of the upstream alexandrevilain/temporal-operator.
Changes Included
New Features
Bug Fixes
CI/Build Improvements
Tests
Other
Files Changed
52 files changed with 3786 insertions and 759 deletions, including new test files for improved coverage.