Skip to content

Conversation

@FxKu
Copy link
Member

@FxKu FxKu commented Jan 9, 2024

#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 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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants