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

Support for an e2e-framework kubetest2 tester #168

Merged

Conversation

vladimirvivien
Copy link
Contributor

@vladimirvivien vladimirvivien commented Nov 11, 2022

This PR implements a kubetest2 tester to run e2e-framework tests can be executed with kubetest2 as:

$> kubetest2 kind --up --down        \
     --test=e2e-framework --         \
     --packages ./cluster            \
     --kubeconfig=$HOME/.kube/config \
     --skip-assessments=pod-count

See README.md for doc.

Closes #132
Closes #174

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 11, 2022
@k8s-ci-robot k8s-ci-robot requested a review from cpanato November 11, 2022 01:08
@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Nov 11, 2022
@k8s-ci-robot k8s-ci-robot requested a review from spiffxp November 11, 2022 01:08
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 11, 2022
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 11, 2022
@vladimirvivien vladimirvivien changed the title [WIP] Support for an e2e-framework kubetest2 tester Support for an e2e-framework kubetest2 tester Dec 11, 2022
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 11, 2022
@vladimirvivien
Copy link
Contributor Author

@cpanato
@harshanarayana
@ShwethaKumbla
@helayoty

Please take a look.

This third-party package implements a Kubernetes-SIGs/[kubetest2](https://github.com/kubernetes-sigs/kubetest2) tester capable of executing tests written using the [e2e-framework](https://github.com/kubernetes-sigs/e2e-framework).

## Kubetest2 installation
First, you must ensure that you have the `kubtest2` binary installed in the environment where you will run your tests :
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
First, you must ensure that you have the `kubtest2` binary installed in the environment where you will run your tests :
First, you must ensure that you have the `kubetest2` binary installed in the environment where you will run your tests :

> See further detail [here](https://github.com/kubernetes-sigs/kubetest2#installation).

## E2E-Framework installation
To build the e2e-framework tester, you can use the following `go intstall` command:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To build the e2e-framework tester, you can use the following `go intstall` command:
To build the e2e-framework tester, you can use the following `go install` command:

--test-flags string Space-separated flags applied to 'go test' command
```

To run a test with kubetest2, you must follow this command format as outline above:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To run a test with kubetest2, you must follow this command format as outline above:
To run a test with kubetest2, you must follow this command format as outlined above:

Let us use `kubetest2` to run a simple test with no deployer and no arguments passed to the test:

```
kubetest2 noop --test=e2e-framework -- --packages ./simple
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
kubetest2 noop --test=e2e-framework -- --packages ./simple
kubetest2 noop --test=e2e-framework -- --packages ./examples/simple

ok sigs.k8s.io/e2e-framework/examples/simple 0.363s
```

You can easily change this command to ask `kubetest` to stand and teardown a Kubernets cluster using kind for instance (kubetest2 support other deployers), as shown:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can easily change this command to ask `kubetest` to stand and teardown a Kubernets cluster using kind for instance (kubetest2 support other deployers), as shown:
You can easily change this command to ask `kubetest2` to stand and teardown a Kubernetes cluster using kind for instance (`kubetest2` also supports other deployers), as shown:

You can easily change this command to ask `kubetest` to stand and teardown a Kubernets cluster using kind for instance (kubetest2 support other deployers), as shown:

```
$> kubetest2 kind --up --down --test=e2e-framework -- packages .
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$> kubetest2 kind --up --down --test=e2e-framework -- packages .
$> kubetest2 kind --up --down --test=e2e-framework -- --packages .



### Passing arguments to e2e-framework tests
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments to your tests to control settings such ass test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments to your tests to control settings such ass test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments to your tests to control settings such as test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:

testCmd.WriteString(" --fail-fast")
}
if t.DisableGracefulTeardown {
testCmd.WriteString("--disable-graceful-shutdown")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
testCmd.WriteString("--disable-graceful-shutdown")
testCmd.WriteString(" --disable-graceful-shutdown")
❯ kubetest2 noop --test=e2e-framework -- --packages ./examples/simple/ --disable-graceful-teardown
I1212 03:47:51.757165   94358 app.go:61] The files in RunDir shall not be part of Artifacts
I1212 03:47:51.757238   94358 app.go:62] pass rundir-in-artifacts flag True for RunDir to be part of Artifacts
I1212 03:47:51.757254   94358 app.go:64] RunDir for this run: "/home/jbpratt/git/kubernetes-sigs/e2e-framework/_rundir/1894a35c-1dc2-46e0-a7aa-334d36fd3690"
I1212 03:47:51.774635   94358 app.go:130] ID for this run: "1894a35c-1dc2-46e0-a7aa-334d36fd3690"
Running:  go test  ./examples/simple/ -args--disable-graceful-shutdown
flag provided but not defined: -args--disable-graceful-shutdown
Usage of /tmp/go-build4037945708/b001/simple.test:
  -test.bench regexp
...

@vladimirvivien
Copy link
Contributor Author

@jbpratt thank you for the great review, PTAL.

@jbpratt
Copy link
Contributor

jbpratt commented Dec 13, 2022

@jbpratt thank you for the great review, PTAL.

@vladimirvivien changes LGTM! Thanks 🐱

Copy link
Contributor

@harshanarayana harshanarayana left a comment

Choose a reason for hiding this comment

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

@vladimirvivien This is wonderful. Just leaving a few comments from my end. PTAL when time permits.


func (t *Tester) Test() error {
testCmd := t.buildCmd()
fmt.Println("Running: ", testCmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be nitpicking here, but should this not be logged only if the tests are being asked to run in -v mode for example ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Good catch, it should be log, not fmt.

ok sigs.k8s.io/e2e-framework/examples/simple 0.363s
```

You can easily change this command to stand up, and teardown, a Kubernetes cluster using KinD for instance (`kubetest2` also supports other deployers):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can easily change this command to stand up, and teardown, a Kubernetes cluster using KinD for instance (`kubetest2` also supports other deployers):
You can easily change this command to stand up and teardown a Kubernetes cluster using KinD for instance (`kubetest2` also supports other deployers):

Deleting cluster "" ...
```

> It should be noted that e2e-framework offers its own programmatic hooks for more flexible controls of pre- and post-test tasks, such as creating and tearing down named clusters (see the examples folder).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> It should be noted that e2e-framework offers its own programmatic hooks for more flexible controls of pre- and post-test tasks, such as creating and tearing down named clusters (see the examples folder).
> It should be noted that e2e-framework offers its own programmatic hooks for more flexible controls of pre and post test tasks, such as creating and tearing down named clusters (see the examples folder).

Minor nitpick here too.

> It should be noted that e2e-framework offers its own programmatic hooks for more flexible controls of pre- and post-test tasks, such as creating and tearing down named clusters (see the examples folder).

### Passing arguments to e2e-framework tests
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments to your tests to control settings such as test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments to your tests to control settings such as test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:
The kubetest2 e2e-framework tester supports the ability to pass CLI arguments for your tests to control settings such as test execution filters and test behaviors. Recall, in order for your e2e-framework tests to receive CLI arguments, you must programmatically create the test environment to receive its configuration from the command-line:

pkg/env/env.go Outdated
if err != nil {
return nil, err
}
env := newTestEnv()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be replaced with return NewWithConfig(cfg), nil right ?

Comment on lines 117 to 121
cfg, err := envconf.NewFromFlags() // important: read config from CLI flags
if err != nil {
log.Fatalf("failed to build config from flags: %s", err)
}
tenv = env.NewWithConfig(cfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be replaced with the new function you have included NewFromFlags to make this much easier I think,

Suggested change
cfg, err := envconf.NewFromFlags() // important: read config from CLI flags
if err != nil {
log.Fatalf("failed to build config from flags: %s", err)
}
tenv = env.NewWithConfig(cfg)
var err error
tenv, err = env.NewFromFlags()
if err != nil {
log.Fatalf("failed to build config from flags: %s", err)
}

t.Fatal(err)
}
if len(deps.Items) == 0 {
t.Fatal("apps found")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this error is right. Why does this say apps found where the check is == 0 ?

Comment on lines +97 to +117
if t.Namespace != "" {
testCmd.WriteString(" --namespace=" + t.Namespace)
}
if t.Kubeconfig != "" {
testCmd.WriteString(" --kubeconfig=" + t.Kubeconfig)
}
if t.Feature != "" {
testCmd.WriteString(" --kubeconfig=" + t.Feature)
}
if t.SkipFeatures != "" {
testCmd.WriteString(" --skip-features=" + t.SkipFeatures)
}
if t.Assess != "" {
testCmd.WriteString(" --assess=" + t.Assess)
}
if t.SkipAssessment != "" {
testCmd.WriteString(" --skip-assessment=" + t.SkipAssessment)
}
if t.Labels != "" {
testCmd.WriteString(" --labels=" + t.Labels)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to simplify this by keeping this set of things co-located in the flags itself ? Will make it easy when someone is adding a new Argument for the e2e-framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have thought about the exact same thing, basically creating a new type (like a map) that has all the info and be used in both places (here and in the pkg/flags packages). One problem is that the flag package works differently then the gpflag package. So some amount of repeat will be needed.

For now, I'd like to keep this implementation simple without spreading more complexity in the PR. This can be revisited in a new PR later.

Copy link
Contributor

Choose a reason for hiding this comment

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

/Ack. We can do that for sure.

Copy link
Contributor

@harshanarayana harshanarayana left a comment

Choose a reason for hiding this comment

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

@vladimirvivien This is wonderful. Just leaving a few comments from my end. PTAL when time permits.

The code in this patch introduces support for github.com/kubernetes-sigs/kubetest2.
It provides a kubetest2-tester binary that allows test writers to run their e2e-framwork tests
using kubetest2.  See README.md for detail.

Signed-off-by: Vladimir Vivien <vladimir.vivien@gmail.com>
@vladimirvivien
Copy link
Contributor Author

@harshanarayana PTAL

Copy link
Contributor

@harshanarayana harshanarayana left a comment

Choose a reason for hiding this comment

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

LGTM

"sigs.k8s.io/kubetest2/pkg/testers"
)

var GitTag string
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a silly question. But where does this get substituted or updated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the kubetest2 framework uses it. I copied from other example, it is worth exploring later whether it is needed or not.

@vladimirvivien
Copy link
Contributor Author

@cpanato can I get an LGTM, it's been reviewed.

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

looks cool! thanks for doing this
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 29, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, vladimirvivien

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 merged commit 502dfc6 into kubernetes-sigs:main Dec 29, 2022
@vladimirvivien vladimirvivien deleted the kubetest2-integration branch June 23, 2023 15:48
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. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to create new test environment from flags Implement a kubetest2 tester for e2e-framework
5 participants