Skip to content

Commit aecd5af

Browse files
committed
fix: make ConfigureInterfaceSysctls tolerate missing sysctl entries
ConfigureInterfaceSysctls previously returned an error whenever any sysctl write failed, which caused VRF creation to fail entirely on nodes running minimal or container kernels where proxy_arp and proxy_ndp entries may not exist. This change makes it match ConfigureTapSysctls behavior by silently skipping missing entries and logging a warning instead. - Skip sysctl write errors instead of returning them, matching ConfigureTapSysctls behavior - Log failed sysctl sets at WARN level for observability without failing the caller - Add package-level logger variable to enable testable log verification - Add unit tests for nonexistent interface scenarios and settings completeness fixes #161
1 parent 46bb2cf commit aecd5af

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

internal/plumbing/sysctl/sysctl.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,15 @@ package sysctl
88

99
import (
1010
"fmt"
11+
"log/slog"
1112

1213
gosysctl "github.com/lorenzosaino/go-sysctl"
1314
)
1415

16+
// logger is the package-level logger. Defaults to slog.Default().
17+
// Override for testing.
18+
var logger *slog.Logger = slog.Default()
19+
1520
var interfaceSettings = []struct {
1621
format string
1722
value string
@@ -25,11 +30,13 @@ var interfaceSettings = []struct {
2530

2631
// ConfigureInterfaceSysctls applies forwarding, rp_filter, and proxy ARP/NDP
2732
// sysctl settings to iface, which are required for correct VRF packet handling.
33+
// Silently skips sysctls that don't exist (e.g., in container environments
34+
// where dynamically created interfaces may not have all sysctl entries).
2835
func ConfigureInterfaceSysctls(iface string) error {
2936
for _, entry := range interfaceSettings {
3037
key := fmt.Sprintf(entry.format, iface)
3138
if err := gosysctl.Set(key, entry.value); err != nil {
32-
return err
39+
logger.Warn("failed to set sysctl (non-fatal)", "sysctl", key, "err", err)
3340
}
3441
}
3542
return nil
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2025 Datum Cloud, Inc.
2+
//
3+
// SPDX-License-Identifier: AGPL-3.0-or-later
4+
5+
package sysctl
6+
7+
import (
8+
"bytes"
9+
"log/slog"
10+
"strings"
11+
"testing"
12+
)
13+
14+
func TestConfigureInterfaceSysctls_nonexistentInterface(t *testing.T) {
15+
// Use a name guaranteed to not exist on any real system.
16+
// gosysctl.Set will fail for every sysctl, but the function
17+
// must still return nil (gracefully skipping missing entries).
18+
const fakeIf = "this_interface_does_not_exist_xk9z2m"
19+
20+
var buf bytes.Buffer
21+
origLogger := logger
22+
logger = slog.New(slog.NewTextHandler(&buf, nil))
23+
defer func() { logger = origLogger }()
24+
25+
if err := ConfigureInterfaceSysctls(fakeIf); err != nil {
26+
t.Fatalf("expected nil error for nonexistent interface, got: %v", err)
27+
}
28+
29+
logs := buf.String()
30+
// Verify that a WARN was logged for each failed sysctl
31+
if !strings.Contains(logs, "failed to set sysctl") {
32+
t.Errorf("expected WARN log for failed sysctl, got: %q", logs)
33+
}
34+
}
35+
36+
func TestConfigureInterfaceSysctls_returnsNil(t *testing.T) {
37+
// Even when some sysctls fail to set, the function must return nil.
38+
const fakeIf = "nonexistent_iface_abc123"
39+
40+
if err := ConfigureInterfaceSysctls(fakeIf); err != nil {
41+
t.Fatalf("ConfigureInterfaceSysctls must always return nil, got error: %v", err)
42+
}
43+
}
44+
45+
func TestConfigureTapSysctls_returnsNil(t *testing.T) {
46+
// Same guarantee: ConfigureTapSysctls must always return nil.
47+
const fakeIf = "nonexistent_tap_xyz789"
48+
49+
if err := ConfigureTapSysctls(fakeIf); err != nil {
50+
t.Fatalf("ConfigureTapSysctls must always return nil, got error: %v", err)
51+
}
52+
}
53+
54+
func TestInterfaceSettings_hasAllEntries(t *testing.T) {
55+
// Verify that interfaceSettings contains all expected sysctl types.
56+
expected := map[string]bool{
57+
"rp_filter": false,
58+
"forwarding": false,
59+
"proxy_arp": false,
60+
"proxy_ndp": false,
61+
}
62+
for _, entry := range interfaceSettings {
63+
for name := range expected {
64+
if strings.Contains(entry.format, name) {
65+
expected[name] = true
66+
}
67+
}
68+
}
69+
for name, found := range expected {
70+
if !found {
71+
t.Errorf("interfaceSettings missing expected sysctl: %s", name)
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)