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

move test/e2e/framework into staging #74352

Closed
pohly opened this issue Feb 21, 2019 · 36 comments
Closed

move test/e2e/framework into staging #74352

pohly opened this issue Feb 21, 2019 · 36 comments
Assignees
Labels
area/code-organization Issues or PRs related to kubernetes code organization help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Milestone

Comments

@pohly
Copy link
Contributor

pohly commented Feb 21, 2019

What would you like to be added:

#66649 and its PR #68483 made it possible to use test/e2e/framework outside of Kubernetes. It would be better to move test/e2e/framework into staging/src/k8s.io/e2e-framework and import it as k8s.io/e2e-framework. This might be a good time to document and clean up the API, if needed. The moved code should pass all of the Kubernetes code quality checks.

Why is this needed:

Including code from k8s.io/kubernetes in other projects is problematic because there is no guarantee about API stability and (relevant for vendoring) no semantic versioning of the API.

xref: #72638

@pohly pohly added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 21, 2019
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Feb 21, 2019
@pohly
Copy link
Contributor Author

pohly commented Feb 21, 2019

/sig testing

@k8s-ci-robot k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 21, 2019
@figo
Copy link
Contributor

figo commented Feb 26, 2019

/cc

@pohly
Copy link
Contributor Author

pohly commented Mar 14, 2019

That change would also be a good time to split the monolithic test/e2e/framework into smaller packages. Allowing users to vendor only the functionality that they really need still has the potential to cut down on the number of packages that get imported, as seen in PR #75340.

There are several utility packages under test/utils which should also be moved.

Here's a rough outline of what the package structure might look like:

  • framework: core code, minimal set of dependencies (client-go, core Kubernetes APIs)
  • framework/XXX: additional helper packages for framework or for working with framework; the key point is that they have the same minimal set of dependencies. To avoid confusion and cycles, these packages
    must not import framework, i.e. the directory tree mirrors the dependency graph.
  • YYY: other helper packages, potentially with larger sets of dependencies. test/utils/crd in PR framework k8s.io/apiextensions-apiserver dependency #75340 is an example for that, with k8s.io/apiextensions-apiserver as additional dependency. They can depend on framework.

We'll probably need checks to ensure that future changes do not accidentally add new and undesirable dependencies. I've done that elsewhere with "go list", see https://github.com/intel/pmem-CSI/blob/586ae281ac2810cb4da6f1e160cf165c7daf0d80/test/test.make#L48-L92

@dims
Copy link
Member

dims commented Mar 14, 2019

@pohly on the last part, we do have hack/verify-import-boss.sh and hack/verify-imports.sh for enforcing restrictions on dependencies :)

@oomichi
Copy link
Member

oomichi commented Mar 18, 2019

/cc @oomichi

@dims dims added the area/code-organization Issues or PRs related to kubernetes code organization label Apr 14, 2019
@dims dims moved this from Backlog to In progress in code-organization subproject Apr 26, 2019
@andrewsykim
Copy link
Member

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jul 11, 2019
@dims
Copy link
Member

dims commented Jul 12, 2019

/assign @andrewsykim

@SataQiu
Copy link
Member

SataQiu commented Jul 18, 2019

/cc

@alejandrox1
Copy link
Contributor

/cc

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 18, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 18, 2019
@mariusgrigoriu
Copy link

How's this effort coming along? The non-importability of the kubernetes repository complicates the build of 3rd party e2e using the framework.

@pohly
Copy link
Contributor Author

pohly commented Jan 13, 2020 via email

@tanjunchen
Copy link
Member

/help
Adding this label for easy search.
See also, kubectl migration doing the same thing:
https://github.com/kubernetes/enhancements/blob/master/keps/sig-cli/kubectl-staging.md
See also, kube-scheduler migration doing the same thing:
https://docs.google.com/document/d/1WO-ixERpqkCSEXEq30YtEH_z_G-BoLKeCbkRJcKq3xA/edit

@k8s-ci-robot
Copy link
Contributor

@tanjunchen:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help
Adding this label for easy search.
See also, kubectl migration doing the same thing:
https://github.com/kubernetes/enhancements/blob/master/keps/sig-cli/kubectl-staging.md
See also, kube-scheduler migration doing the same thing:
https://docs.google.com/document/d/1WO-ixERpqkCSEXEq30YtEH_z_G-BoLKeCbkRJcKq3xA/edit

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 added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Apr 14, 2020
@tanjunchen
Copy link
Member

tanjunchen commented Apr 17, 2020

