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

HDDS-11039. Release initial version of the Helm chart #4

Merged
merged 6 commits into from
Oct 9, 2024

Conversation

dnskr
Copy link
Contributor

@dnskr dnskr commented Jul 8, 2024

What changes were proposed in this pull request?

The PR adds basic workflow to release Helm charts.

The release happens each time the chart version is bumped in Chart.yaml in the main branch.
The workflow turns current GitHub project into a self-hosted Helm chart repo and requires the following:

  • Branch gh-pages (ideally created from the initial commit) to store the published charts.
  • Enabled GitHub Pages with source branch equal to gh-pages (go to Settings/Pages and change the Source Branch).

Caution! The workflow will release the initial version of the chart as soon as it's merged!

Links for more details:

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11039

How was this patch tested?

Tested in the repository fork:

@dnskr dnskr marked this pull request as draft July 8, 2024 13:05
@dnskr
Copy link
Contributor Author

dnskr commented Jul 26, 2024

@adoroszlai Could you please make the following prerequisite steps to proceed with the chart release?

  1. Create gh-pages branch from the initial or second commit.
  2. Enable GitHub Pages with gh-pages source branch.
  3. Provide URL associated with GitHub Pages to add installation instructions to README files.

@adoroszlai
Copy link
Contributor

Thanks @dnskr for working on this. I think we need to discuss whether the project must vote about chart releases, and if so, automating it in Github may not be OK.

@dnskr
Copy link
Contributor Author

dnskr commented Jul 31, 2024

Thanks @dnskr for working on this. I think we need to discuss whether the project must vote about chart releases, and if so, automating it in Github may not be OK.

Got it.

Just for information, the pipeline releases the chart if version value has been bumped in Chart.yaml file, i.e.:

So, we can agree to deny version change in any PRs and bump version only after successful release vote.

@dnskr dnskr force-pushed the add-workflow-to-release-helm-charts branch from 2da21bf to fabfe51 Compare August 5, 2024 12:10
@dnskr
Copy link
Contributor Author

dnskr commented Aug 5, 2024

Updated README files based on the assumption https://apache.github.io/ozone-helm-charts/ is used for GitHub Pages.


Not sure why Test Charts workflow failed. Error message says that actions helm/chart-testing-action and helm/kind-action are not allowed to be used in apache/ozone-helm-charts, but they use SHA and should pass the condition:

or matching the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+

@adoroszlai Please, let me know if I can help to proceed with the PR.

@adoroszlai
Copy link
Contributor

Not sure why Test Charts workflow failed. Error message says that actions helm/chart-testing-action and helm/kind-action are not allowed to be used in apache/ozone-helm-charts, but they use SHA and should pass the condition:

or matching the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+

Please open an INFRA ticket in Apache Jira to help understand why it's getting rejected. Maybe the error description is not up-to-date with the restriction.

@adoroszlai
Copy link
Contributor

I think we need to discuss whether the project must vote about chart releases

Started discussion at https://lists.apache.org/thread/bl1zdbkp0g92b3k02lgv9t7y18xfdqjg

@dnskr
Copy link
Contributor Author

dnskr commented Aug 5, 2024

Not sure why Test Charts workflow failed. Error message says that actions helm/chart-testing-action and helm/kind-action are not allowed to be used in apache/ozone-helm-charts, but they use SHA and should pass the condition:

or matching the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+

Please open an INFRA ticket in Apache Jira to help understand why it's getting rejected. Maybe the error description is not up-to-date with the restriction.

INFRA ticket created - https://issues.apache.org/jira/browse/INFRA-26018

@dnskr dnskr marked this pull request as ready for review September 5, 2024 11:12
@dnskr
Copy link
Contributor Author

dnskr commented Sep 5, 2024

@adoroszlai Could you please rerun Test Charts workflow? Action versions should be pinned now, I'll ping infra team once again if not.

@adoroszlai
Copy link
Contributor

Could you please rerun Test Charts workflow?

I don't see any option to re-run it, probably because it encountered Startup failure. Please push an empty commit to re-trigger.

@adoroszlai
Copy link
Contributor

Could you please make the following prerequisite steps to proceed with the chart release?

  1. Create gh-pages branch from the initial or second commit.
  2. Enable GitHub Pages with gh-pages source branch.
  3. Provide URL associated with GitHub Pages to add installation instructions to README files.

Thanks again @dnskr for the patch. After discussion on the dev mailing list, I think we can use this automated release method. Please let me know if I should go ahead with the gh-pages branch creation.

@dnskr dnskr force-pushed the add-workflow-to-release-helm-charts branch from 91e97df to 767d36b Compare October 5, 2024 22:39
@dnskr
Copy link
Contributor Author

dnskr commented Oct 5, 2024

@adoroszlai Great! We can evaluate release approach later again for 1.0 and further versions.

Unfortunately Test Charts workflow still doesn't work. Action versions should be pinned to appropriate SHA versions, but the workflow fails with the following error:

helm/chart-testing-action@e6669bcd63d7cb57cb4380c33043eebe5d111992 and helm/kind-action@0025e74a8c7512023d06dc019c617aa3cf561fde are not allowed to be used in apache/ozone-helm-charts.
Actions in this workflow must be: within a repository that belongs to your Enterprise account, created by GitHub, verified in the GitHub Marketplace, or matching the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+, AdoptOpenJDK/install-jdk@*, JamesIves/github-pages-deploy-action@5dc1d5a192aeb5ab5b7d5a77b7d36aea4a7f5c92, TobKed/label-when-approved-action@*, actions-cool/issues-helper@*, actions-rs/*, al-cheb/configure-pagefile-action@*, amannn/action-semantic-pull-request@*, apache/*, burrunan/gradle-cache-action@*, bytedeco/javacpp-presets/.github/actions/*, chromaui/action@*, codecov/codecov-action@*, conda-incubator/setup-miniconda@*, container-tools/kind-action@*, container-tools/microshift-action@*, dawidd6/action-download-artifact@*, delaguardo/setup-graalvm@*, docker://j...

Do you know what might be the reason or should I open another issue?
I'm not even sure that */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+ is correct regex expression.

