WIP: add helm test step to gh-actions

[knelasevero]

Also, remove local helm testing in favor of ct in PRs.

Example successful run can be checked here (from my fork to my fork): https://github.com/knelasevero/descheduler/pull/2/checks

Example fail: https://github.com/knelasevero/descheduler/runs/7564334471?check_suite_focus=true

We already get events, describe, and logs of the deployed chart pod in the CI directly.

I am already removing the scripts and references to previous helm testing approach.

So /hold this one until I normalize and merge kubernetes/test-infra#26630

[k8s-ci-robot]

Hi @knelasevero. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign seanmalloy for approval by writing /assign @seanmalloy in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[knelasevero]

@cpanato you mentioned some improvements that could be done to this. If you prefer we can review in this PR, or I can merge this and wait for a follow up PR from you. Your call ❤️

[ingvagabund]

/ok-to-test

[ingvagabund]

This looks really great. I like how easy it is to read the workflow. Also https://github.com/kubernetes-sigs/descheduler/runs/7579750658?check_suite_focus=true looks great with all the events, pod spec and container logs.

Helm / lint-and-test (pull_request) test is currently not required. We need to make sure it is.

[cpanato]

https://github.com/kubernetes-sigs/descheduler/pull/895/files#diff-3be36a9541f01c2e8a31c46ccbbb49265f35f3dc8bb57d01160d4f925961a87cR25-R28

also can be bumped to use helm 3.9.2

[knelasevero]

https://github.com/kubernetes-sigs/descheduler/pull/895/files#diff-3be36a9541f01c2e8a31c46ccbbb49265f35f3dc8bb57d01160d4f925961a87cR25-R28

also can be bumped to use helm 3.9.2

Will do!

[a7i]

this seems odd to do, when we can have a specific values file that can do this.

[knelasevero]

This was only to force ct lint and ct install to show changes, it is unrelated to this value change itself. I want to remove this when the work is finished.

[knelasevero]

Initially I was planning on leaving this, but now I see it is too ugly/confusing 😅

[knelasevero]

I will wait for lgtms and approves, but /hold so I remember to remove this line when reviews are done

[k8s-ci-robot]

@knelasevero: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-descheduler-helm-test	afe1186	link	true	/test pull-descheduler-helm-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[knelasevero]

For some reason when calling from make the helm-extra-set-args from .github/ci/ct.yaml are being ignored... had to set them manually here as arguments of ct.

[knelasevero]

/hold after reviews are done I will remove #895 (comment)

and also need to wait for kubernetes/test-infra#26630

[ingvagabund]

and also need to wait for kubernetes/test-infra#26630

I'd prefer first to introduce a new test, merge this PR, merge kubernetes/test-infra#26630 and then change the make test-helm to run locally.

In overall, this is quite awesome improvement :)

[knelasevero]

and also need to wait for kubernetes/test-infra#26630

I'd prefer first to introduce a new test, merge this PR, merge kubernetes/test-infra#26630 and then change the make test-helm to run locally.

New test? 🤔 You mean this PR itself as the replacement, or something else?

Merge this PR

If we merge this PR first, all new PRs will fail because of inexistent scripts being called on k/test-infra steps.

make test-helm

I've introduced make ct-helm in this PR, and it runs locally against kind

In overall, this is quite awesome improvement :)

🙌🙌

[ingvagabund]

I would like to see the changes in the following order:

1. updating this PR so only a new Helm / lint-and-test (pull_request) test is introduced while still keeping pull-descheduler-helm-test and make test-helm unchanged (merged through WIP: add helm test step to gh-actions #895)
2. making sure the new Helm / lint-and-test (pull_request) test is required and a PR does not merge without the new test going green (this is paramount and needs to be verified)
3. removing pull-descheduler-helm-test test (descheduler: remove helm tests from prow kubernetes/test-infra#26630 gets merged)
4. updating the make test-helm to run the new Helm / lint-and-test (pull_request) test locally (new PR)

[knelasevero]

I would like to see the changes in the following order:

1. updating this PR so only a new Helm / lint-and-test (pull_request) test is introduced while still keeping pull-descheduler-helm-test and make test-helm unchanged (merged through add helm test step to gh-actions #895)
2. making sure the new Helm / lint-and-test (pull_request) test is required and a PR does not merge without the new test going green (this is paramount and needs to be verified)
3. removing pull-descheduler-helm-test test (descheduler: remove helm tests from prow kubernetes/test-infra#26630 gets merged)
4. updating the make test-helm to run the new Helm / lint-and-test (pull_request) test locally (new PR)

You are right, that makes sense. I will convert this PR in a draft/wip to keep it as reference, and open new PRs step by step then. Then we can actually just test if prow ignores or considers gh-actions workflows before merging right after the first merge :)

[k8s-ci-robot]

@knelasevero: PR needs rebase.

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.