-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
✨ test framework: Add kubetest, kubernetesversions, and ginkgo extensions #3652
Conversation
b60e0bd
to
95e8053
Compare
95e8053
to
72953d0
Compare
72953d0
to
69cead5
Compare
/retest |
There was a problem hiding this 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 👀
1eb6652
to
585c2d6
Compare
/test pull-cluster-api-test |
1 similar comment
/test pull-cluster-api-test |
/lgtm |
/lgtm |
/test pull-cluster-api-verify-external-links just want to see if this works 😄 |
There was a problem hiding this 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.
// 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw this, @randomvariable @fabriziopandini we might need MachinePools as well because they use KubeadmConfig not KubeadmConfigTemplate, see how we did it in Azure: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/5f7473fd8221248ca01e35365e4e862a27bf5acf/templates/test/prow-machine-pool-ci-version/patches/machine-pool-ci-version.yaml
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
585c2d6
to
cab26d5
Compare
…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>
cab26d5
to
995578c
Compare
@randomvariable: The following test failed, say
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. |
/milestone v0.3.10 |
Any update on approval? |
/lgtm |
/approve |
[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 |
/override pull-cluster-api-apidiff |
@vincepri: Overrode contexts on behalf of vincepri: pull-cluster-api-apidiff, pull-cluster-api-verify-external-links In response to this:
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. |
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