Removing un-needed code and modify the (Backup/Restore)ItemAction code.#629
Removing un-needed code and modify the (Backup/Restore)ItemAction code.#629blackpiglet wants to merge 3 commits intomainfrom
Conversation
* Change the BIA's logic: if the resource is blocked, add annotation in the item to indicate it should be excluded from the backup. * Modify the code to calculate the plural form of the CRD name. Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
blackpiglet
requested review from
Lyndon-Li,
Copilot,
lubronzhan,
reasonerjt and
xing-yang
There was a problem hiding this comment.
Pull request overview
This PR removes legacy VDDK/astrolabe-based components (backup-driver, data-manager, related controllers/CRDs/build tooling) as Velero moves to the native uploader, and updates the plugin entrypoint to the new Backup/Restore ItemAction flow (including blocklist + RESTMapper initialization).
Changes:
- Removed backup-driver/data-manager codepaths, CRDs, controllers, builders, and associated test/util scripts previously needed for VDDK-based workflows.
- Simplified build/release assets (Makefile, Dockerfiles, CI workflows) to stop packaging/downloading GVDDK/VDDK libs and related binaries.
- Updated the plugin main to initialize blocklist configmap + a cached RESTMapper, and register updated backup/restore item action handlers.
Reviewed changes
Copilot reviewed 73 out of 165 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/install/install.go | Removed legacy install/CRD creation/wait helpers for old components. |
| pkg/install/deployment.go | Removed backup-driver Deployment manifest generation. |
| pkg/install/daemonset.go | Removed data-manager DaemonSet manifest generation. |
| pkg/dataMover/data_mover.go | Removed astrolabe/S3 data mover implementation. |
| pkg/controller/vc_config_controller.go | Removed controller watching VC credential secret and triggering reloads. |
| pkg/controller/interface.go | Removed controller interface abstraction. |
| pkg/controller/generic_controller.go | Removed generic workqueue controller framework. |
| pkg/controller/download_controller_test.go | Removed download controller unit tests (tied to removed CRDs/controllers). |
| pkg/constants/constants.go | Updated restore blocklist entries (removed pods). |
| pkg/common/vsphere/vslm_manager_test.go | Removed vSphere integration tests for VSLM manager. |
| pkg/common/vsphere/vslm_manager.go | Removed VSLM manager wrapper implementation. |
| pkg/common/vsphere/virtual_center_test.go | Removed vCenter integration tests. |
| pkg/common/vsphere/virtual_center.go | Removed vCenter connection/session management implementation. |
| pkg/common/vsphere/vcenter_utils.go | Removed vCenter config parsing/auth-fault helpers. |
| pkg/common/vsphere/cns_manager.go | Removed CNS manager wrapper implementation. |
| pkg/common/config/types.go | Removed legacy gcfg-based config types. |
| pkg/cmd/utils_e2e_test.go | Removed e2e-style test for creating blocklist configmap. |
| pkg/cmd/utils.go | Removed CLI utility helpers (feature flags, version checks, blocklist CM creation, etc.). |
| pkg/cmd/datamgr/datamgr.go | Removed data-manager CLI entrypoint. |
| pkg/cmd/datamgr/cli/server/server.go | Removed data-manager server implementation/controllers wiring. |
| pkg/cmd/datamgr/cli/install/install.go | Removed data-manager install command. |
| pkg/cmd/constants.go | Removed cmd constants used by removed servers/installers. |
| pkg/cmd/backupdriver/cli/server/server.go | Removed backup-driver server implementation/controllers wiring. |
| pkg/cmd/backupdriver/cli/install/install.go | Removed backup-driver install command. |
| pkg/cmd/backupdriver/backupdriver.go | Removed backup-driver CLI entrypoint. |
| pkg/builder/upload_builder.go | Removed builders for removed datamover API types. |
| pkg/builder/snapshot_builder.go | Removed builders for removed backupdriver API types. |
| pkg/builder/object_meta.go | Removed shared objectmeta builder helpers (used by removed builders). |
| pkg/builder/download_builder.go | Removed builders for removed datamover API types. |
| pkg/builder/delete_snapshot_builder.go | Removed builders for removed backupdriver API types. |
| pkg/builder/clone_from_snapshot_builder.go | Removed builders for removed backupdriver API types. |
| pkg/builder/backup_repository_claim_builder.go | Removed builders for removed repository CRDs. |
| pkg/builder/backup_repository_builder.go | Removed builders for removed repository CRDs. |
| pkg/backuprepository/backup_repository_test.go | Removed repository claim tests (tied to removed CRDs). |
| pkg/backuprepository/backup_repository.go | Removed BackupRepository/Claim orchestration logic. |
| pkg/apis/datamover/v1alpha1/zz_generated.deepcopy.go | Removed datamover API types/codegen outputs. |
| pkg/apis/datamover/v1alpha1/upload.go | Removed Upload CRD types. |
| pkg/apis/datamover/v1alpha1/register.go | Removed datamover scheme registration. |
| pkg/apis/datamover/v1alpha1/download.go | Removed Download CRD types. |
| pkg/apis/datamover/v1alpha1/doc.go | Removed datamover API docs markers. |
| pkg/apis/backupdriver/v1alpha1/snapshot.go | Removed Snapshot/Clone/Delete CRD types. |
| pkg/apis/backupdriver/v1alpha1/register.go | Removed backupdriver scheme registration. |
| pkg/apis/backupdriver/v1alpha1/doc.go | Removed backupdriver API docs markers. |
| pkg/apis/backupdriver/v1alpha1/backup_repository.go | Removed BackupRepository/Claim CRD types. |
| hack/update-generated-crd-code.sh | Removed CRD codegen script. |
| hack/test.sh | Removed VDDK libs checks/LD_LIBRARY_PATH setup from tests. |
| hack/crd-gen/main.go | Removed CRD embedding generator tool. |
| hack/backup-driver-sample-crds/snapshot-status-example.yaml | Removed sample CRD manifest. |
| hack/backup-driver-sample-crds/snapshot-example.yaml | Removed sample CRD manifest. |
| go.mod | Bumped Go/tooling + dependencies; removed legacy deps. |
| deployment/create-pv-rbac.yaml | Removed RBAC manifests for removed backupdriver CRDs. |
| deployment/create-pv-providerserviceaccount.yaml | Removed PSA manifest for removed backupdriver CRDs. |
| deployment/create-deployment-for-plugin.yaml | Removed LD_LIBRARY_PATH env var from example deployment. |
| deployment/create-deployment-for-backupdriver.yaml | Removed backup-driver deployment example. |
| deployment/create-deployment-for-backupdriver-guest.yaml | Removed backup-driver guest deployment example. |
| cmd/vsphere-astrolabe/main.go | Removed vsphere-astrolabe binary entrypoint. |
| cmd/velero-plugin-for-vsphere/main.go | Updated plugin initialization + action registration + mapper/blocklist setup. |
| cmd/data-manager-for-plugin/main.go | Removed data-manager-for-plugin binary entrypoint. |
| cmd/backup-driver/main.go | Removed backup-driver binary entrypoint. |
| Makefile | Simplified build targets to produce only plugin binary/image; removed VDDK/Vix libs steps. |
| Dockerfile-plugin | Simplified plugin image packaging; removed VDDK libs + extra binaries. |
| Dockerfile-datamgr | Removed data-manager image build. |
| Dockerfile-backup-driver | Removed backup-driver image build. |
| .github/workflows/release.yml | Removed downloading GVDDK libs in release workflow. |
| .github/workflows/ci.yml | Removed downloading GVDDK libs in CI workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := utils.CreateBlockListConfigMap(logger); err != nil { | ||
| logrus.Fatalf("Failed to create block list config map: %v", err) | ||
| } |
There was a problem hiding this comment.
I prefer to error out on failure because the ConfigMap is not only used for the plugin to read the resource list.
The ConfigMap is also used for the users to customize as their needs.
| var ResourcesToBlockOnRestore = map[string]bool{ | ||
| // Kubernetes with vSphere Supervisor Cluster resources | ||
|
|
||
| // We need to remove some metadata from the Pod resource on | ||
| // Supervisor Cluster, i.e., annotation "vmware-system-vm-uuid" | ||
| // before the restore as the existing VM UUID is associated with | ||
| // the old VM that does not exist any more | ||
| "pods": true, | ||
|
|
||
| // The following resources are backed up everytime when a container | ||
| // is backed up on Supervisor Cluster. | ||
| // We should skip it at restore time. |
There was a problem hiding this comment.
I also don't agree with this proposal, because the pod is also modified during restore.
The only change is that the pod resource is not intercepted by the blocked resource list anymore.
The RIA handles the pod resource specifically.
|
README needs to be updated to reflect the change. |
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Modified the README content. |
blackpiglet
force-pushed
the
velero-dev
branch
2 times, most recently
from
557941b to
ab896ce
Compare
| ## Overview | ||
|
|
||
| This repository contains the Velero Plugin for vSphere. This plugin provides crash-consistent snapshots of vSphere block volumes and backup of volume data into S3 compatible storage. | ||
| This repository contains the Velero Plugin for vSphere. This plugin provides plugins for velero based on [go-plugin](https://github.com/hashicorp/go-plugin). |
There was a problem hiding this comment.
In the very beginning of README, please add a section like this:
The main branch of this repo contains code for the upcoming Velero vSphere Plugin 2.0. This is a breaking change. Velero vSphere Plugin 1.6.x and earlier, including the Backup Driver and the Data Mover, are not supported by Velero vSphere Plugin 2.0. If you need information regarding an earlier release, please refer to https://github.com/vmware-tanzu/velero-plugin-for-vsphere/tree/release-1.6.
|
@xing-yang @lubronzhan |
README.md
Outdated
|
|
||
|  | ||
|
|
||
| The main branch of this repo contains code for the upcoming Velero vSphere Plugin 2.0. This is a breaking change. Velero vSphere Plugin 1.6.x and earlier, including the Backup Driver and the Data Mover, are not supported by Velero vSphere Plugin 2.0. If you need information regarding an earlier release, please refer to https://github.com/vmware-tanzu/velero-plugin-for-vsphere/tree/release-1.6. |
There was a problem hiding this comment.
The link should point to the README.md of release-1.6 branch:
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
|
@xing-yang |
| } | ||
|
|
||
| func (p *NewBackupItemAction) Execute(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { | ||
| blocked, crdName, err := utils.IsObjectBlocked(item, p.Mapper, p.Log) |
There was a problem hiding this comment.
This block list contains virtualmachine, how do we use this plugin to backup virtualMachine in the future
There was a problem hiding this comment.
The velero-plugin-for-vsphere currently lacks specific code to correctly handle VirtualMachine resources, which is why these resources are being ignored.
Since your goal is to intentionally back up and restore VirtualMachine resources, it should be safe to remove them from the block list.
There was a problem hiding this comment.
For VM and VKS we need to exclude them in the future.
With that being said, I don't think exclusion is a good approach. We should let user specify the include-resource list. But I guess this can be improved in 9.2?
What this PR does / why we need it:
Because Velero is adopting the native uploader instead of VDDK, propose this PR to remove the unneeded code from this repository and introduce some changes to meet the current needs.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Yes, this PR does introduce a user-facing change.
The velero-plugin-for-vsphere was released and installed independently.
After this PR, the velero-plugin-for-vsphere is changed to only contain BackupItemAction and RestoreItemAction plugins required for the vSphere environment.
It will only work as a plugin installed by Velero, and it will be integrated by the downstream Velero Carvel package.
The ConfigMap of the blocked resource list is kept unchanged. The user can still uses this ConfigMap to customize the resource they want to block according to their needs.