Skip to content
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

📖 Dynamic Bundle Design Proposal #2671

Closed

Conversation

everettraven
Copy link
Contributor

Description of the change

Add a design for more a more dynamic bundle design

Motivation for the change

Make it easier to define default functionality by implementing a more dynamic style of Bundle

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 3, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @everettraven. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 3, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: everettraven
To complete the pull request process, please assign pwittrock after the PR has been reviewed.
You can assign the PR to them by writing /assign @pwittrock in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. 🥇
Btw, I loved the gifs. :-)
Some questions were added for us better understand your idea.

designs/dynamic-bundle.md Outdated Show resolved Hide resolved
The proposed solution is to create a new form of `Bundle` that is more dynamic and can be used to create a default bundle that contains sensible default plugins that can be used with all plugins. This could be overriden by a command line flag.

## User Stories
- As a Kubebuilder maintainer I would like to have Kubebuilder implement sane default plugins that always run before and/or after plugins, specifying this once and it applying to all uses
Copy link
Member

Choose a reason for hiding this comment

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

could you please suplement the use cases to follow the form: I am as ... would like to .... so that ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will work on making those changes

- A `Bundle` no longer has to be created for each plugin to implement sane defaults

## Examples
Context: Kubebuilder defines that the `kustomize/v1` plugin should always run before any other plugin. Kubebuilder also defines the default plugin to be run if a user does not specify plugins via the `--plugins` flag is `go/v3`
Copy link
Member

Choose a reason for hiding this comment

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

We have the config-gen alpha command which is a solution that replaces kustomize
When it was implemented to the project the idea was users could choose the best scaffold for their needs
So that, they could have a new Golang plugin that uses kustomize or config-gen.

If we move with this idea would that mean remove config-gen and accepted that the only way to configure the project is with kustomize. Am I right? is that something that we really want? Why we would say that a specific plugin is required for any based action done with the CLI? What are pros and cons here? How that can affect others? What is the plan to deprecate and remove config-gen (if that is the case)?

Copy link
Contributor Author

@everettraven everettraven May 5, 2022

Choose a reason for hiding this comment

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

If we move with this idea would that mean remove config-gen and accepted that the only way to configure the project is with kustomize. Am I right? is that something that we really want?

