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

Draft: Separate Helm Charts #92

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Conversation

reedjosh
Copy link

@reedjosh reedjosh commented Dec 30, 2023

Breaks the helm charts out into individual charts with a superchart pattern.

generally by pulling this branch down it can be used in the same manner it always was via:

helm template k8s-mediaserver ./

When changes are made to the sub charts the changes can be pulled in via:

helm dep update

and in the future we can add a GitHub pages helm index for the charts and reference them via that.

@reedjosh
Copy link
Author

@kubealex I'm leaving this in draft state atm, but I'd love if you could do a brief review to make sure you like the direction and to get your overall feedback.

- name: k8s-jackett
version: ^0.9.0
repository: "file://./jackett"
condition: jackett.enabled
Copy link
Author

Choose a reason for hiding this comment

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

The enabled flags move out of values and into the deps section.

### CONFIGMAPS
## INIT-CONTAINER
apiVersion: v1
data:
ServerConfig.json: |
{
"BasePathOverride": "{{ .Values.jackett.ingress.path }}"
Copy link
Author

Choose a reason for hiding this comment

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

Each chart being self contained means qualified paths like this one don't need the .jackett. portion anymore.

@@ -1,12 +1,10 @@
{{ if .Values.jackett.enabled }}
Copy link
Author

Choose a reason for hiding this comment

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

Get this for free from the superchart structure.

@@ -25,8 +23,8 @@ kind: ConfigMap
metadata:
name: jackett-config
data:
PGID: "{{ .Values.general.pgid }}"
PUID: "{{ .Values.general.puid }}"
PGID: "{{ .Values.global.pgid }}"
Copy link
Author

Choose a reason for hiding this comment

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

global is a helm special keyword here, and it achieves what general used to in the old structure.

Copy link
Author

Choose a reason for hiding this comment

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

Probably would be good to unify this file instead of copying it so many times. I'll try to look into that soon.

@reedjosh
Copy link
Author

@kubealex been on hold for the Jellyfin PR #72 for a month now.

Want me to just toss this all together?

Think with quick PR reviews I could get this PR done with Jellyfin within a week.

I'll start the work either way. 🙂 Probably have an update by tomorrow.

@reedjosh
Copy link
Author

image

Got a few minutes not buried under kiddos. Had to modify my current fork slightly, but back into business with Jellyfin.

Will try to clean this up further, and prove that it gens the same stuff. Hopefully then we can just merge this PR all at once to get Jellyfin and individual helm charts.

@kubealex
Copy link
Owner

kubealex commented Feb 8, 2024

Jellyfin has been merged @reedjosh, can you double check if this in any way creates any clash?

@reedjosh
Copy link
Author

reedjosh commented Feb 8, 2024

I will begin cleanup on this branch. Hopefully I'll have a non-draft PR within the next week. : )

@NigelVanHattum
Copy link

@reedjosh Any idea how much cleanup you still want to do? Anywhere I can assist?

@reedjosh
Copy link
Author

@NigelVanHattum I'll try to get back to this sometime soon.

Unfortunately, it will need quite a bit still as the upstream changes will need incorporating and my PR was still not polished even before that.

I do still want this done though, so I'll try to get to it in the next week or so.

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.

None yet

3 participants