✨ New plugin for config-gen as alpha. #2540

Kavinjsir · 2022-03-14T19:41:39Z

In respect to #issue2501, moving the package of config-gen from pkg/cli/alpha towards pkg/plugins.

linux-foundation-easycla · 2022-03-14T19:41:44Z

The committers listed above are authorized under a signed CLA.

✅ login: Kavinjsir / name: TianYi(Tony) (469c3d3, 4db93a5)

k8s-ci-robot · 2022-03-14T19:41:48Z

Welcome @Kavinjsir!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-03-14T19:41:48Z

Hi @Kavinjsir. 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 · 2022-03-14T19:42:01Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Kavinjsir
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:

OWNERS

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

camilamacedo86 · 2022-03-15T11:43:11Z

/ok-to-test

pkg/cli/alpha.go

cmd/main.go

camilamacedo86 · 2022-03-24T07:34:03Z

pkg/cli/alpha/config-gen/README.md

@@ -7,7 +7,7 @@ Supports:
 - Generating CRDs and RBAC from code
 - Generating webhook certificates for development
 - Selectively enabling / disabling components such as prometheus and webhooks
-  - See [types.go](apis/v1alpha1/types.go) for a list of components
+  - See [types.go](types.go) for a list of components


Regards its docs I think we need to move in the direction : #2542

Yeah, that sounds good.
I can make the docs once the implementation is good.
See someone is working on the thread, if there is a successful one give out ahead of this pr, I can try attach the docs based on that.
Otherwise we may have a follow up?
Would you think this work?

Kavinjsir · 2022-03-25T10:14:33Z

pkg/plugins/config-gen/v1alpha/scaffolds/internal/templates/makefile.go

+	}
+
+	// FIXME: How can we customize makefile without overwrite it since it is language based?
+	f.IfExistsAction = machinery.OverwriteFile


Need some help here:
This config-gen plugin will generate a makefile that might overwrite the one from go/v3. So there are two concerns:

Most part of the template from both are duplicate. (The main difference is just make deploy and make install).

It potentially leads contradiction if another plugin is used to generate makefile.

I suppose we may expect this plugin to be a more general one.(So to make it available in sdk)

I will get some references on the plugins from sdk to see how we can adjust this.

The Makefile is part of the language plugin.
See that we have the kustomize and the base.go/v3 which are a composition to generate the = go/v3

IMO:
How it will work? We will need to call base.go. + config-gen such as we do now for go/v3: https://github.com/kubernetes-sigs/kubebuilder/blob/master/cmd/main.go#L35-L39

So, why does config-gen needs to scaffold the Makefile?

The makefile is scaffolded by the Golang one because it has specific targets which are related to the language ( see that the kustomize plugin is used with other languages such as in SDK with Helm/Ansible so that their specific language plugins do not have to go commands on the targets )

More info: See the plugin doc PR : #2569

Please see: https://github.com/kubernetes-sigs/kubebuilder/pull/2540/files#r841070549

Kavinjsir · 2022-03-25T10:17:41Z

test/testdata/generate.sh

+    $kb create api --group crew --version v1 --kind Captain --controller=true --resource=true --make=false
+    $kb create webhook --group crew --version v1 --kind Captain --defaulting --programmatic-validation
+    $kb create api --group crew --version v1 --kind Admiral --plural=admirales --controller=true --resource=true --namespaced=false --make=false
+    $kb create webhook --group crew --version v1 --kind Admiral --plural=admirales --defaulting


Just to clarify, all the data under testdata/project-v3-config-gen is output by local make generatebased on the update here. I am not sure if this is the correct way to generate test cases.

They pass the e2e test on my local laptop, but somehow failed in CI. It seems an issue of go version, so wondering how I can fix that.

IMO here we would add a sample that runs:
`kubebuilder init --plugins=config-gen/v1-alpha,base.go.kubebuilder.io/v3 --domain example.org --repo example.org/guestbook-operator

Such as we are doing in the PR where we propose the new kustomize/v2-alpha see: #2583
`

camilamacedo86 · 2022-04-02T12:10:46Z

@pwittrock could you give a hand on this one?

camilamacedo86 · 2022-04-02T12:22:38Z

testdata/project-v3-config-gen/api/v1/admiral_types.go