This example may be misleading (and I can work on fixing it) because the idea behind this example is that Kubebuilder is defaulting to using kustomize as the way to configure the project. This however can still be overridden by the user if they would prefer to use something like config-gen (this would require them passing a flag to signify they want to override Kubebuilder's default functionality as well as passing the necessary plugin via --plugins)

Why we would say that a specific plugin is required for any based action done with the CLI? What are pros and cons here? How that can affect others?

This allows Kubebuilder to be a tool that focuses on scaffolding based on standards and best practices to develop an operator, while still providing users the ability to override this functionality.

IMO:

  • Pros:

    • Defines default plugins that should be run regardless of what plugins the users pass in, but is able to be overridden by users
    • Enforces standards/best practices
    • Bundles can be defined once and be versatile, applying these defaults to any plugins a user wants to use.
  • Cons:

    • Will require users to get used to a newer CLI interface

What is the plan to deprecate and remove config-gen (if that is the case)?

This should not be necessary


I hope this helps clarify, let me know if you have any more questions!

Copy link
Member

@camilamacedo86 camilamacedo86 May 5, 2022

Choose a reason for hiding this comment

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

Thank you. @everettraven it seems that all pros are already addressed with the current implementation. So, I still trying to figure out what we can do with the proposal that we cannot do today?

The API allows define the default plugins for each project version indeed, see: https://github.com/operator-framework/operator-sdk/blob/master/internal/cmd/operator-sdk/cli/cli.go#L122-L124

Then, we can overwrite them by using the plugins flag to scaffold using any available plugin instead of the default ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API allows define the default plugins for each project version indeed, see: https://github.com/operator-framework/operator-sdk/blob/master/internal/cmd/operator-sdk/cli/cli.go#L122-L124

My understanding is that those plugins are ones to be run if a user does not use the --plugins flag.

The proposal here is to implement a bundle that no matter what plugins a user passes in with the --plugins flag certain plugins are still run unless the user specifically wants to override that functionality. I think of it more as a way to have only one bundle definition that will apply for multiple plugins rather than a new bundle definition for each plugin to implement the same defaults.

- `kubebuilder init --plugins other.plugin/v3`
- Should scaffold using the `kustomize/v1` plugin and the `other.plugin/v3` plugin
- `kubebuilder init --plugins other.plugin/v3 --override-default-plugin-chain`
- Should scaffold using only the `other.plugin/v3` plugin
Copy link
Member

Choose a reason for hiding this comment

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

In this way, we are here proposing that kubebuilder init should only scaffold the kustomize and then we would no longer have the default plugins used by the CLI?

See that we define the default plugin that should be used by Kubebuilder/SDK or any project that uses Kubebuilder CLI Lib so that when a user runs kubebuilder init, that means scaffold the default plugin is. Today it goes/v3.

What are the advantages for kubebuilder users?
How that would make the UX and usability better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this way, we are here proposing that kubebuilder init should only scaffold the kustomize and then we would no longer have the default plugins used by the CLI?

See that we define the default plugin that should be used by Kubebuilder/SDK or any project that uses Kubebuilder CLI Lib so that when a user runs kubebuilder init, that means scaffold the default plugin is. Today it goes/v3.

We should still have the default plugin functionality since Kubebuilder users expect it to function that way. This design should not impact that in anyway.

This is rather proposing that a dynamic bundle be implemented so that there can be default plugins that run no matter what plugins a user passes in. These examples are just meant to show what it would be like if Kubebuilder implemented this functionality.

In these examples, the context is that Kubebuilder wants to make sure the kustomize plugin is always run before any other plugin. This doesn't actually have to be the case if we don't want it to be. This however seems reasonable to me based on the current go/v3 bundle having kustomize scaffold before the go/v3 plugin and the documentation that states that language plugins should follow this format:

  mylanguagev1Bundle, _ := plugin.NewBundle(language.DefaultNameQualifier, plugin.Version{Number: 1},
        kustomizecommonv1.Plugin{}, // extend the common base from Kuebebuilder
        mylanguagev1.Plugin{}, // your plugin language which will do the scaffolds for the specific language on top of the common base
    )

What are the advantages for kubebuilder users?
How that would make the UX and usability better?

The advantages are:

  • Language plugin authors don't have to worry about doing anything in particular to make their language plugin work with Kubebuilder because that functionality is already defaulted in Kubebuilder
  • Kubebuilder users now will only have to define the plugins they would like to use that Kubebuilder doesn't default to. For example if Kubebuilder defined kustomize as a default plugin, kubebuilder init --plugins kustomize/v1,other/v1 could just be kubebuilder init --plugins other/v1

Copy link
Member

Choose a reason for hiding this comment

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

Hi @everettraven:

Following some coments inline:

This is rather proposing that a dynamic bundle be implemented so that there can be default plugins that run no matter what plugins a user passes in. These examples are just meant to show what it would be like if Kubebuilder implemented this functionality.

We have the default plugin option already and it has been used
Also, we can overwrite the default plugin by --plugins informed via the flag

In these examples, the context is that Kubebuilder wants to make sure the kustomize plugin is always run before any other plugin.

Why would we force run kustomize in all cases?
See that API is valid for any plugin/CLI and kustomize does not fit in any scenario.
As shared before, we indeed have a proposal in place to config the projects using the config-gen approach instead of kustomize. Also, we have people that do not use kustomize at all, see: #2350

So, if the main goal of this proposal is to force kustomize ( as it seems to be ) I do not think that we can go along with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of this is not to force kustomize.

The goal is specifically to create a new Bundle format that allows for a single definition that will apply to many different plugins. Kubebuilder does not have to use this new Bundle format to force anything.

Comment on lines +76 to +77
beforePlugins []Plugin
afterPlugins []Plugin
Copy link
Member

Choose a reason for hiding this comment

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

What mean beforePlugins and afterPlugins?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

beforePlugins are plugins that should be run before user defined (or a default if a user does not define) plugins. afterPlugins are plugins that should be run after user defined (or a default if a user does not define) plugins.

This is what the flow would be like:
beforePlugins > user defined (or default) > afterPlugins

Copy link
Member

Choose a reason for hiding this comment

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

We are here trying to spec default pre and post scaffolds for the CLI.
So, it shows me that it would fit under the CLI API domain of responsibility instead of a Bundle API.


File pkg/plugin/bundle.go:
```go
type dynamicBundle struct {
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for the name dynamic?
When/where/why this solution is dynamic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea behind the name dynamic is that there can be defaults that are defined to run before and after all plugins and the user can "inject" plugins into the chain. The idea is that there is one bundle definition for all plugins with a specific supported project version rather than having to repeat the definition for each new language plugin.

"dynamic."+plugins.DefaultNameQualifier,
plugin.Version{Number: 3},
[]plugin.Plugin{
kustomizecommonv1.Plugin{},
Copy link
Member

Choose a reason for hiding this comment

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

What is the main motivation with DynamicBundle?
What are the advantages of using it?
What would allow us to achieve what is not possible be done with Bundle API?

Copy link
Member

Choose a reason for hiding this comment

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

Is the goal allow override the default behaviour done with kubebuilder init?
If yes, what is the difference of:

  1. Create an external plugin for i.e with a bundle of kustomize/v1 and another golang v3 base fake as you did
  2. Then, when we have the external plugins ( phase 2 ) call kubebuilder init --external-plugins=mynewexternalone

That would not scaffold the plugin implemented by you outside?
Is not the Bundle ApI enough for your re-use kustomize and any other plugin of your choice to build new external plugins by composition and re-use the implementations done in Kubebuilder or any other project that follows its interfaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the main motivation with DynamicBundle?
What are the advantages of using it?

I think it is probably best to point to some of my previous answers for these questions:

Is the goal allow override the default behaviour done with kubebuilder init? If yes, what is the difference of:

1. Create an external plugin for i.e with a bundle of kustomize/v1 and another golang v3 base fake as you did

2. Then, when we have the external plugins ( phase 2 ) call kubebuilder init --external-plugins=mynewexternalone

That would not scaffold the plugin implemented by you outside? Is not the Bundle ApI enough for your re-use kustomize and any other plugin of your choice to build new external plugins by composition and re-use the implementations done in Kubebuilder or any other project that follows its interfaces?

I think that you would be able to do it this way but that is causing more work for plugin authors to appease users when this functionality could just be done by Kubebuilder by default. I think the core of this idea is that there is the ability to define some default plugins that are run before and after all other plugins defined by a user (or a default one used). This allows for fewer DynamicBundle definitions that allow flexibility in the language plugins rather than defining a new Bundle for each and every new language plugin.

Copy link
Member

@camilamacedo86 camilamacedo86 May 5, 2022

Choose a reason for hiding this comment

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

@everettraven , so what you are trying to do I think would fit in the CLI API, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/cli.go

That could be new methods which allow us to inform preScaffoldDefaultPlugins and posScaffoldDefaultPlugins for the CLI so that each CLI could have its pre or post scaffold ones by default an option. (at the same way that we have the DefaultPlugin option)

However, I would argue on SDK or Kubebuilder that we should not use it. Why?

  • IHMO, the current implementation, makes very clear what are the plugins used to build/compose each plugin
  • We might have plugins that do not use kustomize at all. (i.e. java plugin available on SDK and for Kubebuilder we want to have config-gen one)
  • We indeed could NOT use the postScaffoldDefault plugin for all specific SDK helpers responsible for doing the customisations on top of SDK because it would break the java plugin for example.

So, I would be OK with adding these options to the API, but I do not see how SDK or Kubebuidler could take advantage of it.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@everettraven everettraven May 7, 2022

Choose a reason for hiding this comment

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

After taking some time to play around with the CLI suggestion I think this definitely makes more sense. You are right that this is the best way to go about allowing this kind of functionality. I am unnecessarily adding complexity here by adding a new Bundle API on top of a CLI API change when all that is necessary is a CLI change.

I also think doing the CLI change properly would prevent any breaking changes being introduced to Kubebuilder and SDK

Thanks for mentioning this and bringing it up @camilamacedo86 !

If this is some functionality that you think may be nice to have in the CLI API (even if Kubebuilder or SDK wouldn't be able to use it) I'd be happy to write up a new design proposal (or modify this one) and PoC for it.


In the above example a plugin `Bundle` is created that ensures that the `kustomize/v1` plugin is run before the `go/v3` plugin.

With the addition of Phase 2 Plugins on the way, users using the `--plugins` flag to utilize external plugins will not get the benefits of using some default functionality that previous Kubebuilder users using the default `go/v3` plugin gained via the `kustomize/v1` plugin.
Copy link
Contributor

Choose a reason for hiding this comment

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

In my understanding, --plugins field would ensure the ordering of plugins by running the plugins in order, irrespective of whether we are including internal plugins or external plugins in the plugin chain. So we will be able to ensure that the kustomize/v1 plugin is run before the go/v3 plugin, just like how it's run in the Bundle concept.

Can you clarify about what is the default functionality and why we are not able to reap those benefits when trying to run external plugins?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was also brought up by @camilamacedo86 previously and I answered here: #2671 (comment)

If there are more questions after looking at that answer let me know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated this section to reflect the actual intentions of this proposal

- As a Kubebuilder user I would like to be able to override the default Kubebuilder plugin chain, scaffolding only exactly what I feel I need

## Goals
- `kubebuilder` is able to define sane defaults for plugins that should always be run before and/or after user specified plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these sane defaults applicable for all plugins in the chain or only to some?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to know some examples of what these sane defaults are going to look like. Are these default applicable for plugins or kubebuilder itself?

If it's for plugins, each plugin might be independent of the other plugins, how can we gather the initial default values used for the plugins? Do we need an API call for that to gather plugin specific info first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally they would apply to all plugins that have the same supported project versions

With the addition of Phase 2 Plugins on the way, users using the `--plugins` flag to utilize external plugins will not get the benefits of using some default functionality that previous Kubebuilder users using the default `go/v3` plugin gained via the `kustomize/v1` plugin.

## Proposal
The proposed solution is to create a new form of `Bundle` that is more dynamic and can be used to create a default bundle that contains sensible default plugins that can be used with all plugins. This could be overriden by a command line flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would like to understand more about the use-cases of the DynamicBundle.

Currently, in Kubebuilder, there are PreScaffold and PostScaffold hooks which can be used by every subcommand (init, create api, create webhook). Pre scaffold logic allows to ensure the correct go version, validating that main.go exists, so on. Similarly, post scaffold logic can execute a set of tasks after the main scaffolding is done, such as updating dependencies. I'm wondering if these can be customized further to set the sane defaults during PostScaffold logic, without having to introduce new default plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the biggest use case is to make it so that we don't have to create a new Bundle every time we create a new language plugin.

The sane defaults are more so default plugins that should be run. Hopefully this comment should clear it up a bit more but let me know if you need more details: #2671 (comment)

Copy link
Contributor

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

Nice proposal @everettraven. Left a few questions to know more the use-cases of DynamicBundle.

@@ -0,0 +1,267 @@
Dynamic Bundle
Copy link
Member

@camilamacedo86 camilamacedo86 May 5, 2022

Choose a reason for hiding this comment

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

Hi @everettraven,

It seems that the primary goal of this proposal would be to force the kustmize plugin to be executed by the CLI always. Then, that does not seem to go along with the project vision:

Additionally, currently, Kubebuilder CLI API allows already:

  • Define default plugins ( which should be the default option when the subcommand tool init is called)
  • Override the default option by passing one or more plugins via the flag --plugins

Then, when we have the plugin phase 2:

  • The API consumer(s) will be able to create their own plugins, which can be built via composition and using kustomize/v1 or alpha-v2 or any version that we or other projects might offer. They also can use the Bundle API to create plugins by composition, and they also can decide to not use the kustomize at all if it does go along with their purpose.
  • Users will be able to override the default behaviour by informing the external plugins. It might use kustomize it might not use it at all.

Because of the above reasons, I think we cannot accept this one. In this way, my vote here would be -1.
However, please let me know if I misunderstood its goals.

And again, thank you for proposing the idea and suggestion.

c/c @rashmigottipati @jmrodri

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that the primary goal of this proposal would be to force the kustmize plugin to be executed by the CLI always.

This is not the goal of this proposal. The goal is to create a new Bundle format that allows for one definition that can apply to multiple plugins.

Some more information:

  1. Kubebuilder does not have to force the kustomize usage with this option. The default plugins can just be left empty.
  2. If Kubebuilder did force kustomize usage it would still be possible to override with another flag.

Additionally, currently, Kubebuilder CLI API allows already:

* Define default plugins ( which should be the default option when the subcommand tool init is called)

* Override the default option by passing one or more plugins via the flag --plugins

Then, when we have the plugin phase 2:

* The API consumer(s) will be able to create plugins via composition to re-use kustomize or not by taking advantage of the Bundle API

* Users will be able to override the default behaviour to inform the external plugins.

My understanding is that the default plugins are just ones that are run unless a user specifies otherwise with the --plugins command.

The idea here is that the DynamicBundle allows for one definition that applies to multiple plugins instead of having to create a new Bundle definition for each new plugin. In the case of Operator SDK, the 4 Bundle definitions for the Go, Ansible, Helm, and Hybrid plugins could be condensed into one DynamicBundle definition.

I may not understand enough about the way that it will work with Phase 2 Plugins so if the Phase 2 Plugins implementation would make this obsolete or unnecessary then this can be ignored since it wouldn't be much of an improvement :)

Copy link
Member

Choose a reason for hiding this comment

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

Please, see: #2671 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Hi @everettraven,

If Kubebuilder did force kustomize usage it would still be possible to override with another flag.

So, we would force and change the default behaviour of overwriting by using the --plugins flag. We would need a new flag to do what --plugins flag does today. Then, how that would work when SDK for i.e scaffold the java plugin? We would need to say for people use override flag instead of the plugins flag used?

c/c @rashmigottipati @jmrodri

Copy link
Contributor Author

@everettraven everettraven May 5, 2022

Choose a reason for hiding this comment

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

That is correct. In the PoC I made this new override flag to be called --override-default-plugin-chain

If a user wanted to ensure that only the plugins they specified to use would be used, they could run something like:

kubebuilder init --plugins someplugin/v1 --override-default-plugin-chain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, using SDK as an example would mean:

* User would no longer be able to have Golang projects scaffolds with operator-sdk init (it would only scaffold what the pre/pos default plugins defined). Then, they would need to `operator-sdk init --plugin=base.go/v3`

If the DynamicBundle is configured to default to scaffolding Golang projects in SDK then running operator-sdk init would still scaffold with the Go plugin.

* Users would no longer be able to scaffold `operator-sdk init --plugin=java/vx`. They would need to do  `operator-sdk init --override-default-plugin-chain=java/vx`

My understanding is that users would still be able to scaffold with the java plugin but it would just result in unused files. In this they would likely want to run something like operator-sdk init --plugins java/v1 --override-default-plugin-chain

Copy link
Contributor Author

@everettraven everettraven May 5, 2022

Choose a reason for hiding this comment

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

Just to break it down even further, the thought behind how the DynamicBundle is structured is this:
(This implements the Bundle interface as well but for this example I wanted to fully expand it to try and provide a further explanation)

type dynamicBundle struct {
	bundle
	name    string
	version Version
	// these would be the plugins that a user wants to use. Can also set a default value for this.
	plugins []Plugin

	supportedProjectVersions []config.Version

	// these would be the plugins that are set to run before any other plugins.
	beforePlugins []Plugin

	// these would be the plugins that are set to run after all other plugins.
	afterPlugins  []Plugin
}

The DynamicBundle implements the Plugin interface and can be used the exact same way as a regular Bundle. For reference, in the PoC the Plugins() function looks like this:

func (db dynamicBundle) Plugins() []Plugin {
	return append(db.beforePlugins, append(db.plugins, db.afterPlugins...)...)
}

This returns a list of plugins just like a regular Bundle. In order to fully implement the idea behind this there would need to be changes in the CLI to modify the DynamicBundle's plugin field based on the user input.

I guess in reality this means that the new DynamicBundle would not in itself make any changes to the CLI, but would require changes to the CLI for it to work the way intended.

All in all, the DynamicBundle by itself does not introduce any new functionality except breaking the plugin chain into a before chain, user defined chain, and an after chain.

The CLI API changes would be what truly makes the DynamicBundle able to function the way this proposal intends by following this kind of logic:

flowchart TD
  A[DynamicBundle is used] --> B{override-default-plugin-chain specified?};
  B -- Yes --> C[Only use plugins specified by user];
  B -- No --> D{plugins specified by user?};
  D -- Yes --> E[Replace DynamicBundle plugins field with the plugins specified by the user];
  D -- No --> F[Use Defaults];
  E --> G[scaffold];
  F --> G;
  C --> G;
Loading

Now I'd like to break this down by each potential scenario and show what each field of a DynamicBundle might look like at every stage of the flow.

Each scenario will start with a DynamicBundle in this state:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

Scenario 1 -- DynamicBundle is used and a user runs operator-sdk init:

Flow stage: override-default-plugin-chain specified?

Because the --override-default-plugin-chain is not specified this evaluates to "No".

The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

Flow stage: plugins specified by user?

Because the --plugins flag is not used this evaluates to "No".

The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
} 

Flow Stage: use defaults

The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
} 

Flow Stage: scaffold

At this stage the DynamicBundle state is:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

This results in a project being scaffolded with kustomize/v1 followed by go/v3 followed by manifests/v2


Scenario 2 -- DynamicBundle is used and a user runs operator-sdk init --plugins other/v1:

Flow stage: override-default-plugin-chain specified?

Because the --override-default-plugin-chain is not specified this evaluates to "No".

The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

Flow stage: plugins specified by user?

Because the --plugins flag is used this evaluates to "Yes".
The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

Flow Stage: Replace DynamicBundle plugins field with the plugins specified by the user

The user has specified --plugins other/v1 so DynamicBundle state becomes:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ other/v1 ],
   afterPlugins: [ manifests/v2 ]
}

Flow Stage: scaffold

At this stage the DynamicBundle state is:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ other/v1 ],
   afterPlugins: [ manifests/v2 ]
}

This results in a project being scaffolded with kustomize/v1 followed by other/v1 followed by manifests/v2


Scenario 3 -- DynamicBundle is used and a user runs operator-sdk init --plugins other/v1 --override-default-plugin-chain:

Flow stage: override-default-plugin-chain specified?

Because the --override-default-plugin-chain is specified this evaluates to "Yes".

The DynamicBundle state remains:

DynamicBundle {
   beforePlugins: [ kustomize/v1 ],
   plugins: [ go/v3 ],
   afterPlugins: [ manifests/v2 ]
}

Flow stage: Only use plugins specified by user

The user has specified --plugins other/v1 so DynamicBundle state becomes:

DynamicBundle {
   beforePlugins: [],
   plugins: [ other/v1 ],
   afterPlugins: []
}

Flow Stage: scaffold

At this stage the DynamicBundle state is:

DynamicBundle {
   beforePlugins: [],
   plugins: [ other/v1 ],
   afterPlugins: []
}

This results in a project being scaffolded with other/v1


From my point of view this makes this proposal a little bit of a CLI API change and a new kind of Bundle API.

I hope that this helps clarify the intentions behind this proposal. Again, if this isn't something that seems fitting or useful to Kubebuilder that is fine - I just wanted to make sure the intentions behind the change is as clear as possible.

/cc @rashmigottipati @jmrodri

Copy link
Member

@camilamacedo86 camilamacedo86 May 6, 2022

Choose a reason for hiding this comment

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

Hi - @everettraven,

  • a) Users will be able to still generating the Helm/Ansible/Go and Java plugins offered by SDK using exactly the same commands and flags and have the same behaviours OR they will need to change it to begin to use override-default-plugin-chain instead?
  • b) Why the new override-default-plugin-chain flag is required? Why do we need a new flag to do what is done via the plugins flag currently?
  • c) So, how would that work if we want Bundles and DynimicBundles for the same CLI?
  • d) How it will work with the subcommands create-api/create-webhook? Today we can kb create-api --plugins=declarative/v1, which will create an API and scaffold the controller using the declarative plugin?
  • e) Would Kubebuilders users still be able to scaffold only the Golang base plugin with kubebuilder init --plugin=base.go/v3 after this implementation OR they would need to use the new flag instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a) Users will be able to still generating the Helm/Ansible/Go and Java plugins offered by SDK using exactly the same commands and flags and have the same behaviours OR they will need to change it to begin to use override-default-plugin-chain instead?

So there is a bit of a conditional with this. IF SDK decided to use the DynamicBundle the same functionality would work for Helm/Ansible/Go plugins. The Java plugin would be where users would have to pass the new --override-default-plugin-chain command.

Just to reiterate, this change does not have to be implemented by either Kubebuilder or SDK. They can keep their current implementation and nothing would change for the users.

Only change that would appear would be the new --override-default-plugin-chain flag. We could even potentially put that flag behind a conditional statement in the CLI logic to obfuscate it if a DynamicBundle is not being used.

b) Why the new override-default-plugin-chain flag is required? Why do we need a new flag to do what is done via the plugins flag currently?

The new --override-default-plugin-chain is because the idea of the DynamicBundle is that there is the ability define the separate layers of the bundle. These layers being:

  • Before
  • Injected
  • After
    These layers are processed in the order of Before -> Injected -> After

My thoughts were that if a DynamicBundle were being used and a user specified the --plugins flag that then those plugins would be used in the Injected layer. This would mean that in order to prevent the DynamicBundle logic of running in the order of Before > Injected > After and only running plugins in the Injected layer users would have to override that logic.

Now that I am thinking more about it, we could also perform this logic in an alternative way that I think would have no impact to the current functionality. This alternative way would instead rely on a different flag called something like --inject-plugins, where a user can define plugins that they would want to inject into the DynamicBundle flow. This would keep the logic that if the --plugins flag is used then completely override and just use the exact plugin chain the user has specified.

What do you think of this potential alternative? (I actually think I prefer this alternative)

c) So, how would that work if we want Bundles and DynimicBundles for the same CLI?

It should work the same because a Bundle just acts as a plugin. I don't immediately see why there would be any major conflicts

d) How it will work with the subcommands create-api/create-webhook? Today we can kb create-api --plugins=declarative/v1, which will create an API and scaffold the controller using the declarative plugin?

The same flag change would likely need to be made for the other subcommands. However if we went with the alternative solution proposed with my answer to your question C, I think it would still function exactly the same.

e) Would Kubebuilders users still be able to scaffold only the Golang base plugin with kubebuilder init --plugin=base.go/v3 after this implementation OR they would need to use the new flag instead?

The originally proposed solution would require the use of the new --override-default-plugin-chain command.

The alternative solution I proposed in response to your question C, this would function exactly the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@camilamacedo86 I just want to highlight a potential alternative solution that I proposed in response to one of your questions in my above response because I think I may actually prefer it because I don't think it would change the existing default functionality when using the --plugins flag, and would instead just introduce one new flag.

Now that I am thinking more about it, we could also perform this logic in an alternative way that I think would have no impact to the current functionality. This alternative way would instead rely on a different flag called something like --inject-plugins, where a user can define plugins that they would want to inject into the DynamicBundle flow. This would keep the logic that if the --plugins flag is used then completely override and just use the exact plugin chain the user has specified.

What do you think of this potential alternative? (I actually think I prefer this alternative)

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
@everettraven
Copy link
Contributor Author

Closing as it was determined that this does not add value to Kubebuilder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants