Skip to content
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.

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

allow delete only if annotations meet configured criteria #1069

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Jul 21, 2020

Right now a Postgres cluster can be easily removed with kubectl delete pg acid-minimal-cluster. If clusters are called very similar it might happen that one accidently removes the wrong resource. While it could be recovered from S3 pretty quickly (depending on the size, of course) a few more steps might be necessary to get back to normal operating mode.

To avoid all the stress, this PRs introduces a simple delete protection mechanism, that checks for existing annotations in the manifest before allowing the delete process to continue. Actually, we would assume this is something to be set in the K8s infrastructure, which blocks a delete request already at the ApiServer level. With this approach here, the CR still gets removed but all managed child resources remain. Either if the cluster was deleted accidentally or the delete annotations were forgotten, one has to recreate the cluster again to continue.

The two delete protection checks are:

  • cluster name must match
  • data must be the current data

The name for the two annotations keys can be configured. Each check can be disabled individually if the option is not set (empty). This is also the default.

This PR also fixes three more things:

  • re-enables all e2e tests
  • some typos on K8s words

@FxKu FxKu added the zalando label Jul 21, 2020
@FxKu FxKu self-assigned this Jul 21, 2020
@FxKu FxKu changed the title [WIP] allow delete only if annotations meet configured criteria allow delete only if annotations meet configured criteria Jul 29, 2020
@FxKu FxKu added this to the 1.6 milestone Jul 29, 2020
// only allow deletion if delete annotations are set and conditions are met
if eventType == EventDelete {
if err := c.meetsClusterDeleteAnnotations(informerOldSpec); err != nil {
c.logger.WithField("cluster-name", clusterName).Warnf("skipping %q event as delete criteria not fulfilled: %s", eventType, err)
Copy link
Member

Choose a reason for hiding this comment

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

skipping %q event as delete criteria not fulfilled: %s
To
Ignoring delete event for cluster %q - manifest does not fulfill delete requirements (annotations): %s

@FxKu FxKu force-pushed the cluster-delete-annotations branch 4 times, most recently from 5e22b01 to 9932757 Compare August 11, 2020 14:27
@FxKu FxKu force-pushed the cluster-delete-annotations branch from 9932757 to 4feb301 Compare August 13, 2020 12:20
@FxKu
Copy link
Member Author

FxKu commented Aug 13, 2020

👍

1 similar comment
@Jan-M
Copy link
Member

Jan-M commented Aug 13, 2020

👍

@FxKu FxKu merged commit 3ddc56e into master Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants