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

⚠️ use kustomize for generating e2e templates #3651

Conversation

fabriziopandini
Copy link
Member

@fabriziopandini fabriziopandini commented Sep 16, 2020

What this PR does / why we need it:
This PR introduces the usage of Kustomize for generating templates for E2E tests.

Additionally

  • MHC-remediation has a new dedicated template
  • all the other templates are without the MachineHealthCheck object so there is no more the risk that remediations kick in the middle of another test

Also, in order to simplify things, this PR drops the distinction between CI and DEV test config

Which issue(s) this PR fixes:
Fixes #3461

Release Note:
If you are using Cluster-API e2e tests:

  • for the e2e kcp-adoption test you show now use different labels in your template; use kcp-adoption.step1 instead of initial and kcp-adoption.step2 instead of kcp
  • for the e2e mhc test; the template for this test should now be called cluster-template-mhc.yaml instead of mhc

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 16, 2020
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 16, 2020
@@ -31,7 +31,7 @@ source "${REPO_ROOT}/hack/ensure-kustomize.sh"
# Configure provider images generation;
# please ensure the generated image name matches image names used in the E2E_CONF_FILE
export REGISTRY=gcr.io/k8s-staging-cluster-api
export TAG=ci
export TAG=dev
Copy link
Member Author

Choose a reason for hiding this comment

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

from @vincepri #3639 (comment)

Why change the tag for the CI?

In order to simplify things, we are now going to maintain a single E2E configuration, instead of maintaining one for CI and one for DEV.
Using TAG=dev also in CI is the simplest option because it is the one used by default by all the make docker-build targets

@@ -28,7 +28,7 @@ export GOPROXY
REPO_ROOT := $(shell git rev-parse --show-toplevel)

help: ## Display this help
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-25s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
Copy link
Member Author

Choose a reason for hiding this comment

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

from @vincepri #3639 (comment)

Was this change intentional? What does it do?

Yes, it simply increases the spaces between the make target and the description in the help so the output is nicer now that there is a new cluster-templates target that is longer than the 15spaces used before

@@ -5,17 +5,17 @@
# - control-plane kubeadm
# - docker

# For creating local dev images run ./scripts/ci-e2e.sh
# For creating local dev images run make docker-build-e2e from the main CAPI repository
Copy link
Member Author

Choose a reason for hiding this comment

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

from @sedefsavas #3639 (comment)

I think we can keep run ./scripts/ci-e2e.sh here, because running make docker-build-e2e will not create images with the tag dev this file is using.

Uhmm, the default in the make file is TAG=dev

TAG ?= dev

Copy link
Member

Choose a reason for hiding this comment

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

LOL, I've done the same in #3593 after much yak shaving.

@@ -0,0 +1,5 @@
bases:
Copy link
Member Author

Choose a reason for hiding this comment

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

from @sedefsavas #3639 (comment)

nit. We can generate cluster-template.yaml on the fly and use that as a base to avoid extra changes we will need to do here when apis change.

I'm not sure to fully understand your suggestion...

Choose a reason for hiding this comment

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

Similar to how CAPV is doing:

https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/de70ff05039df7ab15a4ba11a3f559f4e92f9df7/Makefile#L120

Generate cluster-temlate.yaml and then use that as base, then generate e2e specific templates using Kustomize.

###
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
kind: DockerCluster
apiVersion: v1
Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to gitignore the generated cluster-templates and keep under source control only the bases, but it seems that this makes the apidiff batch to fail 🤔
@vincepri suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

If you get apidiff to run before generating cluster templates, should be ok?

Copy link
Member

Choose a reason for hiding this comment

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

Yes this, if we're making changes to the files and causing a dirty git tree, we should do that after running apidiff

@fabriziopandini
Copy link
Member Author

/area testing
/milestone v0.3.10

@k8s-ci-robot k8s-ci-robot added this to the v0.3.10 milestone Sep 16, 2020
@k8s-ci-robot k8s-ci-robot added the area/testing Issues or PRs related to testing label Sep 16, 2020
@fabriziopandini
Copy link
Member Author

/test pull-cluster-api-e2e-full

test/e2e/Makefile Outdated Show resolved Hide resolved
@randomvariable
Copy link
Member

/lgtm

Need an override on test.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2020
@vincepri
Copy link
Member

/test pull-cluster-api-test
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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 Sep 16, 2020
Co-authored-by: Naadir Jeewa <naadir@randomvariable.co.uk>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2020
@randomvariable
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2020
@k8s-ci-robot k8s-ci-robot merged commit b79d3ec into kubernetes-sigs:master Sep 16, 2020
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/testing Issues or PRs related to testing 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. 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.

[e2e] Use kustomize for generating template flavors for different tests
5 participants