-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Enhanced Plugin Management — Design Proposal #9364
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
Open
abendrothj
wants to merge
8
commits into
vmware-tanzu:main
Choose a base branch
from
abendrothj:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Lyndon-Li <[email protected]> Signed-off-by: Jake Abendroth <[email protected]>
Signed-off-by: Lyndon-Li <[email protected]> Signed-off-by: Jake Abendroth <[email protected]>
- Introduced 'Command' and 'BuiltIn' fields in PluginInfo for better plugin management. - Updated GetInstalledPluginInfo to mark built-in plugins. - Enhanced remove command to prevent removal of built-in plugins. - Modified plugin printer to display built-in status. Signed-off-by: Jake Abendroth <[email protected]>
- Introduced tests for GetInstalledPluginInfo to validate built-in plugin detection. - Added tests for plugin removal logic, ensuring built-in plugins cannot be removed. - Implemented heuristic matching for plugin names in various scenarios. Signed-off-by: Jake Abendroth <[email protected]>
- Add changelog for issue vmware-tanzu#3210 plugin management enhancements - Update overview-plugins.md to document new plugin name removal syntax - Add section explaining built-in vs external plugins - Document new BuiltIn column in 'velero plugin get' output Signed-off-by: Jake Abendroth <[email protected]>
…l plugins - Changed the command paths for the AWS plugin to reflect its status as an external plugin. - Updated test cases to ensure correct detection of built-in and external plugins. - Revised documentation to clarify the distinction between built-in and external plugins, including examples. Signed-off-by: Jake Abendroth <[email protected]>
…removal commands - Adjusted the output of the 'velero plugin get' command to correctly indicate the built-in status of the AWS and pod plugins. - Updated examples for the plugin removal command to demonstrate the inability to remove built-in plugins. Signed-off-by: Jake Abendroth <[email protected]>
3 tasks
Collaborator
|
might want to rebase. design proposal shouldn't contain .go changes. |
Collaborator
|
We can keep code changes in for now and once design is agreed and if implementation still need enhancement after that we can move implementation out to a follow up pr. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status: Draft
Author: Jake Abendroth (abendrothj)
Related issue: #3210
Related implementation PR: #9352
Target release: v1.19 (note: v1.18 feature freeze is near)
Summary
This proposal improves Velero's CLI/plugin UX by clearly indicating which plugins are built into the Velero server binary and by allowing
velero plugin remove <plugin-name>semantics that map user-facing plugin names (e.g., velero.io/aws) to init-container images/names. The changes are intentionally limited to CLI/API metadata and removal UX; they do not change plugin discovery or plugin runtime registration.The implementation work is in #9352 and this design PR is intended to let maintainers review the design independent of the implementation.
Motivation
velero plugin getbut cannot tell which plugins are built-in (and therefore cannot be removed).velero plugin removecurrently requires the init container name or image string; users expect to use plugin names shown invelero plugin getinstead.Goals
velero plugin remove <plugin-name>that resolves to an init container image/name and refuses removal of built-in plugins.velero plugin remove <image|container>behavior for backwards compatibility.Non-Goals
Proposed API changes
Modify the ServerStatusRequest API plugin information to carry additional optional fields:
File: pkg/apis/velero/v1/server_status_request_types.go
These fields are optional to preserve API compatibility with older clients.
Server-side behavior
File: internal/velero/serverstatusrequest.go
CLI output
File: pkg/cmd/util/output/plugin_printer.go
Plugin removal command
File: pkg/cmd/cli/plugin/remove.go
Accept plugin names (e.g., velero.io/aws) as input in addition to existing image/container names.
Workflow:
velero.io/aws->aws) to container name or image substring.--image/--containerflag).Maintain backwards compatibility:
velero plugin remove <image|container>remains valid.Compatibility
velero plugin remove <image|container>continues to work.Security considerations
Tests
velero plugin getshows BuiltIn flags;velero plugin remove <plugin-name>resolves correctly and patches Deployment; refusing removal for BuiltIn plugins.Implementation references