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

Move validation from controllers to pkg/backup, pkg/restore #131

Closed
ncdc opened this issue Oct 11, 2017 · 4 comments
Closed

Move validation from controllers to pkg/backup, pkg/restore #131

ncdc opened this issue Oct 11, 2017 · 4 comments
Labels
Enhancement/Dev Internal or Developer-focused Enhancement to Velero kind/tech-debt
Milestone

Comments

@ncdc
Copy link
Contributor

ncdc commented Oct 11, 2017

Make the validation the first thing we do in Backup() and Restore(). WDYT @skriss?

@skriss
Copy link
Member

skriss commented Oct 11, 2017

yeah, makes sense to me to put it in the library

@rosskukulinski
Copy link
Contributor

Internal development enhancement, but this is P1 because right now we have backup and restore logic split in 2 different packages. In addition, if anyone wants to vendor ark to do backups/restores, it's not pretty. We want a self-contained package for each.

@rosskukulinski rosskukulinski added this to the v1.0.0 milestone Jun 24, 2018
@skriss
Copy link
Member

skriss commented Nov 19, 2018

Here's an initial mini-design for this issue:

Goals

  • Move all backup/restore-processing logic into pkg/backup, pkg/restore respectively so there's a single entrypoint for running a backup/restore which can be more easily tested, vendored, etc.
  • Make controllers "dumb" - only responsible for watching for new/updated custom resources, enqueuing them, and dequeueing them and passing them to the appropriate packages for work

Non-Goals

  • Full refactoring of pkg/backup & pkg/restore. The goal of this initial effort is just to draw a clear boundary around the end-to-end backup/restore processes. From there, integration tests can be written against this boundary to provide guardrails for refactoring the implementations.
  • Similar refactoring for other controllers. This should probably be undertaken later, but first priority is for core backup and restore flows.

Design
Entrypoints to packages should be very simple:

// <pkg/backup>
type Pipeline interface {
    Execute(namespace, name string) error
}
  • Backup pipeline will:
    • Ensure API object exists
    • Ensure API object has the correct phase for processing (blank or New)
    • Default unspecified API object fields
      • .spec.ttl
      • .spec.storageLocation
      • .spec.volumeSnapshotLocations
      • .status.version
    • Validate API object spec
      • included/excluded resources
      • included/excluded namespaces
      • backup storage location
      • volume snapshot locations
    • Update API object's status to either FailedValidation (with errors) or InProgress
    • Instantiate all the things
    • Run API-scraping & related actions/hooks
    • Upload artifacts (archive, log, volumesnapshots file) to storage
    • Update API object's final status
  • Restore pipeline will work similarly (to be detailed after going through backup pipeline & learning from it)

cc @carlisia @nrb @wwitzel3

@ncdc ncdc modified the milestones: v1.0.0, TBD Nov 27, 2018
@skriss skriss self-assigned this Feb 25, 2020
@skriss skriss modified the milestones: TBD, v1.4 Feb 25, 2020
@skriss skriss removed this from the v1.4 milestone Apr 27, 2020
@skriss skriss added this to the v1.6 milestone Jun 9, 2020
@skriss skriss removed their assignment Jun 11, 2020
@nrb nrb added this to To do in v1.6.0 Nov 2, 2020
@nrb nrb added this to To do in v1.7.0 via automation Feb 3, 2021
@nrb nrb removed this from To do in v1.6.0 Feb 3, 2021
@nrb nrb modified the milestones: v1.6.0, v1.7.0 Feb 3, 2021
@dsu-igeek dsu-igeek removed this from To do in v1.7.0 Apr 21, 2021
@dsu-igeek dsu-igeek added this to To do in Velero 1.7.0 via automation Apr 21, 2021
@eleanor-millman
Copy link
Contributor

Closing because it should naturally fall out of the Astrolabe work if we do that.

Velero 1.7.0 automation moved this from To do to Done May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement/Dev Internal or Developer-focused Enhancement to Velero kind/tech-debt
Projects
No open projects
Development

No branches or pull requests

5 participants