Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

CLIPlugin Discovery: Use builder publish to publish artifacts and support Local Discovery and Distribution #972

Merged
merged 4 commits into from
Oct 28, 2021

Conversation

anujc25
Copy link
Contributor

@anujc25 anujc25 commented Oct 25, 2021

What this PR does / why we need it

  1. Make tanzu cli detect plugin commands based on context awareness
    • This change makes sure that only plugins available to the current context are added to tanzu cli when "context-aware-discovery" feature is enabled
  2. Implement plugin install, upgrade, clean, delete commands when the "context-aware-discovery" feature is enabled
  3. Implement plugin artifacts publish command with 'builder' plugin
    • Use artifact output directory generated with tanzu builder compile cli command as an input to the new tanzu builder publish command, and publish the generated artifacts to different discovery and distribution based on the type
    • This change implements the tanzu builder publish command by supporting local discovery/distribution
    • Adds a new Makefile section for "Building and publishing CLIPlugin Discovery resources and binaries

Which issue(s) this PR fixes

Fixes #945 , #958

Describe testing done for PR

Release note

Added support for local discovery and artifact distribution for CLIPlugin Discovery
Implemented `tanzu builder publish` command to publish artifacts and resources for local discovery and distribution

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@anujc25 anujc25 requested review from miclettej and a team as code owners October 25, 2021 17:59
@anujc25 anujc25 added area/cli area/plugin kind/feature Categorizes issue or PR as related to a new feature labels Oct 25, 2021
@anujc25 anujc25 changed the title Support for Local Discovery and Distribution and Use builder publish to publish artifacts CLIPlugin Discovery: Use builder publish to publish artifacts and support Local Discovery and Distribution Oct 25, 2021
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211025181023/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211025181336/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211026011859/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@vuil vuil self-assigned this Oct 26, 2021
@vuil vuil added the ok-to-merge PRs should be labelled with this before merging label Oct 26, 2021
@vuil
Copy link
Contributor

vuil commented Oct 26, 2021

It would have been nice to update pkg/v1/builder/template/plugintemplates/Makefile.tmpl to also include make target for tanzu builder publish eventually, but for now it might be good add some documentation in docs/dev about it.
Also worth documenting is the effect of turning on the context-aware-discovery feature on previously installed plugins, if exist. I believe the effect is that those plugins will essentially be ignored and a new set will be installed based on discovery data into different paths on the file system.

Other nits:
Commit title: Implement missing plugin command for context awareness
s/command/commands/

Copy link
Member

@zjs zjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Left a few nit-picky comments and a couple of questions.

Makefile Show resolved Hide resolved
pkg/v1/cli/common/constants.go Outdated Show resolved Hide resolved
pkg/v1/cli/common/constants.go Outdated Show resolved Hide resolved
pkg/v1/cli/command/core/plugin_manager.go Outdated Show resolved Hide resolved
pkg/v1/builder/command/publish.go Outdated Show resolved Hide resolved
pkg/v1/builder/command/publish/helper.go Outdated Show resolved Hide resolved
pkg/v1/builder/command/publish/helper.go Outdated Show resolved Hide resolved
- This changes makes sure that only plugins available to current
  context are added to tanzu cli when "context-aware-discovery"
  feature is enabled
- Implement plugin install, upgrade, clean, delete commands
  for context awareness
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211028045759/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

- Use artifact output directory generated with `tanzu builder compile cli` command  as an
  input to the new `tanzu builder publish` command, and publish the generated artifacts to
  different discovery and distribution based on the `type`
- This change implements `tanzu builder publish` command by supporting `local` discovery/distribution
- Adds a new `Makefile` section for "Building and publishing CLIPlugin Discovery resources and binaries
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211028145116/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/972/20211028151216/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the changes!

@anujc25 anujc25 merged commit b8b1635 into vmware-tanzu:main Oct 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli area/plugin cla-not-required kind/feature Categorizes issue or PR as related to a new feature ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for Local Discovery and Distribution for CLIPlugin API discovery work
4 participants