Pod isn't restarted when configmap is updated

[jpkrohling]

When the collector's configuration has changed, the pods that are using the respective configmap should be restarted, and it currently isn't.

[bhiravabhatla]

Hey @jpkrohling ,

I would like to work on this. I have gone through the code where we are handling updates to the configmap. I would like to understand couple things.

• Any specific reason we are using Client.Patch instead of Client.Update?

opentelemetry-operator/pkg/collector/reconcile/configmap.go

Line 124 in 16ca4cb

if err := params.Client.Patch(ctx, updated, patch); err != nil {

• Also, I see that we generate the patch for configmap from &params.Instance ( which of type v1alpha1.OpenTelemetryCollector).

opentelemetry-operator/pkg/collector/reconcile/configmap.go

Line 122 in 16ca4cb

patch := client.MergeFrom(&params.Instance)

Dint get the use of this.

For restarting pod on configmap change, I guess we should do following -

• Have a function which could compare the existing and desired configmaps (preferably data) and returns true - if they are different.
• If true, use existing logic to update configmap. Additionally get the pod currently running for operator deployment and delete it. New pod should come up with new config.

Thoughts @jpkrohling ?

[jpkrohling]

Thank you for helping out!

Any specific reason we are using Client.Patch instead of Client.Update?

Patches are less likely to get in conflict with full object updates. See #54 for more info.

Additionally get the pod currently running for operator deployment and delete it. New pod should come up with new config.

Do you have any references on what's the recommendation on Kubernetes' documentation? A rolling update would be my preference, so that pod using the new configuration will have to first get into a running state before they are killed, preventing bad configs from causing an outage of the collector.

[bhiravabhatla]

I was coming from the thought process behind the configmap reloaders out there. For example - https://github.com/stakater/Reloader. This actually does the same, kills the pod when config changes.

But as you said, as deployment itself is created/updated by controller, we could actually do a rolling update - which is better than simply killing the pod. I agree.

To achieve this

• can we may be add/update an annotation in deployment spec with checksum of configmap. If configmap changes, a new rollout will happen.

• Or we can somehow do rollout restart from controller (https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#-em-restart-em-) when configmap is updated

Second point I am not sure how to implement using controller-runtime.

[jpkrohling]

I might need to look into more details, but apparently, the rollout restart command does something similar to the first option:

https://github.com/kubernetes/kubectl/blob/ddb56dde55b6b8de6eba1efbd1d435bed7b40ff4/pkg/cmd/rollout/rollout_restart.go#L159

https://github.com/kubernetes/kubectl/blob/ddb56dde55b6b8de6eba1efbd1d435bed7b40ff4/pkg/polymorphichelpers/interface.go#L109-L110

https://github.com/kubernetes/kubectl/blob/ddb56dde55b6b8de6eba1efbd1d435bed7b40ff4/pkg/polymorphichelpers/objectrestarter.go#L41

[bhiravabhatla]

@jpkrohling , went through above code links, as you said - its doing the same - annotating with timestamp.
Shall I go ahead with first option then. Generate the checksum of configmap and annotate the deployment with same?

[jpkrohling]

Sounds worth a try!