VictoriaMetrics / operator

Kubernetes operator for Victoria Metrics
Apache License 2.0
403 stars 141 forks source link

Broken VMAlert deployment if `VMAlert.spec.extraArgs."notifier.config"` is set #990

Closed tamcore closed 1 day ago

tamcore commented 1 week ago

Since https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2690 we deploy our VMAlert with

apiVersion: operator.victoriametrics.com/v1beta1
kind: VMAlert
spec:
  ....
  extraArgs:
    notifier.config: /etc/vm/secrets/victoria-metrics-agent-vmalert-notifiers/notifier.yaml

and during our upgrade attempt from v1.93.15 LTS to v1.101.0 VMAlert failed on us with failed to init: failed to init notifier: only one of -notifier.config or -notifier.url flags must be specified, since even though we don't define notifier.url, the operator still enforces the parameter.

From the resulting deployment:

        - -httpListenAddr=:8080
        - -notifier.config=/etc/vm/secrets/victoria-metrics-agent-vmalert-notifiers/notifier.yaml
        - -notifier.url=
        - -remoteRead.basicAuth.passwordFile=/etc/vmalert/remote_secrets/REMOTEREAD_BASICAUTHPASSWORD

Removing -notifier.url= from the container's args, seems to work just fine.

tamcore commented 1 week ago

Okay, after looking through the code a bit more, migrating to VMAlert.spec.notifierConfigRef fixes this. Nonetheless, this case should probably be caught :)

Haleygo commented 1 week ago

Hi, yes, -notifier.config should be set in .spec.notifierConfigRef, adding extra check in admission webhook here.

Haleygo commented 1 day ago

The extra check is included in v0.46.1, close as completed. Feel free to reopen if there are further questions.