Skip to content

add unit test and documentation for finalizers #2509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Jan 22, 2024
Merged

add unit test and documentation for finalizers #2509

merged 14 commits into from
Jan 22, 2024

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Jan 9, 2024 •
edited
Loading

#941 added support for using finalizers. However, the PR lacked documentation and a unit test. We decided to merge it anyway due to the age of the PR in favor for having a quick follow-up PR which adds it.

The unit test became larger than anticipated:

  • testing the whole Create() process of cluster.go
  • All finalizer functions were turned private as they are not accessed from anywhere else in the code.
  • Finalizers will not only added on Create and removed on Delerte but also managed sync
  • The introduced JSON patch dependency was removed in favor for marshaling with Go like we do with the status
  • The patch/update of Postgresql resource was moved to it's own function in k8sutil
  • RemoveString function moved into util package and added unit test
  • log messages starting with capital letter were lower cased

This PR also remove a binary debug file which was merged with #2456, unfortunately.

@FxKu FxKu added this to the 2.0 milestone Jan 9, 2024
@FxKu FxKu mentioned this pull request Jan 11, 2024
Merged
@FxKu FxKu requested a review from macedigital as a code owner January 12, 2024 08:27
@jopadi
Copy link
Member

jopadi commented Jan 22, 2024

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Jan 22, 2024

👍

@FxKu FxKu merged commit 4a0c483 into master Jan 22, 2024
@FxKu FxKu deleted the finalizing-finalizers branch January 22, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdudoladov sdudoladov Awaiting requested review from sdudoladov sdudoladov is a code owner

@Jan-M Jan-M Awaiting requested review from Jan-M Jan-M is a code owner

@jopadi jopadi Awaiting requested review from jopadi jopadi is a code owner

@idanovinda idanovinda Awaiting requested review from idanovinda idanovinda is a code owner

@hughcapet hughcapet Awaiting requested review from hughcapet hughcapet is a code owner

@macedigital macedigital Awaiting requested review from macedigital macedigital is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Postgres Operator
Status: Done
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.
2 participants
@FxKu @jopadi