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

feat: enable easier flavor creation #540

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

devigned
Copy link
Contributor

@devigned devigned commented Apr 16, 2020

What this PR does / why we need it:
Flavors are the way clusterctl allows providers to specify different configurations of clusters. This PR makes it easier to create new flavors and to extend existing ones by setting up an opinionated directory structure and a make goal (make generate-flavors) to generate flavors using kustomize.

Below is the proposed structure.

templates
├── addons
│   ├── calico.yaml
│   ├── cloud-controller-manager.yaml
│   └── cloud-node-manager.yaml
├── cluster-template-external-cloud-provider.yaml
├── cluster-template.yaml
├── flavors
│   ├── README.md
│   ├── base
│   │   ├── cluster-template.yaml
│   │   └── kustomization.yaml
│   ├── default
│   │   ├── kustomization.yaml
│   │   └── machine-deployment.yaml
│   └── external-cloud-provider
│       ├── kustomization.yaml
│       └── patches
│           └── external-cloud-provider.yaml
└── test
    ├── cluster-template-conformance-ci-version.yaml
    ├── cluster-template-conformance.yaml
    ├── conformance
    │   ├── kustomization.yaml
    │   └── patches
    │       └── conformance-tags.yaml
    └── conformance-ci-version
        ├── kustomization.yaml
        └── patches
            └── ci-version.yaml
  • templates root: contains the generated flavors starting with cluster-template{-flavor-name}.yaml and the following folders.
    • addons: contains add-on manifests; resources that may or may not be added to a cluster
    • flavors: a list of directories which each contain a kustomization that is generated and added to the ./templates directory as a flavor.
    • test: contains all of the test / conformance kustomizations

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

Enable easier flavor creation. See [flavors documentation](https://github.com/kubernetes-sigs/cluster-api-provider-azure/tree/master/templates/flavors) for more details.

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 16, 2020
@k8s-ci-robot k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 16, 2020
@devigned
Copy link
Contributor Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 16, 2020
@CecileRobertMichon
Copy link
Contributor

@fabriziopandini @detiber @vincepri curious what your thoughts are on this

@devigned
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 16, 2020
@CecileRobertMichon
Copy link
Contributor

/hold

for discussion and reviews

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2020
@CecileRobertMichon
Copy link
Contributor

@devigned maybe I should wait for this to cut a 0.4.2 release so we can try out the "release flavor templates as artifacts" story in real life, wdyt?

@devigned
Copy link
Contributor Author

@CecileRobertMichon great idea!

@devigned devigned force-pushed the template-flavors branch 2 times, most recently from 92d53e5 to f6bb564 Compare April 17, 2020 19:19

.PHONY: release-manifests
release-manifests: $(KUSTOMIZE) $(RELEASE_DIR) ## Builds the manifests to publish with a release
kustomize build config > $(RELEASE_DIR)/infrastructure-components.yaml

.PHONY: release-templates
release-templates: $(RELEASE_DIR)
cp templates/cluster-template* $(RELEASE_DIR)/
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for making this generic so that we don't have to remember to add new templates here every time 💯

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, devigned

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2020
@devigned
Copy link
Contributor Author

nits picked :)

@CecileRobertMichon
Copy link
Contributor

@devigned please add a release note

/hold cancel

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 20, 2020
This directory contains each of the flavors for CAPZ. Each directory besides `base` will be used to
create a flavor by running `kustomize build` on the directory. The name of the directory will be
appended to the end of the cluster-template.yaml, e.g cluster-template-{directory-name}.yaml. That
flavor can be used by specifying `--flavor {directory-name}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

@devigned great PR 👍,
but just a question, I'm a little confused with the difference between base and default, I can see default adds a machine-deployment but when I run make generate-flavors I only get cluster-template.yaml and cluster-template-external-cloud-provider.yaml which are not corresponding to the directories names, so I was wondering if we can combine base and default or clarify it in the readme a little bit more

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are too kind. Thank you.

base is ignored currently as a place to hold shared structures. I hope that it goes away in a future PR where we would take the ./templates/base/cluster-template.yaml and break it up into the individual resources to be used piece by piece to build new flavors. Just thought it would be best to break that change into a couple of PRs.

What do you think?

@nader-ziada
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 21, 2020
@k8s-ci-robot k8s-ci-robot merged commit f960483 into kubernetes-sigs:master Apr 21, 2020
@k8s-ci-robot k8s-ci-robot added this to the v0.5 milestone Apr 21, 2020
@devigned devigned deleted the template-flavors branch April 21, 2020 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants