Skip to content

Commit

Permalink
Plugin refactoring plan
Browse files Browse the repository at this point in the history
First iteration of the plan to split ocp-velero-plugin into
two separate velero plugins:

1) A general openshift plugin, appropriate for situations
   in which velero is used for general backup and restore
   of OpenShift namespaces.
2) A migration plugin, intended to be used only in conjunction
   with the Migration Controller.
  • Loading branch information
sseago committed Jun 12, 2019
1 parent 05910da commit ea2c90c
Showing 1 changed file with 190 additions and 0 deletions.
190 changes: 190 additions & 0 deletions docs/plugin-refactoring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
# ocp-velero-plugin Repo Split

Currently, the contents of ocp-velero-plugin includes a mixture of
actions that are relevant for general backup and restore of OpenShift
namespaces, and actions that only make sense in the context of a
controller-run migration. This document is an attempt to separate the
two purposes in preparation for splitting the repo into two.

## General backup/restore features (and supporting infrastructure):

* Record backup/restore cluster version and registry hostname.
* Don't restore pods that are controlled by higher-level entities (DeploymentConfig, etc.).

## Only needed for migration:

* ConfigureContainerSleep and plugins that deal with this
* ImageStream/ImageStreamTag image copy and "item skipping"
* PV/PVC actions
* Quiesce pods on final migration

## Open questions:

For the following cases, it's not yet clear whether the functionality
belongs in the general plugin or the migration plugin.

### Modifying image references that point to the internal registry:

Should we only do this for migration, or for any backup/restore. In a
non-migration situation, internal images won't be copied to the
destination cluster, if it happens to be different than the source
cluster, so the modified path won't necessarily work. On the other
hand, in a "new cluster" environment, the original path probably won't
work either. In a situation where it's the same cluster but the
configuration has changed (internal registry hostname is different),
though, the updated path might work while the old path may not. Is
there any situation where leaving "backup cluster" image refs is
preferable to changing them to "restore cluster" image refs? If we
eventually remove the SHA on restore for openshift-namespaced images,
we probably don't want to do this in the general case (if we're doing
this at all in the general case)

### Modifying builds:

Similar to "Modifying image references", but different behavior for
local openshift-namespace images. (Might eventually be the same as
above). Same caveat as above, if we're modifying builds for the
general case, we may not want to strip the SHA for
openshift-namespaced builds in that case. The other thing done by the
build plugin is updating `Spec.Output.PushSecret` and
`Spec.Strategy.SourceStrategy.PullSecret` to point to the right
`builder-dockercfg` secret in the destination cluster.

### Modifying routes:

Similar to the "Modifying image references" question. I'm inclined to
say that it's always safer to modify routes to use the src cluster's
domain here. Is this true regardless of whether we're talking about
3.x vs. 4.x clusters, though?

## Post-split:

The general openshift velero plugin will be called
openshift-velero-plugin. The migration plugin will be
openshift-migration-plugin.

Once the repos are split, the migration-specific plugin image will
include the general plugin code (and register any general plugins that
will be used unchanged for migration). As a result, a velero
installation used for migration would only need to have the migration
plugin added, not the general plugin. Another approach would be to
keep them separate and require both plugins to be added, but this
reduces our flexibility to handle situations where we would want a
migration-specific plugin action registered *instead of* a general
plugin rather than *in addition to* it.

We should be able to remove the conditional statements that skip
plugin actions for non-migration use cases, since the migration plugin
won't be registered for general backup restore. This means that most
of the code like this won't be necessary anymore:
```
if input.Restore.Annotations[common.MigrateCopyPhaseAnnotation] != "" {
```
We will still need conditionals where the action differs based on copy
vs. final migrations or copy vs. move behavior.

## File-by-file


* build
* Depends on "modifying builds"
* restore.go, restore_test.go
* clients
* General
* clients.go
* common:
* Depends on "modifying image references"
* backup.go: Needed if we're modifying image
references. Possibly useful in all cases, as all this plugin does is
set annotations to document the kubernetes version and
internal registry hostname of the cluster being backed up.
* restore.go: Needed if we're modifying image
references. Possibly useful in all cases, as all this plugin does is
set annotations to document the kubernetes version and
internal registry hostname of the cluster being restored to.
* types.go:
* const BackupRegistryHostname
* const RestoreRegistryHostname
* util.go
* func ReplaceImageRefPrefix
* func HasImageRefPrefix
* type LocalImageReference
* func ParseLocalImageReference
* func SwapContainerIMageRefs
* func GetSrcAndDestRegistryInfo
* General
* shared.go:
* func getMetadataAndAnnotations
* func GetServerVersion
* func GetRegistryInfo
* types.go:
* type APIServerConfig
* const BackupServerVersion
* const RestoreServerVersion
* util.go
* func GetOwnerReferences
* Migration-specific:
* const MigrationRegistry
* const SwingPVAnnotation
* const MigrateTypeAnnotation
* const MigrateCopyPhaseAnnotation
* const MigrateQuiesceAnnotation
* const PodStageLabel
* const ResticRestoreAnnotationPrefix
* const ResticBackupAnnotation
* util.go
* func ConfigureContainerSleep
* daemonset:
* Depends on "modifying image references"
* restore.go
* deployment:
* Depends on "modifying image references"
* restore.go
* Migration-specific:
* backup.go (quiesce behavior)
* deploymentconfig:
* Depends on "modifying image references"
* restore.go
* Migration-specific:
* backup.go (quiesce behavior)
* imagestream:
* Migration-specific:
* backup.go, restore.go, shared.go (it's possible that we would want a general plugin
for imagestream that's not used for migration, but nothing in these files should be used outside of migration)
* imagestreamtag:
* Migration-specific:
* restore.go (it's possible that we would want a general plugin
for imagestream that's not used for migration, but nothing in this file should be used outside of migration)
* job:
* Depends on "modifying image references"
* restore.go
* pod:
* Depends on "modifying image references"
* restore.go (swap image refs portion)
* Migration-specific:
* restore.go (ConfigureContainerSleep portion)
* Note: here's an example where we need separate Execute action
plugin for general and migration use cases if modifying
image references goes into the general plugin
* pv:
* Migration-specific:
* backup.go
* restore.go
* pvc:
* Migration-specific:
* restore.go
* replicaset:
* Depends on "modifying image references"
* restore.go
* replicationcontroller:
* Depends on "modifying image references"
* restore.go
* route:
* Depends on "modifying routes"
* restore.go, restore_test.go
* statefulset:
* Depends on "modifying image references"
* restore.go
* util/test:
* General:
* test.NewLogger

0 comments on commit ea2c90c

Please sign in to comment.