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

e2e storage tests: usable out-of-tree #70862

Merged
merged 5 commits into from Dec 22, 2018

Conversation

@pohly
Copy link
Contributor

commented Nov 9, 2018

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Not all CSI drivers can be tested in Kubernetes (they might not be open) or should not be (if it does not help develop and enhance Kubernetes). Therefore it is useful to make the tests available to out-of-tree drivers without pulling in Kubernetes E2E specific code and tests.

Does this PR introduce a user-facing change?:

NONE
@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

/test pull-kubernetes-e2e-gce-100-performance

@pohly pohly force-pushed the pohly:e2e-storage-tests branch from ecb1a04 to ab52db2 Nov 9, 2018

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

/hold

I am still testing this myself...

@pohly pohly force-pushed the pohly:e2e-storage-tests branch 3 times, most recently from af7cd36 to 7da9de2 Nov 9, 2018

@k8s-ci-robot k8s-ci-robot added size/XXL and removed size/XL labels Nov 9, 2018

@pohly pohly force-pushed the pohly:e2e-storage-tests branch from 7da9de2 to 0f70ec7 Nov 9, 2018

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/XXL labels Nov 9, 2018

@pohly pohly force-pushed the pohly:e2e-storage-tests branch from 0f70ec7 to 74199a4 Nov 9, 2018

@pohly pohly force-pushed the pohly:e2e-storage-tests branch 2 times, most recently from f2f3b55 to bf39135 Nov 12, 2018

@pohly pohly force-pushed the pohly:e2e-storage-tests branch from bf39135 to 0194ec9 Nov 12, 2018

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Dec 20, 2018

I had to rebase because my other PR for the image versions changed the .yaml files.

@verult can you re-add your lgtm or
@msau42 can you lgtm+approve?

@verult

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 20, 2018

pohly added some commits Nov 9, 2018

e2e/storage: refactor test config
Exposing framework.VolumeTestConfig as part of the testsuite package
API was confusing because it was unclear which of the values in it
really have an effect. How it was set also was a bit awkward: a test
driver had a copy that had to be overwritten at test runtime and then
might have been updated and/or overwritten again by the driver.

Now testsuites has its own test config structure. It contains the
values that might have to be set dynamically at runtime. Instead of
overwriting a copy of that struct inside the test driver, the test
driver takes some common defaults (specifically, the framework pointer
and the prefix) when it gets initialized and then manages its own
copy. For example, the hostpath driver has to lock the pods to a
single node.

framework.VolumeTestConfig is still used internally and test drivers
can decide to run tests with a fully populated instance if needed (for
example, after setting up an NFS server).
e2e: update bazel BUILD files
Generated via hack/update-bazel.sh.

@pohly pohly force-pushed the pohly:e2e-storage-tests branch from 5ce13be to ac8ac8e Dec 21, 2018

@k8s-ci-robot k8s-ci-robot removed the lgtm label Dec 21, 2018

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2018

@msau42 I took the enhancements for multiple nodes out of this PR and re-implemented that in #72002. That PR is now a bit bigger than it needs to be, let me know if you want me to split it up.

Please have another look at the remaining code in this PR.

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2018

/test pull-kubernetes-integration

@liggitt

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

linking PR's that had a test flake because of #71696

cs := f.ClientSet
ns := f.Namespace
n.externalPluginName = fmt.Sprintf("example.com/nfs-%s", ns.Name)

// Reset config. It might have been modified by a previous CreateVolume call.

This comment has been minimized.

Copy link
@msau42

msau42 Dec 21, 2018

Member

Tests can run in parallel. Does each test case get it's own unique object, or share the same object? If each test case has a new object, then this is not needed. If they share the same object, then this may not work very well.

This comment has been minimized.

Copy link
@msau42

msau42 Dec 21, 2018

Member

Opened up #72288 to consider refactoring this later. I don't think this is a clean abstraction

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2018

@msau42

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Dec 21, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, pohly

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

@msau42

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

/priority important-soon

@k8s-ci-robot k8s-ci-robot merged commit e3bf5db into kubernetes:master Dec 22, 2018

19 checks passed

cla/linuxfoundation pohly authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details

pohly added a commit to pohly/kubernetes that referenced this pull request Dec 28, 2018

e2e/storage: reset gcepd config
After PR kubernetes#70862, the dynamic test config no longer gets set for each
test. A driver which modifies the config at runtime must ensure that
those fields get reset before the next test starts, otherwise those
tests can fail.

The intention is to separate the entire test config from the static
driver info (kubernetes#72288),
but for now resetting the config is the fastest way to fix the test
flake.

Fixes: kubernetes#72378

pohly added a commit to pohly/kubernetes that referenced this pull request Dec 28, 2018

e2e/storage: reset gcepd config
After PR kubernetes#70862, the dynamic test config no longer gets set for each
test. A driver which modifies the config at runtime must ensure that
those fields get reset before the next test starts, otherwise those
tests can fail.

The intention is to separate the entire test config from the static
driver info (kubernetes#72288),
but for now resetting the config is the fastest way to fix the test
flake.

Fixes: kubernetes#72378

pohly added a commit to pohly/kubernetes that referenced this pull request Dec 28, 2018

e2e/storage: reset driver config in the test suite
PR kubernetes#70862 made each driver responsible for resetting its config, but
as it turned out, one place was missed in that PR: the in-tree gcepd
sets a node selector. Not resetting that caused other tests to fail
randomly depending on test execution order.

Now the test suite resets the config by taking a copy after setting up
the driver and restoring that copy before each test.

Long term the intention is to separate the entire test config from the
static driver info (kubernetes#72288),
but for now resetting the config is the fastest way to fix the test flake.

Fixes: kubernetes#72378

phenixblue added a commit to phenixblue/kubernetes that referenced this pull request Jan 24, 2019

e2e/storage: reset driver config in the test suite
PR kubernetes#70862 made each driver responsible for resetting its config, but
as it turned out, one place was missed in that PR: the in-tree gcepd
sets a node selector. Not resetting that caused other tests to fail
randomly depending on test execution order.

Now the test suite resets the config by taking a copy after setting up
the driver and restoring that copy before each test.

Long term the intention is to separate the entire test config from the
static driver info (kubernetes#72288),
but for now resetting the config is the fastest way to fix the test flake.

Fixes: kubernetes#72378

pohly added a commit to intel/oim that referenced this pull request Feb 8, 2019

WIP: e2e: use upstream volume testsuites package
The entire volume testing has been refactored upstream. Instead of
just one provisioning test for CSI, the full range of tests that were
previously only available for in-tree volume drivers can now also be
used for CSI.

With some pending
modifications (kubernetes/kubernetes#70862)
these tests can also be used out-of-tree and replace the locally
modified copy of the provisioning test.

The downside is the long runtime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.