WDYT about using git submodules workaround used in Apache Superset apache/superset#14211 ?

@adoroszlai
Copy link
Contributor

Do you know what might be the reason or should I open another issue? I'm not even sure that */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+ is correct regex expression.

According to GitHub Docs, actions can be whitelisted using glob-like pattern that only allows * as wildcard. I think the problem is with the part that tries to match the SHA.

WDYT about using git submodules workaround

It looks like the way we should move forward.

@dnskr
Copy link
Contributor Author

dnskr commented Oct 6, 2024

WDYT about using git submodules workaround

It looks like the way we should move forward.

@adoroszlai Working on it.

Meanwhile, could you please have a look at whitelisted actions in Ozone and Ozone Helm Charts repos?
Here Settings -> Actions -> General -> Allow specified actions and reusable workflows we should have the following actions whitelisted by version an/or SHA accordingly INFRA team response:

  • helm/chart-testing-action
  • helm/kind-action
  • helm/chart-releaser-action

@adoroszlai
Copy link
Contributor

Meanwhile, could you please have a look at whitelisted actions in Ozone and Ozone Helm Charts repos? Here Settings -> Actions -> General -> Allow specified actions and reusable workflows we should have the following actions whitelisted by version an/or SHA accordingly INFRA team response:

Only Infra team has access to repo settings.

@dnskr dnskr force-pushed the add-workflow-to-release-helm-charts branch from 767d36b to bfae1e2 Compare October 7, 2024 17:06
@dnskr dnskr force-pushed the add-workflow-to-release-helm-charts branch from b9b0c1c to 70b6443 Compare October 8, 2024 19:46
@dnskr
Copy link
Contributor Author

dnskr commented Oct 8, 2024

Added chart-releaser-action as git submodule similar to PR #6.
SHA a917fd15b20e8b64b94d9158ad54cd6345335584 points to chart-releaser-action@v1.6.0.

git submodule add --force https://github.com/helm/chart-releaser-action .github/actions/chart-releaser-action
cd .github/actions/chart-releaser-action
git checkout a917fd15b20e8b64b94d9158ad54cd6345335584

Tested changes in fork repo:

Test the chart release

helm install ozone https://github.com/dnskr/ozone-helm-charts/releases/download/ozone-0.1.0/ozone-0.1.0.tgz

kubectl get pod
NAME               READY   STATUS    RESTARTS   AGE
ozone-datanode-0   1/1     Running   0          115s
ozone-datanode-1   1/1     Running   0          113s
ozone-datanode-2   1/1     Running   0          111s
ozone-om-0         1/1     Running   0          115s
ozone-s3g-0        1/1     Running   0          115s
ozone-scm-0        1/1     Running   0          115s

@adoroszlai Please take a look at the changes and I think we can proceed with the release.

@adoroszlai adoroszlai changed the title [DO NOT MERGE] HDDS-11039. Add workflow to release helm charts HDDS-11039. Add workflow to release helm charts Oct 9, 2024
@adoroszlai
Copy link
Contributor

@dnskr gh-pages branch is created from initial commit, and GitHub Pages deployment is set up via .asf.yaml (HDDS-11553). Last step is to merge this, right?

@dnskr dnskr changed the title HDDS-11039. Add workflow to release helm charts HDDS-11039. Release initial version of the Helm chart Oct 9, 2024
@dnskr
Copy link
Contributor Author

dnskr commented Oct 9, 2024

@dnskr gh-pages branch is created from initial commit, and GitHub Pages deployment is set up via .asf.yaml (HDDS-11553). Last step is to merge this, right?

Right, we only need to merge the PR to release the chart and make it available for users.

@adoroszlai adoroszlai merged commit 3616a3e into apache:main Oct 9, 2024
1 check passed
@adoroszlai
Copy link
Contributor

Thanks @dnskr for continued efforts on this.

@pyttel
Copy link
Contributor

pyttel commented Oct 17, 2024

Hi and thank you for the helm chart!
I tried to install the chart with persistence switched on. I get the following error:

PersistentVolumeClaim "YXZ-datanode" is invalid: spec.accessModes: Required value: at least 1 access mode is required

Would it be fixed if accessModes can be set by values.yaml? And is it correct to have only one pvc for 3 instances of OM,SCM,DATANODE and S3G?

@adoroszlai
Copy link
Contributor

Hi and thank you for the helm chart! I tried to install the chart with persistence switched on. I get the following error:

PersistentVolumeClaim "YXZ-datanode" is invalid: spec.accessModes: Required value: at least 1 access mode is required

Would it be fixed if accessModes can be set by values.yaml? And is it correct to have only one pvc for 3 instances of OM,SCM,DATANODE and S3G?

@dnskr can you please check?

@pyttel please report the issue in Jira

@pyttel
Copy link
Contributor

pyttel commented Oct 17, 2024

Yes of cause :) I requested a login over ASF. When it is approved I will add the Ticket.

@pyttel
Copy link
Contributor

pyttel commented Oct 18, 2024

Opened the issue HDDS-11597

@pyttel
Copy link
Contributor

pyttel commented Oct 23, 2024

I created the pull requests. Sorry for the delay but I needed to test a lot manually and the setup takes some time ^^ #9

There is another thing I found: Using more than 1 replicas for OM crashes. In my opinion the env variables from DOC should be applied to OM and SCM. If I'm correct I will open a new Ticket and implement the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants