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

Rancher Fleet support #1255

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Rancher Fleet support #1255

merged 1 commit into from
Sep 7, 2023

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Sep 4, 2023

Rancher fleet installs applications from the existing Helm Chart. During installation it injects at least properties into global.fleet. Because k8gb schema is too strict, helm install does not work because fleet injects its properties We solved this by deleting the schema and rebuilding Chart.

This PR whitelists the fleet object in globals, so k8gb can be installed directly.

k0da
k0da previously approved these changes Sep 4, 2023
Copy link
Collaborator

@k0da k0da left a comment

Choose a reason for hiding this comment

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

LGTM

@k0da
Copy link
Collaborator

k0da commented Sep 4, 2023

@ytsarev @jkremser whats your take on this?

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

allowing totally unrelated value into the schema does not feel right. How exactly does it work with Fleet? Does it mutate the chart?

@kuritka
Copy link
Collaborator Author

kuritka commented Sep 5, 2023

Hi @ytsarev, good question: currently we deal with clusterLabels map injected by fleet, which injects its own values into values.yaml before installing the helmchart.

# values.yaml
global:
  fleet:
    clusterLabels:
      k8gb: "true"
      k8gb-ClusterGeoTag: "us"
      k8gb-ExtClusterGeoTags: "eu"
    ... 
k8gb:
  image: ...
  tag: ...
...

In fact, I don't know exactly what else fleet is able to inject in different edge-cases, so I provided it with an object. Outside of this bounded global.fleet: {} object, fleet can't do anything.

@k0da
Copy link
Collaborator

k0da commented Sep 5, 2023

@ytsarev this is a change to a schema to allow extra values to be added. It doesn't affect helm chart itself, just validation is aware of such case. And enables k8gb to be rollout cross quite huge fleet of the clusters

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

ok, thanks for the clarification. Can we extend the documentation mentioning the Rancher Fleet support, please? It would be great to make people immediately aware of the integration.

@k0da
Copy link
Collaborator

k0da commented Sep 5, 2023

@ytsarev this is not something user controlls. This is internal usage of those fields. A structure evaluated prior helm call and processes helm values based on this. Take it as internal passthrough storage for particular cluster.

without it. Helm just fails to validate values, as schema didn't allow extra values to be passed in.

@ytsarev
Copy link
Member

ytsarev commented Sep 5, 2023

@k0da the statement that k8gb was tested and helm chart adapted for Rancher fleet setup will be anyway useful for users and for the project

@netlify
Copy link

netlify bot commented Sep 6, 2023

Deploy Preview for k8gb-preview ready!

Name Link
🔨 Latest commit dfde856
🔍 Latest deploy log https://app.netlify.com/sites/k8gb-preview/deploys/64f97b3f2a7890000890c805
😎 Deploy Preview https://deploy-preview-1255--k8gb-preview.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k0da
Copy link
Collaborator

k0da commented Sep 6, 2023

👍

k0da
k0da previously approved these changes Sep 6, 2023
@kuritka
Copy link
Collaborator Author

kuritka commented Sep 6, 2023

@k0da , @ytsarev - Hi, I extended with simple doc

k0da
k0da previously approved these changes Sep 6, 2023
Copy link
Member

Choose a reason for hiding this comment

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

this is great. Can we link this manual in the main README, probably extending https://www.k8gb.io/#installation-and-configuration-tutorials ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ytsarev , fixed, thx.

Rancher fleet installs applications from the existing Helm Chart. During installation it injects at least properties into `global.fleet`.
Because k8gb schema is too strict, `helm install` does not work because fleet injects its properties We solved this by deleting the schema and rebuilding Chart.

This PR whitelists the fleet object in globals, so k8gb can be installed directly.

Signed-off-by: Michal Kuritka <[email protected]>
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

looks great, thanks for amending the docs 👍

@kuritka kuritka merged commit a882e03 into master Sep 7, 2023
21 checks passed
@kuritka kuritka deleted the fleet-support branch September 7, 2023 10:31
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