feat: update defaults to Postgres 18, CNPG 1.29, and cert-manager latest#51
Conversation
📝 WalkthroughWalkthroughDefault PostgreSQL image tag changed from 17 to 18 across configs and docs; CloudNativePG references updated to 1.29; cert-manager manifest URLs switched to the moving "latest" release in docs and test manifests. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/usage/postgres_upgrades.md (1)
7-7:⚠️ Potential issue | 🟡 MinorFix remaining version-text mismatches in the upgrade narrative.
A few lines still refer to the old progression and one grammar issue remains, which can confuse operators following this guide.
✏️ Suggested doc patch
-This chart utilizes a mutable tag by default to pull the latest pgEdge Enterprise Postgres image for Postgres 17 and Spock 5. +This chart utilizes a mutable tag by default to pull the latest pgEdge Enterprise Postgres image for Postgres 18 and Spock 5. @@ -Using this approach, you can gradually upgrade to Postgres 17 by removing existing Postgres 16 nodes. +Using this approach, you can gradually upgrade to Postgres 18 by removing existing Postgres 17 nodes. @@ -For example, given the following two node configuration running Postgres 17: +For example, given the following two-node configuration running Postgres 17: @@ -You can perform a major version upgrade from 17 to 18 by specifying a new image for Postgres 17: +You can perform a major version upgrade from 17 to 18 by specifying a new image for Postgres 18:Also applies to: 97-97, 105-105, 121-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/usage/postgres_upgrades.md` at line 7, The upgrade narrative still contains outdated version references and a grammar error — update every occurrence of the old progression language to match the current progression ("Postgres 17 and Spock 5") and remove the ambiguous phrase about a "mutable tag by default"; revise the sentence to read clearly (e.g., "This chart uses a mutable tag by default to pull the latest pgEdge Enterprise Postgres image for Postgres 17 / Spock 5.") and make parallel fixes for the other mismatched lines that mention the previous progression (the similar phrases around the same paragraph and the occurrences corresponding to the other reported locations). Ensure tense and plurality are consistent across the document and that all version mentions use the exact "Postgres 17" and "Spock 5" wording.
🧹 Nitpick comments (2)
docs/install.md (1)
120-120: Update cert-manager to use a pinned version instead oflatestfor reproducible deployments.Line 120 uses
releases/latest, which resolves to different versions over time (currently v1.20.2). This makes production and documentation examples non-reproducible across environments and dates. Pin to a specific release version:Suggested fix
kubectl apply -f \ - https://github.com/cert-manager/cert-manager/releases/latest/download/cert-manager.yaml + https://github.com/cert-manager/cert-manager/releases/download/v1.20.2/cert-manager.yaml(Update this version periodically as new cert-manager releases are tested with pgEdge Helm.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/install.md` at line 120, The docs reference to the cert-manager manifest currently uses the floating URL "releases/latest/download/cert-manager.yaml", which makes installs non-reproducible; update that URL to a pinned release (for example replace "releases/latest" with the specific tag like "releases/download/v1.20.2" or the currently vetted cert-manager version) so the manifest points to an exact version of cert-manager.yaml and document that this pinned version should be updated periodically after testing.test/Makefile (1)
39-40: Pin cert-manager manifest version in test Makefile for deterministic CI test bootstrap.Using
releases/latestcreates a moving target; silent version updates can cause test flakiness when integration tests only verify deployment presence (not behavioral compatibility, seetest/integration/suite_test.golines 74-78). The CNPG manifest is already pinned to v1.29.0 in the same file, demonstrating the team's preference for version pinning in test environments.Add a configurable
CERT_MANAGER_MANIFESTvariable to allow CI to pin a tested version while preserving local override flexibility:Suggested change
+# Allow CI to pin a tested cert-manager manifest while keeping local override flexibility. +CERT_MANAGER_MANIFEST ?= https://github.com/cert-manager/cert-manager/releases/latest/download/cert-manager.yaml + .PHONY: kind-up kind-up: kind delete cluster --name $(KIND_CLUSTER_NAME) 2>/dev/null || true @@ -37,7 +40,7 @@ kind-up: kubectl apply --context kind-$(KIND_CLUSTER_NAME) --server-side -f \ https://raw.githubusercontent.com/pgEdge/pgedge-cnpg-dist/refs/heads/main/manifests/cloudnative-pg/v1.29.0/cnpg-1.29.0.yaml kubectl apply --context kind-$(KIND_CLUSTER_NAME) -f \ - https://github.com/cert-manager/cert-manager/releases/latest/download/cert-manager.yaml + $(CERT_MANAGER_MANIFEST) kubectl --context kind-$(KIND_CLUSTER_NAME) wait --for=condition=Available deployment \ -n cnpg-system cnpg-controller-manager --timeout=120s kubectl --context kind-$(KIND_CLUSTER_NAME) wait --for=condition=Available deployment \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Makefile` around lines 39 - 40, Replace the hardcoded cert-manager manifest URL in the Makefile with a configurable CERT_MANAGER_MANIFEST variable and use that variable in the kubectl apply command; define CERT_MANAGER_MANIFEST with a pinned release URL (e.g., a specific vX.Y.Z release) as the default so CI can override it while still allowing local overrides via make CERT_MANAGER_MANIFEST=...; update the kubectl apply invocation to use $(CERT_MANAGER_MANIFEST) instead of the current releases/latest URL and ensure the variable name appears near CNPG_MANIFEST for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/usage/postgres_upgrades.md`:
- Line 7: The upgrade narrative still contains outdated version references and a
grammar error — update every occurrence of the old progression language to match
the current progression ("Postgres 17 and Spock 5") and remove the ambiguous
phrase about a "mutable tag by default"; revise the sentence to read clearly
(e.g., "This chart uses a mutable tag by default to pull the latest pgEdge
Enterprise Postgres image for Postgres 17 / Spock 5.") and make parallel fixes
for the other mismatched lines that mention the previous progression (the
similar phrases around the same paragraph and the occurrences corresponding to
the other reported locations). Ensure tense and plurality are consistent across
the document and that all version mentions use the exact "Postgres 17" and
"Spock 5" wording.
---
Nitpick comments:
In `@docs/install.md`:
- Line 120: The docs reference to the cert-manager manifest currently uses the
floating URL "releases/latest/download/cert-manager.yaml", which makes installs
non-reproducible; update that URL to a pinned release (for example replace
"releases/latest" with the specific tag like "releases/download/v1.20.2" or the
currently vetted cert-manager version) so the manifest points to an exact
version of cert-manager.yaml and document that this pinned version should be
updated periodically after testing.
In `@test/Makefile`:
- Around line 39-40: Replace the hardcoded cert-manager manifest URL in the
Makefile with a configurable CERT_MANAGER_MANIFEST variable and use that
variable in the kubectl apply command; define CERT_MANAGER_MANIFEST with a
pinned release URL (e.g., a specific vX.Y.Z release) as the default so CI can
override it while still allowing local overrides via make
CERT_MANAGER_MANIFEST=...; update the kubectl apply invocation to use
$(CERT_MANAGER_MANIFEST) instead of the current releases/latest URL and ensure
the variable name appears near CNPG_MANIFEST for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6d109c5-fb3c-47ab-bab0-305e93ac1c3c
📒 Files selected for processing (12)
README.mddocs/configuration.mddocs/install.mddocs/usage/adding_nodes.mddocs/usage/backups.mddocs/usage/connecting.mddocs/usage/monitoring.mddocs/usage/postgres_upgrades.mddocs/usage/standby.mdtest/Makefiletest/unit/cluster_test.govalues.yaml
rshoemaker
left a comment
There was a problem hiding this comment.
Looks good.
Changelog entry now or do you do that as part of the release process?
| pgedge-n1-1 0/66005850 Primary OK BestEffort 1.28.0 aks-agentpool-14750958-vmss000000 | ||
| pgedge-n1-2 0/66005850 Standby (sync) OK BestEffort 1.28.0 aks-agentpool-14750958-vmss000002 | ||
| pgedge-n1-3 0/66005850 Standby (sync) OK BestEffort 1.28.0 aks-agentpool-14750958-vmss000001 |
There was a problem hiding this comment.
do you want to update the sample output to show 1.29.0?
There was a problem hiding this comment.
I'll handle this separately - the output here may have changed with the latest kubectl version, which we are about to publish.
|
I'll update the changelog separately - there's a small fix to the release process that will be needed as well, so I'll bundle it there. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/usage/postgres_upgrades.md (1)
105-105: Minor grammar: hyphenate compound adjective.Consider hyphenating "two-node configuration" since it's a compound adjective modifying a noun.
📝 Suggested fix
-For example, given the following two node configuration running Postgres 17: +For example, given the following two-node configuration running Postgres 17:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/usage/postgres_upgrades.md` at line 105, The phrase "two node configuration" should be hyphenated as a compound adjective; update the text to read "two-node configuration" wherever that phrase appears (e.g., the example sentence currently reading "given the following two node configuration running Postgres 17") to fix the minor grammar issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/usage/postgres_upgrades.md`:
- Line 105: The phrase "two node configuration" should be hyphenated as a
compound adjective; update the text to read "two-node configuration" wherever
that phrase appears (e.g., the example sentence currently reading "given the
following two node configuration running Postgres 17") to fix the minor grammar
issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70e5af82-9d7c-48ac-a6df-8e98d12e78c2
📒 Files selected for processing (1)
docs/usage/postgres_upgrades.md
Summary
17-spock5-standardto18-spock5-standardacross values, docs, and testslatestrelease instead of a pinned versionDetails
Postgres 18
The default
imageNameinvalues.yamlnow uses the18-spock5-standardtag. All documentation examples, code snippets, and unit test assertions have been updated to reflect this change. The major version upgrade docs now use 17→18 as the example scenario (previously 16→17).CNPG 1.29
The Kind cluster setup in
test/Makefilenow installs CNPG 1.29.0. Documentation links to CloudNativePG have been updated fromdocumentation/1.28/todocs/1.29/to match the new URL scheme / docs system.cert-manager latest
Both
test/Makefileanddocs/install.mdnow referencereleases/latest/download/cert-manager.yamlinstead of pinning to v1.19.3, reducing maintenance burden for future upgrades. This was already the case in our docs. Our usage of cert-manager is very straight forward, and customers typically manage this version outside of our chart anyway.