search

kubernetes-retired / multi-tenancy

A working place for multi-tenancy related proposals and prototypes.
Apache License 2.0
954 stars 172 forks source link

kubectl-mtb tests not running on PRs #1523

Closed mac-chaffee closed 2 years ago

mac-chaffee commented 2 years ago

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_multi-tenancy/1522/pull-mtb-test/1467290069042728960

 wrapper.sh] [TEST] Running Test Command: `./benchmarks/kubectl-mtb/hack/ci-test.sh` ...
Running 'make test'
make: Entering directory '/home/prow/go/src/sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb'
make: Nothing to be done for 'test'.
make: Leaving directory '/home/prow/go/src/sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb'
wrapper.sh] [TEST] Test Command exit code: 0

Seems it's been like this for a while: https://github.com/kubernetes-sigs/multi-tenancy/commit/592ffde91c5a22766fe01a495507403fe2cda7e6#diff-90f60acc2a61e4aed81c76af91cbc2f4524d26c183a7479e105e9d91734b7d5bR44

Not sure if the tests fail or not since I don't have docker on my machine, just FYI for now.

mac-chaffee commented 2 years ago

I'm working through the tests, including updating kind and kyverno in the process. Open to tips and advice though :)

Branch is here: https://github.com/kubernetes-sigs/multi-tenancy/compare/master...mac-chaffee:fix-tests

mac-chaffee commented 2 years ago

I think fixing these tests may be beyond my golang capabilities unfortunately :( Here's some recommendations about what to improve.

  1. The setup and teardown for each test consists of 90 lines of code duplicated in 12 different places, and it has a few bugs: https://github.com/kubernetes-sigs/multi-tenancy/blob/master/benchmarks/kubectl-mtb/test/benchmarks/block_access_to_cluster_resources/block_access_to_cluster_resources_test.go#L24-L114
    • In unittestutils/cluster.go, there's a CreateCluster method, but it uses a different cluster name. There's also code to create a kind cluster in the Makefile. Neither of these are used, but could be replacements for the 90 lines of code
    • There are 2 unhandled errors in that code
    • "go test" runs in parallel based on the number of cores, so the tests race each other to create/destroy the cluster unless you use the -parallel=1 flag
    • Some tests don't check preconditions or clean up after themselves. For example, block_privileged_containers_test.go assumes the kyverno policies don't exist yet, but other tests may create the policies before it does, failing the test. We might could get away with not cleaning up policies after tests if we didn't use ClusterPolicies, since every test runs in a randomized namespace already.
    • Given all these bugs, the duplicate code should probably be put in one place so that future bug fixes don't have to be made 12 times.
  2. Many tests don't differentiate between real errors and expected errors, since many test methods just return booleans like block_privileged_containers_test.go:testPreRunWithRole. If that test throws a legitimate error, it will still pass.
  3. The code in unittestutils/create_policy.go:
    • It splits the yaml file on "---", which is not strictly correct since that string can appear inside multi-line yaml strings. In fact, the updated kyverno.yaml file from upstream includes that string in multi-line strings a few times.
    • Might just be better to kubectl apply -f kyverno.yaml before the tests start up since the logic to apply arbitrary k8s objects in client-go can be complex.
  4. Once the above issues are fixed, that may mean that tests are simplified enough to be included in the mtb_builder/tpl.go template.
k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

mac-chaffee commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

mac-chaffee commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 2 years ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes-sigs/multi-tenancy/issues/1523#issuecomment-1304074215): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.