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

✨ test framework: Add kubetest, kubernetesversions, and ginkgo extensions #3652

Merged

Conversation

randomvariable
Copy link
Member

@randomvariable randomvariable commented Sep 16, 2020

Breaking out of #3593

Mostly upstreaming from CAPA.

The kubetest package allows a consumer to run kubetest (i.e. Kubernetes conformance) against
given clusters.

The kubernetesversions package provides helpers to determine the latest CI release of Kubernetes
and adds debian/ubuntu injection script kustomizations for based on prior work by @dims.

ginkgoextensions are common helpers that were added to CAPA, and are also present in the test/e2e
package.

Additionally add helpers to log running commands as well as gather JUnit files in a way that
is compatible with Prow.

Future PRs will add consumption of this in an e2e test and make CAPA consume these.

APIDiff will fail this PR, because a GetRESTConfig function has been added to the cluster proxy interface. This is used so that a client-go discovery client can be used to discover the API server's kubernetes version. Believe this has minimal impact due to no third-party implementations of the interface.

Signed-off-by: Naadir Jeewa jeewan@vmware.com

What this PR does / why we need it:

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

@k8s-ci-robot k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels 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
@randomvariable randomvariable changed the title test framework: Add kubetest, kubernetesversions, and ginkgo extensions test framework: Add kubetest, kubernetesversions, log and ginkgo extensions Sep 16, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Sep 16, 2020
@randomvariable randomvariable changed the title test framework: Add kubetest, kubernetesversions, log and ginkgo extensions ✨ test framework: Add kubetest, kubernetesversions, log and ginkgo extensions Sep 16, 2020
@randomvariable
Copy link
Member Author

/retest

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.

I can take a deeper look later

/assign @sedefsavas
for another set of 👀

test/framework/kubernetesversions/template.go Outdated Show resolved Hide resolved
test/framework/kubernetesversions/template.go Show resolved Hide resolved
test/framework/kubernetesversions/template.go Outdated Show resolved Hide resolved
test/framework/kubernetesversions/template.go Outdated Show resolved Hide resolved
test/framework/kubernetesversions/versions.go Show resolved Hide resolved
test/framework/kubetest/setup.go Outdated Show resolved Hide resolved
test/framework/kubernetesversions/bindata.go Show resolved Hide resolved
test/framework/kubetest/run.go Show resolved Hide resolved
@randomvariable randomvariable force-pushed the test-framework-extensions branch 7 times, most recently from 1eb6652 to 585c2d6 Compare September 17, 2020 11:02
@randomvariable
Copy link
Member Author

/test pull-cluster-api-test

1 similar comment
@randomvariable
Copy link
Member Author

/test pull-cluster-api-test

@sedefsavas
Copy link

/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 17, 2020
@benmoss
Copy link

benmoss commented Sep 23, 2020

/lgtm

@benmoss
Copy link

benmoss commented Sep 23, 2020

/test pull-cluster-api-verify-external-links

just want to see if this works 😄

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.

@randomvariable thanks for this PR and sorry for lagging behind

This PR looks much more cleaner than the original one, and I'm looking forward to see this used by different providers
My only concern is about using this in docker, because in my experience there could be potential resource bottlenecks when executing the install scripts on several containers running on the same machines, and this can lead to flakes. But let's get this merged so we can test this in practice.

Comment on lines +43 to +48
// KubeadmConfigTemplateName is the name of the KubeadmConfigTemplate resource
// that needs to have the Debian install script injected. Defaults to "${CLUSTER_NAME}-md-0".
KubeadmConfigTemplateName string
// KubeadmControlPlaneName is the name of the KubeadmControlPlane resource
// that needs to have the Debian install script injected. Defaults to "${CLUSTER_NAME}-control-plane".
KubeadmControlPlaneName string
Copy link
Member

Choose a reason for hiding this comment

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

Should we add Machine pool as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're only customising the PreKubeadmCommands and Files bits, so we shouldn't need to touch MachinePools do we?

Copy link
Member

Choose a reason for hiding this comment

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

if this is the case, why do we need KubeadmConfigTemplateName and KubeadmControlPlaneName?

Copy link
Member Author

Choose a reason for hiding this comment

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

those are the resources that get customised.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, the KubeadmConfig for a MachinePool. Makes sense. Will PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

test/framework/log/log.go Outdated Show resolved Hide resolved
@randomvariable
Copy link
Member Author

My only concern is about using this in docker, because in my experience there could be potential resource bottlenecks when executing the install scripts on several containers running on the same machines

Same, the other way would be Sonobuoy, but unfortunately we can't simultaneously import Sonobuoy and clusterctl framework because Sonobuoy imports clashing versions of API machinery. We can probably be revisit when we bump controller runtime in v1alpha4.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 24, 2020
…nd additional suite_helpers

Mostly upstreaming from CAPA.

The kubetest package allows a consumer to run kubetest (i.e. Kubernetes conformance) against
given clusters.

The kubernetesversions package provides helpers to determine the latest CI release of Kubernetes
and adds debian/ubuntu injection script kustomizations for  based on prior work by dims.

ginkgoextensions are common helpers that were added to CAPA, and are also present in the test/e2e
package.

Additionally add helpers to log running commands as well as gather JUnit files in a way that
is compatible with Prow.

Future PRs will add consumption of this in an e2e test and make CAPA consume these.

Signed-off-by: Naadir Jeewa <jeewan@vmware.com>
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 24, 2020

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

Test name Commit Details Rerun command
pull-cluster-api-verify-external-links 995578c link /test pull-cluster-api-verify-external-links

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

/milestone v0.3.10

@k8s-ci-robot k8s-ci-robot added this to the v0.3.10 milestone Sep 24, 2020
@randomvariable
Copy link
Member Author

Any update on approval?

@sedefsavas
Copy link

/lgtm

@vincepri
Copy link
Member

/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 25, 2020
@vincepri
Copy link
Member

/override pull-cluster-api-apidiff
/override pull-cluster-api-verify-external-links

@k8s-ci-robot
Copy link
Contributor

@vincepri: Overrode contexts on behalf of vincepri: pull-cluster-api-apidiff, pull-cluster-api-verify-external-links

In response to this:

/override pull-cluster-api-apidiff
/override pull-cluster-api-verify-external-links

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.

@k8s-ci-robot k8s-ci-robot merged commit 59faa7b into kubernetes-sigs:master Sep 25, 2020
@randomvariable randomvariable deleted the test-framework-extensions branch September 28, 2020 10:55
@randomvariable randomvariable changed the title ✨ test framework: Add kubetest, kubernetesversions, log and ginkgo extensions ✨ test framework: Add kubetest, kubernetesversions, and ginkgo extensions Sep 28, 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. 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.

None yet

7 participants