Skip to content

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