alexandrevilain / temporal-operator

Temporal Kubernetes Operator
https://temporal-operator.pages.dev/
Apache License 2.0
160 stars 35 forks source link

Opt-in to tolerate ad-hoc modification in resources #418

Open yujunz opened 1 year ago

yujunz commented 1 year ago

During development, we sometimes want to make ad-hoc changes to temporal instances, e.g. adding an environment variable. The current behavior of the operator is reconciling it to the generated manifests which will reset such changes.

It would be nice to tolerate ad-hoc changes, particularly when it does not conflict with CRD. The feature can be opt-in for development and debugging purpose.

alexandrevilain commented 1 year ago

Hi! The operator role is to always ensure the current state matches the desired state. I personally don't know any operator providing such features, do you know one ? I have no idea how to implement such pattern. Maybe it could be something like clusterAPI with paused clusters.

Adding custom env vars using overrides is not enough for your use case ?

yujunz commented 1 year ago

Maybe not be a perfect example, but argocd app controller does not replace resources by default. It applies the desired manifest as a patch and does not reset the unmanaged fields. There is an explicit option to replace resources if needed.

I'm looking for similar behavior in the operator so that we can have a workaround when some features are not supported yet, e.g. archival support mentioned in #364

alexandrevilain commented 1 year ago

ArgoCD and this operator have completely different behavior for resource management and diff generation.

I have to double check how ArgoCD does this to decide if it can be implemented in this operator and if it's not an anti-pattern.

About the archival feature, I planned it for 0.15.0, I'm about to release 0.14.0 in the next days.

yujunz commented 1 year ago

Maybe it is related to https://kubernetes.io/docs/reference/using-api/server-side-apply/

A fully specified intent is a partial object that only includes the fields and values for which the user has an opinion. That intent either creates a new object or is combined, by the server, with the existing object.

Consider the operator as a kind of user aforementioned.

alexandrevilain commented 1 year ago

SSA could be a good idea !

Looks like server-side apply is not ready yet in controller-runtime see https://github.com/kubernetes-sigs/kubebuilder/issues/2514

Let's mark this issue "on hold" and I'll come back when controller-runtime will provide a clean and easy way to support SSA 🙂

yujunz commented 1 year ago

Digging https://github.com/kubernetes-sigs/kubebuilder/issues/2514 reveals two threads to follow.

yujunz commented 1 year ago

Is the current update logic here? https://github.com/alexandrevilain/controller-tools/blob/dddd43a16d33104cd69a71eeff36a3b0414b14fd/pkg/reconciler/builders_reconciler.go#L156

alexandrevilain commented 1 year ago

@yujunz ! Yes, you're right :)