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 #227

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Add helm chart #227

merged 1 commit into from
Dec 9, 2022

Conversation

waammar
Copy link
Collaborator

@waammar waammar commented Oct 28, 2022

Purpose

Add Helm chart for Ralph

Description

Created the chart under src/helm I tried to migrate the logic from the .j2 tray files.
The version label was replaced as the helm convention for app.kubernetes.io/version
The checksum from the secret name was deleted. The annotation checksum/config: was used instead on the secret, the checksum is automatically generated from vault.yaml it help to update the secret object every time a value is updated.
Note that the .Chart.AppVersion is better used to track the version deployed and it should reflect the image.tag
There is only one vars files values.yaml the vault is under specific file vault.yaml used only for generating the secrets.

Questions

  • Why do you have two service with the same config? the only difference is the label static
  • What is deployment_stamp ?
  • What is the purpose of prefix ?
  • How would you like to manage env.?

ToDo

  • Add README.md

You can use helm template . under ./src/helm/ralph to check the output of the chart, it will show you the final manifest that will be deployed.

@jmaupetit
Copy link
Contributor

Why do you have two service with the same config? the only difference is the label static

To make a long story short: to be able to point to a service that has a fixed name (e.g. without blue-green suffix).
Context: openfun/arnold#427

What is deployment_stamp ?

Our way to distinguish stacks (previous/current/next) in a blue-green deployment strategy. Our playbooks use this selector.

What is the purpose of prefix ?

Prefix for ingresses?

How would you like to manage env.?

We want to use the most relevant/recommended community tools. What do you suggest?

@jmaupetit jmaupetit self-requested a review October 31, 2022 15:30
@jmaupetit jmaupetit assigned waammar and unassigned jmaupetit Oct 31, 2022
@waammar
Copy link
Collaborator Author

waammar commented Oct 31, 2022

Why do you have two service with the same config? the only difference is the label static

To make a long story short: to be able to point to a service that has a fixed name (e.g. without blue-green suffix). Context: openfun/arnold#427

Do you want to keep this strategy? you can still have one service, (one for each blue and green version) and add two host in the ingress for the last working (static) version with a hostname without the suffix blue/green

What is deployment_stamp ?

Our way to distinguish stacks (previous/current/next) in a blue-green deployment strategy. Our playbooks use this selector.

I assume you want to preserve this mechanism? so I need to implement this label on the chart

What is the purpose of prefix ?

Prefix for ingresses?

Yes, I saw it on ingress and services I assume now that the value will be either blue or green?

How would you like to manage env.?

We want to use the most relevant/recommended community tools. What do you suggest?

The convention is to start with one values.yaml file that contains all the default values and config, and add one values file by env. that only change the related configuration. You must then add the correct values file to your deployment argument, in next iteration we may think of using Helmfile tool, that may improve a bit the deploy process to make it more simple, in the long run, it is better to automate the deploy with CD tool like ArgoCD

@jmaupetit
Copy link
Contributor

Why do you have two service with the same config? the only difference is the label static

To make a long story short: to be able to point to a service that has a fixed name (e.g. without blue-green suffix). Context: openfun/arnold#427

Do you want to keep this strategy? you can still have one service, (one for each blue and green version) and add two host in the ingress for the last working (static) version with a hostname without the suffix blue/green

No we don't want to keep Arnold-related strategy in this new standard implementation. From what we understand, flux/flagger can handle the blue-green deployment strategy without having to manage this as we did. As a consequence, only one version of each service is required: the static-one.

What is deployment_stamp ?

Our way to distinguish stacks (previous/current/next) in a blue-green deployment strategy. Our playbooks use this selector.

I assume you want to preserve this mechanism? so I need to implement this label on the chart

Nope.

What is the purpose of prefix ?

Prefix for ingresses?

Yes, I saw it on ingress and services I assume now that the value will be either blue or green?

This is a URL prefix, e.g. previous.my-service.com or next.my-service.com

How would you like to manage env.?

We want to use the most relevant/recommended community tools. What do you suggest?

The convention is to start with one values.yaml file that contains all the default values and config, and add one values file by env. that only change the related configuration. You must then add the correct values file to your deployment argument, in next iteration we may think of using Helmfile tool, that may improve a bit the deploy process to make it more simple, in the long run, it is better to automate the deploy with CD tool like ArgoCD

Duly noted. ArgoCD is in the scope, but for now you don't have to handle this complexity in the current implementation.

@waammar waammar force-pushed the add-helm-chart branch 2 times, most recently from f4086c4 to 798ce65 Compare November 5, 2022 10:32
@waammar waammar marked this pull request as ready for review November 5, 2022 10:33
@waammar
Copy link
Collaborator Author

waammar commented Nov 5, 2022

@jmaupetit Can you please make a full review of this PR? I made the final clean up.

@waammar waammar removed the WIP label Nov 5, 2022
@waammar waammar force-pushed the add-helm-chart branch 2 times, most recently from 44a300f to d6eb141 Compare November 5, 2022 10:46
Copy link
Contributor

@jmaupetit jmaupetit left a comment

Choose a reason for hiding this comment

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

Great! I'll test it by the end of the day and give you more feedback.

@waammar
Copy link
Collaborator Author

waammar commented Nov 7, 2022

I fixed some issues, but I still have one related to the auth.json of LRS, the yaml to json conversion is failing, it is also related to the complex logic here, maybe we should discuss the design of this solution

@waammar waammar force-pushed the add-helm-chart branch 2 times, most recently from 2ea773b to e365bd9 Compare November 8, 2022 20:50
Comment on lines +61 to +62
- name: RALPH_BACKENDS__DATABASE__ES__CLIENT_OPTIONS__ca_certs
value: "/usr/local/share/ca-certificates/es-cluster.pem"
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned yesterday, this can be improved thanks to a ConfigMap (looping over a values.yml file configuration) injected in the environment.

Copy link
Contributor

@jmaupetit jmaupetit left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @waammar!

We already deploy Ralph on Kubernetes using Arnold and a dedicated tray,
but we need to provide more standard options for our users.

This Chart is still experimental. Expect things to change in a near
future.
@jmaupetit jmaupetit merged commit 408bc30 into master Dec 9, 2022
@jmaupetit jmaupetit deleted the add-helm-chart branch December 9, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants