pkg/plugin/v3: add plugin `v3-alpha` #1551

estroz · 2020-06-04T22:39:28Z

A new plugin version v3-alpha will contain all new plugin/scaffold changes/features. Since plugin version v2 is tested with project version 2, we probably should just test the new plugin with project version 3-alpha. This won't necessarily be the case going forward, ex. once 4-alpha is created we should generate separate testdata.

/cc @DirectXMan12 @camilamacedo86 @joelanford @mengqiy

estroz · 2020-06-04T22:49:40Z

/test pull-kubebuilder-e2e-k8s-1-17-0

estroz · 2020-06-04T23:42:03Z

A thought on bumping plugin version stages, ex. alpha to beta and beta to "" (stable): we should probably:

Just change the plugin's stage string instead of creating a new package.
Build a mechanism into pkg/cli that auto-upgrades an alpha or beta to the next stage if available. ~~This should never be an issue because the next stage should be no different from the previous one, just more stable.~~ This could be an issue if we remove something from alpha when upgrading to beta, perhaps a migration guide and something like --latest would work.

Nothing needs to be done in this PR regarding ^.

estroz · 2020-06-05T01:32:02Z

We may want to consider adding e2e test for plugin version v3-alpha too. Will likely follow up with these.

camilamacedo86

Hi @estroz,

As we spoke, following my POV.

We do not need to make v2 plugins support V2 and V3 projects

v2 plugin just support v2 projects
v3-alpha and future versions support V3* projects.

It would make the things simpler and remove the need to change the generate_tesdata.sh script.

We do not copy all plugin/v2 scaffold files to plugin/v3
It is possible to have in plugin/v3 only what is diff as it is done between v1 to v2. See here.

The above suggestions are applied in the PR; #1498 where is possible to check that all will work as expected.

I hope that it helps to reach out to the final decision.

estroz · 2020-06-10T16:20:27Z

Is it reasonable to have an alpha plugin as the default for any project version?
Can we assume that any given project scaffold will not change over time?

If the answer to 1 is no (alpha implies the plugin is unstable), then the previous stable plugin version should be the default (v2 in this case). If yes, then v3-alpha is fine.

If the answer to 2 is no, then we should copy all scaffolds to a new plugin whenever we create one. If yes (or a given scaffold can be considered stable in the long term), then we should selectively copy scaffolds.

Regardless #1498 makes changes unrelated to creating a new plugin version, which we should make separately from creation.

camilamacedo86 · 2020-06-10T17:21:31Z

Hi @estroz,

Just to clarifies. The #1498 was just added to show that it is a possible solution and will work fine for we add new changes to the plugins. However, if you be ok to move with the proposed solution we can split the #1498 as you suggested(add plugin/add new changes)

DirectXMan12

looked over the scaffolding code, only skimmed the templates. comments inline (expect some of it's copy-pasted from v2, but there's some stuff that was surprising)

DirectXMan12 · 2020-06-11T21:28:25Z

pkg/plugin/v3/api.go

+
+// (used only to gen api with --pattern=addon)
+// KbDeclarativePatternVersion is the sigs.k8s.io/kubebuilder-declarative-pattern version
+const KbDeclarativePatternVersion = "v0.0.0-20200522144838-848d48e5b073"


do we need this? I'm assuming going forward we just want --plugin declarative or something

I think this attempts to fix a CI issue where the testdata scaffolded with addons enabled will pull the latest kubebuilder-declarative-pattern commit, which messes up git diff.

We should probably scaffold a tools.go containing a pinned version of this. ~~I could also see kustomize and controller-tools versions being pinned this way, which are currently dealt with in the Makefile.~~

The fix was done in https://github.com/kubernetes-sigs/kubebuilder/pull/1527/files?
I just n understand why we are changing its implementation here. Should not we just keep the scope to add new plugin version in this PR? I mean, why change it here?

This won't be changed here, just added a TODO.

OK. Yep, we have an issue tracked for we create a plugin for that. See: #1543. The solution made was just to fix it until we are able to move forward and provide a plugin for that.

c/c @DirectXMan12

DirectXMan12 · 2020-06-11T21:29:24Z

pkg/plugin/v3/api.go

+	if os.Getenv("KUBEBUILDER_ENABLE_PLUGINS") != "" {
+		fs.StringVar(&p.pattern, "pattern", "",
+			"generates an API following an extension pattern (addon)")
+	}


DirectXMan12 · 2020-06-11T21:30:43Z

pkg/plugin/v3/api.go

+	reader := bufio.NewReader(os.Stdin)
+	if !p.resourceFlag.Changed {
+		fmt.Println("Create Resource [y/n]")
+		p.doResource = util.YesNo(reader)


at some point we should re-evaluate the yes/no stuff. IIRC we had wanted to do it for v2, but didn't want to break compat

pkg/plugin/v3/api.go

DirectXMan12 · 2020-06-11T21:33:04Z

pkg/plugin/v3/init.go

+		return err
+	}
+
+	err = util.RunCmd("Running make", "make")


shouldn't this be conditional on the make flag defined in the other file?

Currently there's no init --make flag unlike create api. Perhaps we should add one.

Should we not here just replicate so the code to v3? IMO we should not change any impl in this pr just add the v3-alpha plugin and then we can check each case/scenario in new pr's.

This won't be changed here, just added a TODO.

DirectXMan12 · 2020-06-11T21:35:06Z

pkg/plugin/v3/scaffolds/api.go

+		s.config.AddResource(s.resource.GVK())
+
+		if err := machinery.NewScaffold(s.plugins...).Execute(
+			s.newUniverse(),


shouldn't we be passing the universe between scaffold calls here?

I think that it also can be a TODO @estroz ?

DirectXMan12 · 2020-06-11T21:36:18Z

pkg/plugin/v3/scaffolds/init.go

+	bpFile.License = s.license
+	bpFile.Owner = s.owner
+	if err := machinery.NewScaffold().Execute(
+		s.newUniverse(""),


ditto here and elsewhere re: re-using the universe

I think that it also can be a TODO @estroz ?

DirectXMan12 · 2020-06-11T21:40:51Z

If the answer to 1 is no (alpha implies the plugin is unstable), then the previous stable plugin version should be the default (v2 in this case). If yes, then v3-alpha is fine.

Per the meeting, IIRC we decided that the answer was "no", and that the default scaffolding for the v3 project version was going to be v2 till v3 stable was ready

testdata: modify project-v3.* testdata to use go/v3-alpha plugin, since project-v2.* already tests plugin go/v2 generate_testdata.sh: add --plugin option

estroz · 2020-06-11T22:24:41Z

@DirectXMan12 fixed what was easy/reasonable and added TODO's for the rest.

camilamacedo86 · 2020-06-12T15:59:50Z

pkg/plugin/v3/init.go

+project will prompt the user to run 'dep ensure' after writing the project files.
+`
+	ctx.Examples = fmt.Sprintf(`  # Scaffold a project using the apache2 license with "The Kubernetes authors" as owners
+  %s init --project-version=2 --domain example.org --license apache2 --owner "The Kubernetes authors"


Suggested change

%s init --project-version=2 --domain example.org --license apache2 --owner "The Kubernetes authors"

%s init --project-version=3-alpha --domain example.org --license apache2 --owner "The Kubernetes authors"

camilamacedo86 · 2020-06-12T16:00:24Z

pkg/plugin/v3/init.go

+)
+
+func (p *initPlugin) UpdateContext(ctx *plugin.Context) {
+	ctx.Description = `Initialize a new project including vendor/ directory and Go package directories.


Suggested change

ctx.Description = `Initialize a new project including vendor/ directory and Go package directories.

ctx.Description = `Initialize a new project including Go package directories.

I should be removed already.

camilamacedo86

Just a few nits. Otherwise

/approve

k8s-ci-robot · 2020-06-12T16:01:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, estroz

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [camilamacedo86]

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

DirectXMan12 · 2020-06-16T22:28:49Z

/lgtm

k8s-ci-robot requested review from camilamacedo86, DirectXMan12, joelanford and mengqiy June 4, 2020 22:39

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 4, 2020

estroz force-pushed the chore/add-plugin-v3 branch from 190ab2f to e0adaee Compare June 5, 2020 01:23

camilamacedo86 reviewed Jun 10, 2020

View reviewed changes

DirectXMan12 requested changes Jun 11, 2020

View reviewed changes

pkg/plugin/v3: add plugin 3-alpha

3eba593

testdata: modify project-v3.* testdata to use go/v3-alpha plugin, since project-v2.* already tests plugin go/v2 generate_testdata.sh: add --plugin option

estroz force-pushed the chore/add-plugin-v3 branch from e0adaee to 3eba593 Compare June 11, 2020 22:23

estroz requested review from DirectXMan12 and camilamacedo86 June 11, 2020 23:27

camilamacedo86 reviewed Jun 12, 2020

View reviewed changes

camilamacedo86 approved these changes Jun 12, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 12, 2020

k8s-ci-robot assigned DirectXMan12 Jun 16, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 16, 2020

k8s-ci-robot merged commit 26e468a into kubernetes-sigs:master Jun 16, 2020

estroz deleted the chore/add-plugin-v3 branch June 17, 2020 23:41

estroz mentioned this pull request Jun 18, 2020

internal/plugins: move go/v2-alpha to its own package operator-framework/operator-sdk#3181

Merged

camilamacedo86 mentioned this pull request Feb 18, 2021

New Release - proposing 3.0.0 stable #2029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/plugin/v3: add plugin `v3-alpha` #1551

pkg/plugin/v3: add plugin `v3-alpha` #1551

estroz commented Jun 4, 2020 •

edited

Loading

estroz commented Jun 4, 2020

estroz commented Jun 4, 2020 •

edited

Loading

estroz commented Jun 5, 2020

camilamacedo86 left a comment

estroz commented Jun 10, 2020 •

edited

Loading

camilamacedo86 commented Jun 10, 2020

DirectXMan12 left a comment

DirectXMan12 Jun 11, 2020

estroz Jun 11, 2020 •

edited

Loading

camilamacedo86 Jun 12, 2020 •

edited

Loading

estroz Jun 12, 2020

camilamacedo86 Jun 12, 2020 •

edited

Loading

DirectXMan12 Jun 11, 2020

DirectXMan12 Jun 11, 2020

DirectXMan12 Jun 11, 2020

estroz Jun 11, 2020

camilamacedo86 Jun 12, 2020

estroz Jun 12, 2020

DirectXMan12 Jun 11, 2020

camilamacedo86 Jun 12, 2020

DirectXMan12 Jun 11, 2020

camilamacedo86 Jun 12, 2020

DirectXMan12 commented Jun 11, 2020

estroz commented Jun 11, 2020

camilamacedo86 Jun 12, 2020

estroz Jun 17, 2020

camilamacedo86 Jun 12, 2020

estroz Jun 17, 2020

camilamacedo86 left a comment

k8s-ci-robot commented Jun 12, 2020

DirectXMan12 commented Jun 16, 2020

	%s init --project-version=2 --domain example.org --license apache2 --owner "The Kubernetes authors"
	%s init --project-version=3-alpha --domain example.org --license apache2 --owner "The Kubernetes authors"

	ctx.Description = `Initialize a new project including vendor/ directory and Go package directories.
	ctx.Description = `Initialize a new project including Go package directories.

pkg/plugin/v3: add plugin v3-alpha #1551

pkg/plugin/v3: add plugin v3-alpha #1551

Conversation

estroz commented Jun 4, 2020 • edited Loading

estroz commented Jun 4, 2020

estroz commented Jun 4, 2020 • edited Loading

estroz commented Jun 5, 2020

camilamacedo86 left a comment

Choose a reason for hiding this comment

estroz commented Jun 10, 2020 • edited Loading

camilamacedo86 commented Jun 10, 2020

DirectXMan12 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estroz Jun 11, 2020 • edited Loading

Choose a reason for hiding this comment

camilamacedo86 Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DirectXMan12 commented Jun 11, 2020

estroz commented Jun 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 12, 2020

DirectXMan12 commented Jun 16, 2020

pkg/plugin/v3: add plugin `v3-alpha` #1551

pkg/plugin/v3: add plugin `v3-alpha` #1551

estroz commented Jun 4, 2020 •

edited

Loading

estroz commented Jun 4, 2020 •

edited

Loading

estroz commented Jun 10, 2020 •

edited

Loading

estroz Jun 11, 2020 •

edited

Loading

camilamacedo86 Jun 12, 2020 •

edited

Loading

camilamacedo86 Jun 12, 2020 •

edited

Loading