✨Add templating interface to clusterctl #3115

wfernandes · 2020-05-28T22:52:16Z

What this PR does / why we need it:

This PR creates an interface (YamlProcessor) to allow for future extensible templating processors.
Currently, there is no change in behavior which clusterctl commands or usage.
The existing templating behavior of variable substitution is encapsulated in the SimpleProcessor which is also the default if no yaml processor are defined.
This PR only exposes YAMLProcessor as part of the clusterctl.GetClusterTemplateOptions, that is, the yaml processor is exposed via the clusterctl client library for clusterctl.GetClusterTemplate(opts).
There are other minor changes and refactors.

Note
I'm happy to break this PR into smaller PRs. As of now, I kinda broke into separate commits.

Release Notes:
TODO

/area clusterctl

vincepri · 2020-05-29T18:47:05Z

/assign

ncdc · 2020-06-08T20:26:47Z

@fabriziopandini would you have time to review?

cmd/clusterctl/client/repository/components.go

cmd/clusterctl/client/repository/template.go

vincepri · 2020-06-11T18:42:52Z

/hold

Holding to review/merge after we cut alpha.0 next week

wfernandes · 2020-06-18T18:56:56Z

/retest

benmoss

Seems pretty good, just some small comments

cmd/clusterctl/client/yamlprocessor/template_processor_simple.go

cmd/clusterctl/client/yamlprocessor/template_processor_simple_test.go

cmd/clusterctl/client/cluster/client_test.go

cmd/clusterctl/client/repository/components_client_test.go

wfernandes · 2020-06-19T00:07:43Z

/test pull-cluster-api-e2e

wfernandes · 2020-06-19T21:26:47Z

@vincepri Can we remove the hold on this PR now that alpha is cut?

fabriziopandini

I'm +1 to squash commits, remove the hold and take the latest steps to get this merged.
I have added a few comments, mostly nits.

cmd/clusterctl/client/client.go

cmd/clusterctl/client/common.go

cmd/clusterctl/client/repository/components.go

cmd/clusterctl/client/repository/template_client_test.go

cmd/clusterctl/client/repository/components_client_test.go

cmd/clusterctl/client/yamlprocessor/processor.go

cmd/clusterctl/client/yamlprocessor/template_processor_simple.go

cmd/clusterctl/client/yamlprocessor/template_processor_simple_test.go

vincepri · 2020-06-22T15:58:57Z

I have some time today to review

vincepri

Code changes look good to me, this looks close to being shipped.

cmd/clusterctl/client/client_test.go

cmd/clusterctl/client/yamlprocessor/processor.go

vincepri · 2020-06-23T18:23:03Z

/milestone v0.3.7

wfernandes · 2020-06-23T20:17:39Z

@fabriziopandini @vincepri I've addressed all of the comments. Thanks for the great feedback!

Whenever y'all get the chance please review the last commit that put I added.
455c4be

This one changes the function prototype of the Process method to not depend on config.VariablesClient.
See this feedback comment thread: #3115 (comment)

UPDATE: Since I squashed the commits you can reference the file simple_processor.go to see the changes to Process.

wfernandes · 2020-06-23T20:18:23Z

/hold cancel

The clusterctl.GetClusterTemplateOptions now includes a YamlProcessor option to specify a YamlProcessor to use. By default it uses the SimpleProcessor. Other notes: - Consolidate all the arguments into a single TemplateClientInput struct - Use SimpleProcessor as default template processor - Use SimpleProcessor for components provider - Adds '$' to SimpleProcessor's variable regex because there was a test case that defined that behavior - Add TemplateInput struct to reduce the number of args - Removes explicit sort.Strings Since the sets.String.List returns a sorted list - Consolidates all the arguments for NewCompoonents into ComponentsInput

fabriziopandini · 2020-06-25T13:34:33Z

@wfernandes outstanding job!
/lgtm

vincepri

Leaving these here for documentation:

sigs.k8s.io/cluster-api/cmd/clusterctl/client
Incompatible changes:

ClusterClientFactory: changed from func(Kubeconfig) (sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster.Client, error) to func(ClusterClientFactoryInput) (sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster.Client, error)
RepositoryClientFactory: changed from func(sigs.k8s.io/cluster-api/cmd/clusterctl/client/config.Provider) (sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository.Client, error) to func(RepositoryClientFactoryInput) (sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository.Client, error)

sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository
Incompatible changes:

NewComponents: changed from func(sigs.k8s.io/cluster-api/cmd/clusterctl/client/config.Provider, sigs.k8s.io/cluster-api/cmd/clusterctl/client/config.Client, []byte, ComponentsOptions) (*components, error) to func(ComponentsInput) (*components, error)

The PR looks good to go to me, leaving final approval to @ncdc

ncdc · 2020-06-25T14:56:19Z

/approve

k8s-ci-robot · 2020-06-25T14:56:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

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:

~~cmd/clusterctl/OWNERS~~ [ncdc]

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

vincepri · 2020-06-25T15:04:08Z

/test pull-cluster-api-test

