VictoriaMetrics / operator

Kubernetes operator for Victoria Metrics
Apache License 2.0
430 stars 144 forks source link

Auto reloading on vminsert configuration change #927

Open leoskyrocker opened 6 months ago

leoskyrocker commented 6 months ago

Currently, the operator is not able to configure the vminsert component to reload new configuration mounted onto the vminsert pods.

Problem

As a result, we have to manually restart the deployment every time a mounted configmap is updated.

Desired behavior

On update of our custom configmap mounted onto vminsert, the operator should be able to either cause vminsert to restart, or somehow hot reload the new configuration.

Workarounds

I have not discovered a reliable workaround at this point.

Option 1 - didn't work

There has been a suggested workaround to install the config-reloader sidecar along with vminsert. However, after some testing[1] and reading up on the raised issue, it turns out that this is not supported due to vminsert not having a reload endpoint.

[1] - I got back an error in the config-reloader from vminsert:

cannot trigger api reload: unexpected status code: 400 for reload api request

Option 2 - almost worked

I tried using a 3rd party solution, i.e. Stakater/reloader, however it requires annotating the vminsert deployment with the annotation reloader.stakater.com/auto: "true". Since we deploy vminsert via thre VMCluster resource, I have not found a way to annotate just the vminsert deployment (without annotating anything else).

Links victoria metrics slack discussion: https://victoriametrics.slack.com/archives/CTC488T1P/p1713174755111179

Haleygo commented 6 months ago

Hi @leoskyrocker, thanks for the detailed issue! About second option, you can add the annotation only for vminsert pod using VMClusterSpec.VMInsert.podMetadata.annotations, like:

  spec:
    replicationFactor: 1
    retentionPeriod: 14d
    vminsert:
      podMetadata:
        annotations:
          reloader.stakater.com/auto: "true"

And for option1, I think it's also rational, I will push it to see if it can be merged.

Update: sorry, I missed the point that the annotation should be added to the deployment instead of pod, then operator needs to add extra fields for vminsert, might be ok but sounds a bit redundant. Let's wait a while to see if /-/reload can be added to vminsert.

Haleygo commented 5 months ago

Hey @leoskyrocker , jfyi, https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3923 has been merged and will be included in the next release, so Option 1 is back to game)