feat: add reconcile strategy field in spec #748
Conversation
The previous commit left out the statefulset volumes and volume mounts
This adds an option for the sidecars that are templated by Amalthea to be versioned. Also it combines all the executables that run in the sidecars that Amalthea adds to live in a single executable.
|
This change is part of the following stack: Change managed by git-spice. |
| @@ -65,6 +65,8 @@ type AmaltheaSessionSpec struct { | |||
|
|
|||
| // +optional | |||
| // Additional volumes to include in the statefulset for a session | |||
| // Volumes used internally by amalthea are all prefixed with 'amalthea-' so as long as you | |||
| // avoid that naming you will avoid conflicts with the volumes that amalthea generates. | |||
There was a problem hiding this comment.
Should we have a validator that ensure it does not happen ?
There was a problem hiding this comment.
@sgaist to do this properly we need a validating webhook. Which is a whole other service that requires its own tls certs and a few other things. For now we can rely on the check that already happens in the statefulset for this. I will open another PR where we need to properly surface a bunch of errors that may occur in child resource in the status of amalthea. I will address this there. And for the validating webhook I will open another issue.
| // The status of the session and all hibernation or auto-cleanup functionality will always work as expected. | ||
| // A few values are possible: | ||
| // - never: Amalthea will never update any of the child resources and will ignore any changes to the CR | ||
| // - always: This is the expected method of operation for an operator, changes to the spec are always reconciled |
There was a problem hiding this comment.
Is there a way to notify the user that the session needs to be restarted ? Otherwise we will have the issue of work getting interrupted at the least good moment.
There was a problem hiding this comment.
@sgaist we will most likely not be running Amalthea with the always strategy in renku. I am not sure who you are referring to by "user" because the only "user" we can reliably notify from Amalthea is the admin/service that created the session in k8s. And we can do this via the status field in the CR. But in the case of always, notifying the user that the session needs to be restarted makes no sense. The whole point of this strategy is that things will just happen immediately without action from the user or admin.
We can add a notification that the session will be updated with the whenHibernatedOrFailed strategy. So that users and admins know that the session is out of date and will be updated when the session is hibernated.
We can do a similar thing for the never strategy.
With always what we can try to do is add something that indicates that the session has been updated.
I will do this in a follow up PR.
olevski
force-pushed
the
feat-improve-reconciliation
branch
from
8968307 to
da54b8c
Compare
feat: add reconcile strategy
chore: cleanup tests