From f39a29b75469d1760af31e49b727d2320318ab05 Mon Sep 17 00:00:00 2001 From: Andrii Chubatiuk Date: Mon, 13 Jul 2026 11:29:52 +0300 Subject: [PATCH] operator: check config reload happened before completing reconcile --- api/operator/v1beta1/vmagent_types.go | 36 +++ api/operator/v1beta1/vmalert_types.go | 18 ++ api/operator/v1beta1/vmalertmanager_types.go | 18 ++ api/operator/v1beta1/vmauth_types.go | 30 +- api/operator/v1beta1/vmextra_types.go | 45 ++- api/operator/v1beta1/vmsingle_types.go | 20 ++ api/operator/v1beta1/zz_generated.deepcopy.go | 5 + config/crd/overlay/crd.descriptionless.yaml | 12 + config/crd/overlay/crd.yaml | 54 +++- docs/CHANGELOG.md | 3 + docs/api.md | 30 +- internal/controller/operator/controllers.go | 3 + .../operator/factory/build/container.go | 64 +++- .../operator/factory/build/vmscrape.go | 99 ++++-- .../operator/factory/build/vmscrape_test.go | 76 +++++ .../operator/factory/k8stools/client_utils.go | 2 +- .../factory/reconcile/config_reload.go | 115 +++++++ .../operator/factory/reconcile/secret.go | 10 +- .../operator/factory/reconcile/secret_test.go | 9 +- .../operator/factory/vlcluster/vmauth_lb.go | 2 +- .../operator/factory/vmagent/rbac.go | 2 +- .../operator/factory/vmagent/scrapes_test.go | 4 +- .../operator/factory/vmagent/vmagent.go | 160 ++++++---- .../operator/factory/vmagent/vmagent_test.go | 90 +++++- .../operator/factory/vmalert/rules.go | 4 + .../operator/factory/vmalert/vmalert.go | 14 +- .../factory/vmalertmanager/statefulset.go | 15 +- .../operator/factory/vmanomaly/config.go | 4 +- .../operator/factory/vmauth/vmauth.go | 47 ++- .../operator/factory/vmauth/vmauth_test.go | 15 + .../operator/factory/vmcluster/vmcluster.go | 2 +- .../operator/factory/vmdistributed/util.go | 250 ---------------- .../factory/vmdistributed/util_test.go | 47 --- .../operator/factory/vmdistributed/vmauth.go | 3 + .../factory/vmdistributed/vmauth_test.go | 18 ++ .../operator/factory/vmdistributed/zone.go | 66 +--- .../operator/factory/vmsingle/scrapes_test.go | 4 +- .../operator/factory/vmsingle/vmsingle.go | 111 ++++--- .../operator/factory/vtcluster/vmauth_lb.go | 2 +- .../controller/operator/vmagent_controller.go | 24 +- .../controller/operator/vmalert_controller.go | 1 + .../operator/vmalertmanager_controller.go | 1 + .../vmalertmanagerconfig_controller.go | 26 +- .../controller/operator/vmauth_controller.go | 1 + .../controller/operator/vmrule_controller.go | 23 +- .../operator/vmsingle_controller.go | 24 +- .../controller/operator/vmuser_controller.go | 23 +- internal/podutil/discover.go | 64 ++++ internal/podutil/discover_test.go | 51 ++++ internal/podutil/metrics.go | 281 ++++++++++++++++++ internal/podutil/metrics_test.go | 106 +++++++ test/e2e/vmauth_test.go | 2 +- 52 files changed, 1545 insertions(+), 591 deletions(-) create mode 100644 internal/controller/operator/factory/reconcile/config_reload.go create mode 100644 internal/podutil/discover.go create mode 100644 internal/podutil/discover_test.go create mode 100644 internal/podutil/metrics.go create mode 100644 internal/podutil/metrics_test.go diff --git a/api/operator/v1beta1/vmagent_types.go b/api/operator/v1beta1/vmagent_types.go index 28c36ba0a..23ce6c5a1 100644 --- a/api/operator/v1beta1/vmagent_types.go +++ b/api/operator/v1beta1/vmagent_types.go @@ -283,11 +283,26 @@ func (cr *VMAgent) GetReloadURL(host string) string { return BuildLocalURL(reloadAuthKeyFlag, host, cr.Spec.Port, reloadPath, cr.Spec.ExtraArgs) } +// GetMetricsURL implements configReloadWaitable interface +func (cr *VMAgent) GetMetricsURL(host string) string { + return BuildLocalURL("", host, cr.Spec.Port, metricsPath, cr.Spec.ExtraArgs) +} + // GetReloaderParams implements reloadable interface func (cr *VMAgent) GetReloaderParams() *CommonConfigReloaderParams { return &cr.Spec.CommonConfigReloaderParams } +// GetConfigReloadMetricNames returns vmagent's config-reload-completion metrics for the given assets, +// or for all assets if none are given. +func (cr *VMAgent) GetConfigReloadMetricNames(assets ...ConfigReloadAsset) []string { + return configReloadMetricNames(map[ConfigReloadAsset]string{ + ConfigReloadAssetMain: "vm_promscrape_config_last_reload_success_timestamp_seconds", + ConfigReloadAssetRelabel: "vmagent_relabel_config_last_reload_success_timestamp_seconds", + ConfigReloadAssetStreamAggr: "vmagent_streamaggr_config_reload_success_timestamp_seconds", + }, assets) +} + // UseProxyProtocol implements build.probeCRD interface func (cr *VMAgent) UseProxyProtocol() bool { return UseProxyProtocol(cr.Spec.ExtraArgs) @@ -710,6 +725,27 @@ func (cr *VMAgent) GetAdditionalService() *AdditionalServiceSpec { return cr.Spec.ServiceSpec } +// HasRemoteWriteSecrets reports whether any remoteWrite entry references a credential secret. +func (cr *VMAgent) HasRemoteWriteSecrets() bool { + for _, rw := range cr.Spec.RemoteWrite { + if rw.BasicAuth != nil && len(rw.BasicAuth.Password.Name) > 0 { + return true + } + if rw.BearerTokenSecret != nil && rw.BearerTokenSecret.Name != "" { + return true + } + if rw.OAuth2 != nil && rw.OAuth2.ClientSecret != nil { + return true + } + } + return false +} + +// HasConfigReloader reports whether vmagent's pod spec includes a config-reloader sidecar. +func (cr *VMAgent) HasConfigReloader() bool { + return !ptr.Deref(cr.Spec.IngestOnlyMode, false) || cr.HasAnyRelabellingConfigs() || cr.HasAnyStreamAggrRule() || cr.HasRemoteWriteSecrets() +} + func checkRelabelConfigs(src []*RelabelConfig) error { // TODO: restore check when issue will be fixed at golang // https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6911 diff --git a/api/operator/v1beta1/vmalert_types.go b/api/operator/v1beta1/vmalert_types.go index dc7beb661..38d466c77 100644 --- a/api/operator/v1beta1/vmalert_types.go +++ b/api/operator/v1beta1/vmalert_types.go @@ -168,11 +168,24 @@ func (cr *VMAlert) GetReloadURL(host string) string { return BuildLocalURL(reloadAuthKeyFlag, host, cr.Spec.Port, reloadPath, cr.Spec.ExtraArgs) } +// GetMetricsURL implements configReloadWaitable interface +func (cr *VMAlert) GetMetricsURL(host string) string { + return BuildLocalURL("", host, cr.Spec.Port, metricsPath, cr.Spec.ExtraArgs) +} + // GetReloaderParams implements reloadable interface func (cr *VMAlert) GetReloaderParams() *CommonConfigReloaderParams { return &cr.Spec.CommonConfigReloaderParams } +// GetConfigReloadMetricNames returns vmalert's config-reload-completion metric. +// vmalert has only one config asset, so assets is accepted but ignored. +func (cr *VMAlert) GetConfigReloadMetricNames(_ ...ConfigReloadAsset) []string { + return []string{ + "vmalert_config_last_reload_success_timestamp_seconds", + } +} + // UseProxyProtocol implements build.probeCRD interface func (cr *VMAlert) UseProxyProtocol() bool { return UseProxyProtocol(cr.Spec.ExtraArgs) @@ -530,6 +543,11 @@ func (cr *VMAlert) IsUnmanaged() bool { return !cr.Spec.SelectAllByDefault && cr.Spec.RuleSelector == nil && cr.Spec.RuleNamespaceSelector == nil } +// HasConfigReloader reports whether vmalert's pod spec includes a config-reloader sidecar. +func (cr *VMAlert) HasConfigReloader() bool { + return !cr.IsUnmanaged() +} + // LastSpecUpdated compares spec with last applied spec stored, replaces old spec and returns true if it's updated func (cr *VMAlert) LastSpecUpdated() bool { updated := cr.Status.LastAppliedSpec == nil || !equality.Semantic.DeepEqual(&cr.Spec, cr.Status.LastAppliedSpec) diff --git a/api/operator/v1beta1/vmalertmanager_types.go b/api/operator/v1beta1/vmalertmanager_types.go index 383cfb338..1a48cce7b 100644 --- a/api/operator/v1beta1/vmalertmanager_types.go +++ b/api/operator/v1beta1/vmalertmanager_types.go @@ -238,11 +238,29 @@ func (cr *VMAlertmanager) GetReloadURL(host string) string { return localReloadURL.String() } +// GetMetricsURL implements configReloadWaitable interface +func (cr *VMAlertmanager) GetMetricsURL(host string) string { + localMetricsURL := &url.URL{ + Scheme: cr.accessScheme(), + Host: fmt.Sprintf("%s:%s", host, cr.Port()), + Path: cr.GetMetricsPath(), + } + return localMetricsURL.String() +} + // GetReloaderParams implements reloadable interface func (cr *VMAlertmanager) GetReloaderParams() *CommonConfigReloaderParams { return &cr.Spec.CommonConfigReloaderParams } +// GetConfigReloadMetricNames returns alertmanager's config-reload-completion metric. +// Alertmanager has only one config asset, so assets is accepted but ignored. +func (cr *VMAlertmanager) GetConfigReloadMetricNames(_ ...ConfigReloadAsset) []string { + return []string{ + "alertmanager_config_last_reload_success_timestamp_seconds", + } +} + // UseProxyProtocol implements build.probeCRD interface func (cr *VMAlertmanager) UseProxyProtocol() bool { return false diff --git a/api/operator/v1beta1/vmauth_types.go b/api/operator/v1beta1/vmauth_types.go index 152e306ea..000a3fdd1 100644 --- a/api/operator/v1beta1/vmauth_types.go +++ b/api/operator/v1beta1/vmauth_types.go @@ -772,11 +772,21 @@ func (cr *VMAuth) IsUnmanaged() bool { // GetReloadURL implements reloadable interface func (cr *VMAuth) GetReloadURL(host string) string { - port := cr.Spec.Port + return BuildLocalURL(reloadAuthKeyFlag, host, cr.metricsPort(), reloadPath, cr.Spec.ExtraArgs) +} + +// metricsPort returns the port vmauth serves /metrics (and other internal +// routes) on: the internal port if configured, else the main port. +func (cr *VMAuth) metricsPort() string { if len(cr.Spec.InternalListenPort) > 0 { - port = cr.Spec.InternalListenPort + return cr.Spec.InternalListenPort } - return BuildLocalURL(reloadAuthKeyFlag, host, port, reloadPath, cr.Spec.ExtraArgs) + return cr.Spec.Port +} + +// GetMetricsURL implements configReloadWaitable interface +func (cr *VMAuth) GetMetricsURL(host string) string { + return BuildLocalURL("", host, cr.metricsPort(), metricsPath, cr.Spec.ExtraArgs) } // GetReloaderParams implements reloadable interface @@ -784,6 +794,20 @@ func (cr *VMAuth) GetReloaderParams() *CommonConfigReloaderParams { return &cr.Spec.CommonConfigReloaderParams } +// GetConfigReloadMetricNames returns vmauth's config-reload-completion metric. +// vmauth has only one config asset, so assets is accepted but ignored. +func (cr *VMAuth) GetConfigReloadMetricNames(_ ...ConfigReloadAsset) []string { + return []string{ + "vmauth_config_last_reload_success_timestamp_seconds", + } +} + +// HasConfigReloader reports whether vmauth's pod spec includes a config-reloader sidecar - +// only true when the operator itself owns and writes the auth config (no SecretRef/LocalPath). +func (cr *VMAuth) HasConfigReloader() bool { + return cr.Spec.SecretRef == nil && cr.Spec.LocalPath == "" +} + // UseProxyProtocol implements build.probeCRD interface func (cr *VMAuth) UseProxyProtocol() bool { hasInternalPorts := len(cr.Spec.InternalListenPort) == 0 diff --git a/api/operator/v1beta1/vmextra_types.go b/api/operator/v1beta1/vmextra_types.go index dc9a4e003..010a7109c 100644 --- a/api/operator/v1beta1/vmextra_types.go +++ b/api/operator/v1beta1/vmextra_types.go @@ -71,6 +71,40 @@ const ( MetadataStrategyMergePromPriority MetadataStrategy = "merge-prometheus-priority" ) +// ConfigReloadAsset identifies a distinct config asset that a CR's config-reloader +// sidecar watches and reloads independently, each tracked by its own +// config-reload-completion metric. +type ConfigReloadAsset string + +const ( + // ConfigReloadAssetMain identifies the primary config asset (e.g. scrape config, + // alerting/recording rules, vmauth config, alertmanager config). + ConfigReloadAssetMain ConfigReloadAsset = "main" + // ConfigReloadAssetRelabel identifies the global/per-target relabeling config asset. + ConfigReloadAssetRelabel ConfigReloadAsset = "relabel" + // ConfigReloadAssetStreamAggr identifies the stream aggregation config asset. + ConfigReloadAssetStreamAggr ConfigReloadAsset = "streamAggr" +) + +// configReloadMetricNames selects metric names from all for the requested assets. +// With no assets requested, metric names for every known asset are returned. +func configReloadMetricNames(all map[ConfigReloadAsset]string, assets []ConfigReloadAsset) []string { + if len(assets) == 0 { + names := make([]string, 0, len(all)) + for _, name := range all { + names = append(names, name) + } + return names + } + names := make([]string, 0, len(assets)) + for _, asset := range assets { + if name, ok := all[asset]; ok { + names = append(names, name) + } + } + return names +} + const ( httpPathPrefixFlag = "http.pathPrefix" httpUseProxyProtocolFlag = "httpListenAddr.useProxyProtocol" @@ -1387,7 +1421,7 @@ type CommonConfigReloaderParams struct { // UseVMConfigReloader replaces prometheus-like config-reloader // with vm one. It uses secrets watch instead of file watch // which greatly increases speed of config updates - // Removed since v0.67.0: this property is ignored and no longer needed + // +notes={removed_in: "v0.67.0"} // +optional UseVMConfigReloader *bool `json:"useVMConfigReloader,omitempty"` // ConfigReloaderImageTag defines image:tag for config-reloader container @@ -1408,9 +1442,16 @@ type CommonConfigReloaderParams struct { ConfigReloaderExtraArgs map[string]string `json:"configReloaderExtraArgs,omitempty"` // ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. // Given secret reference will be added to the application and vm-config-reloader as volume - // available since v0.57.0 version + // +notes={available_from: "v0.57.0"} // +optional ConfigReloadAuthKeySecret *corev1.SecretKeySelector `json:"configReloadAuthKeySecret,omitempty"` + // WaitForConfigReload instructs the operator to verify, via the + // application's own config-reload-completion metric, that every replica + // has actually reloaded before marking dependent objects as applied. + // No-op for application types that don't report such a metric. + // +optional + // +notes={available_from: "v0.74.0"} + WaitForConfigReload *bool `json:"waitForConfigReload,omitempty"` } // CommonAppsParams defines common params diff --git a/api/operator/v1beta1/vmsingle_types.go b/api/operator/v1beta1/vmsingle_types.go index a3b219eab..b149df24e 100644 --- a/api/operator/v1beta1/vmsingle_types.go +++ b/api/operator/v1beta1/vmsingle_types.go @@ -126,6 +126,11 @@ func (cr *VMSingle) HasAnyRelabellingConfigs() bool { return cr.Spec.HasAnyRelabellingConfigs() } +// HasConfigReloader reports whether vmsingle's pod spec includes a config-reloader sidecar. +func (cr *VMSingle) HasConfigReloader() bool { + return !ptr.Deref(cr.Spec.IngestOnlyMode, true) || cr.HasAnyRelabellingConfigs() || cr.HasAnyStreamAggrRule() +} + // ScrapeSelectors gets object and namespace sepectors func (cr *VMSingle) ScrapeSelectors(scrape client.Object) (*metav1.LabelSelector, *metav1.LabelSelector) { return cr.Spec.ScrapeSelectors(scrape) @@ -168,11 +173,26 @@ func (cr *VMSingle) GetReloadURL(host string) string { return BuildLocalURL(reloadAuthKeyFlag, host, cr.Spec.Port, reloadPath, cr.Spec.ExtraArgs) } +// GetMetricsURL implements configReloadWaitable interface +func (cr *VMSingle) GetMetricsURL(host string) string { + return BuildLocalURL("", host, cr.Spec.Port, metricsPath, cr.Spec.ExtraArgs) +} + // GetReloaderParams implements reloadable interface func (cr *VMSingle) GetReloaderParams() *CommonConfigReloaderParams { return &cr.Spec.CommonConfigReloaderParams } +// GetConfigReloadMetricNames returns vmsingle's config-reload-completion metrics for the given assets, +// or for all assets if none are given. +func (cr *VMSingle) GetConfigReloadMetricNames(assets ...ConfigReloadAsset) []string { + return configReloadMetricNames(map[ConfigReloadAsset]string{ + ConfigReloadAssetMain: "vm_promscrape_config_last_reload_success_timestamp_seconds", + ConfigReloadAssetRelabel: "vm_relabel_config_last_reload_success_timestamp_seconds", + ConfigReloadAssetStreamAggr: "vminsert_streamagg_config_last_reload_success_timestamp_seconds", + }, assets) +} + // UseProxyProtocol implements build.probeCRD interface func (cr *VMSingle) UseProxyProtocol() bool { return UseProxyProtocol(cr.Spec.ExtraArgs) diff --git a/api/operator/v1beta1/zz_generated.deepcopy.go b/api/operator/v1beta1/zz_generated.deepcopy.go index bab53d6f6..386722823 100644 --- a/api/operator/v1beta1/zz_generated.deepcopy.go +++ b/api/operator/v1beta1/zz_generated.deepcopy.go @@ -507,6 +507,11 @@ func (in *CommonConfigReloaderParams) DeepCopyInto(out *CommonConfigReloaderPara *out = new(v1.SecretKeySelector) (*in).DeepCopyInto(*out) } + if in.WaitForConfigReload != nil { + in, out := &in.WaitForConfigReload, &out.WaitForConfigReload + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CommonConfigReloaderParams. diff --git a/config/crd/overlay/crd.descriptionless.yaml b/config/crd/overlay/crd.descriptionless.yaml index d79b23945..47d147849 100644 --- a/config/crd/overlay/crd.descriptionless.yaml +++ b/config/crd/overlay/crd.descriptionless.yaml @@ -7637,6 +7637,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean required: - remoteWrite type: object @@ -11682,6 +11684,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean webConfig: properties: basic_auth_users: @@ -12990,6 +12994,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean required: - datasource type: object @@ -16438,6 +16444,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean type: object x-kubernetes-preserve-unknown-fields: true status: @@ -24695,6 +24703,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean type: object x-kubernetes-preserve-unknown-fields: true type: object @@ -45403,6 +45413,8 @@ spec: type: string type: object type: object + waitForConfigReload: + type: boolean type: object status: properties: diff --git a/config/crd/overlay/crd.yaml b/config/crd/overlay/crd.yaml index 3bb417ebb..3bb035b8d 100644 --- a/config/crd/overlay/crd.yaml +++ b/config/crd/overlay/crd.yaml @@ -9431,7 +9431,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must be a @@ -14766,7 +14765,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean vmAgentExternalLabelName: description: |- @@ -15093,6 +15091,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean required: - remoteWrite type: object @@ -20555,7 +20560,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must be a @@ -22358,7 +22362,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean volumeMounts: description: |- @@ -22679,6 +22682,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean webConfig: description: |- WebConfig defines configuration for webserver @@ -23019,7 +23029,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must be a @@ -24826,7 +24835,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean volumeMounts: description: |- @@ -25147,6 +25155,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean required: - datasource type: object @@ -28563,7 +28578,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must be a @@ -31429,7 +31443,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean userNamespaceSelector: description: |- @@ -31851,6 +31864,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean type: object x-kubernetes-preserve-unknown-fields: true status: @@ -38247,7 +38267,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must @@ -46640,7 +46659,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean userNamespaceSelector: description: |- @@ -48972,6 +48990,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean type: object x-kubernetes-preserve-unknown-fields: true type: object @@ -85995,7 +86020,6 @@ spec: description: |- ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests. Given secret reference will be added to the application and vm-config-reloader as volume - available since v0.57.0 version properties: key: description: The key of the secret to select from. Must be a @@ -89457,7 +89481,6 @@ spec: UseVMConfigReloader replaces prometheus-like config-reloader with vm one. It uses secrets watch instead of file watch which greatly increases speed of config updates - Removed since v0.67.0: this property is ignored and no longer needed type: boolean vmAgentExternalLabelName: description: |- @@ -90243,6 +90266,13 @@ spec: type: string type: object type: object + waitForConfigReload: + description: |- + WaitForConfigReload instructs the operator to verify, via the + application's own config-reload-completion metric, that every replica + has actually reloaded before marking dependent objects as applied. + No-op for application types that don't report such a metric. + type: boolean type: object status: description: VMSingleStatus defines the observed state of VMSingle diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 685393a4b..a640a07f8 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,6 +15,9 @@ aliases: * FEATURE: [vlagent](https://docs.victoriametrics.com/operator/resources/vlagent/): add `basicAuth` field support to `remoteWrite` entries. See [#2371](https://github.com/VictoriaMetrics/operator/issues/2371). * FEATURE: [vmuser](https://docs.victoriametrics.com/operator/resources/vmuser/): add `defaultVMAccessClaim` field to `spec.jwt`, mapped to vmauth's `jwt.default_vm_access_claim`. It lets a `VMUser` accept JWTs that don't carry a `vm_access` claim, matching `vmauth` v1.147.0+ behavior. See [#2375](https://github.com/VictoriaMetrics/operator/issues/2375). +* FEATURE: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/), [vmalert](https://docs.victoriametrics.com/operator/resources/vmalert/), [vmalertmanager](https://docs.victoriametrics.com/operator/resources/vmalertmanager/), [vmauth](https://docs.victoriametrics.com/operator/resources/vmauth/): add opt-in `spec.waitForConfigReload` field. When enabled, the operator verifies via the application's own config-reload-completion metric that every ready replica actually picked up a config change before marking dependent objects (e.g. `VMUser`, `VMRule`) as applied, instead of only confirming the config was written to the underlying Secret/ConfigMap. +* FEATURE: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): the internal `VMAuth` load balancer now always waits for a confirmed config reload before zone rotation proceeds, regardless of `spec.vmAuth.spec.waitForConfigReload`. This prevents a zone from being excluded from or restored to the LB backend list while `vmauth` is still routing traffic against its previous, stale configuration. +* FEATURE: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/), [vmalert](https://docs.victoriametrics.com/operator/resources/vmalert/), [vmalertmanager](https://docs.victoriametrics.com/operator/resources/vmalertmanager/), [vmsingle](https://docs.victoriametrics.com/operator/resources/vmsingle/), [vmauth](https://docs.victoriametrics.com/operator/resources/vmauth/): when the config-reloader sidecar is present and `spec.disableSelfServiceScrape` is not set, the operator-managed `VMServiceScrape`/`VMPodScrape` now also scrapes the config-reloader's own `/metrics` endpoint directly by its container port, so config-reload metrics (e.g. reload success/failure counters and timestamps) are collected without any extra user configuration. * BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/), [vmsingle](https://docs.victoriametrics.com/operator/resources/vmsingle/): add missing `list` verb to config-reloader's secrets RBAC rule. See [#2384](https://github.com/VictoriaMetrics/operator/issues/2384). * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): return an error instead of panicking when a `Labels` map value is malformed (missing the `=` separator) during config parsing. diff --git a/docs/api.md b/docs/api.md index e5f5e7dee..65f065cbc 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1594,12 +1594,13 @@ Appears in: [VMAgentSpec](#vmagentspec), [VMAlertSpec](#vmalertspec), [VMAlertma | Field | Description | | --- | --- | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| | configReloaderResources#
_[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#resourcerequirements-v1-core)_ | _(Optional)_
ConfigReloaderResources config-reloader container resource request and limits, https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
if not defined default resources from operator config will be used | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | #### CommonRelabelParams @@ -3717,7 +3718,7 @@ Appears in: [VMAgent](#vmagent) | claimTemplates#
_[PersistentVolumeClaim](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#persistentvolumeclaim-v1-core) array_ | _(Required)_
ClaimTemplates allows adding additional VolumeClaimTemplates for VMAgent in StatefulMode | | componentVersion#
_string_ | _(Optional)_
ComponentVersion defines default images tag for all components.
it can be overwritten with component specific image.tag value. | | configMaps#
_string array_ | _(Optional)_
ConfigMaps is a list of ConfigMaps in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/configs/CONFIGMAP_NAME folder | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| @@ -3823,11 +3824,12 @@ Appears in: [VMAgent](#vmagent) | useDefaultResources#
_boolean_ | _(Optional)_
UseDefaultResources controls resource settings
By default, operator sets built-in resource requirements | | useLegacyNaming#
_boolean_ | _(Optional)_
UseLegacyNaming uses standalone Helm chart naming for managed resources:
the CR name is used directly instead of the default "-" convention.
Available from: v0.73.0 | | useStrictSecurity#
_boolean_ | _(Optional)_
UseStrictSecurity enables strict security mode for component
it restricts disk writes access
uses non-root user out of the box
drops not needed security permissions | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| | vmAgentExternalLabelName#
_string_ | _(Optional)_
VMAgentExternalLabelName Name of vmAgent external label used to denote vmAgent instance
name. Defaults to the value of `prometheus`. External label will
_not_ be added when value is set to empty string (`""`).
Deprecated: since version v0.67.0 will be removed in v0.69.0 use externalLabelName instead
| | volumeMounts#
_[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volumemount-v1-core) array_ | _(Optional)_
VolumeMounts allows configuration of additional VolumeMounts on the output Deployment/StatefulSet definition.
VolumeMounts specified will be appended to other VolumeMounts in the Application container | | volumes#
_[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volume-v1-core) array_ | _(Optional)_
Volumes allows configuration of additional volumes on the output Deployment/StatefulSet definition.
Volumes specified will be appended to other volumes that are generated. | | vpa#
_[EmbeddedVPA](#embeddedvpa)_ | _(Optional)_
Configures vertical pod autoscaling. | +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | #### VMAlert @@ -3913,7 +3915,7 @@ Appears in: [VMAlert](#vmalert) | affinity#
_[Affinity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#affinity-v1-core)_ | _(Optional)_
Affinity If specified, the pod's scheduling constraints. | | componentVersion#
_string_ | _(Optional)_
ComponentVersion defines default images tag for all components.
it can be overwritten with component specific image.tag value. | | configMaps#
_string array_ | _(Optional)_
ConfigMaps is a list of ConfigMaps in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/configs/CONFIGMAP_NAME folder | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| @@ -3980,10 +3982,11 @@ Appears in: [VMAlert](#vmalert) | useDefaultResources#
_boolean_ | _(Optional)_
UseDefaultResources controls resource settings
By default, operator sets built-in resource requirements | | useLegacyNaming#
_boolean_ | _(Optional)_
UseLegacyNaming uses standalone Helm chart naming for managed resources:
the CR name is used directly instead of the default "-" convention.
Available from: v0.73.0 | | useStrictSecurity#
_boolean_ | _(Optional)_
UseStrictSecurity enables strict security mode for component
it restricts disk writes access
uses non-root user out of the box
drops not needed security permissions | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| | volumeMounts#
_[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volumemount-v1-core) array_ | _(Optional)_
VolumeMounts allows configuration of additional VolumeMounts on the output Deployment/StatefulSet definition.
VolumeMounts specified will be appended to other VolumeMounts in the Application container | | volumes#
_[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volume-v1-core) array_ | _(Optional)_
Volumes allows configuration of additional volumes on the output Deployment/StatefulSet definition.
Volumes specified will be appended to other volumes that are generated. | | vpa#
_[EmbeddedVPA](#embeddedvpa)_ | _(Optional)_
Configures vertical pod autoscaling. | +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | #### VMAlertmanager @@ -4062,7 +4065,7 @@ Appears in: [VMAlertmanager](#vmalertmanager) | configMaps#
_string array_ | _(Optional)_
ConfigMaps is a list of ConfigMaps in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/configs/CONFIGMAP_NAME folder | | configNamespaceSelector#
_[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#labelselector-v1-meta)_ | _(Optional)_
ConfigNamespaceSelector defines namespace selector for VMAlertmanagerConfig.
Works in combination with Selector.
NamespaceSelector nil - only objects at VMAlertmanager namespace.
Selector nil - only objects at NamespaceSelector namespaces.
If both nil - behaviour controlled by selectAllByDefault | | configRawYaml#
_string_ | _(Optional)_
ConfigRawYaml - raw configuration for alertmanager,
it helps it to start without secret.
priority -> hardcoded ConfigRaw -> ConfigRaw, provided by user -> ConfigSecret. | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| @@ -4131,10 +4134,11 @@ Appears in: [VMAlertmanager](#vmalertmanager) | useDefaultResources#
_boolean_ | _(Optional)_
UseDefaultResources controls resource settings
By default, operator sets built-in resource requirements | | useLegacyNaming#
_boolean_ | _(Optional)_
UseLegacyNaming uses standalone Helm chart naming for managed resources:
the CR name is used directly instead of the default "-" convention.
Available from: v0.73.0 | | useStrictSecurity#
_boolean_ | _(Optional)_
UseStrictSecurity enables strict security mode for component
it restricts disk writes access
uses non-root user out of the box
drops not needed security permissions | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| | volumeMounts#
_[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volumemount-v1-core) array_ | _(Optional)_
VolumeMounts allows configuration of additional VolumeMounts on the output Deployment/StatefulSet definition.
VolumeMounts specified will be appended to other VolumeMounts in the Application container | | volumes#
_[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volume-v1-core) array_ | _(Optional)_
Volumes allows configuration of additional volumes on the output Deployment/StatefulSet definition.
Volumes specified will be appended to other volumes that are generated. | | vpa#
_[EmbeddedVPA](#embeddedvpa)_ | _(Optional)_
Configures vertical pod autoscaling. | +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | | webConfig#
_[VMAlertmanagerWebConfig](#vmalertmanagerwebconfig)_ | _(Optional)_
WebConfig defines configuration for webserver
https://github.com/prometheus/alertmanager/blob/main/docs/https.md | #### VMAlertmanagerTracingConfig @@ -4266,7 +4270,7 @@ Appears in: [VMAuth](#vmauth), [VMDistributedAuth](#vmdistributedauth) | arbitraryFSAccessThroughSMs#
_[ArbitraryFSAccessThroughSMsConfig](#arbitraryfsaccessthroughsmsconfig)_ | _(Optional)_
ArbitraryFSAccessThroughSMs configures whether configuration
can contain paths to arbitrary files on the file system
e.g bearer token files, basic auth password files, tls certs file paths | | componentVersion#
_string_ | _(Optional)_
ComponentVersion defines default images tag for all components.
it can be overwritten with component specific image.tag value. | | configMaps#
_string array_ | _(Optional)_
ConfigMaps is a list of ConfigMaps in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/configs/CONFIGMAP_NAME folder | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| @@ -4342,12 +4346,13 @@ Appears in: [VMAuth](#vmauth), [VMDistributedAuth](#vmdistributedauth) | useLegacyNaming#
_boolean_ | _(Optional)_
UseLegacyNaming uses standalone Helm chart naming for managed resources:
the CR name is used directly instead of the default "-" convention.
Available from: v0.73.0 | | useProxyProtocol#
_boolean_ | _(Required)_
UseProxyProtocol enables proxy protocol for vmauth
https://www.haproxy.org/download/2.3/doc/proxy-protocol.txt | | useStrictSecurity#
_boolean_ | _(Optional)_
UseStrictSecurity enables strict security mode for component
it restricts disk writes access
uses non-root user out of the box
drops not needed security permissions | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| | userNamespaceSelector#
_[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#labelselector-v1-meta)_ | _(Optional)_
UserNamespaceSelector Namespaces to be selected for VMAuth discovery.
Works in combination with Selector.
NamespaceSelector nil - only objects at VMAuth namespace.
Selector nil - only objects at NamespaceSelector namespaces.
If both nil - behaviour controlled by selectAllByDefault | | userSelector#
_[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#labelselector-v1-meta)_ | _(Optional)_
UserSelector defines VMUser to be selected for config file generation.
Works in combination with NamespaceSelector.
NamespaceSelector nil - only objects at VMAuth namespace.
If both nil - behaviour controlled by selectAllByDefault | | volumeMounts#
_[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volumemount-v1-core) array_ | _(Optional)_
VolumeMounts allows configuration of additional VolumeMounts on the output Deployment/StatefulSet definition.
VolumeMounts specified will be appended to other VolumeMounts in the Application container | | volumes#
_[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volume-v1-core) array_ | _(Optional)_
Volumes allows configuration of additional volumes on the output Deployment/StatefulSet definition.
Volumes specified will be appended to other volumes that are generated. | | vpa#
_[EmbeddedVPA](#embeddedvpa)_ | _(Optional)_
Configures vertical pod autoscaling. | +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | #### VMAuthUnauthorizedUserAccessSpec @@ -4945,7 +4950,7 @@ Appears in: [VMDistributedZoneSingle](#vmdistributedzonesingle), [VMSingle](#vms | arbitraryFSAccessThroughSMs#
_[ArbitraryFSAccessThroughSMsConfig](#arbitraryfsaccessthroughsmsconfig)_ | _(Optional)_
ArbitraryFSAccessThroughSMs configures whether configuration
based on EndpointAuth can access arbitrary files on the file system
of the VMAgent or VMSingle container e.g. bearer token files, basic auth, tls certs | | componentVersion#
_string_ | _(Optional)_
ComponentVersion defines default images tag for all components.
it can be overwritten with component specific image.tag value. | | configMaps#
_string array_ | _(Optional)_
ConfigMaps is a list of ConfigMaps in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/configs/CONFIGMAP_NAME folder | -| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
available since v0.57.0 version | +| configReloadAuthKeySecret#
_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretkeyselector-v1-core)_ | _(Optional)_
ConfigReloadAuthKeySecret defines optional secret reference authKey for /-/reload API requests.
Given secret reference will be added to the application and vm-config-reloader as volume
Available from: v0.57.0 | | configReloaderExtraArgs#
_object (keys:string, values:string)_ | _(Optional)_
ConfigReloaderExtraArgs that will be passed to VMAuths config-reloader container
for example resync-interval: "30s" | | configReloaderImage#
_string_ | _(Optional)_
ConfigReloaderImage defines image:tag for config-reloader container | | configReloaderImageTag#
_string_ | _(Optional)_
ConfigReloaderImageTag defines image:tag for config-reloader container
Deprecated: since version v0.67.0 will be removed in v0.69.0 use configReloaderImage instead
| @@ -5043,12 +5048,13 @@ Appears in: [VMDistributedZoneSingle](#vmdistributedzonesingle), [VMSingle](#vms | useDefaultResources#
_boolean_ | _(Optional)_
UseDefaultResources controls resource settings
By default, operator sets built-in resource requirements | | useLegacyNaming#
_boolean_ | _(Optional)_
UseLegacyNaming uses standalone Helm chart naming for managed resources:
the CR name is used directly instead of the default "-" convention.
Available from: v0.73.0 | | useStrictSecurity#
_boolean_ | _(Optional)_
UseStrictSecurity enables strict security mode for component
it restricts disk writes access
uses non-root user out of the box
drops not needed security permissions | -| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Removed since v0.67.0: this property is ignored and no longer needed | +| useVMConfigReloader#
_boolean_ | _(Optional)_
UseVMConfigReloader replaces prometheus-like config-reloader
with vm one. It uses secrets watch instead of file watch
which greatly increases speed of config updates
Deprecated: will be removed in v0.67.0
| | vmAgentExternalLabelName#
_string_ | _(Optional)_
VMAgentExternalLabelName Name of vmAgent external label used to denote vmAgent instance
name. Defaults to the value of `prometheus`. External label will
_not_ be added when value is set to empty string (`""`).
Deprecated: since version v0.67.0 will be removed in v0.69.0 use externalLabelName instead
| | vmBackup#
_[VMBackup](#vmbackup)_ | _(Optional)_
VMBackup configuration for backup | | volumeMounts#
_[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volumemount-v1-core) array_ | _(Optional)_
VolumeMounts allows configuration of additional VolumeMounts on the output Deployment/StatefulSet definition.
VolumeMounts specified will be appended to other VolumeMounts in the Application container | | volumes#
_[Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#volume-v1-core) array_ | _(Optional)_
Volumes allows configuration of additional volumes on the output Deployment/StatefulSet definition.
Volumes specified will be appended to other volumes that are generated. | | vpa#
_[EmbeddedVPA](#embeddedvpa)_ | _(Optional)_
Configures vertical pod autoscaling. | +| waitForConfigReload#
_boolean_ | _(Optional)_
WaitForConfigReload instructs the operator to verify, via the
application's own config-reload-completion metric, that every replica
has actually reloaded before marking dependent objects as applied.
No-op for application types that don't report such a metric.
Available from: v0.74.0 | #### VMStaticScrape diff --git a/internal/controller/operator/controllers.go b/internal/controller/operator/controllers.go index ea7d74910..395bbe47e 100644 --- a/internal/controller/operator/controllers.go +++ b/internal/controller/operator/controllers.go @@ -41,6 +41,9 @@ var ( maxConcurrency = ptr.To(15) ) +// childReconcileConcurrencyLimit bounds concurrent errgroup fan-out over sibling CRs within a single parent reconcile. +const childReconcileConcurrencyLimit = 5 + var ( optionsInit sync.Once defaultOptions *controller.Options diff --git a/internal/controller/operator/factory/build/container.go b/internal/controller/operator/factory/build/container.go index 68bd06907..74311cb22 100644 --- a/internal/controller/operator/factory/build/container.go +++ b/internal/controller/operator/factory/build/container.go @@ -9,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -326,16 +327,59 @@ func AppendArgsForInsertPorts(args []string, ip *vmv1beta1.InsertPorts) []string return args } -var ( - configReloaderDefaultPort = 8435 - configReloaderContainerProbe = corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Scheme: "HTTP", - Port: intstr.FromInt(configReloaderDefaultPort), +const ( + // ConfigReloaderDefaultPort is the config-reloader container's fixed HTTP listen port, + // exposing its own /health and /metrics endpoints. + ConfigReloaderDefaultPort = 8435 + // ConfigReloaderPortName is the container/service port name for ConfigReloaderDefaultPort. + ConfigReloaderPortName = "reloader-http" +) + +var configReloaderContainerProbe = corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Scheme: "HTTP", + Port: intstr.FromInt(ConfigReloaderDefaultPort), + }, +} + +func configReloaderJobRelabeling() vmv1beta1.EndpointRelabelings { + return vmv1beta1.EndpointRelabelings{ + RelabelConfigs: []*vmv1beta1.RelabelConfig{ + { + SourceLabels: []string{"job"}, + TargetLabel: "job", + Regex: vmv1beta1.StringOrArray{"(.+)"}, + Replacement: ptr.To("${1}-" + ConfigReloaderPortName), + }, }, } -) +} + +// ConfigReloaderVMServiceScrapeEndpoint returns a VMServiceScrape endpoint that scrapes the +// config-reloader sidecar directly by its container port (via TargetPort, resolved from the +// pod's actual EndpointSlice ports), without requiring a matching named ServicePort. +func ConfigReloaderVMServiceScrapeEndpoint() vmv1beta1.Endpoint { + return vmv1beta1.Endpoint{ + TargetPort: ptr.To(intstr.FromInt32(ConfigReloaderDefaultPort)), + EndpointRelabelings: configReloaderJobRelabeling(), + EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ + Path: "/metrics", + }, + } +} + +// ConfigReloaderPodScrapeEndpoint returns a VMPodScrape endpoint that scrapes the +// config-reloader sidecar directly by its container port number. +func ConfigReloaderPodScrapeEndpoint() vmv1beta1.PodMetricsEndpoint { + return vmv1beta1.PodMetricsEndpoint{ + PortNumber: ptr.To(int32(ConfigReloaderDefaultPort)), + EndpointRelabelings: configReloaderJobRelabeling(), + EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ + Path: "/metrics", + }, + } +} type reloadable interface { GetReloaderParams() *vmv1beta1.CommonConfigReloaderParams @@ -421,8 +465,8 @@ func ConfigReloaderContainer(isInit bool, cr reloadable, mounts []corev1.VolumeM // exposes reloader-http port for container func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container) { crContainer.Ports = append(crContainer.Ports, corev1.ContainerPort{ - ContainerPort: int32(configReloaderDefaultPort), - Name: "reloader-http", + ContainerPort: int32(ConfigReloaderDefaultPort), + Name: ConfigReloaderPortName, Protocol: "TCP", }) crContainer.LivenessProbe = &corev1.Probe{ diff --git a/internal/controller/operator/factory/build/vmscrape.go b/internal/controller/operator/factory/build/vmscrape.go index 164f5a47e..39d83db92 100644 --- a/internal/controller/operator/factory/build/vmscrape.go +++ b/internal/controller/operator/factory/build/vmscrape.go @@ -39,12 +39,16 @@ func VMServiceScrape(service *corev1.Service, b scrapeBuilder, additionalPortNam } var extraRelabelingRules vmv1beta1.EndpointRelabelings + path := b.GetMetricsPath() if servicePort.Name != defaultPortName { // check service for extra ports var nameMatched bool for _, filter := range additionalPortNames { if servicePort.Name == filter { nameMatched = true + // sidecars (config-reloader, vmbackupmanager) always expose metrics at the + // literal path below, regardless of the app's own http.pathPrefix + path = "/metrics" // add a relabeling rule to avoid job collision extraRelabelingRules.RelabelConfigs = []*vmv1beta1.RelabelConfig{ { @@ -66,7 +70,7 @@ func VMServiceScrape(service *corev1.Service, b scrapeBuilder, additionalPortNam Port: servicePort.Name, EndpointRelabelings: extraRelabelingRules, EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ - Path: b.GetMetricsPath(), + Path: path, }, } if b.UseTLS() { @@ -134,30 +138,56 @@ func VMServiceScrape(service *corev1.Service, b scrapeBuilder, additionalPortNam return scrape } -// VMPodScrapeForObjectWithSpec build VMPodScrape for given podScrapeBuilder -func VMPodScrape(b podScrapeBuilder, portName string) *vmv1beta1.VMPodScrape { +// VMPodScrape builds a VMPodScrape for given podScrapeBuilder, with portName as the primary +// endpoint and any additionalPortNames (e.g. sidecar metrics ports) appended alongside it. +func VMPodScrape(b podScrapeBuilder, portName string, additionalPortNames ...string) *vmv1beta1.VMPodScrape { extraArgs := b.GetExtraArgs() authKey := extraArgs["metricsAuthKey"] - endpoint := vmv1beta1.PodMetricsEndpoint{ - Port: &portName, - EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ - Path: b.GetMetricsPath(), - }, - } - if b.UseTLS() { - endpoint.Scheme = "https" - // add insecure by default - // if needed user will override it with direct config - endpoint.TLSConfig = &vmv1beta1.TLSConfig{ - InsecureSkipVerify: true, + buildEndpoint := func(name string, isPrimary bool) vmv1beta1.PodMetricsEndpoint { + path := b.GetMetricsPath() + var relabelings vmv1beta1.EndpointRelabelings + if !isPrimary { + // sidecars (e.g. config-reloader) always expose metrics at the literal path + // below, regardless of the app's own http.pathPrefix + path = "/metrics" + relabelings.RelabelConfigs = []*vmv1beta1.RelabelConfig{ + { + SourceLabels: []string{"job"}, + TargetLabel: "job", + Regex: vmv1beta1.StringOrArray{"(.+)"}, + Replacement: ptr.To("${1}-" + name), + }, + } } - } - if len(authKey) > 0 { - endpoint.Params = map[string][]string{ - "authKey": {authKey}, + ep := vmv1beta1.PodMetricsEndpoint{ + Port: ptr.To(name), + EndpointRelabelings: relabelings, + EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ + Path: path, + }, + } + if b.UseTLS() { + ep.Scheme = "https" + // add insecure by default + // if needed user will override it with direct config + ep.TLSConfig = &vmv1beta1.TLSConfig{ + InsecureSkipVerify: true, + } } + if len(authKey) > 0 { + ep.Params = map[string][]string{ + "authKey": {authKey}, + } + } + return ep + } + + endpoints := []vmv1beta1.PodMetricsEndpoint{buildEndpoint(portName, true)} + for _, name := range additionalPortNames { + endpoints = append(endpoints, buildEndpoint(name, false)) } + selectorLabels := b.SelectorLabels() scrape := &vmv1beta1.VMPodScrape{ ObjectMeta: metav1.ObjectMeta{ @@ -167,25 +197,30 @@ func VMPodScrape(b podScrapeBuilder, portName string) *vmv1beta1.VMPodScrape { OwnerReferences: []metav1.OwnerReference{b.AsOwner()}, }, Spec: vmv1beta1.VMPodScrapeSpec{ - Selector: *metav1.SetAsLabelSelector(selectorLabels), - PodMetricsEndpoints: []vmv1beta1.PodMetricsEndpoint{ - endpoint, - }, + Selector: *metav1.SetAsLabelSelector(selectorLabels), + PodMetricsEndpoints: endpoints, }, } serviceScrapeSpec := b.GetServiceScrape() if serviceScrapeSpec != nil { for _, e := range serviceScrapeSpec.Endpoints { - if e.Port == *scrape.Spec.PodMetricsEndpoints[0].Port { - scrape.Spec.PodMetricsEndpoints[0].EndpointScrapeParams = e.EndpointScrapeParams - scrape.Spec.PodMetricsEndpoints[0].EndpointRelabelings = e.EndpointRelabelings - continue + var found bool + for idx := range scrape.Spec.PodMetricsEndpoints { + pep := &scrape.Spec.PodMetricsEndpoints[idx] + if pep.Port != nil && *pep.Port == e.Port { + found = true + pep.EndpointScrapeParams = e.EndpointScrapeParams + pep.EndpointRelabelings = e.EndpointRelabelings + break + } + } + if !found { + scrape.Spec.PodMetricsEndpoints = append(scrape.Spec.PodMetricsEndpoints, vmv1beta1.PodMetricsEndpoint{ + Port: ptr.To(e.Port), + EndpointRelabelings: e.EndpointRelabelings, + EndpointScrapeParams: e.EndpointScrapeParams, + }) } - scrape.Spec.PodMetricsEndpoints = append(scrape.Spec.PodMetricsEndpoints, vmv1beta1.PodMetricsEndpoint{ - Port: ptr.To(e.Port), - EndpointRelabelings: e.EndpointRelabelings, - EndpointScrapeParams: e.EndpointScrapeParams, - }) } scrape.Spec.PodTargetLabels = serviceScrapeSpec.PodTargetLabels scrape.Spec.SampleLimit = serviceScrapeSpec.SampleLimit diff --git a/internal/controller/operator/factory/build/vmscrape_test.go b/internal/controller/operator/factory/build/vmscrape_test.go index d9cc69f1a..8a3f6ab12 100644 --- a/internal/controller/operator/factory/build/vmscrape_test.go +++ b/internal/controller/operator/factory/build/vmscrape_test.go @@ -308,6 +308,60 @@ func TestVMServiceScrapeForServiceWithSpec(t *testing.T) { }, }) + // with a custom http.pathPrefix: the primary port uses it, but an additional (sidecar) + // port always scrapes the literal /metrics path, since sidecars like config-reloader + // are unaffected by the app's own path prefix + f(opts{ + filterPortNames: []string{"reloader-http"}, + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmagent-svc", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "http"}, + {Name: "reloader-http"}, + }, + }, + }, + spec: testScrapeObject{ + extraArgs: map[string]string{"http.pathPrefix": "/prefix"}, + }, + wantServiceScrapeSpec: vmv1beta1.VMServiceScrapeSpec{ + Endpoints: []vmv1beta1.Endpoint{ + { + EndpointRelabelings: vmv1beta1.EndpointRelabelings{ + RelabelConfigs: vmAppRelabel, + }, + EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ + Path: "/prefix/metrics", + }, + Port: "http", + }, + { + EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ + Path: "/metrics", + }, + Port: "reloader-http", + EndpointRelabelings: vmv1beta1.EndpointRelabelings{ + RelabelConfigs: []*vmv1beta1.RelabelConfig{{ + SourceLabels: []string{"job"}, + TargetLabel: "job", + Regex: vmv1beta1.StringOrArray{"(.+)"}, + Replacement: ptr.To("${1}-reloader-http"), + }, victoriaMetricsAppRelabelConfig()}, + }, + }, + }, + Selector: metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{ + Key: vmv1beta1.AdditionalServiceLabel, + Operator: metav1.LabelSelectorOpDoesNotExist, + }}, + }, + }, + }) + // with authKey and tls f(opts{ service: &corev1.Service{ @@ -403,6 +457,28 @@ func TestVMPodScrapeAddsVictoriaMetricsAppLabel(t *testing.T) { } } +func TestVMPodScrapeAdditionalPorts(t *testing.T) { + spec := testScrapeObject{extraArgs: map[string]string{"http.pathPrefix": "/prefix"}} + + scrape := VMPodScrape(&spec, "http", "reloader-http") + + assert.Len(t, scrape.Spec.PodMetricsEndpoints, 2) + primary := scrape.Spec.PodMetricsEndpoints[0] + assert.Equal(t, "http", *primary.Port) + assert.Equal(t, "/prefix/metrics", primary.Path) + + extra := scrape.Spec.PodMetricsEndpoints[1] + assert.Equal(t, "reloader-http", *extra.Port) + // sidecar ports always scrape the literal /metrics path, unaffected by http.pathPrefix + assert.Equal(t, "/metrics", extra.Path) + assert.Contains(t, extra.RelabelConfigs, &vmv1beta1.RelabelConfig{ + SourceLabels: []string{"job"}, + TargetLabel: "job", + Regex: vmv1beta1.StringOrArray{"(.+)"}, + Replacement: ptr.To("${1}-reloader-http"), + }) +} + func TestVMServiceScrapeObjectsAddVictoriaMetricsAppLabel(t *testing.T) { objectMeta := metav1.ObjectMeta{Name: "test", Namespace: "default"} diff --git a/internal/controller/operator/factory/k8stools/client_utils.go b/internal/controller/operator/factory/k8stools/client_utils.go index 180dc2040..2765ada61 100644 --- a/internal/controller/operator/factory/k8stools/client_utils.go +++ b/internal/controller/operator/factory/k8stools/client_utils.go @@ -93,7 +93,7 @@ func UpdatePodAnnotations(ctx context.Context, rclient client.Client, selector m if err != nil { return fmt.Errorf("failed to list pod items: %w", err) } - updateTime := time.Now().Format("2006-01-02T15-04-05") + updateTime := time.Now().Format(time.RFC3339Nano) pt := client.RawPatch(types.MergePatchType, fmt.Appendf(nil, `{"metadata": {"annotations": {"configmap-sync-lastupdate-at": "%s"} } }`, updateTime)) for _, pod := range podsToUpdate.Items { diff --git a/internal/controller/operator/factory/reconcile/config_reload.go b/internal/controller/operator/factory/reconcile/config_reload.go new file mode 100644 index 000000000..c2f1e7ade --- /dev/null +++ b/internal/controller/operator/factory/reconcile/config_reload.go @@ -0,0 +1,115 @@ +package reconcile + +import ( + "context" + "fmt" + "net/http" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" + "github.com/VictoriaMetrics/operator/internal/podutil" +) + +// configReloadWaitable is implemented by CRD types that support verifying a +// config reload was actually picked up by every replica, via their own +// config-reload-completion metric. +type configReloadWaitable interface { + client.Object + GetReloaderParams() *vmv1beta1.CommonConfigReloaderParams + GetConfigReloadMetricNames(assets ...vmv1beta1.ConfigReloadAsset) []string + GetMetricsURL(host string) string + SelectorLabels() map[string]string +} + +var ( + configReloadWaitInterval = 2 * time.Second + configReloadWaitTimeout = 60 * time.Second + configReloadHTTPTimeout = 5 * time.Second +) + +// WaitForConfigReload verifies, by polling each ready replica's own +// config-reload-completion metric, that a config change made at or after +// `after` has actually been picked up by the running process - not just +// that the merged config was written to a Secret. It's a no-op unless +// WaitForConfigReload is enabled on the CR and the CR type reports a +// config-reload-completion metric. assets scopes the wait to specific config +// assets; with none given, metrics for every asset are polled. +func WaitForConfigReload(ctx context.Context, rclient client.Client, cr configReloadWaitable, after time.Time, assets ...vmv1beta1.ConfigReloadAsset) error { + if !ptr.Deref(cr.GetReloaderParams().WaitForConfigReload, false) { + return nil + } + metricNames := cr.GetConfigReloadMetricNames(assets...) + if len(metricNames) == 0 { + return nil + } + httpClient := &http.Client{Timeout: configReloadHTTPTimeout} + // Config-reload-completion metrics only have whole-second precision, so round up to + // the next whole second - otherwise a stale reload from just before `after`, but in + // the same wall-clock second, would be indistinguishable from a fresh one. + afterUnix := after.Unix() + if after.Nanosecond() > 0 { + afterUnix++ + } + selector := labels.SelectorFromSet(cr.SelectorLabels()) + + err := wait.PollUntilContextTimeout(ctx, configReloadWaitInterval, configReloadWaitTimeout, true, func(ctx context.Context) (bool, error) { + var podList corev1.PodList + if err := rclient.List(ctx, &podList, &client.ListOptions{ + LabelSelector: selector, + Namespace: cr.GetNamespace(), + }); err != nil { + return false, fmt.Errorf("cannot list pods for %s/%s: %w", cr.GetNamespace(), cr.GetName(), err) + } + if len(podList.Items) == 0 { + return false, nil + } + for _, pod := range podList.Items { + if !pod.DeletionTimestamp.IsZero() || !PodIsReady(&pod, 0) || pod.Status.PodIP == "" { + return false, nil + } + latest, err := scrapeReloadMetrics(ctx, httpClient, cr.GetMetricsURL(pod.Status.PodIP), metricNames) + if err != nil { + // transient scrape failure - keep polling rather than failing the wait outright + return false, nil + } + for _, name := range metricNames { + if latest[name] < float64(afterUnix) { + return false, nil + } + } + } + return true, nil + }) + if err != nil { + return fmt.Errorf("timed out waiting for %s/%s to reload config: %w", cr.GetNamespace(), cr.GetName(), err) + } + return nil +} + +// scrapeReloadMetrics fetches the given /metrics URL and returns, for each of the given +// metric names, its latest sample value. +func scrapeReloadMetrics(ctx context.Context, httpClient *http.Client, url string, metricNames []string) (map[string]float64, error) { + queries := make([]podutil.MetricQuery, len(metricNames)) + for i, name := range metricNames { + queries[i] = podutil.MetricQuery{Name: name} + } + metrics, err := podutil.FetchMetricsValues(ctx, httpClient, url, queries) + if err != nil { + return nil, err + } + latest := make(map[string]float64, len(metricNames)) + for name, values := range metrics { + for _, v := range values { + if v > latest[name] { + latest[name] = v + } + } + } + return latest, nil +} diff --git a/internal/controller/operator/factory/reconcile/secret.go b/internal/controller/operator/factory/reconcile/secret.go index 1e9fcbb09..6bc5a02f1 100644 --- a/internal/controller/operator/factory/reconcile/secret.go +++ b/internal/controller/operator/factory/reconcile/secret.go @@ -14,12 +14,14 @@ import ( "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" ) -// Secret reconciles secret object -func Secret(ctx context.Context, rclient client.Client, newObj *corev1.Secret, prevMeta *metav1.ObjectMeta, owner *metav1.OwnerReference) error { +// Secret reconciles secret object. The returned bool reports whether the Secret was actually +// created or updated (false if reconciliation found nothing to change). +func Secret(ctx context.Context, rclient client.Client, newObj *corev1.Secret, prevMeta *metav1.ObjectMeta, owner *metav1.OwnerReference) (bool, error) { nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + updated := true removeFinalizer := true rclient.Scheme().Default(newObj) - return retryOnConflict(func() error { + err := retryOnConflict(func() error { var existingObj corev1.Secret if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { @@ -41,10 +43,12 @@ func Secret(ctx context.Context, rclient client.Client, newObj *corev1.Secret, p logMessageMetadata = append(logMessageMetadata, fmt.Sprintf("data_changed=%t", !isDataEqual)) if !needsUpdate { + updated = false return nil } existingObj.Data = newObj.Data logger.WithContext(ctx).Info(fmt.Sprintf("updating Secret %s", strings.Join(logMessageMetadata, ", "))) return rclient.Update(ctx, &existingObj) }) + return updated, err } diff --git a/internal/controller/operator/factory/reconcile/secret_test.go b/internal/controller/operator/factory/reconcile/secret_test.go index c6bf6d9b6..90444e6b2 100644 --- a/internal/controller/operator/factory/reconcile/secret_test.go +++ b/internal/controller/operator/factory/reconcile/secret_test.go @@ -20,6 +20,7 @@ func TestSecretReconcile(t *testing.T) { prevMeta *metav1.ObjectMeta predefinedObjects []runtime.Object actions []k8stools.ClientAction + wantChanged bool } getSecret := func(fns ...func(s *corev1.Secret)) *corev1.Secret { s := &corev1.Secret{ @@ -42,7 +43,9 @@ func TestSecretReconcile(t *testing.T) { ctx := context.Background() cl := k8stools.GetTestClientWithActions(o.predefinedObjects) synctest.Test(t, func(t *testing.T) { - assert.NoError(t, Secret(ctx, cl, o.new, o.prevMeta, nil)) + changed, err := Secret(ctx, cl, o.new, o.prevMeta, nil) + assert.NoError(t, err) + assert.Equal(t, o.wantChanged, changed) assert.Equal(t, o.actions, cl.Actions) }) } @@ -56,6 +59,7 @@ func TestSecretReconcile(t *testing.T) { {Verb: "Get", Kind: "Secret", Resource: nn}, {Verb: "Create", Kind: "Secret", Resource: nn}, }, + wantChanged: true, }) // no updates @@ -82,6 +86,7 @@ func TestSecretReconcile(t *testing.T) { {Verb: "Get", Kind: "Secret", Resource: nn}, {Verb: "Update", Kind: "Secret", Resource: nn}, }, + wantChanged: true, }) // data key added @@ -97,6 +102,7 @@ func TestSecretReconcile(t *testing.T) { {Verb: "Get", Kind: "Secret", Resource: nn}, {Verb: "Update", Kind: "Secret", Resource: nn}, }, + wantChanged: true, }) // data key removed @@ -112,5 +118,6 @@ func TestSecretReconcile(t *testing.T) { {Verb: "Get", Kind: "Secret", Resource: nn}, {Verb: "Update", Kind: "Secret", Resource: nn}, }, + wantChanged: true, }) } diff --git a/internal/controller/operator/factory/vlcluster/vmauth_lb.go b/internal/controller/operator/factory/vlcluster/vmauth_lb.go index 2dd0d1e83..1423541d9 100644 --- a/internal/controller/operator/factory/vlcluster/vmauth_lb.go +++ b/internal/controller/operator/factory/vlcluster/vmauth_lb.go @@ -31,7 +31,7 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) } owner := cr.AsOwner() - if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta, &owner); err != nil { + if _, err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta, &owner); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMauthLBDeployment(cr) diff --git a/internal/controller/operator/factory/vmagent/rbac.go b/internal/controller/operator/factory/vmagent/rbac.go index 20039cf32..f37e4014b 100644 --- a/internal/controller/operator/factory/vmagent/rbac.go +++ b/internal/controller/operator/factory/vmagent/rbac.go @@ -38,7 +38,7 @@ func getScrapeDiscoveryRules() []rbacv1.PolicyRule { // It includes a scoped secrets rule for the config-reloader plus the SD discovery rules. func getSingleNamespaceRules(cr *vmv1beta1.VMAgent) []rbacv1.PolicyRule { ingestOnly := ptr.Deref(cr.Spec.IngestOnlyMode, false) - if ingestOnly && !hasRemoteWriteSecrets(cr) { + if ingestOnly && !cr.HasRemoteWriteSecrets() { return nil } secretsRule := rbacv1.PolicyRule{ diff --git a/internal/controller/operator/factory/vmagent/scrapes_test.go b/internal/controller/operator/factory/vmagent/scrapes_test.go index 938a92ad2..eb734b37e 100644 --- a/internal/controller/operator/factory/vmagent/scrapes_test.go +++ b/internal/controller/operator/factory/vmagent/scrapes_test.go @@ -40,7 +40,7 @@ func TestCreateOrUpdateScrapeConfig(t *testing.T) { } build.AddDefaults(testClient.Scheme()) ac := getAssetsCache(ctx, testClient, o.cr) - _, err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, nil, ac) + _, _, err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, nil, ac) assert.NoError(t, err) var expectSecret corev1.Secret assert.NoError(t, testClient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &expectSecret)) @@ -2164,7 +2164,7 @@ scrape_configs: [] }, } ac := getAssetsCache(ctx, testClient, cr) - _, err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, nil, ac) + _, _, err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, nil, ac) assert.NoError(t, err) var configSecret corev1.Secret assert.NoError(t, testClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cr.PrefixedName()}, &configSecret)) diff --git a/internal/controller/operator/factory/vmagent/vmagent.go b/internal/controller/operator/factory/vmagent/vmagent.go index bb520f777..97258f64b 100644 --- a/internal/controller/operator/factory/vmagent/vmagent.go +++ b/internal/controller/operator/factory/vmagent/vmagent.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" "sync" + "time" "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" @@ -54,14 +55,22 @@ func buildVMAgentServiceScrape(cr *vmv1beta1.VMAgent, svc *corev1.Service) *vmv1 if cr == nil || svc == nil || ptr.Deref(cr.Spec.DisableSelfServiceScrape, false) { return nil } - return build.VMServiceScrape(svc, cr) + scrape := build.VMServiceScrape(svc, cr) + if cr.HasConfigReloader() { + scrape.Spec.Endpoints = append(scrape.Spec.Endpoints, build.ConfigReloaderVMServiceScrapeEndpoint()) + } + return scrape } func buildVMAgentPodScrape(cr *vmv1beta1.VMAgent) *vmv1beta1.VMPodScrape { if cr == nil || ptr.Deref(cr.Spec.DisableSelfServiceScrape, false) { return nil } - return build.VMPodScrape(cr, "http") + scrape := build.VMPodScrape(cr, "http") + if cr.HasConfigReloader() { + scrape.Spec.PodMetricsEndpoints = append(scrape.Spec.PodMetricsEndpoints, build.ConfigReloaderPodScrapeEndpoint()) + } + return scrape } func createOrUpdateService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent) error { @@ -140,7 +149,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.C if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA, &owner); err != nil { return fmt.Errorf("failed create service account: %w", err) } - if !ptr.Deref(cr.Spec.IngestOnlyMode, false) || hasRemoteWriteSecrets(cr) { + if !ptr.Deref(cr.Spec.IngestOnlyMode, false) || cr.HasRemoteWriteSecrets() { if err := createK8sAPIAccess(ctx, rclient, cr, prevCR, cfg.WatchNamespaces); err != nil { return fmt.Errorf("cannot create vmagent role and binding for it, err: %w", err) } @@ -162,19 +171,40 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.C } ac := getAssetsCache(ctx, rclient, cr) - extraCount, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, nil, ac) + writeStartedAt := time.Now() + extraCount, mainChanged, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, nil, ac) if err != nil { return err } - if err := createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, prevCR, ac); err != nil { + relabelChanged, err := createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, prevCR, ac) + if err != nil { return fmt.Errorf("cannot update relabeling asset for vmagent: %w", err) } - if err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac); err != nil { + streamAggrChanged, err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac) + if err != nil { return fmt.Errorf("cannot update stream aggregation config for vmagent: %w", err) } + // wait once for every asset that actually changed, instead of stacking a separate + // up-to-timeout wait per asset. + var changedAssets []vmv1beta1.ConfigReloadAsset + if mainChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetMain) + } + if relabelChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetRelabel) + } + if streamAggrChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetStreamAggr) + } + if len(changedAssets) > 0 { + if err := reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, changedAssets...); err != nil { + return err + } + } + newAppTpl, err := newK8sApp(cr, ac, extraCount) if err != nil { return fmt.Errorf("cannot build new deploy for vmagent: %w", err) @@ -592,7 +622,7 @@ func newPodSpec(cr *vmv1beta1.VMAgent, ac *build.AssetsCache, extraConfigSecretC } ingestOnly := ptr.Deref(cr.Spec.IngestOnlyMode, false) - needsConfigSecret := !ingestOnly || hasRemoteWriteSecrets(cr) + needsConfigSecret := !ingestOnly || cr.HasRemoteWriteSecrets() if !ingestOnly { args = append(args, @@ -738,7 +768,7 @@ func newPodSpec(cr *vmv1beta1.VMAgent, ac *build.AssetsCache, extraConfigSecretC var operatorContainers []corev1.Container var ic []corev1.Container // conditional add config reloader container - if !ingestOnly || cr.HasAnyRelabellingConfigs() || cr.HasAnyStreamAggrRule() || hasRemoteWriteSecrets(cr) { + if cr.HasConfigReloader() { var ss *corev1.SecretKeySelector if needsConfigSecret { ss = &corev1.SecretKeySelector{ @@ -890,22 +920,22 @@ func buildRelabelingsAssets(cr *vmv1beta1.VMAgent, ac *build.AssetsCache) (*core return cfgCM, nil } -// createOrUpdateRelabelConfigsAssets builds relabeling configs for vmagent at separate configmap, serialized as yaml -func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) error { +// createOrUpdateRelabelConfigsAssets builds relabeling configs for vmagent at separate configmap, serialized as yaml. +// It reports whether the configmap content changed, leaving the caller to wait for config reload. +func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) (bool, error) { if !cr.HasAnyRelabellingConfigs() { - return nil + return false, nil } assetsCM, err := buildRelabelingsAssets(cr, ac) if err != nil { - return err + return false, err } var prevConfigMeta *metav1.ObjectMeta if prevCR != nil { prevConfigMeta = ptr.To(build.ResourceMeta(build.RelabelConfigResourceKind, prevCR)) } owner := cr.AsOwner() - _, err = reconcile.ConfigMap(ctx, rclient, assetsCM, prevConfigMeta, &owner) - return err + return reconcile.ConfigMap(ctx, rclient, assetsCM, prevConfigMeta, &owner) } // buildStreamAggrConfig combines all possible stream aggregation configs and adding it to the configmap. @@ -965,23 +995,23 @@ func buildStreamAggrConfig(cr *vmv1beta1.VMAgent, ac *build.AssetsCache) (*corev return cfgCM, nil } -// createOrUpdateStreamAggrConfig builds stream aggregation configs for vmagent at separate configmap, serialized as yaml -func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) error { +// createOrUpdateStreamAggrConfig builds stream aggregation configs for vmagent at separate configmap, serialized as yaml. +// It reports whether the configmap content changed, leaving the caller to wait for config reload. +func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) (bool, error) { // fast path if !cr.HasAnyStreamAggrRule() { - return nil + return false, nil } streamAggrCM, err := buildStreamAggrConfig(cr, ac) if err != nil { - return err + return false, err } var prevConfigMeta *metav1.ObjectMeta if prevCR != nil { - prevConfigMeta = ptr.To(build.ResourceMeta(build.StreamAggrConfigResourceKind, cr)) + prevConfigMeta = ptr.To(build.ResourceMeta(build.StreamAggrConfigResourceKind, prevCR)) } owner := cr.AsOwner() - _, err = reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevConfigMeta, &owner) - return err + return reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevConfigMeta, &owner) } type item struct { @@ -999,21 +1029,6 @@ func sortMap(m map[string]string) []item { return kv } -func hasRemoteWriteSecrets(cr *vmv1beta1.VMAgent) bool { - for _, rw := range cr.Spec.RemoteWrite { - if rw.BasicAuth != nil && len(rw.BasicAuth.Password.Name) > 0 { - return true - } - if rw.BearerTokenSecret != nil && rw.BearerTokenSecret.Name != "" { - return true - } - if rw.OAuth2 != nil && rw.OAuth2.ClientSecret != nil { - return true - } - } - return false -} - func buildRemoteWriteArgs(cr *vmv1beta1.VMAgent, ac *build.AssetsCache) ([]string, error) { maxDiskUsage := defaultMaxDiskUsage var queues int32 @@ -1345,23 +1360,31 @@ func CreateOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr * prevCR.Spec = *cr.Status.LastAppliedSpec } ac := getAssetsCache(ctx, rclient, cr) - if _, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, childObject, ac); err != nil { + writeStartedAt := time.Now() + _, changed, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, childObject, ac) + if err != nil { return err } + if changed { + return reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, vmv1beta1.ConfigReloadAssetMain) + } return nil } -func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, childObject client.Object, ac *build.AssetsCache) (int, error) { +// createOrUpdateScrapeConfig reconciles the main scrape config and reports whether it changed, +// leaving the caller to wait for config reload. +func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, childObject client.Object, ac *build.AssetsCache) (int, bool, error) { if ptr.Deref(cr.Spec.IngestOnlyMode, false) { - if !hasRemoteWriteSecrets(cr) { - return 0, nil + if !cr.HasRemoteWriteSecrets() { + return 0, false, nil } // ingest-only mode with remote write credentials: reconcile a credential-only secret // so vmagent can read credentials from files and the config-reloader can watch for changes. if _, err := buildRemoteWriteArgs(cr, ac); err != nil { - return 0, fmt.Errorf("cannot load remote write assets for ingest-only mode: %w", err) + return 0, false, fmt.Errorf("cannot load remote write assets for ingest-only mode: %w", err) } owner := cr.AsOwner() + var changed bool for kind, secret := range ac.GetOutput() { var prevSecretMeta *metav1.ObjectMeta if prevCR != nil { @@ -1371,24 +1394,31 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, // placeholder gzipped config so config-reloader finds its expected key d, err := build.GzipConfig([]byte{}) if err != nil { - return 0, fmt.Errorf("cannot gzip placeholder config: %w", err) + return 0, false, fmt.Errorf("cannot gzip placeholder config: %w", err) } secret.Data[scrapeGzippedFilename] = d } secret.ObjectMeta = build.ResourceMeta(kind, cr) secret.Annotations = map[string]string{"generated": "true"} - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner); err != nil { - return 0, err + secretChanged, err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner) + if err != nil { + return 0, false, err + } + // TLS assets aren't part of the config-reloader-watched scrape config, and vmagent's + // own reload-completion metrics only advance when the actual scrape config content + // changes - counting a TLS-only change here would wait on a metric that never moves. + if kind == build.SecretConfigResourceKind { + changed = changed || secretChanged } } - return 0, nil + return 0, changed, nil } // HACK: newPodSpec could load content into ac and it must be called // before secret config reconcile // // TODO: @f41gh7 rewrite this section with VLAgent secret assets injection pattern if _, err := newPodSpec(cr, ac, 0); err != nil { - return 0, err + return 0, false, err } pos := &vmscrapes.ParsedObjects{ @@ -1406,20 +1436,20 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, } sp := &cr.Spec.CommonScrapeParams if err := pos.Init(ctx, rclient, sp); err != nil { - return 0, err + return 0, false, err } pos.ValidateObjects(sp) // Build the global config section and the flat list of individual scrape job configs. cfgBase, jobs, err := pos.BuildScrapeJobsConfig(ctx, sp, ac) if err != nil { - return 0, fmt.Errorf("generating config for vmagent failed: %w", err) + return 0, false, fmt.Errorf("generating config for vmagent failed: %w", err) } // Split jobs into gzip-compressed buckets, each fitting within the Kubernetes Secret limit. buckets, err := build.PackItems(jobs, config.MustGetBaseConfig().ConfigDataBudgetBytes, 150) if err != nil { - return 0, fmt.Errorf("splitting scrape config into buckets: %w", err) + return 0, false, fmt.Errorf("splitting scrape config into buckets: %w", err) } extraCount := len(buckets) - 1 @@ -1434,7 +1464,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, mainCfg = append(mainCfg, yaml.MapItem{Key: "scrape_configs", Value: buckets[0]}) generatedConfig, err := yaml.Marshal(mainCfg) if err != nil { - return 0, fmt.Errorf("marshalling config for vmagent: %w", err) + return 0, false, fmt.Errorf("marshalling config for vmagent: %w", err) } owner := cr.AsOwner() @@ -1447,6 +1477,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, return keys[i] < keys[j] }) + var changed bool for _, kind := range keys { secret := secrets[kind] var prevSecretMeta *metav1.ObjectMeta @@ -1456,7 +1487,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, if kind == build.SecretConfigResourceKind { d, err := build.GzipConfig(generatedConfig) if err != nil { - return 0, fmt.Errorf("cannot gzip config for vmagent: %w", err) + return 0, false, fmt.Errorf("cannot gzip config for vmagent: %w", err) } secret.Data[scrapeGzippedFilename] = d } @@ -1464,8 +1495,15 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, secret.Annotations = map[string]string{ "generated": "true", } - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner); err != nil { - return 0, err + secretChanged, err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner) + if err != nil { + return 0, false, err + } + // TLS assets aren't part of the config-reloader-watched scrape config, and vmagent's + // own reload-completion metrics only advance when the actual scrape config content + // changes - counting a TLS-only change here would wait on a metric that never moves. + if kind == build.SecretConfigResourceKind { + changed = changed || secretChanged } } @@ -1476,29 +1514,31 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, idx := i + 1 extraData, err := yaml.Marshal(bucket) if err != nil { - return 0, fmt.Errorf("marshalling extra scrape config bucket %d: %w", idx, err) + return 0, false, fmt.Errorf("marshalling extra scrape config bucket %d: %w", idx, err) } compressed, err := build.GzipConfig(extraData) if err != nil { - return 0, fmt.Errorf("gzipping extra scrape config bucket %d: %w", idx, err) + return 0, false, fmt.Errorf("gzipping extra scrape config bucket %d: %w", idx, err) } s := vmscrapes.BuildExtraConfigSecret(cr, idx, compressed) - if err := reconcile.Secret(ctx, rclient, s, nil, &owner); err != nil { - return 0, err + secretChanged, err := reconcile.Secret(ctx, rclient, s, nil, &owner) + if err != nil { + return 0, false, err } + changed = changed || secretChanged } // Remove any stale extra secrets left over from a previous reconcile with more buckets. if err := vmscrapes.RemoveStaleExtraConfigSecrets(ctx, rclient, cr, extraCount); err != nil { - return 0, err + return 0, false, err } parentName := fmt.Sprintf("%s.%s.vmagent", cr.Name, cr.Namespace) if err := pos.UpdateStatusesForScrapeObjects(ctx, rclient, parentName, childObject); err != nil { - return 0, err + return 0, false, err } - return extraCount, nil + return extraCount, changed, nil } func createOrUpdateHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent) error { diff --git a/internal/controller/operator/factory/vmagent/vmagent_test.go b/internal/controller/operator/factory/vmagent/vmagent_test.go index 95f5d71bc..72f309d6c 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_test.go @@ -2031,6 +2031,7 @@ func TestCreateOrUpdateService(t *testing.T) { }, validate: func(svc *corev1.Service) { assert.Equal(t, "vmagent-base", svc.Name) + // http + influx-tcp + influx-udp assert.Len(t, svc.Spec.Ports, 3) assert.Equal(t, map[string]string{ @@ -2087,6 +2088,7 @@ func TestCreateOrUpdateService(t *testing.T) { }, validate: func(svc *corev1.Service) { assert.Equal(t, "vmagent-base", svc.Name) + // http + influx-tcp + influx-udp assert.Len(t, svc.Spec.Ports, 3) assert.Equal(t, map[string]string{ @@ -2105,6 +2107,42 @@ func TestCreateOrUpdateService(t *testing.T) { }) } +func TestBuildVMAgentServiceScrapeConfigReloaderPort(t *testing.T) { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "vmagent-base"}, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Name: "http"}, + }, + }, + } + + hasReloaderEndpoint := func(scrape *vmv1beta1.VMServiceScrape) bool { + for _, ep := range scrape.Spec.Endpoints { + if ep.TargetPort != nil && ep.TargetPort.IntValue() == build.ConfigReloaderDefaultPort { + return true + } + } + return false + } + + // config-reloader present (default: not ingest-only): scrape includes its endpoint + withReloader := &vmv1beta1.VMAgent{ObjectMeta: metav1.ObjectMeta{Name: "base", Namespace: "default"}} + scrape := buildVMAgentServiceScrape(withReloader, svc) + assert.True(t, hasReloaderEndpoint(scrape)) + + // ingest-only mode with no relabel/streamAggr/remoteWrite secrets: no config-reloader, + // so the scrape must not reference its endpoint + withoutReloader := &vmv1beta1.VMAgent{ + ObjectMeta: metav1.ObjectMeta{Name: "base", Namespace: "default"}, + Spec: vmv1beta1.VMAgentSpec{ + CommonScrapeParams: vmv1beta1.CommonScrapeParams{IngestOnlyMode: ptr.To(true)}, + }, + } + scrape = buildVMAgentServiceScrape(withoutReloader, svc) + assert.False(t, hasReloaderEndpoint(scrape)) +} + func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { type opts struct { cr *vmv1beta1.VMAgent @@ -2116,7 +2154,8 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - assert.NoError(t, createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, nil, ac)) + _, err := createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, nil, ac) + assert.NoError(t, err) var createdCM corev1.ConfigMap assert.NoError(t, cl.Get(ctx, types.NamespacedName{ Namespace: o.cr.Namespace, @@ -2216,7 +2255,8 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - assert.NoError(t, createOrUpdateStreamAggrConfig(ctx, cl, o.cr, nil, ac)) + _, err := createOrUpdateStreamAggrConfig(ctx, cl, o.cr, nil, ac) + assert.NoError(t, err) var createdCM corev1.ConfigMap assert.NoError(t, cl.Get(ctx, types.NamespacedName{ @@ -2394,6 +2434,52 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { }) } +func TestCreateOrUpdateStreamAggrConfigRemovesStaleManagedLabel(t *testing.T) { + newCR := func(managedLabels map[string]string) *vmv1beta1.VMAgent { + return &vmv1beta1.VMAgent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default-vmagent", + Namespace: "default", + }, + Spec: vmv1beta1.VMAgentSpec{ + ManagedMetadata: &vmv1beta1.ManagedObjectsMetadata{ + Labels: managedLabels, + }, + RemoteWrite: []vmv1beta1.VMAgentRemoteWriteSpec{ + { + URL: "localhost:8429", + StreamAggrConfig: &vmv1beta1.StreamAggrConfig{ + Rules: []vmv1beta1.StreamAggrRule{{ + Interval: "1m", + Outputs: []string{"total"}, + }}, + }, + }, + }, + }, + } + } + + ctx := context.TODO() + cl := k8stools.GetTestClientWithObjects(nil) + ac := build.NewAssetsCache(ctx, cl, nil) + + prevCR := newCR(map[string]string{"stale-label": "should-be-removed"}) + _, err := createOrUpdateStreamAggrConfig(ctx, cl, prevCR, nil, ac) + assert.NoError(t, err) + + var cm corev1.ConfigMap + nsn := types.NamespacedName{Namespace: prevCR.Namespace, Name: build.ResourceName(build.StreamAggrConfigResourceKind, prevCR)} + assert.NoError(t, cl.Get(ctx, nsn, &cm)) + assert.Contains(t, cm.Labels, "stale-label") + + cr := newCR(nil) + _, err = createOrUpdateStreamAggrConfig(ctx, cl, cr, prevCR, ac) + assert.NoError(t, err) + assert.NoError(t, cl.Get(ctx, nsn, &cm)) + assert.NotContains(t, cm.Labels, "stale-label") +} + func TestMakeSpecForAgentOk(t *testing.T) { type opts struct { cr *vmv1beta1.VMAgent diff --git a/internal/controller/operator/factory/vmalert/rules.go b/internal/controller/operator/factory/vmalert/rules.go index ba2d1db1c..a86297880 100644 --- a/internal/controller/operator/factory/vmalert/rules.go +++ b/internal/controller/operator/factory/vmalert/rules.go @@ -6,6 +6,7 @@ import ( "hash/fnv" "sort" "strconv" + "time" "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" @@ -55,8 +56,11 @@ func reconcileConfigsData(ctx context.Context, rclient client.Client, cr *vmv1be if needReload { // trigger sync for configmap logger.WithContext(ctx).Info("triggering pod config reload by changing annotation") + writeStartedAt := time.Now() if err := k8stools.UpdatePodAnnotations(ctx, rclient, cr.PodLabels(), cr.Namespace); err != nil { logger.WithContext(ctx).Error(err, "failed to update vmalert pod cm-sync annotation") + } else if err := reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, vmv1beta1.ConfigReloadAssetMain); err != nil { + return nil, err } } return newConfigMapNames, nil diff --git a/internal/controller/operator/factory/vmalert/vmalert.go b/internal/controller/operator/factory/vmalert/vmalert.go index 5a9b294f6..555128fe8 100644 --- a/internal/controller/operator/factory/vmalert/vmalert.go +++ b/internal/controller/operator/factory/vmalert/vmalert.go @@ -43,7 +43,11 @@ func buildScrape(cr *vmv1beta1.VMAlert, svc *corev1.Service) *vmv1beta1.VMServic if cr == nil || svc == nil || ptr.Deref(cr.Spec.DisableSelfServiceScrape, false) { return nil } - return build.VMServiceScrape(svc, cr) + scrape := build.VMServiceScrape(svc, cr) + if cr.HasConfigReloader() { + scrape.Spec.Endpoints = append(scrape.Spec.Endpoints, build.ConfigReloaderVMServiceScrapeEndpoint()) + } + return scrape } // createOrUpdateService creates service for vmalert @@ -321,7 +325,7 @@ func newPodSpec(cr *vmv1beta1.VMAlert, ruleConfigMapNames []string, ac *build.As vmalertContainers = append(vmalertContainers, vmalertContainer) var initContainers []corev1.Container - if !cr.IsUnmanaged() { + if cr.HasConfigReloader() { crc := build.ConfigReloaderContainer(false, cr, crMounts, nil) // rules-out write-side: not in crMounts to avoid --watched-dir causing reload loops. crc.VolumeMounts = append(crc.VolumeMounts, corev1.VolumeMount{ @@ -599,7 +603,11 @@ func createOrUpdateAssets(ctx context.Context, rclient client.Client, cr, prevCR if prevCR != nil { prevSecretMeta = ptr.To(build.ResourceMeta(kind, prevCR)) } - err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner) + // changed is intentionally discarded: unlike rules.go, there's no config-reload + // metric that reflects these TLS/notifier assets specifically - vmalert only bumps + // vmalert_config_last_reload_success_timestamp_seconds when rule content changes, so + // waiting on it here would time out on every credential rotation with no rule change. + _, err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner) if err != nil { return err } diff --git a/internal/controller/operator/factory/vmalertmanager/statefulset.go b/internal/controller/operator/factory/vmalertmanager/statefulset.go index 8354c3521..8a07c14cc 100644 --- a/internal/controller/operator/factory/vmalertmanager/statefulset.go +++ b/internal/controller/operator/factory/vmalertmanager/statefulset.go @@ -8,6 +8,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/Masterminds/semver/v3" appsv1 "k8s.io/api/apps/v1" @@ -116,7 +117,10 @@ func buildScrape(cr *vmv1beta1.VMAlertmanager, svc *corev1.Service) *vmv1beta1.V if cr == nil || svc == nil || ptr.Deref(cr.Spec.DisableSelfServiceScrape, false) { return nil } - return build.VMServiceScrape(svc, cr) + scrape := build.VMServiceScrape(svc, cr) + // vmalertmanager always runs with a config-reloader sidecar + scrape.Spec.Endpoints = append(scrape.Spec.Endpoints, build.ConfigReloaderVMServiceScrapeEndpoint()) + return scrape } func createOrUpdateAlertManagerService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAlertmanager) error { @@ -544,9 +548,16 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be } owner := cr.AsOwner() - if err := reconcile.Secret(ctx, rclient, newAMSecretConfig, prevSecretMeta, &owner); err != nil { + writeStartedAt := time.Now() + changed, err := reconcile.Secret(ctx, rclient, newAMSecretConfig, prevSecretMeta, &owner) + if err != nil { return err } + if changed { + if err := reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, vmv1beta1.ConfigReloadAssetMain); err != nil { + return err + } + } parent := fmt.Sprintf("%s.%s.vmalertmanager", cr.Name, cr.Namespace) diff --git a/internal/controller/operator/factory/vmanomaly/config.go b/internal/controller/operator/factory/vmanomaly/config.go index fb34b8e52..ebdbb470c 100644 --- a/internal/controller/operator/factory/vmanomaly/config.go +++ b/internal/controller/operator/factory/vmanomaly/config.go @@ -51,7 +51,7 @@ func createOrUpdateConfig(ctx context.Context, rclient client.Client, cr, prevCR prevSecretMeta = ptr.To(build.ResourceMeta(kind, prevCR)) } secret.ObjectMeta = build.ResourceMeta(kind, cr) - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner); err != nil { + if _, err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner); err != nil { return err } } @@ -61,7 +61,7 @@ func createOrUpdateConfig(ctx context.Context, rclient client.Client, cr, prevCR prevSecretMeta = ptr.To(build.ResourceMeta(build.SecretConfigResourceKind, prevCR)) } - if err := reconcile.Secret(ctx, rclient, newSecretConfig, prevSecretMeta, &owner); err != nil { + if _, err := reconcile.Secret(ctx, rclient, newSecretConfig, prevSecretMeta, &owner); err != nil { return err } diff --git a/internal/controller/operator/factory/vmauth/vmauth.go b/internal/controller/operator/factory/vmauth/vmauth.go index 501e56afa..ba12bacdd 100644 --- a/internal/controller/operator/factory/vmauth/vmauth.go +++ b/internal/controller/operator/factory/vmauth/vmauth.go @@ -6,7 +6,9 @@ import ( "maps" "path" "sort" + "time" + "github.com/cespare/xxhash/v2" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -419,6 +421,8 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be creds := ac.GetOutput() if secret, ok := creds[build.TLSAssetsResourceKind]; ok { maps.Copy(s.Data, secret.Data) + // forces vmauth's byte-comparison reload check to notice TLS cert/key rotations too + generatedConfig = append(fmt.Appendf(nil, "# tls-assets-hash: %x\n", tlsAssetsHash(secret.Data)), generatedConfig...) } data, err := build.GzipConfig(generatedConfig) @@ -431,9 +435,16 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be prevSecretMeta = ptr.To(buildConfigSecretMeta(prevCR)) } owner := cr.AsOwner() - if err := reconcile.Secret(ctx, rclient, s, prevSecretMeta, &owner); err != nil { + writeStartedAt := time.Now() + changed, err := reconcile.Secret(ctx, rclient, s, prevSecretMeta, &owner) + if err != nil { return err } + if changed { + if err := reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, vmv1beta1.ConfigReloadAssetMain); err != nil { + return err + } + } pos.users.UpdateMetrics(ctx) parentObject := fmt.Sprintf("%s.%s.vmauth", cr.GetName(), cr.GetNamespace()) @@ -448,6 +459,22 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be return nil } +func tlsAssetsHash(data map[string][]byte) uint64 { + keys := make([]string, 0, len(data)) + for k := range data { + keys = append(keys, k) + } + sort.Strings(keys) + h := xxhash.New() + for _, k := range keys { + h.Write([]byte(k)) //nolint:errcheck + h.Write([]byte{0}) //nolint:errcheck + h.Write(data[k]) //nolint:errcheck + h.Write([]byte{0}) //nolint:errcheck + } + return h.Sum64() +} + func buildConfigSecretMeta(cr *vmv1beta1.VMAuth) metav1.ObjectMeta { return metav1.ObjectMeta{ Name: cr.ConfigSecretName(), @@ -673,15 +700,17 @@ func buildScrape(cr *vmv1beta1.VMAuth, svc *corev1.Service) *vmv1beta1.VMService return nil } b := build.VMServiceScrape(svc, cr) - if len(cr.Spec.InternalListenPort) == 0 { - return b - } - for idx := range b.Spec.Endpoints { - ep := &b.Spec.Endpoints[idx] - if ep.Port == "http" { - ep.Port = internalPortName - break + if len(cr.Spec.InternalListenPort) > 0 { + for idx := range b.Spec.Endpoints { + ep := &b.Spec.Endpoints[idx] + if ep.Port == "http" { + ep.Port = internalPortName + break + } } } + if cr.HasConfigReloader() { + b.Spec.Endpoints = append(b.Spec.Endpoints, build.ConfigReloaderVMServiceScrapeEndpoint()) + } return b } diff --git a/internal/controller/operator/factory/vmauth/vmauth_test.go b/internal/controller/operator/factory/vmauth/vmauth_test.go index d1f0d8051..06dd01d41 100644 --- a/internal/controller/operator/factory/vmauth/vmauth_test.go +++ b/internal/controller/operator/factory/vmauth/vmauth_test.go @@ -26,6 +26,21 @@ import ( "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" ) +func TestTLSAssetsHash(t *testing.T) { + // deterministic regardless of map iteration order + a := tlsAssetsHash(map[string][]byte{"ca.crt": []byte("ca"), "tls.crt": []byte("cert")}) + b := tlsAssetsHash(map[string][]byte{"tls.crt": []byte("cert"), "ca.crt": []byte("ca")}) + assert.Equal(t, a, b) + + // changing any value changes the hash + c := tlsAssetsHash(map[string][]byte{"ca.crt": []byte("ca"), "tls.crt": []byte("rotated")}) + assert.NotEqual(t, a, c) + + // changing keys changes the hash even with same values + d := tlsAssetsHash(map[string][]byte{"ca.crt": []byte("ca")}) + assert.NotEqual(t, a, d) +} + func TestCreateOrUpdate(t *testing.T) { type opts struct { cr *vmv1beta1.VMAuth diff --git a/internal/controller/operator/factory/vmcluster/vmcluster.go b/internal/controller/operator/factory/vmcluster/vmcluster.go index c91688479..130fbab64 100644 --- a/internal/controller/operator/factory/vmcluster/vmcluster.go +++ b/internal/controller/operator/factory/vmcluster/vmcluster.go @@ -1689,7 +1689,7 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) } owner := cr.AsOwner() - if err := reconcile.Secret(ctx, rclient, buildVMAuthLBSecret(cr), prevSecretMeta, &owner); err != nil { + if _, err := reconcile.Secret(ctx, rclient, buildVMAuthLBSecret(cr), prevSecretMeta, &owner); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMAuthLBDeployment(cr) diff --git a/internal/controller/operator/factory/vmdistributed/util.go b/internal/controller/operator/factory/vmdistributed/util.go index 2b2684e68..bf338a979 100644 --- a/internal/controller/operator/factory/vmdistributed/util.go +++ b/internal/controller/operator/factory/vmdistributed/util.go @@ -1,263 +1,13 @@ package vmdistributed import ( - "context" "fmt" - "io" - "net/http" - "strconv" - "strings" vmv1alpha1 "github.com/VictoriaMetrics/operator/api/operator/v1alpha1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" ) -func fetchMetricValues(ctx context.Context, httpClient *http.Client, url, metricName, dimension string) (map[string]float64, error) { - request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request for VMAgent at %s: %w", url, err) - } - resp, err := httpClient.Do(request) - if err != nil { - return nil, fmt.Errorf("failed to fetch metrics from VMAgent at %s: %w", url, err) - } - defer resp.Body.Close() - data, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read VMAgent metrics at %s: %w", url, err) - } - s := string(data) - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unexpected response status=%d, body=%q while requesting VMAgent metrics at %s", resp.StatusCode, s, url) - } - values := make(map[string]float64) - if err := unmarshalMetric(values, s, metricName, dimension); err != nil { - return nil, fmt.Errorf("failed to unmarshal metric=%s values at %s: %w", metricName, url, err) - } - return values, nil -} - -func unmarshalMetric(values map[string]float64, s, name, dimension string) error { - for len(s) > 0 { - line := s - if n := strings.IndexByte(s, '\n'); n >= 0 { - line = s[:n] - s = s[n+1:] - } else { - s = "" - } - line = strings.TrimSpace(line) - if len(line) == 0 || strings.HasPrefix(line, "#") { - continue - } - - n := strings.IndexAny(line, "{ \t\r") - metricName := line - if n >= 0 { - metricName = line[:n] - } - if metricName != name { - continue - } - - line = line[len(metricName):] - line = skipLeadingWhitespace(line) - var dimValue string - if len(line) > 0 && line[0] == '{' { - line = line[1:] - var err error - var tags map[string]string - line, tags, err = unmarshalTags(line) - if err != nil { - return fmt.Errorf("cannot unmarshal tags: %w", err) - } - dimValue = tags[dimension] - } - line = skipLeadingWhitespace(line) - if len(line) == 0 { - return fmt.Errorf("value cannot be empty") - } - valStr := line - n = strings.IndexAny(line, " \t\r\n") - if n >= 0 { - valStr = valStr[:n] - } - v, err := strconv.ParseFloat(valStr, 64) - if err != nil { - return fmt.Errorf("cannot parse value %q: %w", line, err) - } - if len(dimValue) > 0 { - values[dimValue] = v - } - } - return nil -} - -func unmarshalTags(s string) (string, map[string]string, error) { - tags := make(map[string]string) - var err error - for { - s = skipLeadingWhitespace(s) - n := strings.IndexByte(s, '"') - if n < 0 { - // end of tags - if len(s) > 0 && s[0] == '}' { - return s[1:], tags, nil - } - return s, tags, fmt.Errorf("missing value for tag %q", s) - } - // Determine if this is a value or quoted label - possibleKey := skipTrailingWhitespace(s[:n]) - possibleKeyLen := len(possibleKey) - key := "" - if possibleKeyLen == 0 { - // Parse quoted label - {"label"="value"} or {"metric"} - key, s, err = unmarshalQuotedString(s) - if err != nil { - return s, tags, err - } - s = skipLeadingWhitespace(s) - if len(s) > 0 { - if s[0] == ',' || s[0] == '}' { - if len(s) > 1 && s[0] == ',' { - s = s[1:] - } - continue - } else if s[0] != '=' { - // We are a quoted label that isn't preceded by a comma or at the end - // of the tags so we must have a value - return s, tags, fmt.Errorf("missing value for quoted tag %q", key) - } - s = skipLeadingWhitespace(s[1:]) - } - // Fall through to parsing value - } else { - c := possibleKey[len(possibleKey)-1] - // unquoted label {label="value"} - if c == '=' { - // Parse unquoted label - key = skipLeadingWhitespace(s[:possibleKeyLen-1]) - key = skipTrailingWhitespace(key) - s = skipLeadingWhitespace(s[possibleKeyLen:]) - } else { - // unquoted tag without a value - return s, tags, fmt.Errorf("missing value for unquoted tag %q", s) - } - } - // Parse value - var value string - value, s, err = unmarshalQuotedString(s) - if err != nil { - return s, tags, err - } - - if len(key) > 0 { - // Allow empty values (len(value)==0) - see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/453 - tags[key] = value - } - s = skipLeadingWhitespace(s) - if len(s) > 0 && s[0] == '}' { - // End of tags found. - return s[1:], tags, nil - } - if len(s) == 0 || s[0] != ',' { - return s, tags, fmt.Errorf("missing comma after tag %s=%q", key, value) - } - s = s[1:] - } -} - -func skipTrailingWhitespace(s string) string { - for len(s) > 0 && (s[len(s)-1] == ' ' || s[len(s)-1] == '\t') { - s = s[:len(s)-1] - } - return s -} - -func skipLeadingWhitespace(s string) string { - for len(s) > 0 && (s[0] == ' ' || s[0] == '\t') { - s = s[1:] - } - return s -} - -func unmarshalQuotedString(s string) (string, string, error) { - if len(s) == 0 || s[0] != '"' { - return "", s, fmt.Errorf("missing starting double quote in string: %q", s) - } - n := findClosingQuote(s) - if n == -1 { - return "", s, fmt.Errorf("missing closing double quote in string: %q", s) - } - return unescapeValue(s[1:n]), s[n+1:], nil -} - -func unescapeValue(s string) string { - n := strings.IndexByte(s, '\\') - if n < 0 { - // Fast path - nothing to unescape - return s - } - b := make([]byte, 0, len(s)) - for { - b = append(b, s[:n]...) - s = s[n+1:] - if len(s) == 0 { - b = append(b, '\\') - break - } - // label_value can be any sequence of UTF-8 characters, but the backslash (\), double-quote ("), - // and line feed (\n) characters have to be escaped as \\, \", and \n, respectively. - // See https://github.com/prometheus/docs/blob/e39897e4ee6e67d49d47204a34d120e3314e82f9/docs/instrumenting/exposition_formats.md - switch s[0] { - case '\\': - b = append(b, '\\') - case '"': - b = append(b, '"') - case 'n': - b = append(b, '\n') - default: - b = append(b, '\\', s[0]) - } - s = s[1:] - n = strings.IndexByte(s, '\\') - if n < 0 { - b = append(b, s...) - break - } - } - return string(b) -} - -func findClosingQuote(s string) int { - if len(s) == 0 || s[0] != '"' { - return -1 - } - off := 1 - s = s[1:] - for { - n := strings.IndexByte(s, '"') - if n < 0 { - return -1 - } - if prevBackslashesCount(s[:n])%2 == 0 { - return off + n - } - off += n + 1 - s = s[n+1:] - } -} - -func prevBackslashesCount(s string) int { - n := 0 - for len(s) > 0 && s[len(s)-1] == '\\' { - n++ - s = s[:len(s)-1] - } - return n -} - func mergeSpecs[T any](a, b *T, name string) (*T, error) { merged, err := k8stools.RenderPlaceholders(a, map[string]string{ vmv1alpha1.ZonePlaceholder: name, diff --git a/internal/controller/operator/factory/vmdistributed/util_test.go b/internal/controller/operator/factory/vmdistributed/util_test.go index c3b878b29..137c8c4e2 100644 --- a/internal/controller/operator/factory/vmdistributed/util_test.go +++ b/internal/controller/operator/factory/vmdistributed/util_test.go @@ -1,12 +1,7 @@ package vmdistributed import ( - "context" - "fmt" - "net/http" - "net/http/httptest" "testing" - "time" "github.com/stretchr/testify/assert" "k8s.io/utils/ptr" @@ -15,48 +10,6 @@ import ( vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" ) -func TestFetchMetricValues(t *testing.T) { - f := func(body, metric, dimension string, expected map[string]float64) { - t.Helper() - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, body) - })) - defer ts.Close() - - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() - - values, err := fetchMetricValues(ctx, ts.Client(), ts.URL, metric, dimension) - assert.NoError(t, err) - - assert.Equal(t, expected, values) - } - - // test metric - f(` -test_metric{path="no path"} 24 -`, "test_metric", "path", map[string]float64{ - "no path": 24, - }) - - // ignores similarly prefixed metrics - f(` -test_metric_total{path="wrong"} 99 -test_metric{path="correct"} 24 -`, "test_metric", "path", map[string]float64{ - "correct": 24, - }) - - // ignores prometheus metadata lines - f(` -# HELP test_metric queue size -# TYPE test_metric gauge -test_metric{path="correct"} 24 -`, "test_metric", "path", map[string]float64{ - "correct": 24, - }) -} - func TestMergeSpecs(t *testing.T) { t.Run("VMCluster", func(t *testing.T) { f := func(common, zone *vmv1beta1.VMClusterSpec, zoneName string, expected *vmv1beta1.VMClusterSpec) { diff --git a/internal/controller/operator/factory/vmdistributed/vmauth.go b/internal/controller/operator/factory/vmdistributed/vmauth.go index 61af4f1e1..b4a62285d 100644 --- a/internal/controller/operator/factory/vmdistributed/vmauth.go +++ b/internal/controller/operator/factory/vmdistributed/vmauth.go @@ -137,5 +137,8 @@ func buildVMAuthLB(cr *vmv1alpha1.VMDistributed, zs *zones, excludeIds ...int) * targetRefs = append(targetRefs, vmBackendTargetRef(zs.backends, kind, paths, &owner, readExcludeIds...)) vmAuth.Spec.DefaultTargetRefs = targetRefs + // Force-enabled regardless of user overrides, so zone rotation only proceeds once vmauth + // has actually reloaded its routing config, not just written it to the API server. + vmAuth.Spec.WaitForConfigReload = ptr.To(true) return &vmAuth } diff --git a/internal/controller/operator/factory/vmdistributed/vmauth_test.go b/internal/controller/operator/factory/vmdistributed/vmauth_test.go index 9946eb3ac..c0097758b 100644 --- a/internal/controller/operator/factory/vmdistributed/vmauth_test.go +++ b/internal/controller/operator/factory/vmdistributed/vmauth_test.go @@ -259,3 +259,21 @@ func TestBuildVMAuthLBVMSingleBackend(t *testing.T) { {Name: "single-a", Namespace: "ns"}, }, vmAuth.Spec.DefaultTargetRefs[1].CRD.Objects) } + +func TestBuildVMAuthLBForcesWaitForConfigReload(t *testing.T) { + cr := &vmv1alpha1.VMDistributed{ + ObjectMeta: metav1.ObjectMeta{Name: "test-dist", Namespace: "ns"}, + Spec: vmv1alpha1.VMDistributedSpec{ + VMAuth: vmv1alpha1.VMDistributedAuth{ + Spec: vmv1beta1.VMAuthSpec{ + CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ + WaitForConfigReload: ptr.To(false), + }, + }, + }, + }, + } + vmAuth := buildVMAuthLB(cr, &zones{}) + assert.NotNil(t, vmAuth) + assert.Equal(t, ptr.To(true), vmAuth.Spec.WaitForConfigReload) +} diff --git a/internal/controller/operator/factory/vmdistributed/zone.go b/internal/controller/operator/factory/vmdistributed/zone.go index 7339c79ff..90a37f95a 100644 --- a/internal/controller/operator/factory/vmdistributed/zone.go +++ b/internal/controller/operator/factory/vmdistributed/zone.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "net/url" "path/filepath" "sort" "strings" @@ -12,13 +11,10 @@ import ( "time" "github.com/cespare/xxhash/v2" - discoveryv1 "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,6 +23,7 @@ import ( vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/reconcile" + "github.com/VictoriaMetrics/operator/internal/podutil" ) // VMBackend is implemented by *vmv1beta1.VMCluster and *vmv1beta1.VMSingle. @@ -375,53 +372,8 @@ func (zs *zones) updateLB(ctx context.Context, rclient client.Client, cr *vmv1al return reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner) } -func getMetricsAddrs(ctx context.Context, rclient client.Client, vmAgent *vmv1beta1.VMAgent) sets.Set[string] { - var esl discoveryv1.EndpointSliceList - o := client.ListOptions{ - LabelSelector: labels.SelectorFromSet(labels.Set{discoveryv1.LabelServiceName: vmAgent.PrefixedName()}), - Namespace: vmAgent.Namespace, - } - if err := rclient.List(ctx, &esl, &o); err != nil { - logger.WithContext(ctx).Error(err, "failed to load endpointslices", "service", vmAgent.PrefixedName()) - return nil - } - if len(esl.Items) == 0 { - return nil - } - addrs := sets.New[string]() - for i := range esl.Items { - es := &esl.Items[i] - var port int32 - for _, p := range es.Ports { - if p.Name != nil && *p.Name == "http" && p.Port != nil { - port = *p.Port - } - } - if port == 0 { - continue - } - for _, ep := range es.Endpoints { - if ep.Conditions.Ready == nil || !*ep.Conditions.Ready { - continue - } - for _, a := range ep.Addresses { - if a == "" { - continue - } - host := fmt.Sprintf("%s:%d", a, port) - if es.AddressType == discoveryv1.AddressTypeIPv6 { - host = fmt.Sprintf("[%s]:%d", a, port) - } - u := &url.URL{ - Host: host, - Scheme: strings.ToLower(vmAgent.ProbeScheme()), - Path: vmAgent.GetMetricsPath(), - } - addrs.Insert(u.String()) - } - } - } - return addrs +var vmAgentQueueMetricQueries = []podutil.MetricQuery{ + {Name: vmAgentQueueMetricName, Dimension: "path"}, } func (zs *zones) waitForEmptyPQ(ctx context.Context, rclient client.Client, interval time.Duration, clusterIdx int) { @@ -435,13 +387,13 @@ func (zs *zones) waitForEmptyPQ(ctx context.Context, rclient client.Client, inte pollMetrics := func(pctx context.Context, nsn types.NamespacedName, addr string) error { return wait.PollUntilContextCancel(pctx, interval, true, func(ctx context.Context) (done bool, err error) { // Query each discovered ip. If any returns non-zero metric, continue polling. - var metricValues map[string]float64 - if metricValues, err = fetchMetricValues(ctx, zs.httpClient, addr, vmAgentQueueMetricName, "path"); err != nil { + metrics, err := podutil.FetchMetricsValues(ctx, zs.httpClient, addr, vmAgentQueueMetricQueries) + if err != nil { logger.WithContext(ctx).Error(err, "attempt to get metrics failed", "url", addr, "name", nsn.String()) // Treat fetch errors as transient -> not ready, continue polling. return false, nil } - for p, v := range metricValues { + for p, v := range metrics[vmAgentQueueMetricName] { pqDir := filepath.Base(p) idx := strings.Index(pqDir, "_") if idx == -1 { @@ -474,7 +426,11 @@ func (zs *zones) waitForEmptyPQ(ctx context.Context, rclient client.Client, inte m := newManager(ctx) wg.Go(func() { wait.UntilWithContext(m.ctx, func(ctx context.Context) { - addrs := getMetricsAddrs(ctx, rclient, vmAgent) + addrs, err := podutil.DiscoverEndpointAddrs(ctx, rclient, vmAgent.Namespace, vmAgent.PrefixedName(), "http", vmAgent.ProbeScheme(), vmAgent.GetMetricsPath()) + if err != nil { + logger.WithContext(ctx).Error(err, "failed to load endpointslices", "service", vmAgent.PrefixedName()) + return + } for addr := range addrs { if m.has(addr) { continue diff --git a/internal/controller/operator/factory/vmsingle/scrapes_test.go b/internal/controller/operator/factory/vmsingle/scrapes_test.go index cdbeb54c5..c1362927c 100644 --- a/internal/controller/operator/factory/vmsingle/scrapes_test.go +++ b/internal/controller/operator/factory/vmsingle/scrapes_test.go @@ -40,7 +40,7 @@ func TestCreateOrUpdateScrapeConfig(t *testing.T) { } build.AddDefaults(testClient.Scheme()) ac := getAssetsCache(ctx, testClient, o.cr) - _, err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, nil, ac) + _, _, err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, nil, ac) assert.NoError(t, err) var expectSecret corev1.Secret nsn := types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()} @@ -2028,7 +2028,7 @@ scrape_configs: [] }, } ac := getAssetsCache(ctx, testClient, cr) - if _, err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, nil, ac); err != nil { + if _, _, err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, nil, ac); err != nil { t.Errorf("createOrUpdateScrapeConfig() error = %s", err) } var configSecret corev1.Secret diff --git a/internal/controller/operator/factory/vmsingle/vmsingle.go b/internal/controller/operator/factory/vmsingle/vmsingle.go index a23bdaf65..462896887 100644 --- a/internal/controller/operator/factory/vmsingle/vmsingle.go +++ b/internal/controller/operator/factory/vmsingle/vmsingle.go @@ -5,6 +5,7 @@ import ( "fmt" "path" "sort" + "time" "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" @@ -121,17 +122,38 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMSingle, rclient client. } ac := getAssetsCache(ctx, rclient, cr) - extraCount, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, nil, ac) + writeStartedAt := time.Now() + extraCount, mainChanged, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, nil, ac) if err != nil { return err } - if err := createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, prevCR, ac); err != nil { + relabelChanged, err := createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, prevCR, ac) + if err != nil { return fmt.Errorf("cannot update relabeling asset for vmsingle: %w", err) } - if err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac); err != nil { + streamAggrChanged, err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac) + if err != nil { return fmt.Errorf("cannot update stream aggregation config for vmsingle: %w", err) } + // wait once for every asset that actually changed, instead of stacking a separate + // up-to-timeout wait per asset. + var changedAssets []vmv1beta1.ConfigReloadAsset + if mainChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetMain) + } + if relabelChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetRelabel) + } + if streamAggrChanged { + changedAssets = append(changedAssets, vmv1beta1.ConfigReloadAssetStreamAggr) + } + if len(changedAssets) > 0 { + if err := reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, changedAssets...); err != nil { + return err + } + } + var prevDeploy *appsv1.Deployment if prevCR != nil { var err error @@ -405,7 +427,7 @@ func newPodSpec(ctx context.Context, cr *vmv1beta1.VMSingle, extraConfigSecretCo containers := []corev1.Container{vmsingleContainer} var ic []corev1.Container - if !ptr.Deref(cr.Spec.IngestOnlyMode, true) || cr.HasAnyRelabellingConfigs() || cr.HasAnyStreamAggrRule() { + if cr.HasConfigReloader() { var ss *corev1.SecretKeySelector if !ptr.Deref(cr.Spec.IngestOnlyMode, true) { ss = &corev1.SecretKeySelector{ @@ -502,7 +524,11 @@ func buildScrape(cr *vmv1beta1.VMSingle, svc *corev1.Service) *vmv1beta1.VMServi if cr == nil || svc == nil || ptr.Deref(cr.Spec.DisableSelfServiceScrape, false) { return nil } - return build.VMServiceScrape(svc, cr, "vmbackupmanager") + scrape := build.VMServiceScrape(svc, cr, "vmbackupmanager") + if cr.HasConfigReloader() { + scrape.Spec.Endpoints = append(scrape.Spec.Endpoints, build.ConfigReloaderVMServiceScrapeEndpoint()) + } + return scrape } func createOrUpdateService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle) error { @@ -596,22 +622,22 @@ func buildRelabelingsAssets(cr *vmv1beta1.VMSingle, ac *build.AssetsCache) (*cor return cm, nil } -// createOrUpdateRelabelConfigsAssets builds relabeling configs for vmsingle at separate configmap, serialized as yaml -func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, ac *build.AssetsCache) error { +// createOrUpdateRelabelConfigsAssets builds relabeling configs for vmsingle at separate configmap, serialized as yaml. +// It reports whether the configmap content changed, leaving the caller to wait for config reload. +func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, ac *build.AssetsCache) (bool, error) { if !cr.HasAnyRelabellingConfigs() { - return nil + return false, nil } assestsCM, err := buildRelabelingsAssets(cr, ac) if err != nil { - return err + return false, err } var prevConfigMeta *metav1.ObjectMeta if prevCR != nil { prevConfigMeta = ptr.To(build.ResourceMeta(build.RelabelConfigResourceKind, prevCR)) } owner := cr.AsOwner() - _, err = reconcile.ConfigMap(ctx, rclient, assestsCM, prevConfigMeta, &owner) - return err + return reconcile.ConfigMap(ctx, rclient, assestsCM, prevConfigMeta, &owner) } // buildStreamAggrConfig build configmap with stream aggregation config for vmsingle. @@ -641,22 +667,22 @@ func buildStreamAggrConfig(cr *vmv1beta1.VMSingle, ac *build.AssetsCache) (*core return cfgCM, nil } -// createOrUpdateStreamAggrConfig builds stream aggregation configs for vmsingle at separate configmap, serialized as yaml -func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, ac *build.AssetsCache) error { +// createOrUpdateStreamAggrConfig builds stream aggregation configs for vmsingle at separate configmap, serialized as yaml. +// It reports whether the configmap content changed, leaving the caller to wait for config reload. +func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, ac *build.AssetsCache) (bool, error) { if !cr.HasAnyStreamAggrRule() { - return nil + return false, nil } streamAggrCM, err := buildStreamAggrConfig(cr, ac) if err != nil { - return err + return false, err } var prevCMMeta *metav1.ObjectMeta if prevCR != nil { prevCMMeta = ptr.To(build.ResourceMeta(build.StreamAggrConfigResourceKind, prevCR)) } owner := cr.AsOwner() - _, err = reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevCMMeta, &owner) - return err + return reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevCMMeta, &owner) } func createOrUpdateVPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle) error { @@ -755,15 +781,22 @@ func CreateOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr * prevCR.Spec = *cr.Status.LastAppliedSpec } ac := getAssetsCache(ctx, rclient, cr) - if _, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, childObject, ac); err != nil { + writeStartedAt := time.Now() + _, changed, err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, childObject, ac) + if err != nil { return err } + if changed { + return reconcile.WaitForConfigReload(ctx, rclient, cr, writeStartedAt, vmv1beta1.ConfigReloadAssetMain) + } return nil } -func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, childObject client.Object, ac *build.AssetsCache) (int, error) { +// createOrUpdateScrapeConfig reconciles the main scrape config and reports whether it changed, +// leaving the caller to wait for config reload. +func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, childObject client.Object, ac *build.AssetsCache) (int, bool, error) { if ptr.Deref(cr.Spec.IngestOnlyMode, true) { - return 0, nil + return 0, false, nil } pos := &vmscrapes.ParsedObjects{ @@ -780,18 +813,18 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, } sp := &cr.Spec.CommonScrapeParams if err := pos.Init(ctx, rclient, sp); err != nil { - return 0, err + return 0, false, err } pos.ValidateObjects(sp) cfgBase, jobs, err := pos.BuildScrapeJobsConfig(ctx, sp, ac) if err != nil { - return 0, fmt.Errorf("generating config for vmsingle failed: %w", err) + return 0, false, fmt.Errorf("generating config for vmsingle failed: %w", err) } buckets, err := build.PackItems(jobs, config.MustGetBaseConfig().ConfigDataBudgetBytes, 150) if err != nil { - return 0, fmt.Errorf("splitting scrape config into buckets for vmsingle: %w", err) + return 0, false, fmt.Errorf("splitting scrape config into buckets for vmsingle: %w", err) } extraCount := len(buckets) - 1 @@ -805,7 +838,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, mainCfg = append(mainCfg, yaml.MapItem{Key: "scrape_configs", Value: buckets[0]}) generatedConfig, err := yaml.Marshal(mainCfg) if err != nil { - return 0, fmt.Errorf("marshalling config for vmsingle: %w", err) + return 0, false, fmt.Errorf("marshalling config for vmsingle: %w", err) } owner := cr.AsOwner() @@ -817,6 +850,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] }) + var changed bool for _, kind := range keys { secret := secrets[kind] var prevSecretMeta *metav1.ObjectMeta @@ -826,7 +860,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, if kind == build.SecretConfigResourceKind { d, err := build.GzipConfig(generatedConfig) if err != nil { - return 0, fmt.Errorf("cannot gzip config for vmsingle: %w", err) + return 0, false, fmt.Errorf("cannot gzip config for vmsingle: %w", err) } secret.Data[scrapeGzippedFilename] = d } @@ -834,8 +868,15 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, secret.Annotations = map[string]string{ "generated": "true", } - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner); err != nil { - return 0, err + secretChanged, err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta, &owner) + if err != nil { + return 0, false, err + } + // TLS assets aren't part of the config-reloader-watched scrape config, and vmsingle's + // own reload-completion metrics only advance when the actual scrape config content + // changes - counting a TLS-only change here would wait on a metric that never moves. + if kind == build.SecretConfigResourceKind { + changed = changed || secretChanged } } @@ -843,26 +884,28 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, idx := i + 1 extraData, err := yaml.Marshal(bucket) if err != nil { - return 0, fmt.Errorf("marshalling extra scrape config bucket %d for vmsingle: %w", idx, err) + return 0, false, fmt.Errorf("marshalling extra scrape config bucket %d for vmsingle: %w", idx, err) } compressed, err := build.GzipConfig(extraData) if err != nil { - return 0, fmt.Errorf("gzipping extra scrape config bucket %d for vmsingle: %w", idx, err) + return 0, false, fmt.Errorf("gzipping extra scrape config bucket %d for vmsingle: %w", idx, err) } s := vmscrapes.BuildExtraConfigSecret(cr, idx, compressed) - if err := reconcile.Secret(ctx, rclient, s, nil, &owner); err != nil { - return 0, err + secretChanged, err := reconcile.Secret(ctx, rclient, s, nil, &owner) + if err != nil { + return 0, false, err } + changed = changed || secretChanged } if err := vmscrapes.RemoveStaleExtraConfigSecrets(ctx, rclient, cr, extraCount); err != nil { - return 0, err + return 0, false, err } parentName := fmt.Sprintf("%s.%s.vmsingle", cr.Name, cr.Namespace) if err := pos.UpdateStatusesForScrapeObjects(ctx, rclient, parentName, childObject); err != nil { - return 0, err + return 0, false, err } - return extraCount, nil + return extraCount, changed, nil } diff --git a/internal/controller/operator/factory/vtcluster/vmauth_lb.go b/internal/controller/operator/factory/vtcluster/vmauth_lb.go index 109153514..90968c78c 100644 --- a/internal/controller/operator/factory/vtcluster/vmauth_lb.go +++ b/internal/controller/operator/factory/vtcluster/vmauth_lb.go @@ -29,7 +29,7 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) } owner := cr.AsOwner() - if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta, &owner); err != nil { + if _, err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta, &owner); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMauthLBDeployment(cr) diff --git a/internal/controller/operator/vmagent_controller.go b/internal/controller/operator/vmagent_controller.go index 140ea1987..0086016a0 100644 --- a/internal/controller/operator/vmagent_controller.go +++ b/internal/controller/operator/vmagent_controller.go @@ -23,6 +23,7 @@ import ( "sync" "github.com/go-logr/logr" + "golang.org/x/sync/errgroup" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -168,13 +169,15 @@ func collectVMAgentScrapes(l logr.Logger, ctx context.Context, rclient client.Cl err = fmt.Errorf("cannot list VMAgents for %T: %w", instance, err) return } + var g errgroup.Group + g.SetLimit(childReconcileConcurrencyLimit) for i := range objects.Items { item := &objects.Items[i] if item.IsUnmanaged(instance) { continue } - l := l.WithValues("vmagent", item.Name, "parent_namespace", item.Namespace) - ctx := logger.AddToContext(ctx, l) + itemLog := l.WithValues("vmagent", item.Name, "parent_namespace", item.Namespace) + itemCtx := logger.AddToContext(ctx, itemLog) // only check selector when deleting object, // since labels can be changed when updating and we can't tell if it was selected before, and we can't tell if it's creating or updating. @@ -186,20 +189,23 @@ func collectVMAgentScrapes(l logr.Logger, ctx context.Context, rclient client.Cl ObjectSelector: objectSelector, DefaultNamespace: instance.GetNamespace(), } - match, err := isSelectorsMatchesTargetCRD(ctx, rclient, instance, item, opts) + match, err := isSelectorsMatchesTargetCRD(itemCtx, rclient, instance, item, opts) if err != nil { - l.Error(err, fmt.Sprintf("cannot match VMAgent and %T", instance)) + itemLog.Error(err, fmt.Sprintf("cannot match VMAgent and %T", instance)) continue } if !match { continue } } - - if configErr := vmagent.CreateOrUpdateScrapeConfig(ctx, rclient, item, instance); configErr != nil { - l.Error(configErr, "cannot update VMAgent scrape configuration") - err = configErr - } + g.Go(func() error { + if configErr := vmagent.CreateOrUpdateScrapeConfig(itemCtx, rclient, item, instance); configErr != nil { + itemLog.Error(configErr, "cannot update VMAgent scrape configuration") + return configErr + } + return nil + }) } + err = g.Wait() return } diff --git a/internal/controller/operator/vmalert_controller.go b/internal/controller/operator/vmalert_controller.go index d5513633b..623c194fb 100644 --- a/internal/controller/operator/vmalert_controller.go +++ b/internal/controller/operator/vmalert_controller.go @@ -69,6 +69,7 @@ func (r *VMAlertReconciler) Scheme() *runtime.Scheme { // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmalerts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmalerts/status,verbs=get;update;patch // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmalerts/finalizers,verbs=* +// +kubebuilder:rbac:groups="",resources=pods,verbs=list;watch func (r *VMAlertReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { l := r.Log.WithValues(r.name, req.Name, "namespace", req.Namespace) ctx = logger.AddToContext(ctx, l) diff --git a/internal/controller/operator/vmalertmanager_controller.go b/internal/controller/operator/vmalertmanager_controller.go index 5a00bc407..01b5c4d2c 100644 --- a/internal/controller/operator/vmalertmanager_controller.go +++ b/internal/controller/operator/vmalertmanager_controller.go @@ -73,6 +73,7 @@ func (r *VMAlertmanagerReconciler) Scheme() *runtime.Scheme { // +kubebuilder:rbac:groups=apps,resources=statefulsets/status,verbs=get;update;patch // +kubebuilder:rbac:groups="",resources=configmaps,verbs=* // +kubebuilder:rbac:groups="",resources=secrets,verbs=* +// +kubebuilder:rbac:groups="",resources=pods,verbs=list;watch func (r *VMAlertmanagerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { l := r.Log.WithValues(r.name, req.Name, "namespace", req.Namespace) ctx = logger.AddToContext(ctx, l) diff --git a/internal/controller/operator/vmalertmanagerconfig_controller.go b/internal/controller/operator/vmalertmanagerconfig_controller.go index 55bdae4a0..9cf3b2b8a 100644 --- a/internal/controller/operator/vmalertmanagerconfig_controller.go +++ b/internal/controller/operator/vmalertmanagerconfig_controller.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/go-logr/logr" + "golang.org/x/sync/errgroup" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" ctrl "sigs.k8s.io/controller-runtime" @@ -92,14 +93,16 @@ func (r *VMAlertmanagerConfigReconciler) Reconcile(ctx context.Context, req ctrl return } + var g errgroup.Group + g.SetLimit(childReconcileConcurrencyLimit) for i := range objects.Items { item := &objects.Items[i] if !item.DeletionTimestamp.IsZero() || (item.Status.ParsingSpecError != "" && !vmv1beta1.HasUnknownFields(item.Status.ParsingSpecError)) || item.IsUnmanaged() { continue } - l := l.WithValues("vmalertmanager", item.Name, "parent_namespace", item.Namespace) - ctx := logger.AddToContext(ctx, l) + itemLog := l.WithValues("vmalertmanager", item.Name, "parent_namespace", item.Namespace) + itemCtx := logger.AddToContext(ctx, itemLog) // only check selector when deleting object, // since labels can be changed when updating and we can't tell if it was selected before, and we can't tell if it's creating or updating. @@ -110,20 +113,27 @@ func (r *VMAlertmanagerConfigReconciler) Reconcile(ctx context.Context, req ctrl ObjectSelector: item.Spec.ConfigSelector, DefaultNamespace: instance.Namespace, } - match, err := isSelectorsMatchesTargetCRD(ctx, r.Client, &instance, item, opts) + match, err := isSelectorsMatchesTargetCRD(itemCtx, r.Client, &instance, item, opts) if err != nil { - l.Error(err, "cannot match alertmanager against selector, probably bug") + itemLog.Error(err, "cannot match alertmanager against selector, probably bug") continue } if !match { continue } } - if configErr := vmalertmanager.CreateOrUpdateConfig(ctx, r.Client, item, &instance); configErr != nil { - l.Error(configErr, "cannot update alertmanager config") - err = configErr - } + // each VMAlertmanager's config reconcile is independent of the others - run them + // concurrently so a slow one (e.g. waiting for its own config reload to be confirmed) + // doesn't serialize the whole reconcile behind every other selected VMAlertmanager. + g.Go(func() error { + if configErr := vmalertmanager.CreateOrUpdateConfig(itemCtx, r.Client, item, &instance); configErr != nil { + itemLog.Error(configErr, "cannot update alertmanager config") + return configErr + } + return nil + }) } + err = g.Wait() return } diff --git a/internal/controller/operator/vmauth_controller.go b/internal/controller/operator/vmauth_controller.go index a0760772e..071b949f3 100644 --- a/internal/controller/operator/vmauth_controller.go +++ b/internal/controller/operator/vmauth_controller.go @@ -69,6 +69,7 @@ func (r *VMAuthReconciler) Scheme() *runtime.Scheme { // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmauths,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=operator.victoriametrics.com,resources=vmauths/status,verbs=get;update;patch // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=pods,verbs=list;watch func (r *VMAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { l := r.Log.WithValues(r.name, req.Name, "namespace", req.Namespace) ctx = logger.AddToContext(ctx, l) diff --git a/internal/controller/operator/vmrule_controller.go b/internal/controller/operator/vmrule_controller.go index d3350f60c..1d7b14fa7 100644 --- a/internal/controller/operator/vmrule_controller.go +++ b/internal/controller/operator/vmrule_controller.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/go-logr/logr" + "golang.org/x/sync/errgroup" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" ctrl "sigs.k8s.io/controller-runtime" @@ -95,13 +96,15 @@ func (r *VMRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return } + var g errgroup.Group + g.SetLimit(childReconcileConcurrencyLimit) for i := range objects.Items { item := &objects.Items[i] if item.DeletionTimestamp != nil || (item.Status.ParsingSpecError != "" && !vmv1beta1.HasUnknownFields(item.Status.ParsingSpecError)) { continue } - l := l.WithValues("vmalert", item.Name, "parent_namespace", item.Namespace) - ctx := logger.AddToContext(ctx, l) + itemLog := l.WithValues("vmalert", item.Name, "parent_namespace", item.Namespace) + itemCtx := logger.AddToContext(ctx, itemLog) // only check selector when deleting object, // since labels can be changed when updating and we can't tell if it was selected before, and we can't tell if it's creating or updating. @@ -112,9 +115,9 @@ func (r *VMRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ObjectSelector: item.Spec.RuleSelector, DefaultNamespace: instance.Namespace, } - match, err := isSelectorsMatchesTargetCRD(ctx, r.Client, &instance, item, opts) + match, err := isSelectorsMatchesTargetCRD(itemCtx, r.Client, &instance, item, opts) if err != nil { - l.Error(err, "cannot match vmalert and vmrule") + itemLog.Error(err, "cannot match vmalert and vmrule") continue } if !match { @@ -122,11 +125,15 @@ func (r *VMRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } } - if _, configErr := vmalert.CreateOrUpdateRuleConfigMaps(ctx, r, item, &instance, item.HasNotifiersConfigured()); configErr != nil { - l.Error(configErr, "cannot update rules configmaps") - err = configErr - } + g.Go(func() error { + if _, configErr := vmalert.CreateOrUpdateRuleConfigMaps(itemCtx, r, item, &instance, item.HasNotifiersConfigured()); configErr != nil { + itemLog.Error(configErr, "cannot update rules configmaps") + return configErr + } + return nil + }) } + err = g.Wait() return } diff --git a/internal/controller/operator/vmsingle_controller.go b/internal/controller/operator/vmsingle_controller.go index 9b95fcf51..e2d7239d1 100644 --- a/internal/controller/operator/vmsingle_controller.go +++ b/internal/controller/operator/vmsingle_controller.go @@ -23,6 +23,7 @@ import ( "sync" "github.com/go-logr/logr" + "golang.org/x/sync/errgroup" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -161,13 +162,15 @@ func collectVMSingleScrapes(l logr.Logger, ctx context.Context, rclient client.C return } + var g errgroup.Group + g.SetLimit(childReconcileConcurrencyLimit) for i := range objects.Items { item := &objects.Items[i] if item.IsUnmanaged(instance) { continue } - l := l.WithValues("vmsingle", item.Name, "parent_namespace", item.Namespace) - ctx := logger.AddToContext(ctx, l) + itemLog := l.WithValues("vmsingle", item.Name, "parent_namespace", item.Namespace) + itemCtx := logger.AddToContext(ctx, itemLog) // only check selector when deleting object, // since labels can be changed when updating and we can't tell if it was selected before, and we can't tell if it's creating or updating. if !instance.GetDeletionTimestamp().IsZero() { @@ -178,20 +181,23 @@ func collectVMSingleScrapes(l logr.Logger, ctx context.Context, rclient client.C ObjectSelector: objectSelector, DefaultNamespace: instance.GetNamespace(), } - match, err := isSelectorsMatchesTargetCRD(ctx, rclient, instance, item, opts) + match, err := isSelectorsMatchesTargetCRD(itemCtx, rclient, instance, item, opts) if err != nil { - l.Error(err, fmt.Sprintf("cannot match VMSingle and %T", instance)) + itemLog.Error(err, fmt.Sprintf("cannot match VMSingle and %T", instance)) continue } if !match { continue } } - - if configErr := vmsingle.CreateOrUpdateScrapeConfig(ctx, rclient, item, instance); configErr != nil { - l.Error(configErr, "cannot update VMSingle scrape configuration") - err = configErr - } + g.Go(func() error { + if configErr := vmsingle.CreateOrUpdateScrapeConfig(itemCtx, rclient, item, instance); configErr != nil { + itemLog.Error(configErr, "cannot update VMSingle scrape configuration") + return configErr + } + return nil + }) } + err = g.Wait() return } diff --git a/internal/controller/operator/vmuser_controller.go b/internal/controller/operator/vmuser_controller.go index d6bc4d805..e3ef0df50 100644 --- a/internal/controller/operator/vmuser_controller.go +++ b/internal/controller/operator/vmuser_controller.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/go-logr/logr" + "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -106,14 +107,16 @@ func (r *VMUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return } + var g errgroup.Group + g.SetLimit(childReconcileConcurrencyLimit) for i := range objects.Items { item := &objects.Items[i] if !item.DeletionTimestamp.IsZero() || (item.Status.ParsingSpecError != "" && !vmv1beta1.HasUnknownFields(item.Status.ParsingSpecError)) || item.IsUnmanaged() { continue } // reconcile users for given vmauth. - l = l.WithValues("vmauth", item.Name, "parent_namespace", item.Namespace) - ctx := logger.AddToContext(ctx, l) + itemLog := l.WithValues("vmauth", item.Name, "parent_namespace", item.Namespace) + itemCtx := logger.AddToContext(ctx, itemLog) // only check selector when deleting object, // since labels can be changed when updating and we can't tell if it was selected before, and we can't tell if it's creating or updating. @@ -124,20 +127,24 @@ func (r *VMUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ObjectSelector: item.Spec.UserSelector, DefaultNamespace: instance.Namespace, } - match, err := isSelectorsMatchesTargetCRD(ctx, r.Client, &instance, item, opts) + match, err := isSelectorsMatchesTargetCRD(itemCtx, r.Client, &instance, item, opts) if err != nil { - l.Error(err, "cannot match vmauth and vmuser") + itemLog.Error(err, "cannot match vmauth and vmuser") continue } if !match { continue } } - if configErr := vmauth.CreateOrUpdateConfig(ctx, r, item, &instance); configErr != nil { - l.Error(configErr, "cannot create or update vmauth deploy for vmuser") - err = configErr - } + g.Go(func() error { + if configErr := vmauth.CreateOrUpdateConfig(itemCtx, r, item, &instance); configErr != nil { + itemLog.Error(configErr, "cannot create or update vmauth deploy for vmuser") + return configErr + } + return nil + }) } + err = g.Wait() return } diff --git a/internal/podutil/discover.go b/internal/podutil/discover.go new file mode 100644 index 000000000..57b50c8e6 --- /dev/null +++ b/internal/podutil/discover.go @@ -0,0 +1,64 @@ +// Package podutil provides shared helpers for discovering pod endpoints via +// EndpointSlices and calling admin/metrics HTTP endpoints on them directly +// (in-cluster pod-to-pod traffic, not routed through the Kubernetes API). +package podutil + +import ( + "context" + "fmt" + "net/url" + "strings" + + discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// DiscoverEndpointAddrs lists ready endpoint addresses for the given Service (identified via +// the standard discoveryv1.LabelServiceName label) and builds a URL for each, using portName +// to pick the right port, scheme for http/https, and path for the request path. +func DiscoverEndpointAddrs(ctx context.Context, rclient client.Client, namespace, serviceName, portName, scheme, path string) (sets.Set[string], error) { + var esl discoveryv1.EndpointSliceList + o := client.ListOptions{ + LabelSelector: labels.SelectorFromSet(labels.Set{discoveryv1.LabelServiceName: serviceName}), + Namespace: namespace, + } + if err := rclient.List(ctx, &esl, &o); err != nil { + return nil, fmt.Errorf("failed to load endpointslices for service=%s: %w", serviceName, err) + } + addrs := sets.New[string]() + for i := range esl.Items { + es := &esl.Items[i] + var port int32 + for _, p := range es.Ports { + if p.Name != nil && *p.Name == portName && p.Port != nil { + port = *p.Port + } + } + if port == 0 { + continue + } + for _, ep := range es.Endpoints { + if ep.Conditions.Ready != nil && !*ep.Conditions.Ready { + continue + } + for _, a := range ep.Addresses { + if a == "" { + continue + } + host := fmt.Sprintf("%s:%d", a, port) + if es.AddressType == discoveryv1.AddressTypeIPv6 { + host = fmt.Sprintf("[%s]:%d", a, port) + } + u := &url.URL{ + Host: host, + Scheme: strings.ToLower(scheme), + Path: path, + } + addrs.Insert(u.String()) + } + } + } + return addrs, nil +} diff --git a/internal/podutil/discover_test.go b/internal/podutil/discover_test.go new file mode 100644 index 000000000..751857397 --- /dev/null +++ b/internal/podutil/discover_test.go @@ -0,0 +1,51 @@ +package podutil + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + discoveryv1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + + "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" +) + +func TestDiscoverEndpointAddrs(t *testing.T) { + f := func(ready *bool, wantAddrs []string) { + t.Helper() + es := &discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-eps", + Namespace: "default", + Labels: map[string]string{discoveryv1.LabelServiceName: "test-svc"}, + }, + AddressType: discoveryv1.AddressTypeIPv4, + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Conditions: discoveryv1.EndpointConditions{Ready: ready}, + }, + }, + Ports: []discoveryv1.EndpointPort{ + {Name: ptr.To("http"), Port: ptr.To[int32](8080)}, + }, + } + rclient := k8stools.GetTestClientWithObjects([]runtime.Object{es}) + + addrs, err := DiscoverEndpointAddrs(context.Background(), rclient, "default", "test-svc", "http", "http", "/metrics") + assert.NoError(t, err) + assert.ElementsMatch(t, wantAddrs, addrs.UnsortedList()) + } + + // ready=true: included + f(ptr.To(true), []string{"http://10.0.0.1:8080/metrics"}) + + // ready=false: excluded + f(ptr.To(false), nil) + + // ready=nil (unknown): EndpointSlice semantics treat this as ready, so it must be included + f(nil, []string{"http://10.0.0.1:8080/metrics"}) +} diff --git a/internal/podutil/metrics.go b/internal/podutil/metrics.go new file mode 100644 index 000000000..d0dfa5750 --- /dev/null +++ b/internal/podutil/metrics.go @@ -0,0 +1,281 @@ +package podutil + +import ( + "bufio" + "context" + "fmt" + "io" + "net/http" + "strconv" + "strings" +) + +// maxMetricLineBytes bounds a single scanned line of a /metrics response. It exists only to +// cap pathological input (e.g. a single unbounded line) - legitimately large /metrics +// responses are handled fine since they're never buffered in full, only scanned line by line. +const maxMetricLineBytes = 1 << 20 // 1MiB + +// maxErrBodyBytes bounds how much of a non-200 response body is read for the error message. +const maxErrBodyBytes = 4 << 10 // 4KiB + +// MetricQuery selects a metric by Name and, optionally, a label (Dimension) whose value is +// used to key each sample of that metric. If Dimension is empty, every sample of the metric +// is kept regardless of its labels, keyed by a synthetic index. +type MetricQuery struct { + Name string + Dimension string +} + +// FetchMetricsValues issues a single HTTP GET to url and extracts every sample matching any +// of the given queries, returning metric name -> (dimension value -> sample value). +func FetchMetricsValues(ctx context.Context, httpClient *http.Client, url string, queries []MetricQuery) (map[string]map[string]float64, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request for %s: %w", url, err) + } + resp, err := httpClient.Do(request) + if err != nil { + return nil, fmt.Errorf("failed to fetch metrics from %s: %w", url, err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrBodyBytes)) + return nil, fmt.Errorf("unexpected response status=%d, body=%q while requesting metrics at %s", resp.StatusCode, body, url) + } + dimensions := make(map[string]string, len(queries)) + for _, q := range queries { + dimensions[q.Name] = q.Dimension + } + values := make(map[string]map[string]float64, len(queries)) + if err := unmarshalMetrics(values, resp.Body, dimensions); err != nil { + return nil, fmt.Errorf("failed to unmarshal metrics values at %s: %w", url, err) + } + return values, nil +} + +// unmarshalMetrics scans r line by line rather than buffering it in full, so a large +// /metrics response never inflates controller memory - only one line is held at a time. +func unmarshalMetrics(values map[string]map[string]float64, r io.Reader, dimensions map[string]string) error { + scanner := bufio.NewScanner(r) + scanner.Buffer(make([]byte, 0, 4096), maxMetricLineBytes) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if len(line) == 0 || strings.HasPrefix(line, "#") { + continue + } + + n := strings.IndexAny(line, "{ \t\r") + metricName := line + if n >= 0 { + metricName = line[:n] + } + dimension, ok := dimensions[metricName] + if !ok { + continue + } + + line = line[len(metricName):] + line = skipLeadingWhitespace(line) + var dimValue string + if len(line) > 0 && line[0] == '{' { + line = line[1:] + var err error + var tags map[string]string + line, tags, err = unmarshalTags(line) + if err != nil { + return fmt.Errorf("cannot unmarshal tags: %w", err) + } + if dimension != "" { + dimValue = tags[dimension] + } + } + line = skipLeadingWhitespace(line) + if len(line) == 0 { + return fmt.Errorf("value cannot be empty") + } + valStr := line + n = strings.IndexAny(line, " \t\r\n") + if n >= 0 { + valStr = valStr[:n] + } + v, err := strconv.ParseFloat(valStr, 64) + if err != nil { + return fmt.Errorf("cannot parse value %q: %w", line, err) + } + metricValues := values[metricName] + if metricValues == nil { + metricValues = make(map[string]float64) + values[metricName] = metricValues + } + if len(dimValue) > 0 { + metricValues[dimValue] = v + } else if dimension == "" { + metricValues[strconv.Itoa(len(metricValues))] = v + } + } + return scanner.Err() +} + +func unmarshalTags(s string) (string, map[string]string, error) { + tags := make(map[string]string) + var err error + for { + s = skipLeadingWhitespace(s) + n := strings.IndexByte(s, '"') + if n < 0 { + // end of tags + if len(s) > 0 && s[0] == '}' { + return s[1:], tags, nil + } + return s, tags, fmt.Errorf("missing value for tag %q", s) + } + // Determine if this is a value or quoted label + possibleKey := skipTrailingWhitespace(s[:n]) + possibleKeyLen := len(possibleKey) + key := "" + if possibleKeyLen == 0 { + // Parse quoted label - {"label"="value"} or {"metric"} + key, s, err = unmarshalQuotedString(s) + if err != nil { + return s, tags, err + } + s = skipLeadingWhitespace(s) + if len(s) > 0 { + if s[0] == ',' || s[0] == '}' { + if len(s) > 1 && s[0] == ',' { + s = s[1:] + } + continue + } else if s[0] != '=' { + // We are a quoted label that isn't preceded by a comma or at the end + // of the tags so we must have a value + return s, tags, fmt.Errorf("missing value for quoted tag %q", key) + } + s = skipLeadingWhitespace(s[1:]) + } + // Fall through to parsing value + } else { + c := possibleKey[len(possibleKey)-1] + // unquoted label {label="value"} + if c == '=' { + // Parse unquoted label + key = skipLeadingWhitespace(s[:possibleKeyLen-1]) + key = skipTrailingWhitespace(key) + s = skipLeadingWhitespace(s[possibleKeyLen:]) + } else { + // unquoted tag without a value + return s, tags, fmt.Errorf("missing value for unquoted tag %q", s) + } + } + // Parse value + var value string + value, s, err = unmarshalQuotedString(s) + if err != nil { + return s, tags, err + } + + if len(key) > 0 { + // Allow empty values (len(value)==0) - see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/453 + tags[key] = value + } + s = skipLeadingWhitespace(s) + if len(s) > 0 && s[0] == '}' { + // End of tags found. + return s[1:], tags, nil + } + if len(s) == 0 || s[0] != ',' { + return s, tags, fmt.Errorf("missing comma after tag %s=%q", key, value) + } + s = s[1:] + } +} + +func skipTrailingWhitespace(s string) string { + for len(s) > 0 && (s[len(s)-1] == ' ' || s[len(s)-1] == '\t') { + s = s[:len(s)-1] + } + return s +} + +func skipLeadingWhitespace(s string) string { + for len(s) > 0 && (s[0] == ' ' || s[0] == '\t') { + s = s[1:] + } + return s +} + +func unmarshalQuotedString(s string) (string, string, error) { + if len(s) == 0 || s[0] != '"' { + return "", s, fmt.Errorf("missing starting double quote in string: %q", s) + } + n := findClosingQuote(s) + if n == -1 { + return "", s, fmt.Errorf("missing closing double quote in string: %q", s) + } + return unescapeValue(s[1:n]), s[n+1:], nil +} + +func unescapeValue(s string) string { + n := strings.IndexByte(s, '\\') + if n < 0 { + // Fast path - nothing to unescape + return s + } + b := make([]byte, 0, len(s)) + for { + b = append(b, s[:n]...) + s = s[n+1:] + if len(s) == 0 { + b = append(b, '\\') + break + } + // label_value can be any sequence of UTF-8 characters, but the backslash (\), double-quote ("), + // and line feed (\n) characters have to be escaped as \\, \", and \n, respectively. + // See https://github.com/prometheus/docs/blob/e39897e4ee6e67d49d47204a34d120e3314e82f9/docs/instrumenting/exposition_formats.md + switch s[0] { + case '\\': + b = append(b, '\\') + case '"': + b = append(b, '"') + case 'n': + b = append(b, '\n') + default: + b = append(b, '\\', s[0]) + } + s = s[1:] + n = strings.IndexByte(s, '\\') + if n < 0 { + b = append(b, s...) + break + } + } + return string(b) +} + +func findClosingQuote(s string) int { + if len(s) == 0 || s[0] != '"' { + return -1 + } + off := 1 + s = s[1:] + for { + n := strings.IndexByte(s, '"') + if n < 0 { + return -1 + } + if prevBackslashesCount(s[:n])%2 == 0 { + return off + n + } + off += n + 1 + s = s[n+1:] + } +} + +func prevBackslashesCount(s string) int { + n := 0 + for len(s) > 0 && s[len(s)-1] == '\\' { + n++ + s = s[:len(s)-1] + } + return n +} diff --git a/internal/podutil/metrics_test.go b/internal/podutil/metrics_test.go new file mode 100644 index 000000000..1237e1e29 --- /dev/null +++ b/internal/podutil/metrics_test.go @@ -0,0 +1,106 @@ +package podutil + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestFetchMetricsValues(t *testing.T) { + f := func(body string, queries []MetricQuery, expected map[string]map[string]float64) { + t.Helper() + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, body) + })) + defer ts.Close() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + values, err := FetchMetricsValues(ctx, ts.Client(), ts.URL, queries) + assert.NoError(t, err) + + assert.Equal(t, expected, values) + } + + // test metric + f(` +test_metric{path="no path"} 24 +`, []MetricQuery{{Name: "test_metric", Dimension: "path"}}, map[string]map[string]float64{ + "test_metric": {"no path": 24}, + }) + + // ignores similarly prefixed metrics + f(` +test_metric_total{path="wrong"} 99 +test_metric{path="correct"} 24 +`, []MetricQuery{{Name: "test_metric", Dimension: "path"}}, map[string]map[string]float64{ + "test_metric": {"correct": 24}, + }) + + // ignores prometheus metadata lines + f(` +# HELP test_metric queue size +# TYPE test_metric gauge +test_metric{path="correct"} 24 +`, []MetricQuery{{Name: "test_metric", Dimension: "path"}}, map[string]map[string]float64{ + "test_metric": {"correct": 24}, + }) + + // empty dimension returns every sample regardless of labels + f(` +test_metric 24 +`, []MetricQuery{{Name: "test_metric"}}, map[string]map[string]float64{ + "test_metric": {"0": 24}, + }) + + // multiple queries are resolved from a single scrape + f(` +metric_a 1 +metric_b{path="x"} 2 +`, []MetricQuery{{Name: "metric_a"}, {Name: "metric_b", Dimension: "path"}}, map[string]map[string]float64{ + "metric_a": {"0": 1}, + "metric_b": {"x": 2}, + }) +} + +func TestFetchMetricsValuesLargeResponse(t *testing.T) { + var body strings.Builder + for range 200_000 { + body.WriteString("unrelated_metric{label=\"x\"} 1\n") + } + body.WriteString(`test_metric{path="correct"} 24` + "\n") + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + io.WriteString(w, body.String()) + })) + defer ts.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + values, err := FetchMetricsValues(ctx, ts.Client(), ts.URL, []MetricQuery{{Name: "test_metric", Dimension: "path"}}) + assert.NoError(t, err) + assert.Equal(t, map[string]map[string]float64{"test_metric": {"correct": 24}}, values) +} + +func TestFetchMetricsValuesLineTooLong(t *testing.T) { + line := `test_metric{label="` + strings.Repeat("x", maxMetricLineBytes) + `"} 24` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + io.WriteString(w, line+"\n") + })) + defer ts.Close() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + _, err := FetchMetricsValues(ctx, ts.Client(), ts.URL, []MetricQuery{{Name: "test_metric", Dimension: "label"}}) + assert.Error(t, err) +} diff --git a/test/e2e/vmauth_test.go b/test/e2e/vmauth_test.go index 2ef277766..345238b1a 100644 --- a/test/e2e/vmauth_test.go +++ b/test/e2e/vmauth_test.go @@ -438,7 +438,7 @@ var _ = Describe("test vmauth Controller", Label("vm", "auth"), func() { Expect(svc.Spec.Ports).To(HaveLen(2)) var vmss vmv1beta1.VMServiceScrape Expect(k8sClient.Get(ctx, nsn, &vmss)).ToNot(HaveOccurred()) - Expect(vmss.Spec.Endpoints).To(HaveLen(1)) + Expect(vmss.Spec.Endpoints).To(HaveLen(2)) ep := vmss.Spec.Endpoints[0] Expect(ep.Port).To(Equal("internal")) },