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.

Sign up for GitHub

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

Jump to bottom

feat: add reusable workflow for Helm Chart #108

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

feat: add reusable workflow for Helm Chart #108

wants to merge 7 commits into from

Conversation

b0l0k
Copy link
Member

@b0l0k b0l0k commented Sep 26, 2023

No description provided.

@b0l0k b0l0k marked this pull request as draft September 26, 2023 14:59
@b0l0k
Copy link
Member Author

b0l0k commented Sep 26, 2023

Waiting that PE team sets CHARTMUSEUM variables at org. level to simplify it.

noirbizarre
noirbizarre reviewed Sep 26, 2023
View reviewed changes
.github/workflows/_reusable_helm.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2 cts: given this is a repository containing only reusable actions and workflows, maybe the reusable_ prefix is a bit redundant (in addition to creating very long workflows name) and maybe _helm.yml is enough

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to separate reusable workflows from the "workflow of the repository".
GitHub is again very limited on that, there are many complaints about it, maybe it'll change in the future.

I'm fine with "_".

or maybe creating another repo just with reusable workflows ?

.github/workflows/_reusable_helm.yml Outdated
Comment on lines 16 to 49
lint-chart:
runs-on: [self-hosted, shared]
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-python@v2
with:
python-version: 3.7
- name: Set up chart-testing
uses: helm/chart-testing-action@v2.2.0
- name: Run chart-testing (lint)
run: ct lint --config .github/ct.yaml --check-version-increment=false

lint-docs:
runs-on: [self-hosted, shared]
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Run helm-docs
env:
HELM_DOCS_VERSION: "1.11.0"
HELM_DOCS_BASE_URL: "https://github.com/norwoodj/helm-docs/releases/download"
run: |
OS=$(uname)
# install helm-docs
curl -sSfL "${HELM_DOCS_BASE_URL}/v${HELM_DOCS_VERSION}/helm-docs_${HELM_DOCS_VERSION}_${OS}_x86_64.tar.gz" |
tar -xzf - helm-docs
# validate docs
./helm-docs
git diff --exit-code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those 2 should move to pre-commit, it would avoid waiting for CI to discover issues while it could have been discovered on commit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it could be an interesting action for the future ✅ but for the current stream, it would be very heavy tasks to setup git hooks for every projects.

.github/workflows/_reusable_helm.yml Outdated Show resolved Hide resolved
@slundy-ledger
Copy link
Contributor

slundy-ledger commented Sep 27, 2023 •
edited

Hello, I have similar remark
It would be nice to split the test chart part and the publish chart part.
The test part can be triggered when modifying the repo /helm folder, the publish part triggered when tagging the repo.
Example for the action python-lib, also it is split in many step. Also having an action in the folder also allow adding a nice readme. What do you think ?

@b0l0k
Copy link
Member Author

b0l0k commented Sep 27, 2023

Hello, I have similar remark It would be nice to split the test chart part and the publish chart part. The test part can be triggered when modifying the repo /helm folder, the publish part triggered when tagging the repo. Example for the action python-lib, also it is split in many step. Also having an action in the folder also allow adding a nice readme. What do you think ?

The problem with Action is that it is very limited, and doesn't support property secrets injections. It is one of the reasons that I decided to use reusable workflow.
We could indeed limit the "linting" when the files are modified, but frankly speaking it takes few seconds and could be interesting to catch issues which are not related to direct modifications such as liniting update.

@b0l0k b0l0k force-pushed the add-reusable-workflow-helm branch 12 times, most recently from a5e3af9 to 135d89a Compare September 30, 2023 18:20
@b0l0k b0l0k force-pushed the add-reusable-workflow-helm branch from 135d89a to a550e97 Compare October 6, 2023 08:44
@b0l0k b0l0k force-pushed the add-reusable-workflow-helm branch 5 times, most recently from 40729be to fd20fce Compare October 17, 2023 10:19
@slundy-ledger slundy-ledger mentioned this pull request Nov 8, 2023
Merged
mkaurkovska-ledger
mkaurkovska-ledger reviewed Nov 16, 2023
View reviewed changes
.github/workflows/_helm_publish.yml
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Push chart to chartmuseum-prd repo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chartmuseum-dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noirbizarre noirbizarre noirbizarre left review comments

@mkaurkovska-ledger mkaurkovska-ledger mkaurkovska-ledger left review comments

@ncendrier-ledger ncendrier-ledger Awaiting requested review from ncendrier-ledger ncendrier-ledger is a code owner

@CedricCabessa CedricCabessa Awaiting requested review from CedricCabessa CedricCabessa is a code owner

@katticot-ledger katticot-ledger Awaiting requested review from katticot-ledger katticot-ledger is a code owner

@fbakoura-ledger fbakoura-ledger Awaiting requested review from fbakoura-ledger

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@b0l0k @slundy-ledger @noirbizarre @mkaurkovska-ledger @rlaschuk-ledger @mmejdoub-ledger