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

Helm Charts maintainer needed #1975

Closed
diemol opened this issue Oct 17, 2023 · 20 comments · Fixed by #2029, #2028, #2027, #2034 or #2036
Closed

Helm Charts maintainer needed #1975

diemol opened this issue Oct 17, 2023 · 20 comments · Fixed by #2029, #2028, #2027, #2034 or #2036

Comments

@diemol
Copy link
Member

diemol commented Oct 17, 2023

Feature and motivation

We have been unable to handle updates and proper testing for the helm chart. We are looking for someone who wants to maintain them long-term.

Usage example

If we do not find someone by the end of November 2023, we will move the chart to the https://github.com/seleniumhq-community organization.

@diemol diemol pinned this issue Oct 17, 2023
@amardeep2006
Copy link
Contributor

I see some good PRs raised in recent times. The helm chart is really useful and I see contributions thriving. I wish I were expert in kubernetes and Helm chart.
Only help I can offer is in testing the helm charts.

@diemol
Copy link
Member Author

diemol commented Nov 6, 2023

The problem is that people come to contribute something and never return, which is OK. That is also part of what open source is. However, we do not have the time to test changes correctly and ensure some level of quality in the chart. We need a GitHub workflow that helps us with that.

@VietND96
Copy link
Member

VietND96 commented Nov 6, 2023

I also tried to investigate the testing by comparing output via helm template, and for syntax via helm lint, and basic setup with default values via simplex K8s env deployed by Minikube. Will come up with a workflow if I can automate those steps

@amardeep2006
Copy link
Contributor

amardeep2006 commented Nov 7, 2023

@VietND96 There are some interesting github actions on marketplace for using in Github workflows.
https://github.com/marketplace/actions/helm-chart-testing
Repo : https://github.com/helm/chart-testing-action

There is no need to install minikube, it spins up KIND for kubernetes cluster. I am ready for collaboration if required since I have good experience in using Github actions.

@amardeep2006
Copy link
Contributor

I tried the above GitHub action and it threw some valid linting issues. After solving those linting issues I am now facing issues in helm chart dependencies.
Here is the github run details.
https://github.com/amardeep2006/docker-selenium/actions/runs/6782807895/job/18435787690

Next step : I probably I need to build helm chart first so that It download keda helm chart. Will keep updated.

@VietND96
Copy link
Member

VietND96 commented Nov 7, 2023

@amardeep2006 , the action looks good. You can go ahead and initialize the workflow since I have not started yet :)
You can check the steps that action support to do this procedure helm repo add dependencies chart repo -> helm repo update -> helm dependencies build -> helm lint -> helm package -> helm install

@amardeep2006
Copy link
Contributor

amardeep2006 commented Nov 7, 2023

@diemol The GitHub action is now passing. Here is the latest run I did against .
https://github.com/amardeep2006/docker-selenium/actions/runs/6783953826/job/18439259939

It needs a Name of maintainer in Helm Chart (GitHub username and email) to pass linting. Do we have a volunteer for this? Or can I put your name for time being ?

@diemol
Copy link
Member Author

diemol commented Nov 8, 2023

Where is this needed?

@amardeep2006
Copy link
Contributor

amardeep2006 commented Nov 8, 2023

@diemol It is required by ct tool in Chart.yml. ct tool has this as linting rule that maintainer must be added as Github id.
It expects maintainers attribute to be present.

selenium-grid => (version: "0.23.1", path: "charts/selenium-grid") > chart doesn't have maintainers

I tried passing dummy value, it failed with error :

Error: failed linting charts: failed processing charts
✖︎ selenium-grid => (version: "0.23.1", path: "charts/selenium-grid") > failed validating maintainer "To Be Decided": 404 Not Found

@VietND96
Copy link
Member

VietND96 commented Nov 9, 2023

I saw most Dockerfile are using the label authors="Selenium [email protected]"
So, I think the same could be used in chart metadata. @amardeep2006, you can input the below info and raise the PR for your workflow then.

maintainers:
  - name: Selenium
    email: [email protected]
sources:
  - https://github.com/SeleniumHQ/docker-selenium

@amardeep2006
Copy link
Contributor

I will raise with following in a day or two. I will use SeleniumHQ because Selenium namespace is taken by someone already in GitHub.

maintainers:
  - name: SeleniumHQ
    email: [email protected]
sources:
  - https://github.com/SeleniumHQ/docker-selenium

@amardeep2006
Copy link
Contributor

amardeep2006 commented Nov 10, 2023

Raised PR #2002 This can act as foundation for tests.
Long run : We can create multiple test values files that can be applied one by one in KIND kube cluster. Helm chart testing is relatively new concept so let's evolve it.

@amardeep2006
Copy link
Contributor

Raised fresh PR as previous PR had some CLA issue. #2003

@amardeep2006
Copy link
Contributor

@VietND96 Thanks for improvements in GitHub Workflow triggers in subsequent PRs. Highly Appreciate.

@amardeep2006
Copy link
Contributor

@diemol Does the workflow contribution is sufficient to keep the helm chart alive as part of main repo ? If you feel something else is needed for testing , please share the needs.

@diemol
Copy link
Member Author

diemol commented Nov 21, 2023

Ideally, we need a flow that starts the chart, runs a few Selenium tests, and checks that everything works. Plus, someone who helps maintain it and triage related issues.

@amardeep2006
Copy link
Contributor

amardeep2006 commented Nov 22, 2023

May be something like this https://helm.sh/docs/topics/chart_tests/
It basically uses helm.sh/hook: test add test cases as container.

PR --> GH Workflow --> spinup KIND --> ct install (which internally invoke helm install and helm test) --> Spins up a test container which has prebuild selenium installed to do testing.

May be a new Docker Image is needed that has all the test cases packages already . OR may be that container can mount test cases from GitHub runner as volume ?

Or may be existing tests may be run against grid ingress for simplicity.

What do you think of this idea @VietND96 . Or there is some better Industry Practice you are aware ?

@amardeep2006
Copy link
Contributor

Ideally, we need a flow that starts the chart, runs a few Selenium tests, and checks that everything works. Plus, someone who helps maintain it and triage related issues.

@diemol @VietND96 I have raised PR #2027 to address this concern. Now on PR Workflow will run existing selenium test cases against the Kubernetes ingress.

On triaging can you please explain the expectations, roles and responsibilities a bit more.

@diemol
Copy link
Member Author

diemol commented Nov 24, 2023

Thanks, @amardeep2006!

On triaging can you please explain the expectations, roles and responsibilities a bit more.

Well, someone who is replying and fixing issues related to the Helm chart. The Helm chart has been a contribution from the community, and while we are happy about it, it has made our maintenance load higher.

@VietND96 VietND96 linked a pull request Dec 2, 2023 that will close this issue
8 tasks
@VietND96 VietND96 unpinned this issue Dec 6, 2023
@VietND96 VietND96 linked a pull request Dec 6, 2023 that will close this issue
8 tasks
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.