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.

Sign up for GitHub

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

Jump to bottom

🌱 e2e tests for clusterctl quickstart flow using managed topologies #5423

Merged
merged 1 commit into from
Oct 23, 2021
Merged

🌱 e2e tests for clusterctl quickstart flow using managed topologies #5423

merged 1 commit into from
Oct 23, 2021

Conversation

ykakarap
Copy link
Contributor

What this PR does / why we need it:
This PR adds an E2E test to test the quickstart flow of ClusterAPI using clusterctl with a flavor of cluster that uses managed topology.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #5348

@k8s-ci-robot 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 Oct 14, 2021
@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from 6bcb80b to 5b65327 Compare October 14, 2021 05:53
@ykakarap
Copy link
Contributor Author

/hold until PR #5351 is meged.

@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 Oct 14, 2021
@ykakarap
Copy link
Contributor Author

/assign @fabriziopandini

@ykakarap ykakarap changed the title 🌱 e2e tests for clusterctl quickstart flow using managed topologies 🌱 WIP: e2e tests for clusterctl quickstart flow using managed topologies Oct 14, 2021
@ykakarap
Copy link
Contributor Author

/retest

@fabriziopandini
Copy link
Member

@killianmuldoon PTAL

fabriziopandini
fabriziopandini reviewed Oct 14, 2021
View reviewed changes
Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

Overall the approach seems ok to me, just a couple of nits from my side.

test/e2e/data/infrastructure-docker/v1beta1/clusterclass-test.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-test.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-test.yaml Outdated Show resolved Hide resolved
test/e2e/config/docker.yaml Outdated Show resolved Hide resolved
test/e2e/config/docker.yaml Outdated Show resolved Hide resolved
test/e2e/config/docker.yaml Show resolved Hide resolved
test/e2e/quick_start_test.go Outdated Show resolved Hide resolved
@fabriziopandini
Copy link
Member

/retest
@ykakarap let's trigger E2E tests manually a couple of times and make sure the new one works in a reliable way

killianmuldoon
killianmuldoon reviewed Oct 19, 2021
View reviewed changes
Copy link
Contributor

@killianmuldoon killianmuldoon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

On my local setup this isn't running while the ordinary cluster quick start test is running and passing. I'm getting an error when trying to access the cluster that there is no /etc/kubernetes/admin.conf created so it seems to be failing early on in the process.

I'm getting a timeout after 600 seconds at STEP: Waiting for one control plane node to exist. The ordinary cluster quickstart is taking about 75 seconds on my machine so something seems to be going wrong.

@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch 2 times, most recently from f66a020 to c70ff15 Compare October 21, 2021 02:45
@ykakarap
Copy link
Contributor Author

On my local setup this isn't running while the ordinary cluster quick start test is running and passing. I'm getting an error when trying to access the cluster that there is no /etc/kubernetes/admin.conf created so it seems to be failing early on in the process.

I'm getting a timeout after 600 seconds at STEP: Waiting for one control plane node to exist. The ordinary cluster quickstart is taking about 75 seconds on my machine so something seems to be going wrong.

I dont seems to have this problem. However I did hit the timeout issue in a few cases and it was mostly CNI not being installed properly or because of the docker pull limit. I have observed a timeout error on the PR too a few times. If the PR pipelines re-hit the timeout issue now it might be similar to the issue you are observing, because I solved the CNI issue.

@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from c70ff15 to 2ea8436 Compare October 21, 2021 02:53
@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@ykakarap ykakarap changed the title 🌱 WIP: e2e tests for clusterctl quickstart flow using managed topologies 🌱 e2e tests for clusterctl quickstart flow using managed topologies Oct 21, 2021
@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from 2ea8436 to 3d2fd1b Compare October 21, 2021 04:26
@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from 3d2fd1b to 464a2cc Compare October 21, 2021 04:34
@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@ykakarap
Copy link
Contributor Author

On my local setup this isn't running while the ordinary cluster quick start test is running and passing. I'm getting an error when trying to access the cluster that there is no /etc/kubernetes/admin.conf created so it seems to be failing early on in the process.

I'm getting a timeout after 600 seconds at STEP: Waiting for one control plane node to exist. The ordinary cluster quickstart is taking about 75 seconds on my machine so something seems to be going wrong.

I dont seems to have this problem. However I did hit the timeout issue in a few cases and it was mostly CNI not being installed properly or because of the docker pull limit. I have observed a timeout error on the PR too a few times. If the PR pipelines re-hit the timeout issue now it might be similar to the issue you are observing, because I solved the CNI issue.

@killianmuldoon The pipeline also seems to be hitting the same issue that you are facing. It is weird that I am not facing this issue on my linux machine. 🤔

sbueringer
sbueringer reviewed Oct 21, 2021
View reviewed changes
Copy link
Member

@sbueringer sbueringer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very nice, I especially like that we can just reuse our existing quickstart test code.

I've left a comment (#5423 (comment)) about the CI failure.

test/e2e/config/docker.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/bases/cluster-with-topology.yaml Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/bases/cluster-with-topology.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/bases/cluster-with-topology.yaml Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start.yaml Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start.yaml Outdated Show resolved Hide resolved
@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from 464a2cc to d65c8a4 Compare October 21, 2021 16:34
@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@ykakarap
Copy link
Contributor Author

All tests are now green. ✅

The PR has a hold right now, waiting for #5351 to be merged first.

@sbueringer
Copy link
Member

All tests are now green. ✅

The PR has a hold right now, waiting for #5351 to be merged first.

Nice, lgtm +/- the hold/rebase thing

@fabriziopandini
Copy link
Member

lgtm pending rebase and another round of pull-cluster-api-e2e-full-main

@killianmuldoon
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 Oct 22, 2021
vincepri
vincepri approved these changes Oct 22, 2021
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@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 Oct 22, 2021
@ykakarap ykakarap force-pushed the clusterctl_clusterclass_quickstart_e2e branch from d65c8a4 to 996d023 Compare October 22, 2021 17:15
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed 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. labels Oct 22, 2021
@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

@sbueringer
Copy link
Member

/lgtm
/hold
@ykakarap Feel free to unhold after the test is green

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 22, 2021
@ykakarap
Copy link
Contributor Author

/retest

@ykakarap
Copy link
Contributor Author

/test pull-cluster-api-e2e-full-main

once more for good measure :)

@ykakarap
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 Oct 23, 2021
@ykakarap
Copy link
Contributor Author

canceled hold as pull-cluster-api-e2e-full-main test ran successfully multiple times.

@k8s-ci-robot k8s-ci-robot merged commit ab0bb57 into kubernetes-sigs:main Oct 23, 2021
@k8s-ci-robot k8s-ci-robot added this to the v0.4 milestone Oct 23, 2021
@killianmuldoon killianmuldoon mentioned this pull request Oct 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@killianmuldoon killianmuldoon killianmuldoon left review comments

@fabriziopandini fabriziopandini fabriziopandini left review comments

@sbueringer sbueringer sbueringer left review comments

@vincepri vincepri vincepri approved these changes

@enxebre enxebre Awaiting requested review from enxebre

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

@vincepri vincepri

@sbueringer sbueringer

@fabriziopandini fabriziopandini

@killianmuldoon killianmuldoon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.4
Development

Successfully merging this pull request may close these issues.

Add e2e tests to the enhanced clusterctl generate cluster
6 participants
@ykakarap @fabriziopandini @sbueringer @killianmuldoon @k8s-ci-robot @vincepri