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

✨ Add Kubetest2 provider #3855

Closed
wants to merge 1 commit into from

Conversation

benmoss
Copy link

@benmoss benmoss commented Oct 22, 2020

What this PR does / why we need it:
Adds a kubetest2 provider for CAPI. It depends on kind, clusterctl, and kubectl to all be on the path.

Usage:
clone this PR, cd to test/kubetest2-capi

GO111MODULE=on go get sigs.k8s.io/kubetest2/...@latest
GO111MODULE=on go get sigs.k8s.io/cluster-api/test/kubetest2-capi/...@latest

KIND_EXPERIMENTAL_DOCKER_NETWORK=bridge kubetest2 capi  --provider docker:v0.3.10 --kubernetes-version 1.19.1 --flavor development --config ~/kind-cluster-with-extramounts.yaml --cni-manifest https://docs.projectcalico.org/v3.12/manifests/calico.yaml --up

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 22, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign justinsb after the PR has been reviewed.
You can assign the PR to them by writing /assign @justinsb 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:

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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 22, 2020

```
export KIND_EXPERIMENTAL_DOCKER_NETWORK=bridge
kubetest2 capi --provider docker --repo-root $CLONEDREPOPATH --build --up --down
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, have you tested this with any other providers?

Copy link
Author

@benmoss benmoss Nov 2, 2020

Choose a reason for hiding this comment

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

The --build parts won't work with other providers today, it only (barely) works with kind and CAPD because CAPD relies on kind images. The Build method on the provider just delegates to the Kind provider, if we really wanted it to work seamlessly we'd probably need to go all the way to integrating image-builder.

Copy link
Author

Choose a reason for hiding this comment

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

Oh and specifically to answer your question, I tested just the --up parts with AWS as well :). This deployer mostly just delegates to kind and clusterctl, so it should work with the same caveats those tools have, ie they need to be on the path, there's some magic environment variables, etc

Copy link
Member

Choose a reason for hiding this comment

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

After this merges in, I'm intending to open a PR to the AWS Cloud Provider repo to start testing the out of tree CPI.

@vincepri
Copy link
Member

vincepri commented Nov 2, 2020

What are we going to use this for? Could we add some documentation on what the purpose of this new code is and how is it different than the current pipelines?

More importantly, do we have folks that can maintain this codebase?

@randomvariable
Copy link
Member

randomvariable commented Nov 3, 2020

What are we going to use this for? Could we add some documentation on what the purpose of this new code is and how is it different than the current pipelines?

This is going to be used in testing across Kubernetes repos, starting with the AWS Cloud Provider and Cluster Autoscaler repos.

It'll provide a cross-cloud way of getting rid of the k/k/cluster directory and improve signal for Cluster API on Kubernetes main branch.

More importantly, do we have folks that can maintain this codebase?

Volunteering, it's a fairly small interface. CCing @elmiko who had previously raised building a kubetest2 deployer in SIG Testing.

@elmiko
Copy link
Contributor

elmiko commented Nov 3, 2020

thanks for the cc @randomvariable

i am interested in this from the cluster-autoscaler side, but i'm a little hazy on the details of how it works as a capi provider.

@vincepri
Copy link
Member

vincepri commented Nov 3, 2020

Awesome thank you all! Let's add an OWNERS file in here with some maintainers + reviewers, and let's add some more documentation as part of this PR before it gets merged

@benmoss
Copy link
Author

benmoss commented Nov 4, 2020

Bump, I added a README with some more deets and an OWNERS file

@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 Nov 6, 2020
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 6, 2020
@benmoss
Copy link
Author

benmoss commented Nov 6, 2020

Changed the base64 encoded Kind config to be inlined instead and then squashed the commits

@k8s-ci-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
pull-cluster-api-test-main d4ea894 link /test pull-cluster-api-test-main

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.

@vincepri
Copy link
Member

/assign @fabriziopandini @randomvariable

@vincepri
Copy link
Member

/milestone v0.4.0
/hold

Until we get more 👀

@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 Dec 15, 2020
@k8s-ci-robot k8s-ci-robot added this to the v0.4.0 milestone Dec 15, 2020
@elmiko
Copy link
Contributor

elmiko commented Dec 15, 2020

Until we get more eyes

i have been trying to get this working locally, but it's been slow going.

Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

i've been able to get this deploying to bring up the kubetest cluster and the workload cluster, as well as bring them down cleanly. i am hitting a couple issues:

when i try to run a test using --test "some test regex", i just get the following output

$ kubetest2 capi  --provider docker:v0.3.10 --kubernetes-version 1.19.1 --flavor development --config ~/kind-cluster-with-extramounts.yaml --cni-manifest https://docs.projectcalico.org/v3.12/manifests/calico.yaml --cluster-name kind-kubetest2 --test "should be submitted and removed"
Error: unable to find tester should be submitted and removed: "kubetest2-tester-should be submitted and removed" not found in PATH, could not locate "should be submitted and removed" tester 

i tried to use the --test with --test_args but i'm guessing that is not present in this implementation?

the other problem i hit was trying to get creds for the workload cluster, i hit this:

$ kubectl --kubeconfig cw.kubeconfig get pods --all-namespaces
Unable to connect to the server: x509: certificate is valid for 10.128.0.1, 172.17.0.3, 172.17.0.2, 127.0.0.1, not 0.0.0.0

this might just be a misconfiguration on my end, not quite sure yet.

i think this is really promising. once i figured out how to build the binary and make kubetest2 happy, things started to go really well. i'm eager to figure out how to get my tests running =)

```
GO111MODULE=on go get sigs.k8s.io/cluster-api/test/kubetest2-capi@latest
```

Copy link
Contributor

Choose a reason for hiding this comment

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

i think it would be nice to add a paragraph here about building locally using go build in the kubetest2-capi directory. i know it won't make a big difference once this is deployed, but for developers i think it's nice to give a little extra direction on how to get started with just the code.

@benmoss
Copy link
Author

benmoss commented Jan 4, 2021

I'm not gonna be updating this, if someone else wants to take over this work I'll leave this fork up and they can use it as a starting point.

@benmoss benmoss closed this Jan 4, 2021
@randomvariable
Copy link
Member

I'll take this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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.

8 participants