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

COSI-33: Add Helm Charts and Update Release Stage to release helm package #13

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

anurag4DSB
Copy link
Collaborator

@anurag4DSB anurag4DSB commented Nov 13, 2024

Note to reviewers: Tests for helm coming in next PR
Sample release: 0.1.0-PW.11

Generated using co-pilot:

This PR introduces Helm chart packaging and updates the release pipeline to support multi-platform builds and Helm chart distribution. Key changes include:

  1. Added Helm Charts for COSI Driver

    • Introduced a Helm chart for the Scality COSI Driver located in the ./helm/scality-cosi-driver directory.
    • Helm chart is packaged and pushed to GitHub Container Registry (GHCR) as an OCI artifact.

  2. Updated Release Pipeline

    • Modified the release stage to:
    • Support multi-platform builds (linux/amd64, linux/arm64).
    • Default to building for linux/amd64 unless the all_platforms input is set to true.
    • Add a Helm chart packaging step to create and upload the chart to GHCR.

  3. New GitHub Actions Jobs

    • package-helm-chart: This job packages the Helm chart, uploads it as an artifact, and pushes it to GHCR for distribution.
    • create-github-release: Continues to handle release notes and tagging.

Summary:

•	Added Helm chart for the Scality COSI Driver.
•	Updated release pipeline to handle multi-platform builds and Helm chart packaging.
•	Helm chart is now pushed to GHCR for easy installation and distribution.

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.52%. Comparing base (c0f97c6) to head (f5a8ee7).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #13   +/-   ##
=======================================
  Coverage   75.52%   75.52%           
=======================================
  Files           4        4           
  Lines         237      237           
=======================================
  Hits          179      179           
  Misses         51       51           
  Partials        7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anurag4DSB anurag4DSB force-pushed the feature/COSI-33-add-helm-charts branch from 5f37e37 to 7b77327 Compare November 13, 2024 16:47
@anurag4DSB anurag4DSB marked this pull request as ready for review November 13, 2024 16:49
@anurag4DSB anurag4DSB force-pushed the feature/COSI-17-add-dev-container-to-cosi-driver branch from d39c1ba to 1636d3f Compare November 13, 2024 16:58
@anurag4DSB anurag4DSB force-pushed the feature/COSI-33-add-helm-charts branch from 7b77327 to 92f88b5 Compare November 13, 2024 16:58
@anurag4DSB
Copy link
Collaborator Author

rebased and pushed

@anurag4DSB anurag4DSB force-pushed the feature/COSI-33-add-helm-charts branch 3 times, most recently from e6ceba7 to 90a6b03 Compare November 14, 2024 06:51
@fredmnl fredmnl self-requested a review November 14, 2024 08:57
helm/README.md Outdated
Copy link
Collaborator Author

@anurag4DSB anurag4DSB Nov 14, 2024

Choose a reason for hiding this comment

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

We will fill this once we have a release, till then we have install-helm.md
I need to add features supported docs

@anurag4DSB anurag4DSB force-pushed the feature/COSI-17-add-dev-container-to-cosi-driver branch from 1eb8281 to dae6da4 Compare November 14, 2024 16:26
@anurag4DSB anurag4DSB force-pushed the feature/COSI-33-add-helm-charts branch from 90a6b03 to f00a568 Compare November 14, 2024 16:27
@anurag4DSB
Copy link
Collaborator Author

rebased as merging other PR's review comments.

@anurag4DSB anurag4DSB force-pushed the feature/COSI-17-add-dev-container-to-cosi-driver branch from dae6da4 to c0f97c6 Compare November 14, 2024 16:32
Install command:

```bash
make container

helm install scality-cosi-driver ./helm/scality-cosi-driver \
    --namespace scality-object-storage \
    --create-namespace \
    --set image.tag=latest
```
@anurag4DSB anurag4DSB force-pushed the feature/COSI-33-add-helm-charts branch from f00a568 to b638c74 Compare November 14, 2024 16:33
Base automatically changed from feature/COSI-17-add-dev-container-to-cosi-driver to main November 14, 2024 16:37
Copy link

@fredmnl fredmnl left a comment

Choose a reason for hiding this comment

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

LGTM (M being still a super k8s noob).

We talked in other PRs about the fact that COSI retrieves AK/SK as k8s secrets, I would have expected them to be mentioned in the helm chart, why am I wrong?

run: helm show all oci://ghcr.io/${{ github.repository }}/helm-charts/scality-cosi-driver --version ${{ inputs.tag }}

- name: Template the helm chart
run: helm template oci://ghcr.io/${{ github.repository }}/helm-charts/scality-cosi-driver --version ${{ inputs.tag }}
Copy link

Choose a reason for hiding this comment

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

Aren't show and template kind of redundant? (Since template also shows the chart)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They actually serve different purpose

  • helm template command renders the Helm chart templates into Kubernetes manifest files (YAML), without actually installing the chart in a Kubernetes cluster.
  • Test: ensure that the templates are correctly rendered into valid Kubernetes manifests.
  • helm show command displays information about a chart, such as its metadata, values, or dependencies. It doesn’t render the chart into manifests like helm template but instead provides insight into the structure of the chart.
  • Test: show to check chart details (metadata, values).

helm template is focused on rendering and validating the Kubernetes manifests, which is the primary output of a Helm chart. helm show, on the other hand, is about inspecting the chart’s metadata and configuration options. One would need both to ensure the chart is both well-formed (via helm show) and generates the right manifests (via helm template).

@anurag4DSB
Copy link
Collaborator Author

anurag4DSB commented Nov 14, 2024

LGTM (M being still a super k8s noob).

We talked in other PRs about the fact that COSI retrieves AK/SK as k8s secrets, I would have expected them to be mentioned in the helm chart, why am I wrong?

We can further enhance cosi driver by adding that option, and limit one COSI driver to 1 AK/SK, I am open to that.

@anurag4DSB anurag4DSB merged commit 3410ee2 into main Nov 14, 2024
7 checks passed
@anurag4DSB anurag4DSB deleted the feature/COSI-33-add-helm-charts branch November 14, 2024 18:23
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