Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vmalertmanager: fix extraArgs, add two dashes #503

Merged
merged 2 commits into from Jul 22, 2022

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Jul 22, 2022

addExtraArgsOverrideDefaults: add dashes argument

Set it to `-` for all VictoriaMetrics processes, and to `--` for
alertmanager.

This fixes ExtraArgs in the VMAlertmanager resource. Previously, their
keys had to start with an extra `-`.

--

api/*/vmalertmanager_types.go: fix description for ExtraArgs, ExtraEnvs

This is not about VMAuth, but VMAlertmanager.

Fixes #502.

@flokli flokli marked this pull request as draft July 22, 2022 04:38
@flokli flokli force-pushed the vmalertmanager-extra-args branch from e0e7c1e to 52c31bf Compare July 22, 2022 04:41
Set it to `-` for all VictoriaMetrics processes, and to `--` for
alertmanager.

This fixes ExtraArgs in the VMAlertmanager resource. Previously, their
keys had to start with an extra `-`.

Fixes VictoriaMetrics#502.
@flokli flokli force-pushed the vmalertmanager-extra-args branch from 52c31bf to e84da67 Compare July 22, 2022 04:47
@flokli flokli marked this pull request as ready for review July 22, 2022 04:48
@flokli
Copy link
Contributor Author

flokli commented Jul 22, 2022

I couldn't get make manifests to work, my controller-gen 0.8.0 failed to run for me:

Error: unable to parse option "crd:trivialVersions=true,crdVersions=v1beta1": [unknown argument "trivialVersions" (at <input>:1:16) extra arguments provided: "true,crdVersions=v1beta1" (at <input>:1:17)]
Usage:
  controller-gen [flags]

I edited the generated CRDs by hand, please re-run and squash any whitespace/reflow changes into this.

@flokli flokli changed the title Vmalertmanager extra args vmalertmanager: fix extraArgs, add two dashes Jul 22, 2022
Copy link
Collaborator

@f41gh7 f41gh7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@f41gh7 f41gh7 merged commit 7f8de4b into VictoriaMetrics:master Jul 22, 2022
@f41gh7
Copy link
Collaborator

f41gh7 commented Jul 22, 2022

Thanks for contribution!

@f41gh7
Copy link
Collaborator

f41gh7 commented Jul 22, 2022

I couldn't get make manifests to work, my controller-gen 0.8.0 failed to run for me:

Error: unable to parse option "crd:trivialVersions=true,crdVersions=v1beta1": [unknown argument "trivialVersions" (at <input>:1:16) extra arguments provided: "true,crdVersions=v1beta1" (at <input>:1:17)]
Usage:
  controller-gen [flags]

I edited the generated CRDs by hand, please re-run and squash any whitespace/reflow changes into this.

I think, all code generation must be moved into docker containers, as VictoriaMetrics does for other components. Will work on it.

@flokli flokli deleted the vmalertmanager-extra-args branch July 22, 2022 08:33
@flokli
Copy link
Contributor Author

flokli commented Jul 22, 2022

I think, all code generation must be moved into docker containers, as VictoriaMetrics does for other components. Will work on it.

I don't think moving everything into docker containers makes things easier. What's probably missing here is a stronger pinning of the version of generator binaries used. Looks like the makefile doesn't install controller-gen if some controller-gen binary is in $PATH, but doesn't do any further checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VMAlertmanager: ExtraArgs needs to include an additional -
2 participants