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

Adding basic HashR support #140

Merged
merged 11 commits into from
Apr 24, 2024
Merged

Conversation

jkppr
Copy link
Collaborator

@jkppr jkppr commented Apr 23, 2024

Description of the change

This PR adds the first helm chart to support the HashR project in osdfir infrastructure.

  • Uses a postgresql container as storage for HashR results
  • Runs the HashR workload as a CronJob
  • Provides a data manager pod that allows to kubectl cp the necessary data into the storage volume.

Applicable issues

Additional information

This is submitted as a draft PR for initial review before I continue to add charts for other supported HashR processors (e.g. iso9660, GCP, AWS, etc.)

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Newly added variables are documented in the values.yaml
  • Title of the pull request is descriptive

@jkppr jkppr requested a review from wajihyassine April 23, 2024 17:12
@jkppr jkppr self-assigned this Apr 23, 2024
@jkppr jkppr marked this pull request as ready for review April 24, 2024 12:12
Copy link
Member

@wajihyassine wajihyassine left a comment

Choose a reason for hiding this comment

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

LGTM, thank you so much @jkppr !!!

@wajihyassine
Copy link
Member

Oh wait, before merging can you run helm dependency build it should create a Chart.lock file that needs to be merged in

@jkppr jkppr changed the title Adding basic HashR support for deb files Adding basic HashR support Apr 24, 2024
@jkppr jkppr mentioned this pull request Apr 24, 2024
Copy link
Member

@wajihyassine wajihyassine left a comment

Choose a reason for hiding this comment

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

Overall LGTM, left a note about the GCP service account and using workload identity but we can do that in another PR.

- {{ .Values.hashr.importers.gcp.hashr_gcs_bucket | quote }}
env:
- name: GOOGLE_APPLICATION_CREDENTIALS
# Store your SA key in the hashrvolume/creds/ folder via "kubectl cp"!
Copy link
Member

Choose a reason for hiding this comment

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

In the future we should consider using GCP workload identity which is the recommended method. That way no need to export keys and can use the init-gke.sh script to bootstrap the service account.

@wajihyassine wajihyassine merged commit 177b3bf into google:main Apr 24, 2024
1 check passed
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.

2 participants