Skip to content

Verify whether the seaweedfs-db CNPG failed health-check predicate is reachable #3807

Open

Description

@myasnikovdaniil

packages/extra/seaweedfs/templates/seaweedfs-db.yaml wraps a CloudNativePG Cluster in a HelmRelease. Under helm-controller v1.5.0 a HelmRelease whose manifest is a custom resource would otherwise report Ready the moment helm applies the CR, before the operator has written any status, so the template sets waitStrategy.name: poller and supplies CEL healthCheckExprs that gate the release on the Cluster's own conditions. The three predicates, verbatim from lines 30-35:

  healthCheckExprs:
    - apiVersion: postgresql.cnpg.io/v1
      kind: Cluster
      inProgress: has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'False' && e.reason == 'ClusterIsNotReady')
      failed:     has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'False' && e.reason != 'ClusterIsNotReady')
      current:    has(status.conditions) && status.conditions.exists(e, e.type == 'Ready' && e.status == 'True')

The intent is readable from the comment above them: CNPG sets Ready=False with reason=ClusterIsNotReady during ordinary bootstrap, so treating any Ready=False as a failure would flag the release broken within seconds of install. inProgress therefore claims the bootstrap reason and failed is meant to catch every other Ready=False reason.

The concern is that failed may never be able to fire. The CHANGES_REQUESTED review on #3264 states that CNPG writes reason: ClusterIsNotReady for every non-healthy Ready=False, and that the differentiated cause goes into status.phase rather than into reason. If that is right, e.reason != 'ClusterIsNotReady' is unsatisfiable and the failed predicate is dead code.

The consequence, if the predicate is dead, is a gate that can stall but never fail. A genuinely broken Postgres cluster keeps matching inProgress instead of failed, so the HelmRelease sits in progress until timeout: expires rather than failing loudly with the operator's own diagnosis. Because the same release sets install.remediation.retries: -1 and upgrade.remediation.retries: -1, it then retries indefinitely, so the failure mode is a silent retry loop rather than a surfaced error. Anything gated on this release through dependsOn waits behind it.

This is already merged and running in production. It landed in #2601 (commit 29c6afc84) and is on origin/main today. It is also the pattern #3264 proposes to copy to packages/apps/harbor and to extend inside packages/extra/monitoring, which is what makes it worth settling before the pattern spreads.

The claim is not verified. Three things stand between the review's finding and a confirmed bug. The review cited CloudNativePG 1.27.1, while packages/system/postgres-operator/charts/cloudnative-pg/Chart.yaml:2 pins appVersion 1.28.2 on main, so the behaviour may differ from the version the finding was based on. CNPG is consumed as an upstream chart and image and its Go source is not vendored in this repository, so the condition-setting code was not readable during this audit. And no live cluster observation was made. Treat this issue as "this predicate may be dead code, here is how to find out" rather than as a confirmed defect.

What would settle it: read what CloudNativePG 1.28.2 actually writes into status.conditions[type=Ready].reason for a non-bootstrap failure, a cluster that has finished bootstrapping and then breaks, rather than one that never came up. If every Ready=False carries reason: ClusterIsNotReady, the failed predicate is unreachable and should key on status.phase instead, or drop the reason discriminator. If 1.28.2 emits a second reason, the predicate works as written and this issue closes with the version noted. The upstream function named in the review is SetClusterReadyCondition. Reading it in the 1.28.2 tree, or breaking a healthy cluster on a dev stand and inspecting kubectl get cluster -o jsonpath='{.status.conditions}', both answer the question.

Related but separate. The same review flagged a second predicate: the EtcdCluster gate status.brokenMembers > 0 proposed in #3264 is reported as always zero for the default PVC-backed storage, with the vendored CRD description ("While the auto-replacement predicate is a stub it is always 0") and upstream etcd-operator behaviour cited as evidence, and Degraded=True, reason=QuorumLost suggested as the replacement signal. That predicate is not merged, it exists only in the #3264 draft, so it stays inside that PR's review scope and is not part of this issue.

Refs #2642, #3264, #2601.

Activity

  1. added
    kind/bugCategorizes issue or PR as related to a bug
    area/databaseIssues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse)
    triage/needs-informationIndicates an issue needs more information in order to work on it
    area/extraIssues or PRs related to tenant-specific modules (packages/extra/)
    priority/important-longtermImportant over the long term, but may not be staffed and/or may need multiple releases to complete
    on Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/databaseIssues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse)area/extraIssues or PRs related to tenant-specific modules (packages/extra/)kind/bugCategorizes issue or PR as related to a bugpriority/important-longtermImportant over the long term, but may not be staffed and/or may need multiple releases to completetriage/needs-informationIndicates an issue needs more information in order to work on it

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions