From bcf9853e67f0da9e61a80adced8b6381f9ae4a3f Mon Sep 17 00:00:00 2001 From: Ivan Kolesnikov Date: Fri, 4 Sep 2026 20:33:52 +0300 Subject: [PATCH 1/3] fix: derive config-reloader port from http.listenAddr Parse ConfigReloaderExtraArgs["http.listenAddr"] into ContainerPort, probes, named scrape targets, and WaitForConfigReloadHash so hostNetwork DaemonSets can use distinct reloader ports without scheduler conflicts. Fixes #2584 --- docs/CHANGELOG.md | 1 + .../operator/factory/build/container.go | 62 ++++-- .../operator/factory/build/container_test.go | 191 ++++++++++++++++++ .../factory/reconcile/config_reload.go | 38 +++- .../factory/reconcile/config_reload_test.go | 88 +++++--- .../operator/factory/vmagent/vmagent_test.go | 2 +- test/e2e/vmauth_test.go | 2 +- 7 files changed, 330 insertions(+), 54 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b1323c8676..f3b9d0c0bd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -39,6 +39,7 @@ aliases: * 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). ## [v0.74.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.74.1) **Release date:** 04 Aug 2026 diff --git a/internal/controller/operator/factory/build/container.go b/internal/controller/operator/factory/build/container.go index c918a2a399..b2c154b808 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,39 @@ 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 configReloaderProbeHandler(port int32) corev1.ProbeHandler { + return corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Scheme: "HTTP", + Port: intstr.FromInt32(port), + }, + } } func configReloaderJobRelabeling() vmv1beta1.EndpointRelabelings { @@ -355,11 +377,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 +389,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 +474,18 @@ 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 := configReloaderProbeHandler(port) crContainer.Ports = append(crContainer.Ports, corev1.ContainerPort{ - ContainerPort: int32(ConfigReloaderDefaultPort), + ContainerPort: port, Name: ConfigReloaderPortName, Protocol: "TCP", }) @@ -472,7 +494,7 @@ func addPortProbesToConfigReloaderContainer(crContainer *corev1.Container) { SuccessThreshold: 1, FailureThreshold: 3, PeriodSeconds: 10, - ProbeHandler: configReloaderContainerProbe, + ProbeHandler: probe, } crContainer.ReadinessProbe = &corev1.Probe{ InitialDelaySeconds: 5, @@ -480,7 +502,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..df3754cbb4 100644 --- a/internal/controller/operator/factory/build/container_test.go +++ b/internal/controller/operator/factory/build/container_test.go @@ -1006,4 +1006,195 @@ 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, + }, + }, + }) + + // IPv6 http.listenAddr + f(opts{ + cr: &vmv1beta1.VMAlert{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "listen-ipv6", + }, + Spec: vmv1beta1.VMAlertSpec{ + CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ + ConfigReloaderExtraArgs: map[string]string{ + "http.listenAddr": "[::1]:8437", + }, + }, + }, + }, + expectedContainer: corev1.Container{ + Name: "config-reloader", + Args: []string{ + "--http.listenAddr=[::1]:8437", + "--reload-url=http://127.0.0.1:/-/reload", + "--webhook-method=POST", + }, + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + Ports: []corev1.ContainerPort{ + { + Name: "reloader-http", + Protocol: corev1.ProtocolTCP, + ContainerPort: 8437, + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8437), + Scheme: "HTTP", + }, + }, + TimeoutSeconds: 1, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8437), + Scheme: "HTTP", + }, + }, + InitialDelaySeconds: 5, + TimeoutSeconds: 1, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + }, + }) + + // invalid http.listenAddr falls back to default port + f(opts{ + cr: &vmv1beta1.VMAlert{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "listen-invalid", + }, + Spec: vmv1beta1.VMAlertSpec{ + CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ + ConfigReloaderExtraArgs: map[string]string{ + "http.listenAddr": "not-a-host-port", + }, + }, + }, + }, + expectedContainer: corev1.Container{ + Name: "config-reloader", + Args: []string{ + "--http.listenAddr=not-a-host-port", + "--reload-url=http://127.0.0.1:/-/reload", + "--webhook-method=POST", + }, + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + Ports: []corev1.ContainerPort{ + { + Name: "reloader-http", + Protocol: corev1.ProtocolTCP, + ContainerPort: 8435, + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8435), + Scheme: "HTTP", + }, + }, + TimeoutSeconds: 1, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8435), + 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..834781bb7c 100644 --- a/internal/controller/operator/factory/reconcile/config_reload.go +++ b/internal/controller/operator/factory/reconcile/config_reload.go @@ -29,11 +29,35 @@ 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))) + } + configReloadNewHTTPClient = func() *http.Client { + return &http.Client{Timeout: configReloadHTTPTimeout} } ) +// configReloaderPortFromPod returns the config-reloader HTTP port from the pod's container ports, +// preferring the named reloader-http port. Falls back to ConfigReloaderDefaultPort. +func configReloaderPortFromPod(pod *corev1.Pod) int { + for _, c := range pod.Spec.Containers { + if c.Name != "config-reloader" { + continue + } + for _, p := range c.Ports { + if p.Name == build.ConfigReloaderPortName && p.ContainerPort > 0 { + return int(p.ContainerPort) + } + } + for _, p := range c.Ports { + if p.ContainerPort > 0 { + return int(p.ContainerPort) + } + } + } + return build.ConfigReloaderDefaultPort +} + const ( contentHashMetricName = "configreloader_reload_content_hash" mainContentHashKey = "main" @@ -43,7 +67,7 @@ const ( // confirmed applying content matching hash, an exact CRC32 match rather than a wall-clock // heuristic. Skipped rather than blocking if a pod's sidecar predates this metric. func WaitForConfigReloadHash(ctx context.Context, rclient client.Client, cr configReloadWaitable, hash uint32) error { - httpClient := &http.Client{Timeout: configReloadHTTPTimeout} + httpClient := configReloadNewHTTPClient() selector := labels.SelectorFromSet(cr.SelectorLabels()) listOpts := &client.ListOptions{ LabelSelector: selector, @@ -70,7 +94,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 +116,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..2d8733b8dc 100644 --- a/internal/controller/operator/factory/reconcile/config_reload_test.go +++ b/internal/controller/operator/factory/reconcile/config_reload_test.go @@ -3,9 +3,9 @@ package reconcile import ( "context" "fmt" + "io" "net/http" - "net/http/httptest" - "net/url" + "strings" "testing" "time" @@ -20,9 +20,27 @@ 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{})) + + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "config-reloader", + Ports: []corev1.ContainerPort{{ + Name: build.ConfigReloaderPortName, + ContainerPort: 8436, + }}, + }}, + }, + } + assert.Equal(t, 8436, configReloaderPortFromPod(pod)) } func TestWaitForConfigReloadHash_NoPodsIsNoop(t *testing.T) { @@ -40,6 +58,15 @@ func TestWaitForConfigReloadHash_NoPodsIsNoop(t *testing.T) { func readyPod(name, namespace, ip string, labels map[string]string) *corev1.Pod { return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, Labels: labels}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "config-reloader", + Ports: []corev1.ContainerPort{{ + Name: build.ConfigReloaderPortName, + ContainerPort: int32(build.ConfigReloaderDefaultPort), + }}, + }}, + }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, PodIP: ip, @@ -50,29 +77,46 @@ func readyPod(name, namespace, ip string, labels map[string]string) *corev1.Pod } } -func withConfigReloaderMetricsURL(t *testing.T, ts *httptest.Server) { +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +// withConfigReloaderMetricsResponse stubs the sidecar /metrics HTTP response in-process +// (avoids httptest, which is unreliable when loopback is broken in the test environment). +func withConfigReloaderMetricsResponse(t *testing.T, body string) { t.Helper() - orig := configReloaderMetricsURL - u, err := url.Parse(ts.URL) - assert.NoError(t, err) - configReloaderMetricsURL = func(string) string { return "http://" + u.Host + "/metrics" } - t.Cleanup(func() { configReloaderMetricsURL = orig }) + origURL := configReloaderMetricsURL + origClient := configReloadNewHTTPClient + configReloaderMetricsURL = func(string, int) string { return "http://config-reloader.test/metrics" } + configReloadNewHTTPClient = func() *http.Client { + return &http.Client{ + Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + Header: make(http.Header), + }, nil + }), + } + } + t.Cleanup(func() { + configReloaderMetricsURL = origURL + configReloadNewHTTPClient = origClient + }) } func TestWaitForConfigReloadHash(t *testing.T) { cr := &vmv1beta1.VMAuth{ ObjectMeta: metav1.ObjectMeta{Name: "vmauth", Namespace: "default"}, } - labels := cr.SelectorLabels() - pod := readyPod("vmauth-0", cr.Namespace, "10.0.0.1", labels) + sel := cr.SelectorLabels() + pod := readyPod("vmauth-0", cr.Namespace, "10.0.0.1", sel) // exact hash match succeeds immediately. t.Run("match succeeds", func(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - fmt.Fprintf(w, "configreloader_reload_content_hash{key=\"main\"} %d\n", 42) - })) - defer ts.Close() - withConfigReloaderMetricsURL(t, ts) + withConfigReloaderMetricsResponse(t, fmt.Sprintf("configreloader_reload_content_hash{key=\"main\"} %d\n", 42)) fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) start := time.Now() @@ -82,9 +126,7 @@ func TestWaitForConfigReloadHash(t *testing.T) { // metric absent entirely (sidecar predates it) - skipped rather than blocking. t.Run("absent metric is skipped", func(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {})) - defer ts.Close() - withConfigReloaderMetricsURL(t, ts) + withConfigReloaderMetricsResponse(t, "") fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) start := time.Now() @@ -99,11 +141,7 @@ func TestWaitForConfigReloadHash(t *testing.T) { configReloadWaitTimeout = 20 * time.Millisecond defer func() { configReloadWaitInterval, configReloadWaitTimeout = origInterval, origTimeout }() - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - fmt.Fprintf(w, "configreloader_reload_content_hash{key=\"main\"} %d\n", 99) - })) - defer ts.Close() - withConfigReloaderMetricsURL(t, ts) + withConfigReloaderMetricsResponse(t, fmt.Sprintf("configreloader_reload_content_hash{key=\"main\"} %d\n", 99)) fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) assert.Error(t, WaitForConfigReloadHash(context.Background(), fclient, cr, 42)) 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)))) }, }, ), From 214324ef3146858d71ab57c7baa8ea9f4abb68cb Mon Sep 17 00:00:00 2001 From: Ivan Kolesnikov Date: Mon, 7 Sep 2026 12:02:52 +0300 Subject: [PATCH 2/3] fix: simplify config-reloader port lookup Find the config-reloader HTTP port by its stable reloader-http name. Drop the container-name dependency and arbitrary first-port fallback. --- .../operator/factory/build/container.go | 18 +-- .../operator/factory/build/container_test.go | 120 ------------------ .../factory/reconcile/config_reload.go | 12 +- .../factory/reconcile/config_reload_test.go | 49 ++++--- 4 files changed, 42 insertions(+), 157 deletions(-) diff --git a/internal/controller/operator/factory/build/container.go b/internal/controller/operator/factory/build/container.go index b2c154b808..3a90d10045 100644 --- a/internal/controller/operator/factory/build/container.go +++ b/internal/controller/operator/factory/build/container.go @@ -353,16 +353,6 @@ func configReloaderHTTPPort(extraArgs map[string]string) int32 { return int32(port) } -func configReloaderProbeHandler(port int32) corev1.ProbeHandler { - return corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Scheme: "HTTP", - Port: intstr.FromInt32(port), - }, - } -} - func configReloaderJobRelabeling() vmv1beta1.EndpointRelabelings { return vmv1beta1.EndpointRelabelings{ RelabelConfigs: []*vmv1beta1.RelabelConfig{ @@ -483,7 +473,13 @@ func ConfigReloaderContainer(isInit bool, cr reloadable, mounts []corev1.VolumeM // 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 := configReloaderProbeHandler(port) + probe := corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Scheme: "HTTP", + Port: intstr.FromInt32(port), + }, + } crContainer.Ports = append(crContainer.Ports, corev1.ContainerPort{ ContainerPort: port, Name: ConfigReloaderPortName, diff --git a/internal/controller/operator/factory/build/container_test.go b/internal/controller/operator/factory/build/container_test.go index df3754cbb4..0b8b0fa08d 100644 --- a/internal/controller/operator/factory/build/container_test.go +++ b/internal/controller/operator/factory/build/container_test.go @@ -1066,126 +1066,6 @@ func TestBuildConfigReloaderContainer(t *testing.T) { }, }, }) - - // IPv6 http.listenAddr - f(opts{ - cr: &vmv1beta1.VMAlert{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "listen-ipv6", - }, - Spec: vmv1beta1.VMAlertSpec{ - CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ - ConfigReloaderExtraArgs: map[string]string{ - "http.listenAddr": "[::1]:8437", - }, - }, - }, - }, - expectedContainer: corev1.Container{ - Name: "config-reloader", - Args: []string{ - "--http.listenAddr=[::1]:8437", - "--reload-url=http://127.0.0.1:/-/reload", - "--webhook-method=POST", - }, - TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, - Ports: []corev1.ContainerPort{ - { - Name: "reloader-http", - Protocol: corev1.ProtocolTCP, - ContainerPort: 8437, - }, - }, - LivenessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Port: intstr.FromInt32(8437), - Scheme: "HTTP", - }, - }, - TimeoutSeconds: 1, - PeriodSeconds: 10, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Port: intstr.FromInt32(8437), - Scheme: "HTTP", - }, - }, - InitialDelaySeconds: 5, - TimeoutSeconds: 1, - PeriodSeconds: 10, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - }, - }) - - // invalid http.listenAddr falls back to default port - f(opts{ - cr: &vmv1beta1.VMAlert{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "listen-invalid", - }, - Spec: vmv1beta1.VMAlertSpec{ - CommonConfigReloaderParams: vmv1beta1.CommonConfigReloaderParams{ - ConfigReloaderExtraArgs: map[string]string{ - "http.listenAddr": "not-a-host-port", - }, - }, - }, - }, - expectedContainer: corev1.Container{ - Name: "config-reloader", - Args: []string{ - "--http.listenAddr=not-a-host-port", - "--reload-url=http://127.0.0.1:/-/reload", - "--webhook-method=POST", - }, - TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, - Ports: []corev1.ContainerPort{ - { - Name: "reloader-http", - Protocol: corev1.ProtocolTCP, - ContainerPort: 8435, - }, - }, - LivenessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Port: intstr.FromInt32(8435), - Scheme: "HTTP", - }, - }, - TimeoutSeconds: 1, - PeriodSeconds: 10, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Port: intstr.FromInt32(8435), - Scheme: "HTTP", - }, - }, - InitialDelaySeconds: 5, - TimeoutSeconds: 1, - PeriodSeconds: 10, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - }, - }) } func TestConfigReloaderHTTPPort(t *testing.T) { diff --git a/internal/controller/operator/factory/reconcile/config_reload.go b/internal/controller/operator/factory/reconcile/config_reload.go index 834781bb7c..eeaffa6d13 100644 --- a/internal/controller/operator/factory/reconcile/config_reload.go +++ b/internal/controller/operator/factory/reconcile/config_reload.go @@ -37,23 +37,15 @@ var ( } ) -// configReloaderPortFromPod returns the config-reloader HTTP port from the pod's container ports, -// preferring the named reloader-http port. Falls back to ConfigReloaderDefaultPort. +// 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 { - if c.Name != "config-reloader" { - continue - } for _, p := range c.Ports { if p.Name == build.ConfigReloaderPortName && p.ContainerPort > 0 { return int(p.ContainerPort) } } - for _, p := range c.Ports { - if p.ContainerPort > 0 { - return int(p.ContainerPort) - } - } } return build.ConfigReloaderDefaultPort } diff --git a/internal/controller/operator/factory/reconcile/config_reload_test.go b/internal/controller/operator/factory/reconcile/config_reload_test.go index 2d8733b8dc..a3ce5b6a97 100644 --- a/internal/controller/operator/factory/reconcile/config_reload_test.go +++ b/internal/controller/operator/factory/reconcile/config_reload_test.go @@ -29,10 +29,10 @@ func TestConfigReloaderMetricsURL(t *testing.T) { func TestConfigReloaderPortFromPod(t *testing.T) { assert.Equal(t, build.ConfigReloaderDefaultPort, configReloaderPortFromPod(&corev1.Pod{})) - pod := &corev1.Pod{ + namedOnOtherContainer := &corev1.Pod{ Spec: corev1.PodSpec{ Containers: []corev1.Container{{ - Name: "config-reloader", + Name: "sidecar", Ports: []corev1.ContainerPort{{ Name: build.ConfigReloaderPortName, ContainerPort: 8436, @@ -40,7 +40,33 @@ func TestConfigReloaderPortFromPod(t *testing.T) { }}, }, } - assert.Equal(t, 8436, configReloaderPortFromPod(pod)) + 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) { @@ -58,15 +84,6 @@ func TestWaitForConfigReloadHash_NoPodsIsNoop(t *testing.T) { func readyPod(name, namespace, ip string, labels map[string]string) *corev1.Pod { return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, Labels: labels}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: "config-reloader", - Ports: []corev1.ContainerPort{{ - Name: build.ConfigReloaderPortName, - ContainerPort: int32(build.ConfigReloaderDefaultPort), - }}, - }}, - }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, PodIP: ip, @@ -83,8 +100,8 @@ func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } -// withConfigReloaderMetricsResponse stubs the sidecar /metrics HTTP response in-process -// (avoids httptest, which is unreliable when loopback is broken in the test environment). +// withConfigReloaderMetricsResponse stubs the sidecar /metrics body in-process. httptest is +// unreliable here when loopback is broken, so the wait tests drive an injected Transport. func withConfigReloaderMetricsResponse(t *testing.T, body string) { t.Helper() origURL := configReloaderMetricsURL @@ -111,8 +128,8 @@ func TestWaitForConfigReloadHash(t *testing.T) { cr := &vmv1beta1.VMAuth{ ObjectMeta: metav1.ObjectMeta{Name: "vmauth", Namespace: "default"}, } - sel := cr.SelectorLabels() - pod := readyPod("vmauth-0", cr.Namespace, "10.0.0.1", sel) + labels := cr.SelectorLabels() + pod := readyPod("vmauth-0", cr.Namespace, "10.0.0.1", labels) // exact hash match succeeds immediately. t.Run("match succeeds", func(t *testing.T) { From 9212efb00f105474a5256a4cb1ee41ac99acb097 Mon Sep 17 00:00:00 2001 From: Ivan Kolesnikov Date: Mon, 7 Sep 2026 14:37:59 +0300 Subject: [PATCH 3/3] fix: restore config reload HTTP tests Keep the existing httptest pattern and remove the local HTTP transport workaround from the config-reloader port change. --- .../factory/reconcile/config_reload.go | 5 +- .../factory/reconcile/config_reload_test.go | 53 ++++++++----------- 2 files changed, 22 insertions(+), 36 deletions(-) diff --git a/internal/controller/operator/factory/reconcile/config_reload.go b/internal/controller/operator/factory/reconcile/config_reload.go index eeaffa6d13..a79e643e1e 100644 --- a/internal/controller/operator/factory/reconcile/config_reload.go +++ b/internal/controller/operator/factory/reconcile/config_reload.go @@ -32,9 +32,6 @@ var ( configReloaderMetricsURL = func(podIP string, port int) string { return fmt.Sprintf("http://%s/metrics", net.JoinHostPort(podIP, strconv.Itoa(port))) } - configReloadNewHTTPClient = func() *http.Client { - return &http.Client{Timeout: configReloadHTTPTimeout} - } ) // configReloaderPortFromPod returns the HTTP port named reloader-http from any container in the @@ -59,7 +56,7 @@ const ( // confirmed applying content matching hash, an exact CRC32 match rather than a wall-clock // heuristic. Skipped rather than blocking if a pod's sidecar predates this metric. func WaitForConfigReloadHash(ctx context.Context, rclient client.Client, cr configReloadWaitable, hash uint32) error { - httpClient := configReloadNewHTTPClient() + httpClient := &http.Client{Timeout: configReloadHTTPTimeout} selector := labels.SelectorFromSet(cr.SelectorLabels()) listOpts := &client.ListOptions{ LabelSelector: selector, diff --git a/internal/controller/operator/factory/reconcile/config_reload_test.go b/internal/controller/operator/factory/reconcile/config_reload_test.go index a3ce5b6a97..607a173760 100644 --- a/internal/controller/operator/factory/reconcile/config_reload_test.go +++ b/internal/controller/operator/factory/reconcile/config_reload_test.go @@ -3,9 +3,9 @@ package reconcile import ( "context" "fmt" - "io" "net/http" - "strings" + "net/http/httptest" + "net/url" "testing" "time" @@ -94,34 +94,13 @@ func readyPod(name, namespace, ip string, labels map[string]string) *corev1.Pod } } -type roundTripFunc func(*http.Request) (*http.Response, error) - -func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { - return f(r) -} - -// withConfigReloaderMetricsResponse stubs the sidecar /metrics body in-process. httptest is -// unreliable here when loopback is broken, so the wait tests drive an injected Transport. -func withConfigReloaderMetricsResponse(t *testing.T, body string) { +func withConfigReloaderMetricsURL(t *testing.T, ts *httptest.Server) { t.Helper() - origURL := configReloaderMetricsURL - origClient := configReloadNewHTTPClient - configReloaderMetricsURL = func(string, int) string { return "http://config-reloader.test/metrics" } - configReloadNewHTTPClient = func() *http.Client { - return &http.Client{ - Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(strings.NewReader(body)), - Header: make(http.Header), - }, nil - }), - } - } - t.Cleanup(func() { - configReloaderMetricsURL = origURL - configReloadNewHTTPClient = origClient - }) + orig := configReloaderMetricsURL + u, err := url.Parse(ts.URL) + assert.NoError(t, err) + configReloaderMetricsURL = func(string, int) string { return "http://" + u.Host + "/metrics" } + t.Cleanup(func() { configReloaderMetricsURL = orig }) } func TestWaitForConfigReloadHash(t *testing.T) { @@ -133,7 +112,11 @@ func TestWaitForConfigReloadHash(t *testing.T) { // exact hash match succeeds immediately. t.Run("match succeeds", func(t *testing.T) { - withConfigReloaderMetricsResponse(t, fmt.Sprintf("configreloader_reload_content_hash{key=\"main\"} %d\n", 42)) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, "configreloader_reload_content_hash{key=\"main\"} %d\n", 42) + })) + defer ts.Close() + withConfigReloaderMetricsURL(t, ts) fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) start := time.Now() @@ -143,7 +126,9 @@ func TestWaitForConfigReloadHash(t *testing.T) { // metric absent entirely (sidecar predates it) - skipped rather than blocking. t.Run("absent metric is skipped", func(t *testing.T) { - withConfigReloaderMetricsResponse(t, "") + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {})) + defer ts.Close() + withConfigReloaderMetricsURL(t, ts) fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) start := time.Now() @@ -158,7 +143,11 @@ func TestWaitForConfigReloadHash(t *testing.T) { configReloadWaitTimeout = 20 * time.Millisecond defer func() { configReloadWaitInterval, configReloadWaitTimeout = origInterval, origTimeout }() - withConfigReloaderMetricsResponse(t, fmt.Sprintf("configreloader_reload_content_hash{key=\"main\"} %d\n", 99)) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, "configreloader_reload_content_hash{key=\"main\"} %d\n", 99) + })) + defer ts.Close() + withConfigReloaderMetricsURL(t, ts) fclient := k8stools.GetTestClientWithObjects([]runtime.Object{pod}) assert.Error(t, WaitForConfigReloadHash(context.Background(), fclient, cr, 42))