[Feature]: bundled Percona versions move with an NVSentinel release, unguarded and unannounced, and can skip a minor or roll the datastore

[lfriedman-netllama]

Prerequisites

• I searched existing issues
• I can reproduce this issue

Summary

An NVSentinel release can move the bundled Percona versions, and nothing coordinates, validates or announces it. The result is that adopting a release can silently perform a Percona upgrade path that Percona forbids, and can silently roll the operator's replica set.

This is the structural version of #1737. That issue reported the bundled subcharts as internally mismatched (operator 1.23.0 against crVersion 1.21.1) and #1741 fixed it by aligning them, which was the right call. But for a deployment already running the old values, that alignment became an unannounced two-minor jump (1.21 -> 1.23), and Percona's own upgrade docs forbid skipping a minor. A correct consistency fix upstream turned into an unsupported upgrade downstream.

The mechanics

Four version knobs, all independent, with no coordinating input:

Value	Location
operator image tag	psmdb-operator.image.tag
crVersion	psmdb-db.crVersion
mongod image tag	psmdb-db.image.tag
init image tag	psmdb-db.initImage.tag

Verified in the v1.22.0 chart: there is no psmdbVersion-style single input, and no template guard anywhere in charts/mongodb-store/ that validates these against each other.

Four consequences, each confirmed on a live 288-node GB200 cluster:

1. A release bump moves three of the four at once, because they are subchart defaults.
2. Two of them roll the replica set. Any change to a field in the PerconaServerMongoDB CR makes the operator run SmartUpdate over the whole replica set, and crVersion and initImage both live there. So a release adoption can include a datastore roll that appears nowhere in the release notes. We saw this concretely: a single added replication.oplogSizeMB restarted all three mongod pods.
3. Operator/mongod compatibility is a real matrix and nothing checks it. From Percona's version service, operator 1.22.0 certifies mongod up to 8.0.19-7; 8.0.26-11 appears only from 1.23.0. A chart that paired operator 1.22 with mongod 8.0.26-11 would render happily and run an uncertified combination.
4. The bundled psmdb version is not discoverable from the NVSentinel version. NVSentinel v1.22.0 bundles psmdb-db 1.23.0. The only way to know is to read the subchart's Chart.yaml inside the packaged tarball.

Separately, and quietly: leaving initImage unset lets the operator derive it, which can resolve to an unmirrored percona/percona-server-mongodb-operator:<crVersion> pulled from Docker Hub. On a mirror-only or air-gapped cluster that fails at pod start with no configuration having changed.

What it costs

To move one Percona minor safely on our cluster we ran a four-rung ladder: operator 1.21->1.22, then crVersion+initImage 1.21.1->1.22.0, then operator ->1.23, then crVersion+initImage ->1.23.0. Four PRs, two deliberate datastore rolls, and roughly a day, most of it spent establishing facts this issue is asking to be made explicit: which minor was reachable, which mongod each operator certifies, and which fields roll the datastore.

The two failure modes this creates:

• For operators unaware of the coupling: adopting a release skips a Percona minor and rolls their datastore without warning. Neither is visible until something breaks, and the datastore holds every health event.
• For operators who are aware: four pins to carry, a manual ladder per minor, and the pin set drifting further from the chart with every release deferred.

Requests, cheapest first

1. Call out bundled-psmdb version changes in the release notes, explicitly stating that crVersion or initImage changing will roll the replica set. This alone removes most of the surprise, and chore: upgrade percona database #1741's release note is a good precedent, since it did carry an upgrade warning, which is how we caught this in time.
2. Fail the render when the jump is unsupported. When the deployed crVersion is more than one minor behind the chart's operator version, refuse rather than proceed. There is good precedent for this pattern in the chart already: nvsentinel.pcAuth.enabled fails the render rather than guessing on a non-boolean.
3. Expose one coordinated version input so operators pin a single value instead of four, and so the chart cannot ship an internally inconsistent set again. That would have prevented [Bug]: bundled psmdb subcharts are mismatched: operator 1.23.0 with crVersion 1.21.1, and the jump skips a Percona minor #1737 by construction.

Happy to take (1) or (2) as a PR if that helps. (2) in particular is a small named-template guard, and we have the deployed evidence to write a realistic test for it.

[lalitadithya]

hey @lfriedman-netllama , good points, I know this is a nuclear option, but what are your thoughts on not bundling the database in the chart and instead recommending that users set up the database out of band and we provide support for this as an external database? I suspect the longer we bundle this, the more we are going to keep running into this issue as we have clearly learnt that percona is more complex than bitnami

cc/ @kwombach12 , @KaivalyaMDabhadkar

[lfriedman-netllama]

Strongly in favour of external being a first-class supported path. Less so of making it the recommendation. Three points, then what I would do.

The escape hatch already exists, and it is good. global.mongodbStore.enabled: false plus datastore.credentialsFromSecret, and docs/external-datastore.md covering four providers. That doc already carries the thing unbundling actually requires, which is the compatibility contract: the engine floor of 5.0 for change streams, that AWS DocumentDB needs change streams explicitly enabled, and that Cosmos DB RU is unusable because it has none. So the mechanism is largely built. What is missing is not code.

Why I would still keep bundled as the default:

1. It is the only GitOps-representable option. We run 288 nodes under ArgoCD, and the reason we use the bundled datastore is that the CR lives in the chart, so drift is a diff and a rollback is a revert. External means a second lifecycle maintained by hand outside Argo. Recommending it moves work out of one chart and into every operator's runbook, and most will do it less carefully than the chart does.
2. It relocates the version coupling instead of removing it, and makes it fail later. Today the constraint is a real requirement expressed badly at render time. External, it becomes a runtime requirement discovered when a change stream watcher will not start. Your doc handles that well for the four services it names, and nothing handles it for the fifth.
3. Mirror-only clusters get worse, not better. Every image we run comes from our ECR mirror because the chart names it. If external becomes the recommendation, each operator sources their own MongoDB, and the ones without mirror discipline end up pulling from Docker Hub.

What I would do instead: keep bundled as the default, keep external as the supported alternative it already is, and do the three cheap things in this issue. None of them is wasted if you later flip the recommendation, and they matter precisely because bundled stays the default for most operators.

If you want a fourth, and it is the one I would prioritise under either strategy: a startup assertion in store-client on connect that checks the server version and that a change stream can actually be opened, failing with a message that names docs/external-datastore.md. Right now an unsupported datastore surfaces as a watcher that will not start. That check is the difference between "external is supported" and "external is supportable", and it is mandatory if external becomes the recommendation. Happy to PR it. Worth saying it does not belong in preflight: I looked, and that chart is a mutating webhook plus an image-cache DaemonSet, not an environment checker.

One caveat on framing. I do not think the lesson here is that Percona is more complex than bitnami. A chart that bundles any stateful operator inherits that operator's upgrade rules while presenting them as ordinary Helm values, and a bundled bitnami MongoDB would hand you a different instance of the same class of problem. "Do not bundle a stateful operator" is a defensible principle. "Percona specifically is the problem" would lead to swapping it for something that lands you back here.

[lalitadithya]

I had a huddle with Kai, and we agree that with your suggestions! Thanks again for raising them!

I do not think the lesson here is that Percona is more complex than bitnami. A chart that bundles any stateful operator inherits that operator's upgrade rules while presenting them as ordinary Helm values, and a bundled bitnami MongoDB would hand you a different instance of the same class of problem. "Do not bundle a stateful operator" is a defensible principle. "Percona specifically is the problem" would lead to swapping it for something that lands you back here.

agreed, apologies, I meant the same which is that bundling a stateful operator is not ideal for a helm chart.

The escape hatch already exists, and it is good. global.mongodbStore.enabled: false plus datastore.credentialsFromSecret, and docs/external-datastore.md covering four providers

I was thinking along the lines of adding percona/any other in cluster database to that list, so this allows customers to install and manage the lifecycle of the databases outside of NVSentinel while feeding it with minimal configuration. So, this would also allow the users to manage the in-cluster database using any gitops tool of their choosing.

agreed on points 2 and 3. 3 is a supply chain risk as well if the pulls are configured correctly. I also agree that keeping the bundle the default makes sense and it is also the easiest way for customers to get started with tool and this is not an option that we'd want to remove now or ever.

do the three cheap things in this issue.

agreed, as you mentioned earlier, can you open up PRs for 1 and 2? Also, would it be possible to do 3 without resulting in changes to the vendored files for the database install?

One thing that we wanted to ask you is, what are your thoughts on using a tool like AICR (https://github.com/NVIDIA/aicr) to install and manage the lifecycle of NVSentinel? The reason we ask this is that they are working on the NVSentinel integration as a part of NVIDIA/aicr#2623 and they are likely going to take the route of manage percona independently of the NVSentinel install. One followup question we have is also what are your thoughts on a NVSentinel operator much like GPU operator which would manage the upgrade, install and configuration of NVSentinel.

[lfriedman-netllama]

Thanks, and no apology needed on the bitnami point, we were saying the same thing.

Happy to take 1 and 2. I spent some time in the chart before answering because your question about the vendored files has a clean answer, and because testing turned up one constraint that changes what 2 can be.

2 cannot read deployed state, so it has to split

My original wording was "fail when the deployed crVersion is more than one minor behind the chart's operator". That cannot work as a render guard. lookup returns empty during helm template, which I confirmed:

lookupIsEmpty: "true"      # helm template, psmdb CR present in the cluster or not

ArgoCD renders with helm template, so a lookup-based guard would silently pass for every GitOps user, which is exactly the population most at risk. So:

• 2a, the render guard, which is the PR I would open. Validate the values the render is about to produce: operator tag against crVersion against mongod tag, chart defaults and operator overrides alike. Deterministic, no cluster access, behaves identically under helm template, helm upgrade and Argo. It makes [Bug]: bundled psmdb subcharts are mismatched: operator 1.23.0 with crVersion 1.21.1, and the jump skips a Percona minor #1737 impossible by construction and it catches a partial pin edit, which is the mistake an operator carrying pins actually makes.
• 2b, the deployed-state skip, belongs where something can read the cluster. The bootstrap Job already runs with datastore access on every sync, so comparing the live CR's crVersion against the chart's operator version there is the natural fit. Separate PR, and I would want your call first on whether failing that Job is acceptable given it gates the install.

3 is doable with zero changes to the vendored files

charts/mongodb-store/templates/ is yours; psmdb-operator and psmdb-db are the vendored Percona charts. A template in the former can read the effective values of the latter. Probing from a throwaway template in mongodb-store/templates/, with nothing set on the command line:

operatorTag: "1.23.0"      # index .Values "psmdb-operator" "image" "tag"
crVersion:   "1.23.0"      # index .Values "psmdb-db" "crVersion"
mongodTag:   "8.0.26-11"   # index .Values "psmdb-db" "image" "tag"

and with --set mongodb-store.psmdb-db.crVersion=9.9.9 it came through as 9.9.9, so the guard sees operator input too, not just your defaults.

The one thing that is genuinely not possible without touching psmdb-db/templates/cluster.yaml: mechanically feeding one value into the four subchart keys. Helm subchart values are static data, not templated, so a psmdbVersion input cannot rewrite psmdb-db.crVersion on its own.

So I would define "one input" as one declared version that the render enforces, not one value that feeds the subcharts. Add mongodb-store.psmdbVersion, and have the 2a guard fail when any of the four knobs disagrees with it. An operator sets one value, leaves three alone, and a partial edit is refused rather than deployed. That is better for you than a vendored patch would be anyway, since a patched cluster.yaml has to be re-applied every time you re-vendor a Percona release, and this does not.

There is already precedent for where it lives: mongodb-store.validateOplogStartup in charts/mongodb-store/templates/_helpers.tpl is chart-level validation in exactly that spot.

For 1, RELEASE.md has a Quality Gates section that looks like the right home for "a bundled datastore version change requires a release-note callout, stating that a crVersion or initImage change will roll the replica set".

Percona as an in-cluster external database

Agreed, and it is closer than it looks. global.mongodbStore.enabled is already false in values.yaml and in values-full.yaml; only the Tilt files and values-remediation.yaml set it true. So the bundled datastore is already opt-in at the top level, and what is missing is a documented recipe rather than a structural change: a section in docs/external-datastore.md for "run psmdb yourself, point NVSentinel at it", reusing the replica-set and change-stream requirements the doc already states. The detail I would check while writing it is the bootstrap Job's schema init against a database whose credentials NVSentinel did not create.

AICR

I read the README and epic #2623. No objection at all, and I would happily consume its recipes. It is a configuration generator that renders for Argo CD, Flux and Helmfile and explicitly is not a lifecycle manager, so it does not compete with our GitOps. #2623's success criteria also match the posture we arrived at independently: monitoring-only unless explicitly opted in, and a documented progression from STORE_ONLY through quarantine and draining to remediation. Their managing Percona independently is the same conclusion this issue reaches.

One ask: keep plain Helm a first-class path, and keep the version constraints in the chart rather than only in AICR recipes. Otherwise the compatibility knowledge moves into a tool that some operators do not run, and the chart stays exactly as unguarded as it is today. That is the argument for 2 and 3 regardless of what AICR does.

An NVSentinel operator

It would not fix this problem, so I would not do it for this reason. Version compatibility is a render-time constraint and belongs in the chart, which is where 2 and 3 put it. An operator earns its keep for what Helm cannot express: sequencing an upgrade against node drain state, coordinating with GPU or driver upgrades, staging a rollout by node pool. Those are real, and none of them is this.

If you do build one, three requirements from an operator's seat:

1. No autonomous upgrades and no phone-home. The first two things we had to switch off in Percona's operator were upgradeOptions.apply: disabled and DISABLE_TELEMETRY=true. An NVSentinel operator that upgrades itself on a schedule gets disabled on day one in every environment that cares, and then you are maintaining a feature nobody runs.
2. The CR carries the whole configuration, so it stays one object in git. The failure mode is an operator that writes back into fields of its own CR, which then fights the GitOps owner forever.
3. It must not become the only supported path, same as above.

Also worth designing against from the start: an operator whose CRD version is coupled to the app version reintroduces this exact issue one layer up.

I can start on 1 and 2a whenever you are happy with the 2a/2b split.

[lalitadithya]

agreed on 1 and 2a

[lfriedman-netllama]

Thanks. Starting on 1 now, since it is independent of everything below.

Two things I need from you before I write 2a, because they change what the template does rather than just how it is written.

1. Should ask 3 fold into the same PR as 2a? They are the same named template, so doing them together is less work than doing them in sequence and avoids writing the guard twice.

• 2a alone: the guard checks the four knobs against each other, which prevents a [Bug]: bundled psmdb subcharts are mismatched: operator 1.23.0 with crVersion 1.21.1, and the jump skips a Percona minor #1737-style internally inconsistent chart and catches a partial pin edit.
• 2a plus 3: add a mongodb-store.psmdbVersion input that declares the intended version, and have the guard fail when any of the four knobs disagrees with it. An operator then sets one value rather than four, and a half-finished edit is refused. Strictly more useful, and the same amount of template.

My preference is together, but it is your call since 3 adds a new public value to the chart and that is an interface decision, not just a correctness one.

2. For 2b, is failing the bootstrap Job acceptable? This is the one that catches the case in the original report, an operator whose deployed crVersion is more than one minor behind the chart's operator, which the render cannot see because lookup is empty under helm template.

The bootstrap Job is the natural place: it already runs with datastore access on every sync, so it can read the live CR. The question is what it should do when it finds an unsupported jump:

• Fail the Job. Loud and unmissable, but the Job gates the install, so this blocks a deployment that would otherwise have come up. That is arguably correct for an unsupported Percona upgrade path, and arguably too aggressive for a tool that is meant to be monitoring the cluster.
• Warn only, logging plainly and incrementing a metric, and let the install proceed. Safe, but easy to miss, which is how the original problem happened.

I lean towards failing, on the grounds that a silently unsupported datastore upgrade is worse than a blocked sync, but you own the product decision about whether NVSentinel should ever refuse to install itself. Tell me which and I will write it as a separate PR after 1 and 2a.

One thing that is not in question either way: whatever 2b does, it cannot be the only guard, because an air-gapped or first-time install has no live CR to compare against. 2a has to stand on its own.