There are some refrence to 'k8s.io/kubernetes/pkg' in 'test/e2e/framework' now , such as :

metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/dockershim/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
metrics/kubelet_metrics.go:	// Taken from k8s.io/kubernetes/pkg/kubelet/metrics
service/jig.go:// It is copied from "k8s.io/kubernetes/pkg/registry/core/service/portallocator"
providers/kubemark/kubemark.go:	"k8s.io/kubernetes/pkg/kubemark"
kubelet/config.go:	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
kubelet/config.go:	kubeletconfigscheme "k8s.io/kubernetes/pkg/kubelet/apis/config/scheme"
kubelet/stats.go:	kubeletstatsv1alpha1 "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
kubelet/stats.go:	// copied from k8s.io/kubernetes/pkg/kubelet/dockershim/metrics
kubelet/stats.go:	// copied from k8s.io/kubernetes/pkg/kubelet/dockershim/metrics
kubelet/stats.go:	// copied from k8s.io/kubernetes/pkg/kubelet/dockershim/metrics
pods.go:	// the status of container event, copied from k8s.io/kubernetes/pkg/kubelet/events
pods.go:	// the status of container event, copied from k8s.io/kubernetes/pkg/kubelet/events
pods.go:	// the status of container event, copied from k8s.io/kubernetes/pkg/kubelet/events
pods.go:	// it is copied from k8s.io/kubernetes/pkg/kubelet/sysctl
test_context.go:	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
resource_usage_gatherer.go:	kubeletstatsv1alpha1 "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
pv/pv.go:	// it is copied from k8s.io/kubernetes/pkg/volume/util VolumeGidAnnotationKey

The draft of moving e2eframework into Staging , please see https://docs.google.com/document/d/1QgYEmdrdIirRCGBi2j55yb88WCZOGBqOJw7xj3s05PI/edit# , all feedback will be welcomed !

@jtslear
Copy link

jtslear commented May 28, 2020

Hello, @jtslear here from Bug Triage! This issue has not been updated for a long time, so I'd like to check on the status. The code freeze is starting June 25th and while there is still plenty of time, we want to ensure that each PR has a chance to be merged on time.

As the issue is tagged for 1.19, is it still planned for this release?

@jtslear
Copy link

jtslear commented Jun 15, 2020

@kubernetes/sig-testing

PR kubernetes/enhancements#1746 has been pushed to v1.20

PR #90832 is not assigned to any milestone.

Should this issue be pushed to v1.20?

@spiffxp
Copy link
Member

spiffxp commented Jun 16, 2020

/milestone v1.20
Pushed

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.19, v1.20 Jun 16, 2020
@alejandrox1 alejandrox1 added this to To do in Testing Commons Sep 2, 2020
@alejandrox1
Copy link
Contributor

This has been mentioned in a couple places: the work require to stage the framework is more than the work required to build a custom framework. Thus the proposal from testing commons was to provide documentation and examples to help people build their own kubernetes-like framework.
What do people think? Leaning towards closing this issue and moving on with some other work.

/remove-lifecycle frozen

@k8s-ci-robot k8s-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Sep 10, 2020
@timoreimann
Copy link
Contributor

@alejandrox1 I'm not 100% sure I understand what the proposal to build a custom, Kubernetes-like framework is exactly about. Could you share more details?

@alejandrox1
Copy link
Contributor

@alejandrox1 I'm not 100% sure I understand what the proposal to build a custom, Kubernetes-like framework is exactly about. Could you share more details?

There was some digging done to see what work would need to be done to stage the kubernetes e2e framework (detailing dependencies and what to do to resolve them https://docs.google.com/document/d/1ASPcrEC3iSFqD2K2ANaljfLeBZUPxKr-8nrJpRpH9rI/edit?usp=sharing ).
The e2e framework seems to have a couple tools integrated into it that are very intertwined with internal (non-public) Kubernetes APIs (one of my favourite examples is the use of the kubelet stats API for gethering metrics during test runs).
Doing work to remove that sort of dependencies seems very non-trivial.

Taking a step back and looking to why someone may want to use the framework, it seems that some out-of-tree kubernetes providers may benefit because then they can move code from k/k to their own repos with little effort. It could also help other projects build test suites that mirror Kubernetes CI very well.

For such an occasion, creating a "custom" framework is not too terribly complicated - at the core of it it is about integrating with ginkgo to configure an environment for e2e test (i.e., creating pod security policies, namespaces).
There is also the benefit that a custom framework wouldn't be as bulky as the kubernetes one (people just need to add what they actually need instead of supporting every possible use case currently implemented).

For a more concrete example, there is this "toy framework" that was built as a proof of concept
https://github.com/contributing-to-kubernetes/gnosis/tree/master/stories/e2e-tests#running-your-own-tests-smile

and would definitely loveto hear more feedback on this all @timoreimann

@timoreimann
Copy link
Contributor

timoreimann commented Sep 28, 2020

Sorry for the late response @alejandrox1, things have been busy (like everywhere and always :) ).

I work almost exclusively out-of-tree (because my employer DigitalOcean only jumped onto the Kubernetes wagon after new in-tree provider introductions had already been eschewed). My primary motivation is to be able to use a solid, battle-tested, and comprehensive e2e test suite that serves a variety of needs. The upstream e2e framework fulfills these goals by constantly being consumed (and thereby extended and improved) by the major cloud providers.

I can see a hypothetical scenario where a custom e2e testing framework reaches a level of traction, robustness, and support to the extent that it becomes a good fit for out-of-tree consumers. Practically speaking though, I'm concerned that such a tool will always trail behind any in-tree version as that's going to continue to be the primary toolkit, at least as long as the major cloud providers still operate in-tree. I feel like the time and energy is better spent working on a single framework that can be consumed both internally and externally, which would also help to establish a clearer separation in terms of design and abstractions.

Speaking of: I understand all in-tree providers will eventually have to move out-of-tree. Would that not require making the e2e framework externally consumable anyway? The doc you shared seems to point at this circumstance, so I'm not sure if I'm misunderstanding something here.

@BenTheElder
Copy link
Member

My primary motivation is to be able to use a solid, battle-tested, and comprehensive e2e test suite that serves a variety of needs.

I strongly disagree with "battle-tested". "battle-scarred" maybe. The upstream tests are flaky and full of problematic behavior. It's been expensive to deal with. I would not hold e2e.test up as a great example of how to do ... anything?

The upstream tests are difficult to invoke, difficult to debug, flaky, and full of awkward coupling to things like cloud providers or multiple concepts of node level SSH access.

The upstream e2e framework fulfills these goals by constantly being consumed (and thereby extended and improved) by the major cloud providers.

I think very little work on the framework is cloud provider specific at this point, it's all cleanup or work testing core (non provider specific) features (as no new features should be added in-tree unless they're generic).
I've not seen cloud providers 'extending and improving' this substantially.

Practically speaking though, I'm concerned that such a tool will always trail behind any in-tree version as that's going to continue to be the primary toolkit, at least as long as the major cloud providers still operate in-tree.

All cloud providers are expected to move out of tree. The In-tree providers are allowed to maintain in-tree for now, but new major features are expected to be out of tree. The in-tree framework isn't going anywhere due to gravity of the large existing pile of tests, but that also makes it expensive to improve generally.

I feel like the time and energy is better spent working on a single framework that can be consumed both internally and externally, which would also help to establish a clearer separation in terms of design and abstractions.

Internal testing can and will use functionality that external consumers are NOT allowed to depend on, including private APIs.
Testing upstream will always be different.

Speaking of: I understand all in-tree providers will eventually have to move out-of-tree. Would that not require making the e2e framework externally consumable anyway?

No it does not. Most of the cloud provider specific tests ... import the cloud providers indirectly via the framework... Which is going to be a huge no-no. They're probably going to write new tests (AUIU some of them already have), not necessarily using this framework.

@sayanchowdhury
Copy link
Member

👋🏽 Hey! I'm from the Bug Triage team. This issue has not been updated for a while, so I'd like to check on the status. The code freeze is starting November 12th (about 3 weeks from now) and while there is still plenty of time, we want to ensure that each PR has a chance to be merged on time.

As the issue is tagged for 1.20, is it still planned for this release?

@droslean
Copy link
Member

droslean commented Nov 3, 2020

@pohly What is left here? Shall we move this to 1.21?

@andrewsykim
Copy link
Member

I'm gonna close this since I don't think we want to externalize the existing e2e framework as-is, instead some folks are focusing efforts on a new framework that external projects like CSI drivers can use https://github.com/kubernetes-sigs/e2e-framework

/close

@k8s-ci-robot
Copy link
Contributor

@andrewsykim: Closing this issue.

In response to this:

I'm gonna close this since I don't think we want to externalize the existing e2e framework as-is, instead some folks are focusing efforts on a new framework that external projects like CSI drivers can use https://github.com/kubernetes-sigs/e2e-framework

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-organization Issues or PRs related to kubernetes code organization help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Projects
Development

No branches or pull requests