remove e2e tests from helm github action

[a7i]

Rationale:

1. e2e tests are not executed against a Descheduler install. If anything, we could end up with a race condition between a Descheduler run and the e2e tests. For example, notice here that the test runs Descheduler itself.
2. We already run e2e tests against 3 Kubernetes Versions

[a7i]

/cc @ingvagabund @JaneLiuL

[JaneLiuL]

/lgtm

[damemi]

does running the e2e after install just serve as a way to verify that the helm chart was properly installed and the descheduler is actually working?

Maybe there is a simpler test we could run to verify that, but the general purpose for having some functionality test post-install makes sense to me

[a7i]

does running the e2e after install just serve as a way to verify that the helm chart was properly installed and the descheduler is actually working?

Maybe there is a simpler test we could run to verify that, but the general purpose for having some functionality test post-install makes sense to me

AFAIK, the e2e tests do not rely on the Descheduler install. If anything, we could see a race condition between the Descheduler helm install and the e2e Descheduler run.

[damemi]

ah yeah. it makes sense then to remove it to avoid that race condition. But we should add a follow up to add some small test to verify descheduler is actually working after helm install

[knelasevero]

Yeah, I saw the other issue mentioning the additional button step. #946

Having these a bit separate was what made me consider the e2e tests in the helm workflow as useful: #895 (comment)

If we have a contribution that is not from an org member, and only has changes in the chart, we could avoid running /ok-to-test for the 3 versions, and just press the button and only get the feedback that is relevant (1 lint and chart install test + 1 latest e2e). [from non-org-members that are not first time contributors helm workflow would just run, and /ok-to-test would not be required]

But since we want to unify that (based on comments of #946) my rationale wouldn't make sense indeed, and e2e tests in helm workflow becomes redundant.

And yes, it does not run against the pod installed, ct install creates a ns, installs everything, checks results, and deletes the namespace. To make it run against an install we would have to change how e2e tests are implemented and maybe see if ct allows us to skip the deletion.

[ingvagabund]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund

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:

• OWNERS [ingvagabund]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[knelasevero]

Just sharing an update on this, I talked with @cpanato and ct install does not have a skipp cleanup option, but he thinks it is a valid new feature if we want to contribute to the chart test repository. That would be useful for doing that simple tests mentioned here with the pod running and not deleted after ct install.

[ingvagabund]

But we should add a follow up to add some small test to verify descheduler is actually working after helm install

I wonder whether we can create a new CI job that will build a descheduler image from the current PR. An image which we can then use in all the pull-descheduler-test-e2e-k8s-BRANCH-VERSION jobs as e.g. an env which will get injected into make e2e. Deploying a descheduler as a pod right before running individial e2e. Having each e2e to only change the descheduler configuration, wait until a descheduler is restarted and running again and then running a test.

Helm e2e would "just" replace the step of deploying a descheduler as a pod. So all e2e would be interacting with the helm-based deployment of the descheduler.

When it comes to https://github.com/kubernetes-sigs/descheduler/blob/master/test/e2e/e2e_leaderelection_test.go#L105 the test would reuse the running instance instead of creating two new instances.

[knelasevero]

But we should add a follow up to add some small test to verify descheduler is actually working after helm install

I wonder whether we can create a new CI job that will build a descheduler image from the current PR. An image which we can then use in all the pull-descheduler-test-e2e-k8s-BRANCH-VERSION jobs as e.g. an env which will get injected into make e2e. Deploying a descheduler as a pod right before running individial e2e. Having each e2e to only change the descheduler configuration, wait until a descheduler is restarted and running again and then running a test.

Helm e2e would "just" replace the step of deploying a descheduler as a pod. So all e2e would be interacting with the helm-based deployment of the descheduler.

When it comes to https://github.com/kubernetes-sigs/descheduler/blob/master/test/e2e/e2e_leaderelection_test.go#L105 the test would reuse the running instance instead of creating two new instances.

In External Secrets Operator we programaticaly install the chart during our e2e tests, but we have some hard to read setup there... https://github.com/external-secrets/external-secrets/blob/main/e2e/framework/addon/chart.go

And as you say, there we also only do the chart linting, because the chart install is being tested in our e2e tests altogether.

[ingvagabund]

Checking logs of pull-kubernetes-e2e-kind and pull-kubernetes-e2e-gce-ubuntu-containerd jobs in kubernetes/kubernetes repo each job builds its own kubernetes [kind] image. Meaning each job we run would have to build its own descheduler image and deploy it.

And as you say, there we also only do the chart linting, because the chart install is being tested in our e2e tests altogether.

We could follow the same approach:

1. build a descheduler image
2. deploy a descheduler as a pod/deployment through chart install
3. run e2e and have each e2e update the running descheduler configuration instead of running the plugins directly/deploying new descheduler instances

[knelasevero]

Oh, if we want to keep it still in gh-actions: @cpanato just added that feature 😃❤️:
helm/chart-testing#478

helm/chart-testing@0e8be34

[k8s-triage-robot]

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

This bot triages PRs 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 PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

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

/lifecycle stale

[ingvagabund]

/remove-lifecycle stale

[knelasevero]

Let's merge this. @a7i would you please rebase?

[a7i]

/retest

[knelasevero]

/lgtm