Skip to content

deprecate: mark postgres_keep_pvc_after_upgrade deprecated and remove implementation#352

Open
jamesmarshall24 wants to merge 1 commit into
mainfrom
AAP-75917
Open

deprecate: mark postgres_keep_pvc_after_upgrade deprecated and remove implementation#352
jamesmarshall24 wants to merge 1 commit into
mainfrom
AAP-75917

Conversation

@jamesmarshall24
Copy link
Copy Markdown
Contributor

@jamesmarshall24 jamesmarshall24 commented May 29, 2026

Summary

Deprecates postgres_keep_pvc_after_upgrade in the CRD and removes the
implementation. The field has no effect going forward; old Postgres PVCs are
always preserved after upgrade, consistent with standard behavior across
platform operators.

  • Mark field as (Deprecated) in CRD description
  • Remove PVC deletion task from upgrade_postgres.yml
  • Remove CSV specDescriptor entry
  • Remove role default
  • Remove upgrading.md usage example

Summary by CodeRabbit

Release Notes

  • Deprecations

    • The postgres_keep_pvc_after_upgrade configuration field is now marked as deprecated.
  • Changes

    • PostgreSQL upgrade process no longer automatically removes old persistent volume claims after upgrades.
    • Updated documentation clarifying deprecated features and backward compatibility guidance.

… implementation

Mark postgres_keep_pvc_after_upgrade as (Deprecated) in the CRD spec. Remove
the PVC deletion task, CSV specDescriptor, role default, and upgrading.md usage
example. The field remains in the CRD to avoid breaking existing CRs. Old
Postgres PVC is always preserved after upgrade, consistent with standard
behavior across platform operators.

Assisted by: Claude

Signed-off-by: James Marshall <jamarsha@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

This pull request deprecates the postgres_keep_pvc_after_upgrade field throughout the EDA operator. The field is marked deprecated in the CRD schema definition, removed from the UI manifest descriptor, documented as deprecated in upgrade guidance, and finally removed from both the Postgres role defaults and the upgrade playbook's PVC cleanup task.

Changes

PostgreSQL PVC Retention Deprecation

Layer / File(s) Summary
Mark field deprecated in API schema
config/crd/bases/eda.ansible.com_edas.yaml, config/manifests/bases/eda-server-operator.clusterserviceversion.yaml
The spec.database.postgres_keep_pvc_after_upgrade field description is prefixed with "(Deprecated)" in the CRD schema, and the corresponding specDescriptor entry is removed from the CSV manifest UI metadata.
Update upgrade documentation
docs/upgrade/upgrading.md
Upgrade documentation is clarified to note that deprecated CRD fields remain in the schema for backward compatibility while no longer being actively used, with a reference to the Redis Deprecation Notice.
Remove deprecated functionality from implementation
roles/postgres/defaults/main.yml, roles/postgres/tasks/upgrade_postgres.yml
The postgres_keep_pvc_after_upgrade default variable is removed from Postgres role configuration, and the conditional task performing old-PVC deletion during managed-database upgrades is removed from the upgrade playbook.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main changes: deprecating the postgres_keep_pvc_after_upgrade field and removing its implementation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch AAP-75917

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/crd/bases/eda.ansible.com_edas.yaml`:
- Line 2392: Update the deprecated description string "(Deprecated) Specify
whether or not to keep the old PVC after PostgreSQL upgrades" to explicitly
state that the field is now ignored and describe the new default behavior (for
example: "Deprecated — this field is ignored; old PVCs are no longer retained
after PostgreSQL upgrades by default"). Reference the PostgreSQL PVC-related
field (the description line containing "keep the old PVC after PostgreSQL
upgrades") when editing so the wording matches other deprecated entries (e.g.,
the Redis deprecation style) and clearly communicates that users can remove this
setting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: c5083864-5999-4429-b9f4-cf1a761ef8d1

📥 Commits

Reviewing files that changed from the base of the PR and between 27522e9 and fb99460.

📒 Files selected for processing (5)
  • config/crd/bases/eda.ansible.com_edas.yaml
  • config/manifests/bases/eda-server-operator.clusterserviceversion.yaml
  • docs/upgrade/upgrading.md
  • roles/postgres/defaults/main.yml
  • roles/postgres/tasks/upgrade_postgres.yml
💤 Files with no reviewable changes (4)
  • roles/postgres/tasks/upgrade_postgres.yml
  • config/manifests/bases/eda-server-operator.clusterserviceversion.yaml
  • roles/postgres/defaults/main.yml
  • docs/upgrade/upgrading.md

type: string
postgres_keep_pvc_after_upgrade:
description: Specify whether or not to keep the old PVC after PostgreSQL upgrades
description: "(Deprecated) Specify whether or not to keep the old PVC after PostgreSQL upgrades"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Consider clarifying that the field is ignored and what the new behavior is.

The deprecation notice could be more explicit about the field being ignored and the new default behavior. This would be consistent with how other deprecated fields are documented in this file (e.g., Redis fields at lines 102-105, 2143) and prevent user confusion.

📝 Suggested improvement for clarity
-                    description: "(Deprecated) Specify whether or not to keep the old PVC after PostgreSQL upgrades"
+                    description: "(Deprecated) This field is ignored. PostgreSQL PVCs are now always preserved after upgrades."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
description: "(Deprecated) Specify whether or not to keep the old PVC after PostgreSQL upgrades"
description: "(Deprecated) This field is ignored. PostgreSQL PVCs are now always preserved after upgrades."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/crd/bases/eda.ansible.com_edas.yaml` at line 2392, Update the
deprecated description string "(Deprecated) Specify whether or not to keep the
old PVC after PostgreSQL upgrades" to explicitly state that the field is now
ignored and describe the new default behavior (for example: "Deprecated — this
field is ignored; old PVCs are no longer retained after PostgreSQL upgrades by
default"). Reference the PostgreSQL PVC-related field (the description line
containing "keep the old PVC after PostgreSQL upgrades") when editing so the
wording matches other deprecated entries (e.g., the Redis deprecation style) and
clearly communicates that users can remove this setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant