Skip to content

Commit

Permalink
Merge pull request #7656 from blackpiglet/csi_doc_change
Browse files Browse the repository at this point in the history
CSI doc change and remove the CSI feature verifier
  • Loading branch information
qiuming-best committed Apr 12, 2024
2 parents 3c377bc + 59eeec2 commit 43aa892
Show file tree
Hide file tree
Showing 22 changed files with 79 additions and 412 deletions.
1 change: 0 additions & 1 deletion .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ jobs:

- name: 'use gcloud CLI'
run: |
gcloud auth login
gcloud info
- name: Set up QEMU
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ Below are actions from Velero and DMP:
**BIA Execute**
This is the existing logic in Velero. For a source PVC/PV, Velero delivers it to the corresponding BackupItemAction plugin, the plugin then takes the related actions to back it up.
For example, the existing CSI plugin takes a CSI snapshot to the volume represented by the PVC and then returns additional items (i.e., VolumeSnapshot, VolumeSnapshotContent and VolumeSnapshotClass) for Velero to further backup.
To support data movement, we will use BIA V2 which supports asynchronized operation management. Here is the Execute method from BIA V2:
To support data movement, we will use BIA V2 which supports asynchronous operation management. Here is the Execute method from BIA V2:
```
Execute(item runtime.Unstructured, backup *api.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, string, []velero.ResourceIdentifier, error)
```
Besides ```additionalItem``` (as the 2nd return value), Execute method will return one more resource list called ```itemToUpdate```, which means the items to be updated and persisted when the async operation completes. For details, visit [general progress monitoring design][2].
Specifically, this mechanism will be used to persist DUCR into the persisted backup data, in another words, DUCR will be returned as ```itemToUpdate``` from Execute method. DUCR contains all the information the restore requires, so during restore, DUCR will be extracted from the backup data.
Additionally, in the same way, a DMP could add any other items into the persisted backup data.
Execute method also returns the ```operationID``` which uniquely identifies the asynchronized operation. This ```operationID``` is generated by plugins. The [general progress monitoring design][2] doesn't restrict the format of the ```operationID```, for Velero CSI plugin, the ```operationID``` is a combination of the backup CR UID and the source PVC (represented by the ```item``` parameter) UID.
Execute method also returns the ```operationID``` which uniquely identifies the asynchronous operation. This ```operationID``` is generated by plugins. The [general progress monitoring design][2] doesn't restrict the format of the ```operationID```, for Velero CSI plugin, the ```operationID``` is a combination of the backup CR UID and the source PVC (represented by the ```item``` parameter) UID.

**Create Snapshot**
The DMP creates a snapshot of the requested volume and deliver it to DM through DUCR. After that, the DMP leaves the snapshot and its related objects (e.g., VolumeSnapshot and VolumeSnapshotContent for CSI snapshot) to the DM, DM then has full control of the snapshot and its related objects, i.e., deciding whether to delete the snapshot or its related objects and when to do it.
Expand Down Expand Up @@ -930,12 +930,20 @@ For 3, Velero leverage on DMs to decide how to save the log, but they will not g
## Installation
DMs need to be configured during installation so that they can be installed. Plugin DMs may have their own configuration, for VGDM, the only requirement is to install Velero node-agent.
Moreover, the DMP is also required during the installation.
From release-1.14, the `github.com/vmware-tanzu/velero-plugin-for-csi` repository, which is the Velero CSI plugin, is merged into the `github.com/vmware-tanzu/velero` repository.
The reason to merge the CSI plugin is:
* The VolumeSnapshot data mover depends on the CSI plugin, it's reasonabe to integrate them.
* This change reduces the Velero deploying complexity.
* This makes performance tuning easier in the future.
As a result, no need to install Velero CSI plugin anymore.
For example, to move CSI snapshot through VBDM, below is the installation script:
```
velero install \
--provider \
--image \
--plugins velero/velero-plugin-for-csi:xxx \
--features=EnableCSI \
--use-node-agent \
```
Expand Down
18 changes: 8 additions & 10 deletions pkg/backup/actions/csi/volumesnapshot_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ func (p *volumeSnapshotBackupItemAction) Execute(
return nil, nil, "", nil, errors.WithStack(err)
}

p.log.Infof("Returning from VolumeSnapshotBackupItemAction",
"with %d additionalItems to backup", len(additionalItems))
p.log.Infof("Returning from VolumeSnapshotBackupItemAction with %d additionalItems to backup",
len(additionalItems))
for _, ai := range additionalItems {
p.log.Debugf("%s: %s", ai.GroupResource.String(), ai.Name)
}
Expand Down Expand Up @@ -294,22 +294,20 @@ func (p *volumeSnapshotBackupItemAction) Progress(
}

if vs.Status == nil {
p.log.Debugf("VolumeSnapshot %s/%s has an empty status.",
"Skip progress update.", vs.Namespace, vs.Name)
p.log.Debugf("VolumeSnapshot %s/%s has an empty status. Skip progress update.", vs.Namespace, vs.Name)
return progress, nil
}

if boolptr.IsSetToTrue(vs.Status.ReadyToUse) {
p.log.Debugf("VolumeSnapshot %s/%s is ReadyToUse.",
"Continue on querying corresponding VolumeSnapshotContent.",
p.log.Debugf("VolumeSnapshot %s/%s is ReadyToUse. Continue on querying corresponding VolumeSnapshotContent.",
vs.Namespace, vs.Name)
} else if vs.Status.Error != nil {
errorMessage := ""
if vs.Status.Error.Message != nil {
errorMessage = *vs.Status.Error.Message
}
p.log.Warnf("VolumeSnapshot has a temporary error %s.",
"Snapshot controller will retry later.", errorMessage)
p.log.Warnf("VolumeSnapshot has a temporary error %s. Snapshot controller will retry later.",
errorMessage)

return progress, nil
}
Expand All @@ -328,8 +326,8 @@ func (p *volumeSnapshotBackupItemAction) Progress(
}

if vsc.Status == nil {
p.log.Debugf("VolumeSnapshotContent %s has an empty Status.",
"Skip progress update.", vsc.Name)
p.log.Debugf("VolumeSnapshotContent %s has an empty Status. Skip progress update.",
vsc.Name)
return progress, nil
}

Expand Down
9 changes: 0 additions & 9 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ type server struct {
mgr manager.Manager
credentialFileStore credentials.FileStore
credentialSecretStore credentials.SecretStore
featureVerifier features.Verifier
}

func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*server, error) {
Expand Down Expand Up @@ -326,12 +325,6 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
return nil, err
}

featureVerifier := features.NewVerifier(pluginRegistry)

if _, err := featureVerifier.Verify(velerov1api.CSIFeatureFlag); err != nil {
logger.WithError(err).Warn("CSI feature verification failed, the feature may not be ready.")
}

// cancelFunc is not deferred here because if it was, then ctx would immediately
// be canceled once this function exited, making it useless to any informers using later.
// That, in turn, causes the velero server to halt when the first informer tries to use it.
Expand Down Expand Up @@ -425,7 +418,6 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s
mgr: mgr,
credentialFileStore: credentialFileStore,
credentialSecretStore: credentialSecretStore,
featureVerifier: featureVerifier,
}

return s, nil
Expand Down Expand Up @@ -973,7 +965,6 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
s.kubeClient.CoreV1().RESTClient(),
s.credentialFileStore,
s.mgr.GetClient(),
s.featureVerifier,
)

cmd.CheckError(err)
Expand Down
43 changes: 0 additions & 43 deletions pkg/features/mocks/PluginFinder.go

This file was deleted.

49 changes: 0 additions & 49 deletions pkg/features/mocks/Verifier.go

This file was deleted.

71 changes: 0 additions & 71 deletions pkg/features/verify.go

This file was deleted.

61 changes: 0 additions & 61 deletions pkg/features/verify_test.go

This file was deleted.

4 changes: 0 additions & 4 deletions pkg/plugin/clientmgmt/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ func (r *mockRegistry) Get(kind common.PluginKind, name string) (framework.Plugi
return id, args.Error(1)
}

func (r *mockRegistry) Find(kind common.PluginKind, name string) bool {
return false
}

func TestNewManager(t *testing.T) {
logger := test.NewLogger()
logLevel := logrus.InfoLevel
Expand Down
9 changes: 0 additions & 9 deletions pkg/plugin/clientmgmt/process/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ type Registry interface {
List(kind common.PluginKind) []framework.PluginIdentifier
// Get returns the PluginIdentifier for kind and name.
Get(kind common.PluginKind, name string) (framework.PluginIdentifier, error)

// Find checks if the specified plugin exists in the registry
Find(kind common.PluginKind, name string) bool
}

// KindAndName is a convenience struct that combines a PluginKind and a name.
Expand Down Expand Up @@ -128,12 +125,6 @@ func (r *registry) Get(kind common.PluginKind, name string) (framework.PluginIde
return p, nil
}

// Contain if the specified plugin exists in the registry
func (r *registry) Find(kind common.PluginKind, name string) bool {
_, found := r.pluginsByID[KindAndName{Kind: kind, Name: name}]
return found
}

// readPluginsDir recursively reads dir looking for plugins.
func (r *registry) readPluginsDir(dir string) ([]string, error) {
if _, err := r.fs.Stat(dir); err != nil {
Expand Down
Loading

0 comments on commit 43aa892

Please sign in to comment.