diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d51ffb15b4..e91bc81631 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -38,6 +38,8 @@ aliases: * BUGFIX: [vmcluster](https://docs.victoriametrics.com/operator/resources/vmcluster/), [vmalertmanager](https://docs.victoriametrics.com/operator/resources/vmalertmanager/): reject `spec.serviceSpec.type` overrides on `vmselect`, `vmstorage`, and `vmalertmanager` when `useAsDefault` is set, since their default `Service` must stay headless for cluster-native communication. `vminsert` is unaffected, as its default `Service` isn't headless. See [#2487](https://github.com/VictoriaMetrics/operator/issues/2487). * BUGFIX: [vmauth](https://docs.victoriametrics.com/operator/resources/vmauth/): allow `spec.unauthorizedUserAccessSpec` with only `access_log` set, without requiring `url_map`, `url_prefix`, or `targetRefs`. See [#2551](https://github.com/VictoriaMetrics/operator/issues/2551). * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): a `VolumeClaimTemplate` size decrease, which Kubernetes cannot apply in-place to a bound `PersistentVolumeClaim`, was only logged and otherwise reported as a fully successful, `operational` reconcile. It now surfaces as a failed reconcile with the decline reason in `status.reason`, so the divergence between spec and actual PVC size is queryable and can be alerted on. See [#2512](https://github.com/VictoriaMetrics/operator/issues/2512). +* BUGFIX: [vmanomaly](https://docs.victoriametrics.com/operator/resources/vmanomaly/): pass previously skipped spec.extraEnvsFrom to anomaly pods. See [#2567](https://github.com/VictoriaMetrics/operator/issues/2567). +* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): derive config-reloader ContainerPort, probes, scrape targets, and WaitForConfigReloadHash from configReloaderExtraArgs http.listenAddr instead of always using 8435. See [#2584](https://github.com/VictoriaMetrics/operator/issues/2584). * BUGFIX: [vmanomaly](https://docs.victoriametrics.com/operator/resources/vmanomaly/): fix propagation of `spec.extraEnvsFrom` to anomaly pods, previously it was omitted. See [#2567](https://github.com/VictoriaMetrics/operator/issues/2567). * BUGFIX: [vmalertmanager](https://docs.victoriametrics.com/operator/resources/vmalertmanager/): fix propagation of `spec.extraEnvsFrom` to vmalertmanager pod, previously it was omitted. See [#2582](https://github.com/VictoriaMetrics/operator/issues/2582). * BUGFIX: [vmauth](https://docs.victoriametrics.com/operator/resources/vmauth/): fix unmarshalling of `spec.httpRoute.extraRules` with multiple rules. Previously, a rule could inherit fields from the previous one, e.g. a rule without `filters` silently got `filters` from the rule above it. This bug was introduced in [v0.66.0](https://github.com/VictoriaMetrics/operator/releases/tag/v0.66.0), when `spec.httpRoute` was added. See [#2605](https://github.com/VictoriaMetrics/operator/issues/2605). diff --git a/internal/controller/operator/factory/build/container.go b/internal/controller/operator/factory/build/container.go index c918a2a399..3a90d10045 100644 --- a/internal/controller/operator/factory/build/container.go +++ b/internal/controller/operator/factory/build/container.go @@ -2,8 +2,10 @@ package build import ( "fmt" + "net" "path/filepath" "sort" + "strconv" "strings" corev1 "k8s.io/api/core/v1" @@ -326,19 +328,29 @@ func AppendArgsForInsertPorts(args []string, ip *vmv1beta1.InsertPorts) []string } const ( - // ConfigReloaderDefaultPort is the config-reloader container's fixed HTTP listen port, + // ConfigReloaderDefaultPort is the config-reloader container's default HTTP listen port, // exposing its own /health and /metrics endpoints. ConfigReloaderDefaultPort = 8435 - // ConfigReloaderPortName is the container/service port name for ConfigReloaderDefaultPort. + // ConfigReloaderPortName is the container/service port name for the config-reloader HTTP port. ConfigReloaderPortName = "reloader-http" ) -var configReloaderContainerProbe = corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Scheme: "HTTP", - Port: intstr.FromInt(ConfigReloaderDefaultPort), - }, +// configReloaderHTTPPort returns the HTTP listen port from ConfigReloaderExtraArgs["http.listenAddr"] +// (forms :PORT, IP:PORT, [IPv6]:PORT). Missing or invalid values fall back to ConfigReloaderDefaultPort. +func configReloaderHTTPPort(extraArgs map[string]string) int32 { + addr, ok := extraArgs["http.listenAddr"] + if !ok || addr == "" { + return ConfigReloaderDefaultPort + } + _, portStr, err := net.SplitHostPort(addr) + if err != nil { + return ConfigReloaderDefaultPort + } + port, err := strconv.Atoi(portStr) + if err != nil || port < 1 || port > 65535 { + return ConfigReloaderDefaultPort + } + return int32(port) } func configReloaderJobRelabeling() vmv1beta1.EndpointRelabelings { @@ -355,11 +367,10 @@ func configReloaderJobRelabeling() vmv1beta1.EndpointRelabelings { } // 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. +// config-reloader sidecar by named container port, so scrapes follow an overridden listen port. func ConfigReloaderVMServiceScrapeEndpoint() vmv1beta1.Endpoint { return vmv1beta1.Endpoint{ - TargetPort: ptr.To(intstr.FromInt32(ConfigReloaderDefaultPort)), + TargetPort: ptr.To(intstr.FromString(ConfigReloaderPortName)), EndpointRelabelings: configReloaderJobRelabeling(), EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ Path: "/metrics", @@ -368,10 +379,10 @@ func ConfigReloaderVMServiceScrapeEndpoint() vmv1beta1.Endpoint { } // ConfigReloaderPodScrapeEndpoint returns a VMPodScrape endpoint that scrapes the -// config-reloader sidecar directly by its container port number. +// config-reloader sidecar by named container port. func ConfigReloaderPodScrapeEndpoint() vmv1beta1.PodMetricsEndpoint { return vmv1beta1.PodMetricsEndpoint{ - PortNumber: ptr.To(int32(ConfigReloaderDefaultPort)), + Port: ptr.To(ConfigReloaderPortName), EndpointRelabelings: configReloaderJobRelabeling(), EndpointScrapeParams: vmv1beta1.EndpointScrapeParams{ Path: "/metrics", @@ -453,17 +464,24 @@ func ConfigReloaderContainer(isInit bool, cr reloadable, mounts []corev1.VolumeM if !isInit { c.Name = "config-reloader" c.TerminationMessagePolicy = corev1.TerminationMessageFallbackToLogsOnError - addPortProbesToConfigReloaderContainer(&c) + addPortProbesToConfigReloaderContainer(&c, configReloaderHTTPPort(p.ConfigReloaderExtraArgs)) addConfigReloadAuthKeyToReloader(&c, p) } return c } -// addPortProbesToConfigReloaderContainer conditionally adds readiness and liveness probes to the custom config-reloader image -// exposes reloader-http port for container -func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container) { +// addPortProbesToConfigReloaderContainer adds readiness/liveness probes and the reloader-http +// container port, using the resolved HTTP listen port from configReloaderExtraArgs. +func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container, port int32) { + probe := corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Scheme: "HTTP", + Port: intstr.FromInt32(port), + }, + } crContainer.Ports = append(crContainer.Ports, corev1.ContainerPort{ - ContainerPort: int32(ConfigReloaderDefaultPort), + ContainerPort: port, Name: ConfigReloaderPortName, Protocol: "TCP", }) @@ -472,7 +490,7 @@ func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container) { SuccessThreshold: 1, FailureThreshold: 3, PeriodSeconds: 10, - ProbeHandler: configReloaderContainerProbe, + ProbeHandler: probe, } crContainer.ReadinessProbe = &corev1.Probe{ InitialDelaySeconds: 5, @@ -480,7 +498,7 @@ func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container) { SuccessThreshold: 1, FailureThreshold: 3, PeriodSeconds: 10, - ProbeHandler: configReloaderContainerProbe, + ProbeHandler: probe, } } diff --git a/internal/controller/operator/factory/build/container_test.go b/internal/controller/operator/factory/build/container_test.go index 8c890b0048..0b8b0fa08d 100644 --- a/internal/controller/operator/factory/build/container_test.go +++ b/internal/controller/operator/factory/build/container_test.go @@ -1006,4 +1006,75 @@ func TestBuildConfigReloaderContainer(t *testing.T) { }, }, }) + + // overridden http.listenAddr + f(opts{ + cr: &vmv1beta1.VMAlert{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "listen-override", + }, + Spec: vmv1beta1.VMAlertSpec{ + CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ + ConfigReloaderExtraArgs: map[string]string{ + "http.listenAddr": "127.0.0.1:8436", + }, + }, + }, + }, + expectedContainer: corev1.Container{ + Name: "config-reloader", + Args: []string{ + "--http.listenAddr=127.0.0.1:8436", + "--reload-url=http://127.0.0.1:/-/reload", + "--webhook-method=POST", + }, + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + Ports: []corev1.ContainerPort{ + { + Name: "reloader-http", + Protocol: corev1.ProtocolTCP, + ContainerPort: 8436, + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8436), + Scheme: "HTTP", + }, + }, + TimeoutSeconds: 1, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8436), + Scheme: "HTTP", + }, + }, + InitialDelaySeconds: 5, + TimeoutSeconds: 1, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + }, + }) +} + +func TestConfigReloaderHTTPPort(t *testing.T) { + assert.Equal(t, int32(8435), configReloaderHTTPPort(nil)) + assert.Equal(t, int32(8435), configReloaderHTTPPort(map[string]string{})) + assert.Equal(t, int32(8436), configReloaderHTTPPort(map[string]string{"http.listenAddr": "127.0.0.1:8436"})) + assert.Equal(t, int32(8436), configReloaderHTTPPort(map[string]string{"http.listenAddr": ":8436"})) + assert.Equal(t, int32(8437), configReloaderHTTPPort(map[string]string{"http.listenAddr": "[::1]:8437"})) + assert.Equal(t, int32(8435), configReloaderHTTPPort(map[string]string{"http.listenAddr": "hostname-only"})) + assert.Equal(t, int32(8435), configReloaderHTTPPort(map[string]string{"http.listenAddr": "127.0.0.1:0"})) + assert.Equal(t, int32(8435), configReloaderHTTPPort(map[string]string{"http.listenAddr": "127.0.0.1:70000"})) } diff --git a/internal/controller/operator/factory/reconcile/config_reload.go b/internal/controller/operator/factory/reconcile/config_reload.go index f190313844..a79e643e1e 100644 --- a/internal/controller/operator/factory/reconcile/config_reload.go +++ b/internal/controller/operator/factory/reconcile/config_reload.go @@ -29,11 +29,24 @@ var ( configReloadWaitInterval = 2 * time.Second configReloadWaitTimeout = 60 * time.Second configReloadHTTPTimeout = 5 * time.Second - configReloaderMetricsURL = func(podIP string) string { - return fmt.Sprintf("http://%s/metrics", net.JoinHostPort(podIP, strconv.Itoa(build.ConfigReloaderDefaultPort))) + configReloaderMetricsURL = func(podIP string, port int) string { + return fmt.Sprintf("http://%s/metrics", net.JoinHostPort(podIP, strconv.Itoa(port))) } ) +// configReloaderPortFromPod returns the HTTP port named reloader-http from any container in the +// pod. Falls back to ConfigReloaderDefaultPort when that named port is absent. +func configReloaderPortFromPod(pod *corev1.Pod) int { + for _, c := range pod.Spec.Containers { + for _, p := range c.Ports { + if p.Name == build.ConfigReloaderPortName && p.ContainerPort > 0 { + return int(p.ContainerPort) + } + } + } + return build.ConfigReloaderDefaultPort +} + const ( contentHashMetricName = "configreloader_reload_content_hash" mainContentHashKey = "main" @@ -70,7 +83,7 @@ func WaitForConfigReloadHash(ctx context.Context, rclient client.Client, cr conf if !pod.DeletionTimestamp.IsZero() || !PodIsReady(&pod, 0) || pod.Status.PodIP == "" { return false, nil } - got, found, err := scrapeContentHash(ctx, httpClient, pod.Status.PodIP) + got, found, err := scrapeContentHash(ctx, httpClient, &pod) if err != nil { // transient scrape failure - keep polling rather than failing the wait outright return false, nil @@ -92,11 +105,11 @@ func HashBytes(data []byte) uint32 { return crc32.ChecksumIEEE(data) } -// scrapeContentHash fetches the config-reloader sidecar's own metrics endpoint on podIP and +// scrapeContentHash fetches the config-reloader sidecar's own metrics endpoint on the pod and // returns the confirmed content hash it currently exposes, if any (a sidecar predating this // metric reports nothing, distinguished from a genuine mismatch via the found return value). -func scrapeContentHash(ctx context.Context, httpClient *http.Client, podIP string) (uint32, bool, error) { - values, err := podutil.FetchMetricsValues(ctx, httpClient, configReloaderMetricsURL(podIP), []podutil.MetricQuery{ +func scrapeContentHash(ctx context.Context, httpClient *http.Client, pod *corev1.Pod) (uint32, bool, error) { + values, err := podutil.FetchMetricsValues(ctx, httpClient, configReloaderMetricsURL(pod.Status.PodIP, configReloaderPortFromPod(pod)), []podutil.MetricQuery{ {Name: contentHashMetricName, Dimension: "key"}, }) if err != nil { diff --git a/internal/controller/operator/factory/reconcile/config_reload_test.go b/internal/controller/operator/factory/reconcile/config_reload_test.go index 2f18995759..607a173760 100644 --- a/internal/controller/operator/factory/reconcile/config_reload_test.go +++ b/internal/controller/operator/factory/reconcile/config_reload_test.go @@ -20,9 +20,53 @@ import ( ) func TestConfigReloaderMetricsURL(t *testing.T) { - assert.Equal(t, fmt.Sprintf("http://10.0.0.1:%d/metrics", build.ConfigReloaderDefaultPort), configReloaderMetricsURL("10.0.0.1")) + assert.Equal(t, fmt.Sprintf("http://10.0.0.1:%d/metrics", build.ConfigReloaderDefaultPort), configReloaderMetricsURL("10.0.0.1", build.ConfigReloaderDefaultPort)) // IPv6 must be bracketed, otherwise the port separator is ambiguous with the address itself - assert.Equal(t, fmt.Sprintf("http://[2001:db8::1]:%d/metrics", build.ConfigReloaderDefaultPort), configReloaderMetricsURL("2001:db8::1")) + assert.Equal(t, fmt.Sprintf("http://[2001:db8::1]:%d/metrics", build.ConfigReloaderDefaultPort), configReloaderMetricsURL("2001:db8::1", build.ConfigReloaderDefaultPort)) + assert.Equal(t, "http://10.0.0.1:8436/metrics", configReloaderMetricsURL("10.0.0.1", 8436)) +} + +func TestConfigReloaderPortFromPod(t *testing.T) { + assert.Equal(t, build.ConfigReloaderDefaultPort, configReloaderPortFromPod(&corev1.Pod{})) + + namedOnOtherContainer := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "sidecar", + Ports: []corev1.ContainerPort{{ + Name: build.ConfigReloaderPortName, + ContainerPort: 8436, + }}, + }}, + }, + } + assert.Equal(t, 8436, configReloaderPortFromPod(namedOnOtherContainer)) + + namedAfterExtraPort := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "sidecar", + Ports: []corev1.ContainerPort{ + {Name: "extra", ContainerPort: 9999}, + {Name: build.ConfigReloaderPortName, ContainerPort: 8436}, + }, + }}, + }, + } + assert.Equal(t, 8436, configReloaderPortFromPod(namedAfterExtraPort)) + + noNamedPort := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "sidecar", + Ports: []corev1.ContainerPort{{ + Name: "extra", + ContainerPort: 9999, + }}, + }}, + }, + } + assert.Equal(t, build.ConfigReloaderDefaultPort, configReloaderPortFromPod(noNamedPort)) } func TestWaitForConfigReloadHash_NoPodsIsNoop(t *testing.T) { @@ -55,7 +99,7 @@ func withConfigReloaderMetricsURL(t *testing.T, ts *httptest.Server) { orig := configReloaderMetricsURL u, err := url.Parse(ts.URL) assert.NoError(t, err) - configReloaderMetricsURL = func(string) string { return "http://" + u.Host + "/metrics" } + configReloaderMetricsURL = func(string, int) string { return "http://" + u.Host + "/metrics" } t.Cleanup(func() { configReloaderMetricsURL = orig }) } diff --git a/internal/controller/operator/factory/vmagent/vmagent_test.go b/internal/controller/operator/factory/vmagent/vmagent_test.go index 022c72dfed..eb97d62485 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_test.go @@ -2122,7 +2122,7 @@ func TestBuildVMAgentServiceScrapeConfigReloaderPort(t *testing.T) { hasReloaderEndpoint := func(scrape *vmv1beta1.VMServiceScrape) bool { for _, ep := range scrape.Spec.Endpoints { - if ep.TargetPort != nil && ep.TargetPort.IntValue() == build.ConfigReloaderDefaultPort { + if ep.TargetPort != nil && ep.TargetPort.StrVal == build.ConfigReloaderPortName { return true } } diff --git a/test/e2e/vmauth_test.go b/test/e2e/vmauth_test.go index d745865b26..c4d4b3a4f2 100644 --- a/test/e2e/vmauth_test.go +++ b/test/e2e/vmauth_test.go @@ -441,7 +441,7 @@ var _ = Describe("test vmauth Controller", Label("vm", "auth"), func() { Expect(k8sClient.Get(ctx, nsn, &vmss)).ToNot(HaveOccurred()) Expect(vmss.Spec.Endpoints).To(HaveLen(2)) Expect(vmss.Spec.Endpoints[0].Port).To(Equal("internal")) - Expect(vmss.Spec.Endpoints[1].TargetPort).To(Equal(ptr.To(intstr.FromInt32(int32(build.ConfigReloaderDefaultPort))))) + Expect(vmss.Spec.Endpoints[1].TargetPort).To(Equal(ptr.To(intstr.FromString(build.ConfigReloaderPortName)))) }, }, ),