Skip to content

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

Merged
adoroszlai merged 6 commits into
apache:mainfrom
dnskr:add-workflow-to-release-helm-charts
Oct 9, 2024
Merged

HDDS-11039. Release initial version of the Helm chart#4
adoroszlai merged 6 commits into
apache:mainfrom
dnskr:add-workflow-to-release-helm-charts

Conversation

@dnskr

@dnskr dnskr commented Jul 8, 2024

Copy link
Copy Markdown
Contributor

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

dnskr commented Jul 26, 2024

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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

dnskr commented Jul 31, 2024

Copy link
Copy Markdown
Contributor Author

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

dnskr commented Aug 5, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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

dnskr commented Aug 5, 2024

Copy link
Copy Markdown
Contributor Author

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

dnskr commented Sep 5, 2024

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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
Copy Markdown
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

dnskr commented Oct 5, 2024

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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

dnskr commented Oct 6, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

dnskr commented Oct 8, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

dnskr commented Oct 9, 2024

Copy link
Copy Markdown
Contributor Author

@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
@adoroszlai

Copy link
Copy Markdown
Contributor

Thanks @dnskr for continued efforts on this.

@pyttel

pyttel commented Oct 17, 2024

Copy link
Copy Markdown
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?

@adoroszlai

Copy link
Copy Markdown
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

pyttel commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

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

@pyttel

pyttel commented Oct 18, 2024

Copy link
Copy Markdown
Contributor

Opened the issue HDDS-11597

@pyttel

pyttel commented Oct 23, 2024

Copy link
Copy Markdown
Contributor

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