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

Add Helm chart for CDash #2700

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zackgalbreath
Copy link
Contributor

No description provided.

Co-authored-by: William Allen <[email protected]>
@zackgalbreath zackgalbreath marked this pull request as draft February 5, 2025 19:45
Copy link
Collaborator

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

It would be good to do some basic testing for this chart eventually. I don't necessarily mind that being left for a future PR. In addition to testing, I think it would be worthwhile to run helm lint to catch basic issues and enforce Helm's preferred style rules.

# on the CDash webserver instead, you should specify:
# REMOTE_WORKERS: "true"
env: |-
#REMOTE_WORKERS: "true"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We talked about it earlier, but I don't think we should support remote workers here. S3 is recommended, and we provide minio support out of the box for anyone who doesn't have an S3 compatible service available. I'm open to other ideas if you feel strongly that we should support remote workers though.

args: [ "start-website" ]
env:
- name: "APP_URL"
value: "https://{{ .Values.website.url }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the protocol guaranteed to be HTTPS?

Comment on lines +8 to +12
DB_DATABASE: "cdash"
DB_PORT: "5432"
DB_HOST: "{{ .Release.Name }}-postgresql"
DB_CONNECTION: "pgsql"
DB_LOGIN: "postgres"
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we discussed in person a few minutes ago, we should add a check here to conditionally set these values based on whether an external database is being used or not.

# REMOTE_WORKERS: "true"
env: |-
#REMOTE_WORKERS: "true"
#BACKUP_TIMEFRAME: 36
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is BACKUP_TIMEFRAME useful here?

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