fix(operator/cnpg): raise admission webhook timeouts for hosted control planes - #19
fix(operator/cnpg): raise admission webhook timeouts for hosted control planes#19mariomeyer wants to merge 1 commit into
Conversation
…ol planes
CNPG ships its admission webhooks with timeouts tuned for a control plane
co-located with the cluster — 10s mutating, 15s validating, both with
failurePolicy: Fail. On a hosted control plane the API server runs outside
the cluster, so every admission call tunnels back in (konnectivity on
Rackspace Spot) and the round trip costs seconds rather than milliseconds.
Measured on iad-1, ten consecutive mcluster.cnpg.io probes:
2.0s 2.2s 3.0s 3.1s 3.4s 4.7s 6.5s 7.2s 7.8s 10.2s
One in ten already exceeded the 10s ceiling. The operator was idle
throughout (8 millicores, 103Mi, no restarts) and all konnectivity agents
were healthy with no restarts in 14-34 days, so the latency is in the
network path, not the operator. Concurrent admission — a Pulumi preview
dry-running several resources at once — pushes more calls over the limit,
and with failurePolicy: Fail that aborts the run partway through an
update. It presented as an intermittent
failed calling webhook "mcluster.cnpg.io": context deadline exceeded
that a pod restart appeared to fix, because a fresh pod re-establishes the
tunnel, only for it to drift back within minutes.
Raised to 30s, roughly 3x the observed p100. failurePolicy stays Fail, so
nothing bypasses admission — this only stops well-formed calls being cut
off in flight.
The chart exposes webhook.{mutating,validating}.{create,failurePolicy} but
not timeoutSeconds, so this cannot go through Helm values. Applied as
MutatingWebhookConfigurationPatch / ValidatingWebhookConfigurationPatch
instead: `webhooks` is a listType=map keyed by `name`, so each entry merges
by key and no other field is touched. retainOnDelete keeps the raised
timeout in place if the patch resource is removed, rather than reverting a
live cluster to a value that breaks it.
Applies only to cloudnative-pg operator installs.
Note the CNPG operator co-owns f:webhooks (it injects the CA bundle), so
whether it reconciles timeoutSeconds back down needs watching after the
first deploy. If it does, the fix belongs upstream in the chart.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52bf898389
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "mscheduledbackup.cnpg.io", | ||
| ].map((name) => ({ name, timeoutSeconds: webhookTimeoutSeconds })), | ||
| }, | ||
| { provider, dependsOn: [helmRelease], retainOnDelete: true } |
There was a problem hiding this comment.
On an initial deployment, these patches and the CNPG Cluster resources are siblings that depend only on helmRelease: createSingleCnpgCluster() still builds its dependency list from the release and namespace, while neither patch resource is retained or passed into that path. Pulumi may therefore submit a Cluster admission request concurrently with, or before, the timeout patch, leaving the first update exposed to the same 10-second failure this change is intended to prevent. Collect both patch resources and include them in the dependencies used by CNPG custom resources.
Useful? React with 👍 / 👎.
| // the field instead: `webhooks` is a listType=map keyed by `name`, so each | ||
| // entry merges by key and no other field of the configuration is touched. | ||
| // In particular failurePolicy stays Fail — nothing bypasses admission. | ||
| if (type === "cloudnative-pg" && !skipOperatorInstall) { |
There was a problem hiding this comment.
When callers set config.values.webhook.mutating.create or config.values.webhook.validating.create to false, the chart deliberately omits the corresponding configuration, but this condition still registers both partial patch resources. With no Helm-created object to merge into, the update fails because the patch either cannot find its target or attempts to create an invalid configuration containing incomplete webhook entries. Gate each patch on the corresponding chart setting.
Useful? React with 👍 / 👎.
| webhooks: [ | ||
| "mbackup.cnpg.io", | ||
| "mcluster.cnpg.io", | ||
| "mdatabase.cnpg.io", |
There was a problem hiding this comment.
For a caller pinning config.version to a CNPG chart from before the Database webhook was introduced, mdatabase.cnpg.io is not an existing list-map entry. Server-side apply then treats this partial {name, timeoutSeconds} value as a new webhook rather than a field update, and Kubernetes rejects it because required fields such as clientConfig, sideEffects, and admissionReviewVersions are missing. The validating list has the analogous vdatabase.cnpg.io problem, so the implementation should constrain supported chart versions or patch only entries known to exist in the selected version.
Useful? React with 👍 / 👎.
1 participant
Problem
Pulumi operations against a CNPG-backed stack fail intermittently with:
With
failurePolicy: Fail, this aborts the run partway through an update — leaving the stack half-applied. Oniad-1it killed three separatepulumi upruns in one session, each time after some resources had already been modified.It looks like a broken operator, and a pod restart appears to fix it. Neither is true.
Root cause
CNPG ships its admission webhooks with timeouts tuned for a control plane co-located with the cluster — 10s mutating, 15s validating. On a hosted control plane the API server runs outside the cluster, so every admission call has to tunnel back in (konnectivity, on Rackspace Spot). That round trip costs seconds, not milliseconds.
Ten consecutive
mcluster.cnpg.ioprobes oniad-1:Range 2.0–10.2s, median ~4.5s, against a 10s ceiling. One in ten already exceeded it. For reference, an in-cluster admission webhook normally answers in single-digit milliseconds — this is ~1000× that.
The operator is not the bottleneck:
The latency is in the network path. Concurrent admission — a Pulumi preview dry-running several resources at once — pushes more calls past the ceiling, which is why it correlates with
pulumiruns and not with cluster load.Restarting the operator "fixes" it only because a fresh pod re-establishes the tunnel; it drifts back within minutes. Confirmed twice.
Change
Raise all CNPG admission webhook timeouts to 30s (~3× observed p100):
cnpg-mutating-webhook-configurationmbackup,mcluster,mdatabase,mscheduledbackupcnpg-validating-webhook-configurationvbackup,vcluster,vdatabase,vpooler,vscheduledbackupScoped to
cloudnative-pgoperator installs.Why a Patch and not Helm values
The chart exposes only
webhook.{mutating,validating}.{create,failurePolicy}— nottimeoutSeconds. It cannot be expressed through Helm values, so this usesMutatingWebhookConfigurationPatch/ValidatingWebhookConfigurationPatch.webhooksis alistType=mapkeyed byname, so each entry merges by key and no other field of the configuration is touched.Deliberate choices
failurePolicystaysFail. Nothing bypasses admission. This only stops well-formed calls being cut off in flight — it does not weaken validation.retainOnDelete: true. If the patch resource is removed, the raised timeout stays. Reverting a live cluster to a value that demonstrably breaks it is worse than leaving a harmless orphan.Verification
Known unknown
The CNPG operator co-owns
f:webhooks— it writes there to inject the CA bundle:Whether it also reconciles
timeoutSecondsback down is untested. Under server-side apply the patch should claim only that field and hold, but that is a prediction, not a verified fact. Worth watching after the first deploy — if the operator reverts it, the fix belongs upstream in the chart instead.