vincepri · 2020-06-25T15:08:26Z

We need to wait for #3253 to merge before tests here can pass

ncdc · 2020-06-25T15:28:31Z

/retest

k8s-ci-robot · 2020-06-25T15:32:08Z

@wfernandes: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cluster-api-apidiff	`e87da61`	link	`/test pull-cluster-api-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot added area/clusterctl Issues or PRs related to clusterctl do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 28, 2020

k8s-ci-robot requested review from CecileRobertMichon and JoelSpeed May 28, 2020 22:52

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 May 28, 2020

k8s-ci-robot assigned vincepri May 29, 2020

wfernandes changed the title ~~WIP: ✨Add templating interface to clusterctl~~ ✨Add templating interface to clusterctl Jun 3, 2020

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jun 3, 2020

wfernandes force-pushed the clusterctl-templating-default-yaml-processor branch from c65b79c to 6b5e144 Compare June 8, 2020 16:35

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 8, 2020

fabriziopandini reviewed Jun 9, 2020

View reviewed changes

cmd/clusterctl/client/repository/components.go Outdated Show resolved Hide resolved

cmd/clusterctl/client/repository/template.go Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2020

CecileRobertMichon mentioned this pull request Jun 15, 2020

[clusterctl] Allow providers to specify variable defaults for templates #3189

Closed

wfernandes force-pushed the clusterctl-templating-default-yaml-processor branch from a93ff23 to 450e9e3 Compare June 18, 2020 18:11

benmoss reviewed Jun 18, 2020

View reviewed changes

fabriziopandini reviewed Jun 22, 2020

View reviewed changes

vincepri approved these changes Jun 22, 2020

View reviewed changes

cmd/clusterctl/client/client_test.go Outdated Show resolved Hide resolved

cmd/clusterctl/client/client_test.go Outdated Show resolved Hide resolved

cmd/clusterctl/client/yamlprocessor/processor.go Outdated Show resolved Hide resolved

k8s-ci-robot added this to the v0.3.7 milestone Jun 23, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 23, 2020

CecileRobertMichon mentioned this pull request Jun 24, 2020

[clusterctl] Support pluggable templating engines for provider yaml (core, bootstrap, kcp, infra) #3247

Closed

wfernandes force-pushed the clusterctl-templating-default-yaml-processor branch from 455c4be to 4db9565 Compare June 24, 2020 20:05

wfernandes force-pushed the clusterctl-templating-default-yaml-processor branch from 4db9565 to e87da61 Compare June 24, 2020 20:26

CecileRobertMichon mentioned this pull request Jun 24, 2020

[clusterctl] proposal: support a more expressive templating engine beyond simple variable substitution #3252

Closed

k8s-ci-robot assigned fabriziopandini Jun 25, 2020

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

vincepri approved these changes Jun 25, 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 25, 2020

k8s-ci-robot merged commit e874a89 into kubernetes-sigs:master Jun 25, 2020

wfernandes deleted the clusterctl-templating-default-yaml-processor branch June 25, 2020 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨Add templating interface to clusterctl #3115

✨Add templating interface to clusterctl #3115

wfernandes commented May 28, 2020

vincepri commented May 29, 2020

ncdc commented Jun 8, 2020

vincepri commented Jun 11, 2020

wfernandes commented Jun 18, 2020

benmoss left a comment

wfernandes commented Jun 19, 2020

wfernandes commented Jun 19, 2020

fabriziopandini left a comment

vincepri commented Jun 22, 2020

vincepri left a comment

vincepri commented Jun 23, 2020

wfernandes commented Jun 23, 2020 •

edited

Loading

wfernandes commented Jun 23, 2020

fabriziopandini commented Jun 25, 2020

vincepri left a comment

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020

vincepri commented Jun 25, 2020

vincepri commented Jun 25, 2020

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020 •

edited

Loading

✨Add templating interface to clusterctl #3115

✨Add templating interface to clusterctl #3115

Conversation

wfernandes commented May 28, 2020

vincepri commented May 29, 2020

ncdc commented Jun 8, 2020

vincepri commented Jun 11, 2020

wfernandes commented Jun 18, 2020

benmoss left a comment

Choose a reason for hiding this comment

wfernandes commented Jun 19, 2020

wfernandes commented Jun 19, 2020

fabriziopandini left a comment

Choose a reason for hiding this comment

vincepri commented Jun 22, 2020

vincepri left a comment

Choose a reason for hiding this comment

vincepri commented Jun 23, 2020

wfernandes commented Jun 23, 2020 • edited Loading

wfernandes commented Jun 23, 2020

fabriziopandini commented Jun 25, 2020

vincepri left a comment

Choose a reason for hiding this comment

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020

vincepri commented Jun 25, 2020

vincepri commented Jun 25, 2020

ncdc commented Jun 25, 2020

k8s-ci-robot commented Jun 25, 2020 • edited Loading

wfernandes commented Jun 23, 2020 •

edited

Loading

k8s-ci-robot commented Jun 25, 2020 •

edited

Loading