+}
+
+func init() {
+	SchemeBuilder.Register(&Admiral{}, &AdmiralList{})


Why config-gen needs to scaffold the files that are done by the go.base.v3 language?
See: https://github.com/kubernetes-sigs/kubebuilder/pull/2582/files

I'd expected, run :

kubebuilder init --plugins=config-gen/v1-alpha,base.go.kubebuilder.io/v3 --domain example.org --repo example.org/guestbook-operator

Then:

The init command of config-gen would scaffold only the files which are required and specific for this plugin

After that, in the chain the init subcommand of the base go v3 plugin will be called and scaffold all language files ( apis, controllers, makefile )

In this way we do not hurt concepts such as single responsibility, each plugin does only what is its responsibility.

camilamacedo86 · 2022-04-02T12:39:25Z

pkg/plugins/config-gen/v1alpha/scaffolds/init.go

+		&templates.Makefile{
+			BoilerplatePath:          s.boilerplatePath,
+			ControllerToolsVersion:   ControllerToolsVersion,
+			KustomizeVersion:         KustomizeVersion,
+			ControllerRuntimeVersion: ControllerRuntimeVersion,
+		},


That shows not required for config-gen

camilamacedo86 · 2022-04-02T12:39:51Z

pkg/plugins/config-gen/v1alpha/scaffolds/init.go

+const (
+	// ControllerRuntimeVersion is the kubernetes-sigs/controller-runtime version to be used in the project
+	ControllerRuntimeVersion = "v0.11.0"
+	// ControllerToolsVersion is the kubernetes-sigs/controller-tools version to be used in the project
+	ControllerToolsVersion = "v0.8.0"
+	// KustomizeVersion is the kubernetes-sigs/kustomize version to be used in the project
+	KustomizeVersion = "v4.0.5"
+)


That seems not required ether

camilamacedo86 · 2022-04-02T12:41:26Z

pkg/plugins/config-gen/v1alpha/scaffolds/internal/templates/makefile.go

+}
+
+//nolint:lll
+const makefileTemplate = `


What should be diff for config-gen in the Makefile?
Have we anything that is specific for config-gen that we need to change?
Have we anything that we should remove ?

c/c @pwittrock

Hey @camilamacedo86 , thx for the review!

So this is the main part that confuses me of how such alpha config-gen get migrated to the plugin and worked by the integration with language specific plugins.

Because, once it becomes a plugin, the function of config-gen can't be actively triggered.
Especially when we'd like to use it as:

kubebuilder init --plugins=config-gen/v1-alpha,base.go.kubebuilder.io/v3 --domain example.org --repo example.org/guestbook-operator

I understand that we don't expect the plugin to "update" makefile, as that is defined as "language specific".
And, it seems that most of the makefile templates(go/v3, go/v2, helm) have in common with the following lines:

.PHONY: install install: kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config. $(KUSTOMIZE) build config/crd | kubectl apply -f - ... .PHONY: deploy deploy: kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} $(KUSTOMIZE) build config/default | kubectl apply -f - ...

So, if I'm understanding correctly, the language specific plugin:

assumes there is a config/ folder

then defines Makefile

and determine how the CRD and its controller get installed.

In general, the config/ is generated by kustomize plugin. (Though the language specific plugin does not care who create that folder.)

Consequently, I have this basic question:

Once the alpha plugin get triggered by kb init and generates:

kubebuilderconfiggen.yaml

controller_manager_config.yaml

How can users use these files to apply their configured controllers to the cluster?

The temporary approach in the this commit is:
Add this Makefile-Scaffolding template to the alpha plugin. Then the ultimate Makefile will have a difference in its make deploy:

deploy: ## Deploy controller to the K8s cluster specified in ~/.kube/config. kubebuilder alpha config-gen ./kubebuilderconfiggen.yaml | kubectl apply -f -

That's how config-gen get used.

This might not be a good one considering on the usage as:

kubebuilder init --plugins=config-gen/v1-alpha,<language-specific-plugin> ...

That makes the alpha plugin an alternative to the kustomize plugin.

IIUIC, the language plugins expect the config/ folder, regardless of who create that.

So maybe the alpha plugin should generate the config/.

Then the language specific plugins won't necessary do/care additional things.

What do you think?

Hi @Kavinjsir,

Thank you for working on this one, Following the comments inline.

I understand that we don't expect the plugin to "update" makefile, as that is defined as "language specific".
And, it seems that most of the makefile templates(go/v3, go/v2, helm) have in common with the following lines:

.The Makefile is scaffold by the language plugin.
Why it has specific targets which are by language (e.g controller-gen only work with Go, go fmt, and etc..)

This might not be a good one considering on the usage as:

kubebuilder init --plugins=config-gen/v1-alpha, ...
That makes the alpha plugin an alternative to the kustomize plugin.

It is the goal. The purpose of config-gen would be to replace kustomize
That means users could choose if prefer the config-gen or kustomize approach.

k8s-ci-robot · 2022-05-13T23:34:33Z

@Kavinjsir: PR needs rebase.

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-triage-robot · 2022-08-12T00:22:32Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

camilamacedo86 · 2022-09-05T19:13:19Z

@Kavinjsir,

I spoke with its author and he is ok with us deleting or moving config-gen from the project (looking for maintainability since this option does not seems that won atraction from the users). We will need to discuss the possibilities in the community meetings.

However, I understand that with your help we came to the conclusion that we cannot easily move this implementation to a plugin since we need to generate/execute the config-gen to do the actions.

In this way, I think we can close this PR and if we need we can re-open it.
Feel free to re-open it if you see that it fits.

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 14, 2022

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 14, 2022

k8s-ci-robot requested review from joelanford and pwittrock March 14, 2022 19:42

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 14, 2022

Kavinjsir mentioned this pull request Mar 14, 2022

Move config-gen to a alpha plugin design #2501

Closed

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 15, 2022

camilamacedo86 reviewed Mar 15, 2022

View reviewed changes

pkg/cli/alpha.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2022

Kavinjsir force-pushed the refactor/cleanup-configgen branch from 4db93a5 to 2e8b509 Compare March 21, 2022 22:01

camilamacedo86 reviewed Mar 24, 2022

View reviewed changes

cmd/main.go Show resolved Hide resolved

camilamacedo86 reviewed Mar 24, 2022

View reviewed changes

Kavinjsir force-pushed the refactor/cleanup-configgen branch from 86765d5 to 8a812c3 Compare March 25, 2022 08:42

Kavinjsir changed the title ~~🌱 Move config-gen to a alpha plugin~~ ✨ New plugin for config-gen as alpha. Mar 25, 2022

Kavinjsir force-pushed the refactor/cleanup-configgen branch from 8a812c3 to db95b77 Compare March 25, 2022 09:40

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 25, 2022

Kavinjsir commented Mar 25, 2022

View reviewed changes

camilamacedo86 closed this Mar 31, 2022

camilamacedo86 reopened this Mar 31, 2022

camilamacedo86 reviewed Apr 2, 2022

View reviewed changes

✨ Add new plugin config-gen/v1alpha

7dd528d

Kavinjsir force-pushed the refactor/cleanup-configgen branch from db95b77 to 7dd528d Compare April 13, 2022 17:25

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 12, 2022

camilamacedo86 closed this Sep 5, 2022

camilamacedo86 mentioned this pull request Oct 5, 2022

Create a new plugin OR a new project with config-gen option #3000

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ New plugin for config-gen as alpha. #2540

✨ New plugin for config-gen as alpha. #2540

Kavinjsir commented Mar 14, 2022 •

edited

Loading

linux-foundation-easycla bot commented Mar 14, 2022 •

edited

Loading

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 14, 2022

camilamacedo86 commented Mar 15, 2022

camilamacedo86 Mar 24, 2022

Kavinjsir Mar 25, 2022

Kavinjsir Mar 25, 2022

Kavinjsir Mar 25, 2022

camilamacedo86 Apr 2, 2022 •

edited

Loading

camilamacedo86 Apr 2, 2022

Kavinjsir Mar 25, 2022 •

edited

Loading

camilamacedo86 Apr 2, 2022

camilamacedo86 commented Apr 2, 2022

camilamacedo86 Apr 2, 2022

camilamacedo86 Apr 2, 2022

camilamacedo86 Apr 2, 2022

camilamacedo86 Apr 2, 2022

camilamacedo86 Apr 2, 2022

Kavinjsir Apr 9, 2022

Kavinjsir Apr 9, 2022 •

edited

Loading

Kavinjsir Apr 9, 2022

camilamacedo86 Apr 9, 2022

camilamacedo86 Apr 9, 2022

k8s-ci-robot commented May 13, 2022

k8s-triage-robot commented Aug 12, 2022

camilamacedo86 commented Sep 5, 2022

✨ New plugin for config-gen as alpha. #2540

✨ New plugin for config-gen as alpha. #2540

Conversation

Kavinjsir commented Mar 14, 2022 • edited Loading

linux-foundation-easycla bot commented Mar 14, 2022 • edited Loading

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 14, 2022

k8s-ci-robot commented Mar 14, 2022

camilamacedo86 commented Mar 15, 2022

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 Apr 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kavinjsir Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 commented Apr 2, 2022

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

Kavinjsir Apr 9, 2022 • 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

k8s-ci-robot commented May 13, 2022

k8s-triage-robot commented Aug 12, 2022

camilamacedo86 commented Sep 5, 2022

Kavinjsir commented Mar 14, 2022 •

edited

Loading

linux-foundation-easycla bot commented Mar 14, 2022 •

edited

Loading

camilamacedo86 Apr 2, 2022 •

edited

Loading

Kavinjsir Mar 25, 2022 •

edited

Loading

Kavinjsir Apr 9, 2022 •

edited

Loading