From afe9d0a72c95a02504c65fbdaf5898c4182e0553 Mon Sep 17 00:00:00 2001 From: Raman Kumar Date: Fri, 19 Jun 2026 13:47:39 +0530 Subject: [PATCH 1/2] fix(opencost): make agent OpenCost connection detection reliable The agent reports opencostConnection=true only when it has a non-empty OpenCost URL and /healthz returns 2xx. Three defects kept that false even when OpenCost was healthy: 1. URL resolved once at boot. opencostURL was discovered a single time at startup and captured by the telemetry closure, while every other datasource in that closure is re-probed per tick. If OpenCost was not deployed/ready at boot, the URL stayed empty forever and opencostConnection never recovered without a pod restart. Resolve it per tick instead (cached ~CacheTTL by the Discoverer, so no extra API load) so the agent self-heals. 2. Discovery picked the Service's first port. findOne hardcoded Ports[0]. OpenCost's Service can expose both the cost-model API (9003, serves /healthz) and a UI port (9090); if the UI port sorts first, /healthz was probed on it and a healthy OpenCost reported down. Add FindFirstPreferPort and resolve OpenCost preferring port 9003, falling back to the first port otherwise. 3. No endpoint override. The chart wired only OPENCOST_ENABLED, leaving the agent fully dependent on the two label selectors. Wire optional opencost.endpoint -> OPENCOST_ENDPOINT for non-standard installs (the binary already honoured the env). Adds unit tests for port preference (selectPort, FindFirstPreferPort) covering the multi-port and fallback cases. Co-Authored-By: Claude Opus 4.8 (1M context) --- charts/nudgebee-agent/templates/runner.yaml | 5 ++ charts/nudgebee-agent/values.yaml | 5 ++ runner/cmd/agent/main.go | 31 +++++---- runner/pkg/svcdiscover/discover.go | 35 +++++++++- runner/pkg/svcdiscover/discover_test.go | 75 +++++++++++++++++++++ 5 files changed, 136 insertions(+), 15 deletions(-) diff --git a/charts/nudgebee-agent/templates/runner.yaml b/charts/nudgebee-agent/templates/runner.yaml index 22df8de4..8fb300c3 100644 --- a/charts/nudgebee-agent/templates/runner.yaml +++ b/charts/nudgebee-agent/templates/runner.yaml @@ -94,6 +94,11 @@ stringData: # disabled the runner must NOT autodiscover a neighbouring namespace's OpenCost, # so the server side detects cost is off and takes over centrally. OPENCOST_ENABLED: {{ .Values.opencost.enabled | quote }} + {{- if .Values.opencost.endpoint }} + # Explicit OpenCost cost-model API endpoint (serves /healthz). Overrides + # in-cluster autodiscovery; set when OpenCost runs under non-standard labels. + OPENCOST_ENDPOINT: {{ .Values.opencost.endpoint | quote }} + {{- end }} {{- if .Values.globalConfig.prometheus_url }} PROMETHEUS_URL: {{ .Values.globalConfig.prometheus_url | quote }} {{- end }} diff --git a/charts/nudgebee-agent/values.yaml b/charts/nudgebee-agent/values.yaml index 0b87463c..8bba4ed6 100644 --- a/charts/nudgebee-agent/values.yaml +++ b/charts/nudgebee-agent/values.yaml @@ -220,6 +220,11 @@ nodeAgent: # `--set opencost.enabled=true`. opencost: enabled: false + # Optional explicit OpenCost cost-model API endpoint (the one that serves + # /healthz, typically port 9003). Leave empty to autodiscover the in-cluster + # OpenCost Service by label; set it when OpenCost runs under non-standard + # labels that autodiscovery won't match. Maps to OPENCOST_ENDPOINT. + endpoint: "" nameOverride: "" fullnameOverride: "" opencost: diff --git a/runner/cmd/agent/main.go b/runner/cmd/agent/main.go index 9630d606..fb98bccb 100644 --- a/runner/cmd/agent/main.go +++ b/runner/cmd/agent/main.go @@ -235,18 +235,13 @@ func run(ctx context.Context, logger *slog.Logger, cfg *config.Config) error { logger.Warn("invalid OPENCOST_ENABLED, defaulting to enabled", "value", v, "err", err) } } - opencostURL := "" - if opencostEnabled { - // Mirrors OpenCostDiscovery.find_open_cost_url. - // OPENCOST_ENDPOINT env wins; falls back to in-cluster Service lookup. - opencostURL = os.Getenv("OPENCOST_ENDPOINT") - if opencostURL == "" { - if u := disc.FindFirst(ctx, svcdiscover.OpencostSelectors); u != "" { - opencostURL = u - logger.Info("opencost auto-discovered", "url", u) - } - } - } else { + // OPENCOST_ENDPOINT (if set) wins over autodiscovery; read once since env + // doesn't change. The URL itself is resolved per telemetry tick inside the + // Datasources closure below (not here) so the agent self-heals when OpenCost + // is deployed after the agent boots — resolving only at startup would latch + // the boot-time result (empty → opencostConnection stuck false) forever. + opencostEndpoint := os.Getenv("OPENCOST_ENDPOINT") + if !opencostEnabled { logger.Info("opencost disabled (OPENCOST_ENABLED=false); skipping discovery and cost polling") } @@ -923,6 +918,18 @@ func run(ctx context.Context, logger *slog.Logger, cfg *config.Config) error { logsProvider, logsURL, logsOK, logCfg := probeLogsProvider(probeCtx, cfg) as := telemetry.DetectAutoScaler(probeCtx, typedKube, providerInfo.Provider, logger) clickhouseStatus := probeClickhouse(probeCtx, probeClient, clickhouseHost, clickhousePort) + // Resolve OpenCost per tick (cached ~CacheTTL by the Discoverer) so a + // late-deployed OpenCost is picked up without an agent restart. Mirrors + // OpenCostDiscovery.find_open_cost_url: OPENCOST_ENDPOINT wins, else + // autodiscover — preferring the cost-model API port (9003, serves + // /healthz) over a UI port. Empty when OpenCost is disabled or absent. + opencostURL := "" + if opencostEnabled { + opencostURL = opencostEndpoint + if opencostURL == "" { + opencostURL = disc.FindFirstPreferPort(probeCtx, svcdiscover.OpencostSelectors, 9003) + } + } return telemetry.Datasources{ PrometheusURL: cfg.PrometheusURL, AlertManagerURL: cfg.AlertManagerURL, diff --git a/runner/pkg/svcdiscover/discover.go b/runner/pkg/svcdiscover/discover.go index 60be3df3..4bc48fc8 100644 --- a/runner/pkg/svcdiscover/discover.go +++ b/runner/pkg/svcdiscover/discover.go @@ -19,6 +19,7 @@ import ( "sync" "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -106,10 +107,24 @@ func New(cs kubernetes.Interface, clusterDomain string) *Discoverer { // or "" if none. Negative results are cached too so we don't keep listing // services on every call. func (d *Discoverer) FindFirst(ctx context.Context, selectors []string) string { + return d.FindFirstPreferPort(ctx, selectors) +} + +// FindFirstPreferPort is FindFirst with port preference: when a matched Service +// exposes multiple ports, the first port whose number appears in preferredPorts +// (in order) is used, falling back to the Service's first port. This matters for +// OpenCost, whose Service can expose both the cost-model API (9003, which serves +// /healthz) and a UI port (9090) — probing /healthz on the UI port reports an +// otherwise-healthy OpenCost as down. Results (including misses) are cached per +// (selectors, preferredPorts) for CacheTTL. +func (d *Discoverer) FindFirstPreferPort(ctx context.Context, selectors []string, preferredPorts ...int32) string { if d == nil || d.cs == nil { return "" } cacheKey := strings.Join(selectors, "|") + for _, p := range preferredPorts { + cacheKey += fmt.Sprintf("#%d", p) + } now := time.Now() d.mu.Lock() @@ -121,7 +136,7 @@ func (d *Discoverer) FindFirst(ctx context.Context, selectors []string) string { url := "" for _, sel := range selectors { - if u := d.findOne(ctx, sel); u != "" { + if u := d.findOne(ctx, sel, preferredPorts); u != "" { url = u break } @@ -133,7 +148,7 @@ func (d *Discoverer) FindFirst(ctx context.Context, selectors []string) string { return url } -func (d *Discoverer) findOne(ctx context.Context, selector string) string { +func (d *Discoverer) findOne(ctx context.Context, selector string, preferredPorts []int32) string { list, err := d.cs.CoreV1().Services("").List(ctx, metav1.ListOptions{LabelSelector: selector}) if err != nil || len(list.Items) == 0 { return "" @@ -142,10 +157,24 @@ func (d *Discoverer) findOne(ctx context.Context, selector string) string { if len(svc.Spec.Ports) == 0 { return "" } - port := svc.Spec.Ports[0].Port + port := selectPort(svc.Spec.Ports, preferredPorts) return fmt.Sprintf("http://%s.%s.svc.%s:%d", svc.Name, svc.Namespace, d.clusterDomain, port) } +// selectPort returns the first port whose number is in preferredPorts (in +// preference order); if none match — or preferredPorts is empty — it returns the +// Service's first port, preserving the original single-port behaviour. +func selectPort(ports []corev1.ServicePort, preferredPorts []int32) int32 { + for _, want := range preferredPorts { + for i := range ports { + if ports[i].Port == want { + return want + } + } + } + return ports[0].Port +} + // Coalesce returns the first non-empty value. Used at startup when wiring // configured envs against autodiscovered URLs: // diff --git a/runner/pkg/svcdiscover/discover_test.go b/runner/pkg/svcdiscover/discover_test.go index 122478f2..ac23a11d 100644 --- a/runner/pkg/svcdiscover/discover_test.go +++ b/runner/pkg/svcdiscover/discover_test.go @@ -101,6 +101,70 @@ func TestNilDiscovererReturnsEmpty(t *testing.T) { } } +// TestFindFirstPreferPort_PicksAPIPortOverUI seeds an OpenCost Service whose +// first port is the UI (9090) and second is the cost-model API (9003, /healthz). +// Without a preference the first port wins and /healthz is probed on the UI port +// — reporting a healthy OpenCost as down. With prefer 9003 the API port wins. +func TestFindFirstPreferPort_PicksAPIPortOverUI(t *testing.T) { + cs := fake.NewClientset( + mkServicePorts("opencost", "opencost", map[string]string{"app": "opencost"}, 9090, 9003), + ) + d := New(cs, "cluster.local") + + // Default behaviour: first port (UI) — the bug. + if got, want := d.FindFirst(context.Background(), OpencostSelectors), + "http://opencost.opencost.svc.cluster.local:9090"; got != want { + t.Errorf("FindFirst got %q; want %q", got, want) + } + // With preference the cost-model API port wins. + if got, want := d.FindFirstPreferPort(context.Background(), OpencostSelectors, 9003), + "http://opencost.opencost.svc.cluster.local:9003"; got != want { + t.Errorf("FindFirstPreferPort got %q; want %q", got, want) + } +} + +// TestFindFirstPreferPort_FallsBackToFirstPort verifies that when none of the +// preferred ports are exposed, selection falls back to the Service's first port. +func TestFindFirstPreferPort_FallsBackToFirstPort(t *testing.T) { + cs := fake.NewClientset( + mkServicePorts("opencost", "opencost", map[string]string{"app": "opencost"}, 9003), + ) + d := New(cs, "cluster.local") + if got, want := d.FindFirstPreferPort(context.Background(), OpencostSelectors, 12345), + "http://opencost.opencost.svc.cluster.local:9003"; got != want { + t.Errorf("got %q; want %q", got, want) + } +} + +func TestSelectPort(t *testing.T) { + ports := func(ns ...int32) []corev1.ServicePort { + out := make([]corev1.ServicePort, len(ns)) + for i, n := range ns { + out[i] = corev1.ServicePort{Port: n} + } + return out + } + cases := []struct { + name string + ports []corev1.ServicePort + preferred []int32 + want int32 + }{ + {"empty preference uses first", ports(9090, 9003), nil, 9090}, + {"preferred present", ports(9090, 9003), []int32{9003}, 9003}, + {"preference order honoured", ports(9090, 9003), []int32{9003, 9090}, 9003}, + {"preferred absent falls back", ports(9090, 9003), []int32{8080}, 9090}, + {"second preference matches", ports(9090, 9003), []int32{8080, 9090}, 9090}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := selectPort(tc.ports, tc.preferred); got != tc.want { + t.Errorf("selectPort = %d; want %d", got, tc.want) + } + }) + } +} + func mkService(name, namespace string, port int32, labels map[string]string) *corev1.Service { return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, Labels: labels}, @@ -109,3 +173,14 @@ func mkService(name, namespace string, port int32, labels map[string]string) *co }, } } + +func mkServicePorts(name, namespace string, labels map[string]string, ports ...int32) *corev1.Service { + sp := make([]corev1.ServicePort, len(ports)) + for i, p := range ports { + sp[i] = corev1.ServicePort{Port: p} + } + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, Labels: labels}, + Spec: corev1.ServiceSpec{Ports: sp}, + } +} From e9eb94194fc2664ba89520ce71e52a216d0e69ec Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 19 Jun 2026 08:18:40 +0000 Subject: [PATCH 2/2] chore: update image tags for main release --- charts/nudgebee-agent/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/nudgebee-agent/values.yaml b/charts/nudgebee-agent/values.yaml index 8bba4ed6..f056a0ef 100644 --- a/charts/nudgebee-agent/values.yaml +++ b/charts/nudgebee-agent/values.yaml @@ -61,7 +61,7 @@ runnerServiceAccount: runner: image: repository: ghcr.io/nudgebee/nudgebee-agent - tag: 2026-06-18T08-03-27_f048f62d0348572793afa98b8a9b1cc383115071 + tag: 2026-06-19T07-29-09_0c10bd2144db2a151cd74f2b5997347cebd67745 # Image template the pod_profiler action launches debugger pods from. # The agent substitutes `{}` for the variant (bpf, jvm, python, perf, ruby). # Surfaces as PROFILER_IMAGE; leave empty to fall back to the binary default.