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: single sidecars exe and versioned templates #745

Merged
merged 17 commits into from
Oct 23, 2024
Merged

Conversation

olevski
Copy link
Member

@olevski olevski commented Oct 9, 2024

There are a few things that happen here:

  • combine utilities is a single executable that contains the proxy and cloning utilities we run in sidecars
  • enable templating of the cloning and auth proxy containers/sidecars that is based on the version of the image passed in the CRD

This later part about "versioned templating" is required to prevent the following scenario:

  • we will publish the sidecar image with every release
  • the sidecar image is hardcoded as a const in the code
  • someone updates this hardcoded constant
  • redeploys amalthea on a cluster that already has many sessions
  • amalthea restarts and patches all existing images to use the new sidecar image
  • this causes all existing running sessions in the cluster to restart

There are other reasons too. For example we upgrade the version of the sidecar image but its api has changed. Then either new sessions with the new sidecar work or the old ones are patched restarted but now when they start back up they keep crashing because the sidecars image API has changed.

@olevski olevski requested review from a team as code owners October 9, 2024 12:11
@olevski olevski marked this pull request as draft October 9, 2024 12:14
@olevski olevski force-pushed the chore-combine-modules branch 3 times, most recently from 778f090 to 0326146 Compare October 9, 2024 13:14
@olevski olevski marked this pull request as ready for review October 9, 2024 13:19
@olevski olevski force-pushed the chore-combine-modules branch from 0326146 to 053b757 Compare October 9, 2024 13:23
Copy link
Collaborator

@sgaist sgaist left a comment

Choose a reason for hiding this comment

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

It's looking good however I am just not sure about what "template" refers to in this context.

There's the fact that these are template functions but in the k8s context, some resources such as deployment uses template in the sense of "reference pod declaration to be used when creating pods".

@olevski
Copy link
Member Author

olevski commented Oct 10, 2024

It's looking good however I am just not sure about what "template" refers to in this context.

There's the fact that these are template functions but in the k8s context, some resources such as deployment uses template in the sense of "reference pod declaration to be used when creating pods".

In this case "template" means the real k8s children resources I generate from the CR specification. I.e. the PVC, Ingress, Service, Statefulset, etc.

@olevski olevski requested a review from sgaist October 10, 2024 15:21
@olevski
Copy link
Member Author

olevski commented Oct 18, 2024

This change is part of the following stack:

Change managed by git-spice.

@olevski olevski merged commit 9494c23 into main Oct 23, 2024
17 checks passed
@olevski olevski deleted the chore-combine-modules branch October 23, 2024 11:30
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