Add Helm Test To Descheduler Automated CI Checks

[seanmalloy]

The descheduler CI checks currently only run helm lint. It would be great to also run helm test as part of CI. The Makefile will need to be updated to add support for helm test. It would probably also be good to add this as a separate CI check, see below URL.

https://github.com/kubernetes/test-infra/tree/master/config/jobs/kubernetes-sigs/descheduler

[seanmalloy]

/kind cleanup

[seanmalloy]

This documentation looks useful ... https://helm.sh/docs/topics/chart_tests/

[fejta-bot]

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-contributor-experience at kubernetes/community.
/lifecycle stale

[seanmalloy]

/remove-lifecycle stale

[pravarag]

Hi @damemi @seanmalloy I would like to work on this issue if it's not already taken by someone else.

[seanmalloy]

Hi @damemi @seanmalloy I would like to work on this issue if it's not already taken by someone else.

@pravarag no one is working on this. Please feel free to submit a pull request. Let me know if you have any questions.

Thanks!

[pravarag]

@damemi @seanmalloy I was going through the current usage of helm lint as mentioned here and as I can understand linting doesn't require chart to be installed in a namespace. But, for helm test <release_name> we'll have to first install the chart in a namespace as suggested here . So, do we have a K8s cluster where we can first install the release and then utilize it to perform the helm test or there might be other ways to achieve the same(maybe a dry-run)?

[seanmalloy]

@pravarag it will be very similar to the e2e tests. The helm tests will need to run in a kind cluster.

descheduler/Makefile

Lines 114 to 115 in 22fe589

	test-e2e:
	./test/run-e2e-tests.sh

https://github.com/kubernetes-sigs/descheduler/blob/master/test/run-e2e-tests.sh

[seanmalloy]

I'm thinking there would be a new Makefile target created named test-helm and it would run a script ./test/run-helm-tests.sh

[pravarag]

@pravarag it will be very similar to the e2e tests. The helm tests will need to run in a kind cluster.

Thanks @seanmalloy for sharing the above details and I can see the kind cluster creation step here . So, I wanted to check whether I can use the same kind cluster to run helm tests or I'll be needing to create a new one? Also, I don't see a kind delete cluster mentioned anywhere in the scripts so are we not deleting that kind cluster?

[seanmalloy]

@pravarag it will be very similar to the e2e tests. The helm tests will need to run in a kind cluster.

Thanks @seanmalloy for sharing the above details and I can see the kind cluster creation step here . So, I wanted to check whether I can use the same kind cluster to run helm tests or I'll be needing to create a new one? Also, I don't see a kind delete cluster mentioned anywhere in the scripts so are we not deleting that kind cluster?

I think having the script test/run-helm-tests.sh create a separate kind cluster should be fine. I see you added the code in your PR to delete the kind cluster, which is also fine.

Let me know if you have any other questions.

Thanks!

[pravarag]

@seanmalloy I need your help regarding this comment for helm tests: #527 (comment). I wanted to understand the functionality of the test which we'll be writing as part of this issue.

[pravarag]

@seanmalloy please provide your review comments on these changes I've made at the latest here: #527 (comment) . Let me know if those are good to go with or not 🙂

[seanmalloy]

/reopen

[k8s-ci-robot]

@seanmalloy: Reopened this issue.

In response to this:

/reopen

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.

[pravarag]

@seanmalloy thanks for merging the PR. Let me know if I can proceed with adding CI check here: https://github.com/kubernetes/test-infra/tree/master/config/jobs/kubernetes-sigs/descheduler or at any other place as required.

[pravarag]

@seanmalloy @damemi I've few doubts w.r.t adding a CI check for helm test changes.

I've added a CI check with the other checks here: pravarag/test-infra@6e8d626 but I wanted to check how to approach with unit or integration tests as mentioned in other CI checks in the same directory. Do I need to write new unit tests or integration tests for same or there are already few which I can utilize here.

Also let me know if it's okay to create a draft PR with the current changes under kubernetes/test-infra repo or not with work-in-progress so that I can share further updates with easy references.

[damemi]

@pravarag it's always okay to open a WIP PR 🙂 feel free to go ahead, that will make it easier to discuss the changes

[pravarag]

@seanmalloy @damemi can we close this issue as the test-infra PR: kubernetes/test-infra#22391 is also merged now 🙂

[damemi]

@pravarag yup, this should be all set now. Thanks!
/close

[k8s-ci-robot]

@damemi: Closing this issue.

In response to this:

@pravarag yup, this should be all set now. Thanks!
/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.