Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

EVEREST-107: config and secrets consistency via owner references #303

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Nov 10, 2023

EVEREST-107 Powered by Pull Request Badge

CHANGE DESCRIPTION

Problem:
EVEREST-107

Short explanation of the problem.
When it comes to create/update/delete backup storage and the associated secret, we have issues with data consistency and keeping secrets around if they're not needed.

Solution:
This PR aims to resolve the issues. The benefits include:

  1. Delete requires removal of the backup storage only. Secrets are deleted via owner references by k8s
  2. Update makes sure the data is always in a consistent state by creating a new secret and then replacing it in backup storage
  3. Create does not leave secret in k8s in case backup storage cannot be created properly

Create backup storage

  1. Create backup storage in Initializing state
  2. Create a secret with owner reference to backup storage
  3. Update backup storage with secret name and set state to Ready

Update backup storage

  1. Create new secret - if needed
  2. Update backup storage
  3. Delete old secret

Delete backup storage

  1. Delete backup storage
  2. k8s deletes all related secrets

Open topics
Listed in the code with TODO comments.

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?

Tests

  • Is an Integration test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
api/backup_storage.go Show resolved Hide resolved
@ghost ghost changed the title EVEREST-107: config and secrets consistency EVEREST-107: config and secrets consistency via owner references Nov 10, 2023
@recharte
Copy link
Collaborator

Create backup storage

  1. Create backup storage in Initializing state
  2. Create a secret with owner reference to backup storage
  3. Update backup storage with secret name and set state to Ready

I like the idea of using the status field to help us manage what is currently an inconsistent state and I think we can leverage this to achieve the data consistency we're looking for.
However, I don't like the fact that the backend is the entity responsible for setting this status.
For all intents and purposes, the backend is just a client of the everest CRDs and as such should only be responsible for setting their spec and never a status.

Base automatically changed from EVEREST-107-backup-refactor to main November 14, 2023 08:27
})
}

bs.Status.Status = "Ready"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have the possible statuses defined as constants in one place

BucketName: bs.Spec.Bucket,
Region: bs.Spec.Region,
Url: &bs.Spec.EndpointURL,
Status: bs.Status.Status,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to expose the status to the API? Looks like the "Initializing" will never returned to the API user anyway.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants