Skip to content

fix(database): remove 'deleteDatabase' finalizer from orphan Database CRD#5596

Merged
mnencia merged 9 commits intocloudnative-pg:mainfrom
EnterpriseDB:dev/5351
Oct 9, 2024
Merged

fix(database): remove 'deleteDatabase' finalizer from orphan Database CRD#5596
mnencia merged 9 commits intocloudnative-pg:mainfrom
EnterpriseDB:dev/5351

Conversation

@gabriele-wolfox
Copy link
Contributor

@gabriele-wolfox gabriele-wolfox commented Sep 24, 2024 •
edited by mnencia
Loading

Closes #5593

@cnpg-bot cnpg-bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.22 release-1.23 release-1.24 labels Sep 24, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Sep 24, 2024

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@gabriele-wolfox gabriele-wolfox force-pushed the dev/5351 branch 2 times, most recently from a0191d2 to 63ac06a Compare September 24, 2024 12:04
@gabriele-wolfox gabriele-wolfox marked this pull request as ready for review September 24, 2024 14:40
@gabriele-wolfox gabriele-wolfox requested a review from a team as a code owner September 24, 2024 14:40
@gabriele-wolfox gabriele-wolfox deleted the dev/5351 branch September 24, 2024 14:43
@gabriele-wolfox gabriele-wolfox restored the dev/5351 branch September 24, 2024 14:43
@github-actions
Copy link
Contributor

github-actions bot commented Sep 24, 2024

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2024

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@jsilvela
Copy link
Collaborator

jsilvela commented Sep 25, 2024

I saw the other PR that was doing finalizer cleanup, #5581 , has been closed as unnecessary.
Given that, isn't having a dedicated file internal/controller/finalizers_delete.go a bit too big?
We already have internal/controller/cluster_delete.go which seems perfect for this.
And similar with the new dedicated file pkg/utils/finalizers.go. A new file just for one entry?

@gabriele-wolfox
Copy link
Contributor Author

gabriele-wolfox commented Sep 25, 2024

I saw the other PR that was doing finalizer cleanup, #5581 , has been closed as unnecessary. Given that, isn't having a dedicated file internal/controller/finalizers_delete.go a bit too big? We already have internal/controller/cluster_delete.go which seems perfect for this. And similar with the new dedicated file pkg/utils/finalizers.go. A new file just for one entry?

@jsilvela , I like the idea to split the work around finalizers inside separated files.
But if you prefer differently, feel free to move/refactor the code around.

@jsilvela
Copy link
Collaborator

jsilvela commented Sep 25, 2024

No strong feeling there @gabriele-wolfox , will leave to the second reviewer. This PR looks good.

@mnencia mnencia force-pushed the dev/5351 branch 3 times, most recently from 830524d to 0cd7702 Compare October 8, 2024 06:56
@mnencia
Copy link
Member

mnencia commented Oct 8, 2024

/test d=push tl=4

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

@mnencia, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/11231286355

@cnpg-bot cnpg-bot added the ok to merge 👌 This PR can be merged label Oct 8, 2024
@gabriele-wolfox @mnencia
feat: remove 'deleteDatabase' finalizer from orphan Database CRD
0a04dc3 
Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
@gabriele-wolfox @mnencia
test: add unit tests
67dc41f 
Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
@gabriele-wolfox @mnencia
fix: linter issues
0909bdd 
Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
jsilvela and others added 6 commits October 8, 2024 18:49
@jsilvela @mnencia
chore: clarifications
e2c7c80 
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
@jsilvela @mnencia
chore: nit
01675f5 
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
@jsilvela @mnencia
chore: us the new SetupLogger
343aa1b 
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
@jsilvela @mnencia
chore: fix comment
8eceab4 
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
@mnencia
chore: review
ae2ee24 
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia
chore: setupLogger should be called only once
725b4bd 
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia mnencia changed the title feat: remove 'deleteDatabase' finalizer from orphan Database CRD fix: remove 'deleteDatabase' finalizer from orphan Database CRD Oct 9, 2024
@mnencia mnencia changed the title fix: remove 'deleteDatabase' finalizer from orphan Database CRD fix(database): remove 'deleteDatabase' finalizer from orphan Database CRD Oct 9, 2024
@mnencia mnencia merged commit 9c3105f into cloudnative-pg:main Oct 9, 2024
@mnencia mnencia added do not backport This PR must not be backported - it will be in the next minor release and removed backport-requested ◀️ This pull request should be backported to all supported releases release-1.22 release-1.23 release-1.24 labels Oct 9, 2024
bentastic27 pushed a commit to bentastic27/cloudnative-pg that referenced this pull request Oct 11, 2024
@gabriele-wolfox @jsilvela
@mnencia @bentastic27
fix(database): remove 'deleteDatabase' finalizer from orphan Database…
e90a622 
… CRD (cloudnative-pg#5596)

Closes cloudnative-pg#5593

Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Ben Healey <bentastic27@gmail.com>
bentastic27 pushed a commit to bentastic27/cloudnative-pg that referenced this pull request Oct 11, 2024
@gabriele-wolfox @jsilvela
@mnencia @bentastic27
fix(database): remove 'deleteDatabase' finalizer from orphan Database…
dbf3588 
… CRD (cloudnative-pg#5596)

Closes cloudnative-pg#5593

Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Ben Healey <bentastic27@gmail.com>
@mnencia mnencia deleted the dev/5351 branch December 24, 2024 09:07
@ardentperf ardentperf mentioned this pull request Sep 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnencia mnencia mnencia approved these changes

@jsilvela jsilvela jsilvela approved these changes

Assignees

No one assigned

Labels

do not backport This PR must not be backported - it will be in the next minor release ok to merge 👌 This PR can be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[split from #4679]: Operator should prune finalizers from Database when orphaned

4 participants

@gabriele-wolfox @jsilvela @mnencia @cnpg-bot