Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(reload,health): re-initialize monitors on config change; guard probe bind ordering by mattmattox · Pull Request #40 · SupportTools/node-doctor · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions cmd/node-doctor/bind_ordering_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
package main

import (
"context"
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
"time"

prometheusexporter "github.com/supporttools/node-doctor/pkg/exporters/prometheus"
"github.com/supporttools/node-doctor/pkg/types"
)

// TestHealthEndpointServesBeforeNetworkedExporters is the behavioural
// regression guard for the ordering fix in PR #24 (#node-doctor-246).
//
// The incident: on a degraded node a networked exporter's Start() BLOCKS
// (cluster-DNS or API-server reachability, informer cache sync). Before the
// fix, the health server was created AFTER those exporters, so the probe
// listener never opened inside the kubelet's startup-probe budget → the probe
// failed → the kubelet killed the container → crashloop, on exactly the nodes
// node-doctor exists to observe (a1pinode01 crashlooped 125x).
//
// This test pins the invariant directly: while phase 2 is blocked, the health
// endpoint must ALREADY be answering probes over the per-pod unix socket. If
// anyone reorders createExporters so networked init runs first, this test hangs
// on an unservable socket and fails.
func TestHealthEndpointServesBeforeNetworkedExporters(t *testing.T) {
socket := filepath.Join(t.TempDir(), "health.sock")

// Phase 2 blocks until we release it — standing in for a wedged exporter
// Start() on a degraded node.
release := make(chan struct{})
entered := make(chan struct{})

original := startNetworkedExportersFn
startNetworkedExportersFn = func(_ context.Context, _ *types.NodeDoctorConfig) ([]ExporterLifecycle, []types.Exporter, *prometheusexporter.PrometheusExporter) {
close(entered)
<-release
return nil, nil, nil
}
t.Cleanup(func() { startNetworkedExportersFn = original })

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

config := &types.NodeDoctorConfig{
Exporters: types.ExporterConfigs{
Kubernetes: &types.KubernetesExporterConfig{Enabled: true},
},
}

done := make(chan struct{})
go func() {
defer close(done)
_, _, _, _, _ = createExporters(ctx, config, nil, socket)
}()

// Wait until phase 2 is definitely underway and stuck.
select {
case <-entered:
case <-time.After(10 * time.Second):
t.Fatal("networked exporter phase never started")
}

// THE ASSERTION: the probe must already succeed even though the networked
// phase is wedged. runHealthCheck is the exact code path the kubelet exec
// probe runs, so this exercises the real production probe mechanism.
if code := runHealthCheck(socket, "/healthz"); code != 0 {
t.Errorf("liveness probe exit code = %d, want 0. The health server must bind BEFORE "+
"networked exporter init; otherwise a blocked exporter on a degraded node prevents "+
"the probe listener from ever opening and the kubelet crashloops the pod.", code)
}

// Readiness must also be reachable (it returns 503 until a monitor reports,
// but the endpoint must be SERVING, not absent).
if code := runHealthCheck(socket, "/ready"); code != 1 {
t.Errorf("readiness probe exit code = %d, want 1 (endpoint serving, reporting NotReady "+
"because no monitor has run yet)", code)
}

close(release)
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("createExporters did not return after phase 2 was released")
}
}

// TestCreateExportersSourceOrdering is a static lint guard over
// cmd/node-doctor/main.go.
//
// The behavioural test above proves the property for the current structure; this
// one catches a subtler regression: someone inlining a networked exporter
// constructor back into createExporters ahead of the health server, which would
// reintroduce the crashloop while potentially still passing a test that stubs
// the phase-2 seam.
func TestCreateExportersSourceOrdering(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "createExporters")
if fn == nil {
t.Fatal("createExporters not found in main.go")
}

// Constructors/starters that touch the network during startup and can block.
networkedMarkers := []string{
"NewKubernetesExporter",
"NewHTTPExporter",
"NewPrometheusExporter",
"startNetworkedExporters",
}

var healthPos, firstNetworkedPos token.Pos
var firstNetworkedName string

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
if name == "" {
return true
}

if name == "startHealthServer" && healthPos == token.NoPos {
healthPos = call.Pos()
}
for _, marker := range networkedMarkers {
if strings.Contains(name, marker) && firstNetworkedPos == token.NoPos {
firstNetworkedPos = call.Pos()
firstNetworkedName = name
}
}
return true
})

if healthPos == token.NoPos {
t.Fatal("createExporters must call startHealthServer — the health listener has to be bound " +
"before any networked initialization")
}
if firstNetworkedPos == token.NoPos {
t.Fatal("expected createExporters to perform networked exporter initialization")
}

if firstNetworkedPos < healthPos {
t.Errorf("networked init %q at %s runs BEFORE the health server is started at %s. "+
"On a degraded node that exporter's Start() can block, the probe listener never opens "+
"within the startup-probe budget, and the kubelet crashloops the pod (#node-doctor-246). "+
"Move the health server creation back to the top of createExporters.",
firstNetworkedName, fset.Position(firstNetworkedPos), fset.Position(healthPos))
}
}

// TestStartHealthServerDoesNotTouchNetworkedExporters guards the other half of
// the invariant: phase 1 must stay free of any networked exporter construction,
// or "health first" becomes meaningless.
func TestStartHealthServerDoesNotTouchNetworkedExporters(t *testing.T) {
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "main.go", nil, 0)
if err != nil {
t.Fatalf("parse main.go: %v", err)
}

fn := findFunc(file, "startHealthServer")
if fn == nil {
t.Fatal("startHealthServer not found in main.go")
}

forbidden := []string{"NewKubernetesExporter", "NewHTTPExporter", "NewPrometheusExporter"}

ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
name := callName(call)
for _, f := range forbidden {
if strings.Contains(name, f) {
t.Errorf("startHealthServer must not construct networked exporters, found %q at %s. "+
"Phase 1 exists precisely to bind the probe listener before anything that can block.",
name, fset.Position(call.Pos()))
}
}
return true
})
}

// findFunc locates a top-level function declaration by name.
func findFunc(file *ast.File, name string) *ast.FuncDecl {
for _, decl := range file.Decls {
fn, ok := decl.(*ast.FuncDecl)
if ok && fn.Name.Name == name {
return fn
}
}
return nil
}

// callName renders the called function's name, including a package or receiver
// qualifier when present (e.g. "health.NewServer", "healthServer.Start").
func callName(call *ast.CallExpr) string {
switch f := call.Fun.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok {
return x.Name + "." + f.Sel.Name
}
return f.Sel.Name
}
return ""
}
Loading
Loading