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

feat: Helm charts created #35

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Helion55
Copy link

About this PR

This PR will add Helm Charts for the application. Deployment, Service, and Ingress manifests are created. The values.yaml file contains all the values for the manifests. NOTES.txt I have left empty because I feel maintainers can describe or change any stuff as required and this will be much more convenient to the end user. 
In the Ingress, I have configured it to use with SSL certificates, and the required values can be filled during the deployment; otherwise, it can be ignored, and the service can be exposed in the classic way. 

Note

  1. Deployment, Service, and Ingress manifests are created.
  2. values.yaml file have four sections: Common Values, Deployment Values, Service Values, and Ingress Values.
  3. Environment variables for deployment are filled using the docker-compose.yaml file.
  4. Notes.txt is fully customizable.
  5.  Not used the Charts folder as there are no dependencies for this helm deployment.

Feature Added

Solved #32 

Signed-off-by: Rakesh Bajpayee <[email protected]>
@jleach jleach changed the title Helm charts created feat: Helm charts created Jan 16, 2025
Signed-off-by: Rakesh Bajpayee <[email protected]>
Signed-off-by: Rakesh Bajpayee <[email protected]>
Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for this! Maybe you could add some simple docs on how to use this?

And maybe @jleach should also take a look? I'm not experienced in kubernetes setup

Comment on lines +2 to +3
name: helm
description: A Helm chart for Kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we give these mediator specific names?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I will write the docs with the helm commands to install the application.

For the name: and description: I have left it default, but we can change that any time. Also suggest to me any note we should display after the helm deployment; I will write that too.

Comment on lines +19 to +28
AGENT_NAME: Mediator
WALLET_NAME: mediator
WALLET_KEY: ${WALLET_KEY}
POSTGRES_USER: ${POSTGRES_USER}
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
POSTGRES_HOST: ${POSTGRES_HOST}
POSTGRES_ADMIN_USER: ${POSTGRES_ADMIN_USER}
POSTGRES_ADMIN_PASSWORD: ${POSTGRES_ADMIN_PASSWORD}
AGENT_ENDPOINTS: "https://my-mediator.com,wss://my-mediator.com"
LOG_LEVEL: 2
Copy link
Contributor

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 be able to override all values. (Also wallet_name)

Copy link
Author

Choose a reason for hiding this comment

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

I have written the values from the docker-compose.yaml file, it should be left with only "{}" this.
Like,
AGENT_NAME: {}
WALLET_NAME: {}
...
...

For reference I have done that. You can override the values as needed or can set the values with helm --set flag during helm install. You can refer to this https://helm.sh/docs/helm/helm_install/

Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want these to be overridable also:

  AGENT_ENDPOINTS: "https://my-mediator.com,wss://my-mediator.com"

@Helion55 Helion55 requested a review from TimoGlastra January 19, 2025 07:52
helm/README.md Outdated Show resolved Hide resolved
Co-authored-by: Timo Glastra <[email protected]>
Signed-off-by: Rakesh Bajpayee <[email protected]>
@jleach
Copy link
Contributor

jleach commented Jan 20, 2025

@Helion55 We can start with this although we'll need to make some notable changes. I could list 'em off but I think this would turn into an epic. We can either keep this alive and keep chipping away at it or merge and I can start contributing to the helm charts also. Thoughts @TimoGlastra ?

@Helion55 Helion55 closed this Jan 21, 2025
@Helion55 Helion55 reopened this Jan 21, 2025
@Helion55 Helion55 closed this Jan 21, 2025
@Helion55 Helion55 reopened this Jan 21, 2025
Signed-off-by: Rakesh Bajpayee <[email protected]>
- Quick Note
- Helm Commands


Copy link
Author

Choose a reason for hiding this comment

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

Is that fine?

@Helion55
Copy link
Author

If other k8s objects are required, I will work on them. So @jleach, if you kindly mention some of the tasks on which I can work, please let me know; this way the whole Helm Chart will be completed soon.

imagePullPolicy: Always
ports:
- containerPort: {{ .Values.container.port }}
env:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can add the existing health check. Something like:

          livenessProbe:
            httpGet:
              path: /health
              port: http
            initialDelaySeconds: 45
            periodSeconds: 3
          readinessProbe:
            httpGet:
              path: /health
              port: http
            initialDelaySeconds: 45
            timeoutSeconds: 3

value: "{{ .Values.environment.AGENT_NAME }}"
- name: WALLET_NAME
value: "{{ .Values.environment.WALLET_NAME }}"
- name: WALLET_KEY
Copy link
Contributor

Choose a reason for hiding this comment

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

Wallet key, postgres password, are better stored in secrets. Let's move them and reverence them accordingly:

          envFrom:
            - secretRef:
                name: {{ include "mediator-agent.fullname" . }}-postgresql-creds
            - secretRef:
                name: {{ include "mediator-agent.fullname" . }}-creds

containers:
- name: {{ .Values.container.name }}
image: " {{ .Values.image.name }}:{{ .Values.image.tag }} "
imagePullPolicy: Always
Copy link
Contributor

@jleach jleach Jan 21, 2025

Choose a reason for hiding this comment

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

I think we should add the ability to control resource usage. Able to add that to the deployment?

          resources:
            {{- toYaml .Values.resources | nindent 12 }}

Then add values_dev.yml with the following:

resources:
  requests:
    memory: 256Mi
    cpu: 20m
  limits:
    memory: 256Mi
    cpu: 100m

This way we add different values for dev, test, stage, prod. Whatever different teams want to use.

containers:
- name: {{ .Values.container.name }}
image: " {{ .Values.image.name }}:{{ .Values.image.tag }} "
imagePullPolicy: Always
Copy link
Contributor

Choose a reason for hiding this comment

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

Always is the default. The best practices is to not put in default values. Let's remove Always.

spec:
containers:
- name: {{ .Values.container.name }}
image: " {{ .Values.image.name }}:{{ .Values.image.tag }} "
Copy link
Contributor

Choose a reason for hiding this comment

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

The mediator needs a caches directory, I'm not 100% sure why but it does. Let's add that as an emptyDir:

      volumes:
        - name: cache-volume
          emptyDir:
            sizeLimit: 1Gi

and

          volumeMounts:
            - name: cache-volume
              mountPath: /.cache

Copy link
Contributor

Choose a reason for hiding this comment

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

I might add an example of how a secret should be created to work with this helm chart. Not sure what to do with it. We only want it to run when a chart is installed, not updated so for now it can just live in notes unless you know a good way to do this:

---
apiVersion: v1
kind: Secret
metadata:
  name: {{ include "mediator-agent.fullname" . }}-creds
  labels: {{- include "mediator-agent.labels" . | nindent 4}}
type: Opaque
data:
  WALLET_NAME: {{.Values.wallet_user | default "mediator" | b64enc}} 
  WALLET_KEY: {{.Values.wallet_key | default (randAlphaNum 16) | b64enc}} 

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