Merge plugin manifest automatically when building individual plugins with the `builder` plugin #454

anujc25 · 2023-08-02T06:38:33Z

What this PR does / why we need it

Merge plugin_manifest.yaml automatically when building individual plugins with the builder plugin
The builder plugin v1.0.0 or greater, supports generating the correct plugin_manifest.yaml and
plugin_bundle.tar.gz when the user runs make plugin-build PLUGIN_NAME="<plugin-name>" individually
multiple times to build different plugins possibly with different plugin versions. For example:

# Build `foo`. This will generate `plugin_bundle.tar.gz` containing just `foo:v1.0.0` plugin
make plugin-build PLUGIN_NAME="foo" PLUGIN_BUILD_VERSION=v1.0.0

# Build `bar`. This will generate `plugin_bundle.tar.gz` containing `foo:v1.0.0` and `bar:v2.0.0`
make plugin-build PLUGIN_NAME="bar" PLUGIN_BUILD_VERSION=v2.0.0

# Build `baz`. This will generate `plugin_bundle.tar.gz` containing `foo:v1.0.0`, `bar:v2.0.0`, `baz:v3.0.0`
make plugin-build PLUGIN_NAME="baz" PLUGIN_BUILD_VERSION=v3.0.0

Note: Because the builder plugin will automatically create merged plugin bundle if the plugin_manifest.yaml already exists,
To remove all the already built plugins and start from fresh, please configure environment variable PLUGIN_BUNDLE_OVERWRITE=true.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

$ make prepare-builder
cd cmd/plugin/builder && go build -o /Users/anujc/Documents/code/tanzu-cli/bin/builder .
$ rm -rf artifacts/
$ $ git describe --tags 
v1.0.0-dev-37-g569df492

Build just the builder plugin and notice the plugin manifest file contains only the builder plugin with the correct version

$ make plugin-build PLUGIN_NAME=builder
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-01T23:39:14.758537-07:00
plugins:
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v1.0.0-dev

Now, build just test plugin and notice that test plugin gets added to the plugin_manifest file along with builder

$ make plugin-build PLUGIN_NAME=test
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-01T23:41:02.373134-07:00
plugins:
    - name: test
      target: global
      description: Test the CLI
      versions:
        - v1.0.0-dev
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v1.0.0-dev

Now, force CLI to build plugins with version v2.0.0 and build only builder plugin and notice that builder plugin version in plugin_manifest gets updated

$ export PLUGIN_BUILD_VERSION=v2.0.0
$ make plugin-build PLUGIN_NAME=builder
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-01T23:42:39.901108-07:00
plugins:
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v2.0.0
    - name: test
      target: global
      description: Test the CLI
      versions:
        - v1.0.0-dev

While keeping the PLUGIN_BUILD_VERSION=v2.0.0 run make a target to build all plugins and notice both the plugin entry is updated in plugin_manifest with version v2.0.0

$ make plugin-build
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-01T23:43:49.704985-07:00
plugins:
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v2.0.0
    - name: test
      target: global
      description: Test the CLI
      versions:
        - v2.0.0

Run the make plugin-build again after upsetting the PLUGIN_BUILD_VERSION and verify plugin_manifest gets updated again with the right plugin versions.

# Unset PLUGIN_BUILD_VERSION so CLI uses the default git tag
$ make plugin-build
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-01T23:45:08.591372-07:00
plugins:
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v1.0.0-dev
    - name: test
      target: global
      description: Test the CLI
      versions:
        - v1.0.0-dev

Use PLUGIN_BUNDLE_OVERWRITE to overwrite the plugin bundle and manifest file.

$ make plugin-build PLUGIN_NAME=builder PLUGIN_BUNDLE_OVERWRITE=true
.
.
$ cat artifacts/plugins/plugin_manifest.yaml
created: 2023-08-02T11:35:49.47775-07:00
plugins:
    - name: builder
      target: global
      description: Build Tanzu components
      versions:
        - v1.0.0-dev

Release note

Merge `plugin_manifest.yaml` automatically when building individual plugins with the `builder` plugin

Additional information

Special notes for your reviewer

marckhouzam · 2023-08-02T12:14:03Z

Thanks for doing this.
I'm trying to understand the developer workflow here.

The flow you are trying to address I assume is:

in a repo with plugins A and B
build plugin A only
then build plugin B only
then publish both plugins together

But what if the flow is:

in a repo with plugins A and B
build plugin A only
do development for a couple of weeks on plugin B
build plugin B only
in an attempt to publish B only, a development version of A gets published as well

If this is a valid case but less troublesome, we can address it with documentation and/or a printout.

Another question, but not for this PR, should the artifacts/plugin_bundle.tar.gz only contain the plugins that are part of the manifest? Is there value in bundling all versions of all plugins if they are not in the manifest?

marckhouzam

LGTM
Only nits about comments.

cmd/plugin/builder/command/cli_compile.go

anujc25 · 2023-08-02T16:16:41Z

Another question, but not for this PR, should the artifacts/plugin_bundle.tar.gz only contain the plugins that are part of the manifest? Is there value in bundling all versions of all plugins if they are not in the manifest?

Good point. I agree that it does not make much sense to bundle plugin binaries that are not mentioned under plugin_manifest.yaml. I think we can improve this as part of a separate PR. I will file an issue to track this enhancement.

anujc25 · 2023-08-02T16:41:08Z

Thanks for doing this. I'm trying to understand the developer workflow here.

The flow you are trying to address I assume is:

in a repo with plugins A and B

build plugin A only

then build plugin B only

then publish both plugins together

But what if the flow is:

in a repo with plugins A and B

build plugin A only

do development for a couple of weeks on plugin B

build plugin B only

in an attempt to publish B only, a development version of A gets published as well

If this is a valid case but less troublesome, we can address it with documentation and/or a printout.

Thanks for the detailed example. Yes, I feel this scenario is more of a corner case as this should not be an issue for CI systems which are used mostly for publishing plugins. But as you mentioned I also feel that adding documentation clearly explaining this is important. So, I have updated docs/plugindev/README.md with the additional details explaining this.

marckhouzam

Nice doc! Thanks

LGTM

docs/plugindev/README.md

marckhouzam

Nice improvement with PLUGIN_BUNDLE_OVERWRITE
LGTM

…ifest in PluginBundle

chandrareddyp

LGTM!

vuil · 2023-08-03T21:14:55Z

cmd/plugin/builder/command/cli_compile.go

+	return mergedManifest
+}
+
+// findpluginInManifest checks if the plugin already specified in the manifest or not


nit: findPluginInManifest

plugin already -> "plugin is already"

maybe just take a note to update in the followup.

anujc25 requested a review from a team as a code owner August 2, 2023 06:38

vmwclabot added the cla-not-required label Aug 2, 2023

marckhouzam approved these changes Aug 2, 2023

View reviewed changes

anujc25 force-pushed the merge-plugin-manifest-automatically branch from 569df49 to af1fa21 Compare August 2, 2023 16:14

anujc25 force-pushed the merge-plugin-manifest-automatically branch from af1fa21 to 0fb40a2 Compare August 2, 2023 16:38

marckhouzam approved these changes Aug 2, 2023

View reviewed changes

anujc25 force-pushed the merge-plugin-manifest-automatically branch from 0fb40a2 to 91d25be Compare August 2, 2023 17:01

anujc25 commented Aug 2, 2023

View reviewed changes

docs/plugindev/README.md Outdated Show resolved Hide resolved

Merge plugin manifest automatically when building individual plugins

f2fb524

anujc25 force-pushed the merge-plugin-manifest-automatically branch from 93f362d to 754ebcd Compare August 2, 2023 18:39

marckhouzam approved these changes Aug 2, 2023

View reviewed changes

anujc25 force-pushed the merge-plugin-manifest-automatically branch from 754ebcd to 80bbad3 Compare August 2, 2023 19:02

Expose PLUGIN_BUNDLE_OVERWRITE to forcefully overwrite the plugin_man…

d425828

…ifest in PluginBundle

anujc25 force-pushed the merge-plugin-manifest-automatically branch from 80bbad3 to d425828 Compare August 2, 2023 19:04

chandrareddyp approved these changes Aug 2, 2023

View reviewed changes

anujc25 merged commit 2cb95d0 into vmware-tanzu:main Aug 2, 2023

anujc25 mentioned this pull request Aug 2, 2023

[Release-1.0] Merge plugin manifest automatically when building individual plugins with the builder plugin #456

Merged

vuil reviewed Aug 3, 2023

View reviewed changes

marckhouzam added this to the v1.0.0 milestone Oct 20, 2023

anujc25 mentioned this pull request Jan 11, 2024

Merge plugin-group manifest automatically when building individual plugins with the builder plugin #644

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge plugin manifest automatically when building individual plugins with the `builder` plugin #454

Merge plugin manifest automatically when building individual plugins with the `builder` plugin #454

anujc25 commented Aug 2, 2023 •

edited

Loading

marckhouzam commented Aug 2, 2023 •

edited

Loading

marckhouzam left a comment

anujc25 commented Aug 2, 2023

anujc25 commented Aug 2, 2023

marckhouzam left a comment

marckhouzam left a comment

chandrareddyp left a comment

vuil Aug 3, 2023

Merge plugin manifest automatically when building individual plugins with the builder plugin #454

Merge plugin manifest automatically when building individual plugins with the builder plugin #454

Conversation

anujc25 commented Aug 2, 2023 • edited Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

Additional information

Special notes for your reviewer

marckhouzam commented Aug 2, 2023 • edited Loading

marckhouzam left a comment

Choose a reason for hiding this comment

anujc25 commented Aug 2, 2023

anujc25 commented Aug 2, 2023

marckhouzam left a comment

Choose a reason for hiding this comment

marckhouzam left a comment

Choose a reason for hiding this comment

chandrareddyp left a comment

Choose a reason for hiding this comment

vuil Aug 3, 2023

Choose a reason for hiding this comment

Merge plugin manifest automatically when building individual plugins with the `builder` plugin #454

Merge plugin manifest automatically when building individual plugins with the `builder` plugin #454

anujc25 commented Aug 2, 2023 •

edited

Loading

marckhouzam commented Aug 2, 2023 •

edited

Loading