From 4250186d574a79889eacd8a9ad0de3b1f8f099be Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 12:29:32 -0500 Subject: [PATCH 1/9] modify alerts for externally managed clients --- rocketpool/node/check-port-connectivity.go | 73 ++++++++++++------- shared/services/config/alertmanager-config.go | 19 +++++ .../install/alerting/rules/default.tmpl | 12 --- .../install/alerting/rules/high-storage.tmpl | 22 ++++++ 4 files changed, 86 insertions(+), 40 deletions(-) create mode 100644 shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl diff --git a/rocketpool/node/check-port-connectivity.go b/rocketpool/node/check-port-connectivity.go index c4975d5a9..e2d978d96 100644 --- a/rocketpool/node/check-port-connectivity.go +++ b/rocketpool/node/check-port-connectivity.go @@ -74,57 +74,74 @@ func (t *checkPortConnectivity) run() error { } t.log.Print("Checking port connectivity...") + isLocalEc := t.cfg.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local + isLocalCc := t.cfg.ConsensusClientMode.Value.(config.Mode) == config.Mode_Local + + if !isLocalEc && !isLocalCc { + return nil + } + ecOpen := false ccOpen := false ecP2PPort := t.cfg.ExecutionCommon.P2pPort.Value.(uint16) ccP2PPort := t.cfg.ConsensusCommon.P2pPort.Value.(uint16) publicIP, err := getPublicIP() if err == nil { - ecOpen = isPortReachableNATReflection(publicIP, ecP2PPort) - ccOpen = isPortReachableNATReflection(publicIP, ccP2PPort) + if isLocalEc { + ecOpen = isPortReachableNATReflection(publicIP, ecP2PPort) + } + if isLocalCc { + ccOpen = isPortReachableNATReflection(publicIP, ccP2PPort) + } } - if !ecOpen { + + if isLocalEc && !ecOpen { // Fallback to using an external service ecOpen, _, err = isPortReachableExternalService(ecP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } } - if ecOpen { - t.log.Printf("Port %d is OPEN.", ecP2PPort) - if !t.wasEth1PortOpen { - t.log.Printlnf("Execution client P2P port %d is now accessible from the internet.", ecP2PPort) - } - } else { - if t.wasEth1PortOpen { - t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) - } - if err := alerting.AlertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { - t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) + if isLocalEc { + if ecOpen { + t.log.Printf("Port %d is OPEN.", ecP2PPort) + if !t.wasEth1PortOpen { + t.log.Printlnf("Execution client P2P port %d is now accessible from the internet.", ecP2PPort) + } + } else { + if t.wasEth1PortOpen { + t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) + } + if err := alerting.AlertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { + t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) + } } + t.wasEth1PortOpen = ecOpen } - t.wasEth1PortOpen = ecOpen - if !ccOpen { + + if isLocalCc && !ccOpen { // Fallback to using an external service ccOpen, _, err = isPortReachableExternalService(ccP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } } - if ccOpen { - t.log.Printf("Port %d is OPEN.", ccP2PPort) - if !t.wasBeaconP2POpen { - t.log.Printlnf("Consensus client P2P port %d is now accessible from the internet.", ccP2PPort) - } - } else { - if t.wasBeaconP2POpen { - t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) - } - if err := alerting.AlertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { - t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) + if isLocalCc { + if ccOpen { + t.log.Printf("Port %d is OPEN.", ccP2PPort) + if !t.wasBeaconP2POpen { + t.log.Printlnf("Consensus client P2P port %d is now accessible from the internet.", ccP2PPort) + } + } else { + if t.wasBeaconP2POpen { + t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) + } + if err := alerting.AlertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { + t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) + } } + t.wasBeaconP2POpen = ccOpen } - t.wasBeaconP2POpen = ccOpen return nil } diff --git a/shared/services/config/alertmanager-config.go b/shared/services/config/alertmanager-config.go index 35c2d0149..e687a73f5 100644 --- a/shared/services/config/alertmanager-config.go +++ b/shared/services/config/alertmanager-config.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "os" "github.com/mitchellh/go-homedir" "golang.org/x/text/cases" @@ -20,6 +21,9 @@ const AlertmanagerConfigFile string = "alerting/alertmanager.yml" const AlertingRulesConfigTemplate string = "alerting/rules/default.tmpl" const AlertingRulesConfigFile string = "alerting/rules/default.yml" +const HighStorageRulesConfigTemplate string = "alerting/rules/high-storage.tmpl" +const HighStorageRulesConfigFile string = "alerting/rules/high-storage.yml" + // Defaults const defaultAlertmanagerPort uint16 = 9093 const defaultAlertmanagerHost string = "localhost" @@ -372,6 +376,21 @@ func (cfg *AlertmanagerConfig) UpdateConfigurationFiles(configPath string) error if err != nil { return fmt.Errorf("error processing alerting rules template: %w", err) } + + isLocalEc := cfg.Parent.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local + isLocalCc := cfg.Parent.ConsensusClientMode.Value.(config.Mode) == config.Mode_Local + + if isLocalEc || isLocalCc { + err = cfg.processTemplate(configPath, HighStorageRulesConfigTemplate, HighStorageRulesConfigFile, "{{{", "}}}") + if err != nil { + return fmt.Errorf("error processing high-storage alerting rules template: %w", err) + } + } else { + if fileToRemove, err := homedir.Expand(fmt.Sprintf("%s/%s", configPath, HighStorageRulesConfigFile)); err == nil { + os.Remove(fileToRemove) + } + } + return nil } diff --git a/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl b/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl index 4fdd1ef65..0505ee76e 100644 --- a/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl +++ b/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl @@ -92,18 +92,6 @@ groups: description: "{{ $labels.instance }} has low disk space. Currently has {{ humanize $value }} GB free." {{{- end }}} - {{{- if .AlertEnabled_LowDiskSpaceCritical.Value }}} - - alert: LowDiskSpaceCritical - # NOTE: 50GB taken from PruneFreeSpaceRequired in rocketpool-cli's nethermind pruning (it won't prune below 50GB) - expr: node_filesystem_avail_bytes{job="node", mountpoint="/"} / 1024^3 < 50 - labels: - severity: critical - job: node - annotations: - summary: "Device {{ $labels.device }} on instance {{ $labels.instance }} has critically low disk space" - description: "{{ $labels.instance }} has critically low disk space. Currently has {{ humanize $value }} GB free." - {{{- end }}} - {{{- if .AlertEnabled_OSUpdatesAvailable.Value }}} - alert: OSUpdatesAvailable expr: max(os_upgrades_pending{job="node"}) > 0 diff --git a/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl b/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl new file mode 100644 index 000000000..ba5a333b0 --- /dev/null +++ b/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl @@ -0,0 +1,22 @@ +# Autogenerated - DO NOT MODIFY THIS FILE DIRECTLY If you want to overwrite some +# of these values with your own customizations, please disable the alerts in the +# TUI and then add your own rule files to the /alerting/rules +# directory. +# +# NOTE: This file uses non-default go template delimiters (triple braces) to avoid +# conflicts with the default delimiters used in the alerting rules. + +groups: + - name: HighStorage + rules: + {{{- if .AlertEnabled_LowDiskSpaceCritical.Value }}} + - alert: LowDiskSpaceCritical + # NOTE: 50GB taken from PruneFreeSpaceRequired in rocketpool-cli's nethermind pruning (it won't prune below 50GB) + expr: node_filesystem_avail_bytes{job="node", mountpoint="/"} / 1024^3 < 50 + labels: + severity: critical + job: node + annotations: + summary: "Device {{ $labels.device }} on instance {{ $labels.instance }} has critically low disk space" + description: "{{ $labels.instance }} has critically low disk space. Currently has {{ humanize $value }} GB free." + {{{- end }}} From b1510da58f6e1eb0e50b1ebef7b0d84bc25866de Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 12:46:51 -0500 Subject: [PATCH 2/9] add Alertmanager high storage rules configuration tests --- .../config/alertmanager_config_test.go | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 shared/services/config/alertmanager_config_test.go diff --git a/shared/services/config/alertmanager_config_test.go b/shared/services/config/alertmanager_config_test.go new file mode 100644 index 000000000..cc7d182f5 --- /dev/null +++ b/shared/services/config/alertmanager_config_test.go @@ -0,0 +1,124 @@ +package config + +import ( + "io" + "os" + "path/filepath" + "testing" + + cfgtypes "github.com/rocket-pool/smartnode/shared/types/config" +) + +// installAlertingTemplates copies the alerting template files from the source +// tree into dst, mimicking what `rocketpool service install` does at runtime. +// When `go test` runs, the working directory is the package directory +// (shared/services/config), so we walk up three levels to reach the repo root. +func installAlertingTemplates(t *testing.T, dst string) { + t.Helper() + + repoRoot := filepath.Join("..", "..", "..") + srcBase := filepath.Join(repoRoot, "shared", "services", "rocketpool", "assets", "install", "alerting") + dstBase := filepath.Join(dst, "alerting") + + err := filepath.Walk(srcBase, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(srcBase, path) + if err != nil { + return err + } + target := filepath.Join(dstBase, rel) + if info.IsDir() { + return os.MkdirAll(target, 0755) + } + return copyFile(target, path) + }) + if err != nil { + t.Fatalf("failed to install alerting templates: %v", err) + } +} + +func copyFile(dst, src string) error { + if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + return err + } + defer out.Close() + _, err = io.Copy(out, in) + return err +} + +// TestUpdateConfigurationFiles_HighStorageRules verifies that high-storage.yml +// is created when at least one client is locally managed, and deleted when +// both clients are externally managed. +func TestUpdateConfigurationFiles_HighStorageRules(t *testing.T) { + tests := []struct { + name string + ecMode cfgtypes.Mode + ccMode cfgtypes.Mode + expectCreated bool + }{ + { + name: "local EC + local CC creates high-storage.yml", + ecMode: cfgtypes.Mode_Local, + ccMode: cfgtypes.Mode_Local, + expectCreated: true, + }, + { + name: "external EC + external CC removes high-storage.yml", + ecMode: cfgtypes.Mode_External, + ccMode: cfgtypes.Mode_External, + expectCreated: false, + }, + { + name: "local EC + external CC creates high-storage.yml", + ecMode: cfgtypes.Mode_Local, + ccMode: cfgtypes.Mode_External, + expectCreated: true, + }, + { + name: "external EC + local CC creates high-storage.yml", + ecMode: cfgtypes.Mode_External, + ccMode: cfgtypes.Mode_Local, + expectCreated: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Use a fresh temp dir for each sub-test so they are independent. + testDir := t.TempDir() + installAlertingTemplates(t, testDir) + + cfg := NewRocketPoolConfig(testDir, false) + cfg.ExecutionClientMode.Value = tc.ecMode + cfg.ConsensusClientMode.Value = tc.ccMode + cfg.Alertmanager.EnableAlerting.Value = true + + if err := cfg.Alertmanager.UpdateConfigurationFiles(testDir); err != nil { + t.Fatalf("UpdateConfigurationFiles returned error: %v", err) + } + + highStorageFile := filepath.Join(testDir, HighStorageRulesConfigFile) + _, statErr := os.Stat(highStorageFile) + exists := !os.IsNotExist(statErr) + + if exists != tc.expectCreated { + if tc.expectCreated { + t.Errorf("expected high-storage.yml to be created, but it was not") + } else { + t.Errorf("expected high-storage.yml to be deleted, but it still exists") + } + } + }) + } +} From 269a3d784c68d6a13938cbea92258b9733fcafa8 Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:06:06 -0500 Subject: [PATCH 3/9] check-port-connectivity test --- rocketpool/node/node.go | 6 +- .../connectivity}/check-port-connectivity.go | 55 ++++--- .../check-port-connectivity_test.go | 142 ++++++++++++++++++ 3 files changed, 178 insertions(+), 25 deletions(-) rename {rocketpool/node => shared/services/connectivity}/check-port-connectivity.go (77%) create mode 100644 shared/services/connectivity/check-port-connectivity_test.go diff --git a/rocketpool/node/node.go b/rocketpool/node/node.go index 741c4af24..f92535948 100644 --- a/rocketpool/node/node.go +++ b/rocketpool/node/node.go @@ -17,6 +17,7 @@ import ( "github.com/rocket-pool/smartnode/rocketpool/node/collectors" "github.com/rocket-pool/smartnode/shared/services" "github.com/rocket-pool/smartnode/shared/services/alerting" + "github.com/rocket-pool/smartnode/shared/services/connectivity" "github.com/rocket-pool/smartnode/shared/services/state" "github.com/rocket-pool/smartnode/shared/services/wallet/keystore/lighthouse" "github.com/rocket-pool/smartnode/shared/services/wallet/keystore/nimbus" @@ -189,7 +190,8 @@ func run(c *cli.Context) error { if err != nil { return err } - checkPorts, err := newCheckPortConnectivity(c, log.NewColorLogger(CheckPortConnectivityColor)) + var checkPorts *connectivity.CheckPortConnectivity + checkPorts, err = connectivity.NewCheckPortConnectivity(c, cfg, log.NewColorLogger(CheckPortConnectivityColor)) if err != nil { return err } @@ -326,7 +328,7 @@ func run(c *cli.Context) error { time.Sleep(taskCooldown) // Run the port connectivity check - if err := checkPorts.run(); err != nil { + if err := checkPorts.Run(); err != nil { errorLog.Println(err) } time.Sleep(taskCooldown) diff --git a/rocketpool/node/check-port-connectivity.go b/shared/services/connectivity/check-port-connectivity.go similarity index 77% rename from rocketpool/node/check-port-connectivity.go rename to shared/services/connectivity/check-port-connectivity.go index e2d978d96..cba37c23e 100644 --- a/rocketpool/node/check-port-connectivity.go +++ b/shared/services/connectivity/check-port-connectivity.go @@ -1,4 +1,4 @@ -package node +package connectivity import ( "context" @@ -13,9 +13,9 @@ import ( "github.com/urfave/cli" - "github.com/rocket-pool/smartnode/shared/services" "github.com/rocket-pool/smartnode/shared/services/alerting" - "github.com/rocket-pool/smartnode/shared/services/config" + cfg "github.com/rocket-pool/smartnode/shared/services/config" + cfgtypes "github.com/rocket-pool/smartnode/shared/types/config" "github.com/rocket-pool/smartnode/shared/utils/log" ) @@ -36,36 +36,45 @@ var publicIPResolvers = []struct { } // Check port connectivity task -type checkPortConnectivity struct { +type CheckPortConnectivity struct { c *cli.Context log log.ColorLogger - cfg *config.RocketPoolConfig + cfg *cfg.RocketPoolConfig // Track previous state to avoid flooding with repeated alerts wasEth1PortOpen bool wasBeaconP2POpen bool + + // Function pointers for network and alerting actions (to facilitate testing) + GetPublicIP func() (string, error) + IsPortReachableNATReflection func(string, uint16) bool + IsPortReachableExternalService func(uint16) (bool, string, error) + AlertEth1P2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error + AlertBeaconP2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error } // Create check port connectivity task -func newCheckPortConnectivity(c *cli.Context, logger log.ColorLogger) (*checkPortConnectivity, error) { - cfg, err := services.GetConfig(c) - if err != nil { - return nil, err - } - - return &checkPortConnectivity{ +func NewCheckPortConnectivity(c *cli.Context, config *cfg.RocketPoolConfig, logger log.ColorLogger) (*CheckPortConnectivity, error) { + return &CheckPortConnectivity{ c: c, log: logger, - cfg: cfg, + cfg: config, // Assume ports are open at startup to avoid spurious alerts on first cycle wasEth1PortOpen: true, wasBeaconP2POpen: true, + + // Default implementations + GetPublicIP: getPublicIP, + IsPortReachableNATReflection: isPortReachableNATReflection, + IsPortReachableExternalService: isPortReachableExternalService, + AlertEth1P2PPortNotOpen: alerting.AlertEth1P2PPortNotOpen, + AlertBeaconP2PPortNotOpen: alerting.AlertBeaconP2PPortNotOpen, }, nil } // Check whether the configured execution/consensus client P2P ports are // reachable from the internet. Sends an alert the first time either port is detected as closed. -func (t *checkPortConnectivity) run() error { +func (t *CheckPortConnectivity) Run() error { if t.cfg.Alertmanager.EnableAlerting.Value != true { return nil } @@ -74,8 +83,8 @@ func (t *checkPortConnectivity) run() error { } t.log.Print("Checking port connectivity...") - isLocalEc := t.cfg.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local - isLocalCc := t.cfg.ConsensusClientMode.Value.(config.Mode) == config.Mode_Local + isLocalEc := t.cfg.ExecutionClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local + isLocalCc := t.cfg.ConsensusClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local if !isLocalEc && !isLocalCc { return nil @@ -85,19 +94,19 @@ func (t *checkPortConnectivity) run() error { ccOpen := false ecP2PPort := t.cfg.ExecutionCommon.P2pPort.Value.(uint16) ccP2PPort := t.cfg.ConsensusCommon.P2pPort.Value.(uint16) - publicIP, err := getPublicIP() + publicIP, err := t.GetPublicIP() if err == nil { if isLocalEc { - ecOpen = isPortReachableNATReflection(publicIP, ecP2PPort) + ecOpen = t.IsPortReachableNATReflection(publicIP, ecP2PPort) } if isLocalCc { - ccOpen = isPortReachableNATReflection(publicIP, ccP2PPort) + ccOpen = t.IsPortReachableNATReflection(publicIP, ccP2PPort) } } if isLocalEc && !ecOpen { // Fallback to using an external service - ecOpen, _, err = isPortReachableExternalService(ecP2PPort) + ecOpen, _, err = t.IsPortReachableExternalService(ecP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } @@ -112,7 +121,7 @@ func (t *checkPortConnectivity) run() error { if t.wasEth1PortOpen { t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) } - if err := alerting.AlertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { + if err := t.AlertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) } } @@ -121,7 +130,7 @@ func (t *checkPortConnectivity) run() error { if isLocalCc && !ccOpen { // Fallback to using an external service - ccOpen, _, err = isPortReachableExternalService(ccP2PPort) + ccOpen, _, err = t.IsPortReachableExternalService(ccP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } @@ -136,7 +145,7 @@ func (t *checkPortConnectivity) run() error { if t.wasBeaconP2POpen { t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) } - if err := alerting.AlertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { + if err := t.AlertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) } } diff --git a/shared/services/connectivity/check-port-connectivity_test.go b/shared/services/connectivity/check-port-connectivity_test.go new file mode 100644 index 000000000..5b8e40722 --- /dev/null +++ b/shared/services/connectivity/check-port-connectivity_test.go @@ -0,0 +1,142 @@ +package connectivity + +import ( + "testing" + + cfg "github.com/rocket-pool/smartnode/shared/services/config" + cfgtypes "github.com/rocket-pool/smartnode/shared/types/config" + log "github.com/rocket-pool/smartnode/shared/utils/log" +) + +func TestCheckPortConnectivity_Run(t *testing.T) { + logger := log.NewColorLogger(0) + tests := []struct { + name string + ecMode cfgtypes.Mode + ccMode cfgtypes.Mode + enableAlerting bool + portAlertingEnabled bool + expectNetCalls bool + }{ + { + name: "local EC + local CC -> performs checks", + ecMode: cfgtypes.Mode_Local, + ccMode: cfgtypes.Mode_Local, + enableAlerting: true, + portAlertingEnabled: true, + expectNetCalls: true, + }, + { + name: "external EC + external CC -> skips checks", + ecMode: cfgtypes.Mode_External, + ccMode: cfgtypes.Mode_External, + enableAlerting: true, + portAlertingEnabled: true, + expectNetCalls: false, + }, + { + name: "local EC + external CC -> performs checks (for EC)", + ecMode: cfgtypes.Mode_Local, + ccMode: cfgtypes.Mode_External, + enableAlerting: true, + portAlertingEnabled: true, + expectNetCalls: true, + }, + { + name: "alerting disabled -> skips checks", + ecMode: cfgtypes.Mode_Local, + ccMode: cfgtypes.Mode_Local, + enableAlerting: false, + portAlertingEnabled: true, + expectNetCalls: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + config := cfg.NewRocketPoolConfig("", false) + config.ExecutionClientMode.Value = tc.ecMode + config.ConsensusClientMode.Value = tc.ccMode + config.Alertmanager.EnableAlerting.Value = tc.enableAlerting + config.Alertmanager.AlertEnabled_PortConnectivityCheck.Value = tc.portAlertingEnabled + + netCallsMade := false + mockGetPublicIP := func() (string, error) { + netCallsMade = true + return "1.2.3.4", nil + } + mockIsPortReachable := func(host string, port uint16) bool { + netCallsMade = true + return true + } + mockExternalCheck := func(port uint16) (bool, string, error) { + netCallsMade = true + return true, "Success", nil + } + mockAlert := func(cfg *cfg.RocketPoolConfig, port uint16) error { + return nil + } + + task := &CheckPortConnectivity{ + cfg: config, + log: logger, + GetPublicIP: mockGetPublicIP, + IsPortReachableNATReflection: mockIsPortReachable, + IsPortReachableExternalService: mockExternalCheck, + AlertEth1P2PPortNotOpen: mockAlert, + AlertBeaconP2PPortNotOpen: mockAlert, + } + + err := task.Run() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if netCallsMade != tc.expectNetCalls { + t.Errorf("expected network calls: %v, got: %v", tc.expectNetCalls, netCallsMade) + } + }) + } +} + +func TestCheckPortConnectivity_SkipSpecificPort(t *testing.T) { + config := cfg.NewRocketPoolConfig("", false) + config.ExecutionClientMode.Value = cfgtypes.Mode_External + config.ConsensusClientMode.Value = cfgtypes.Mode_Local + config.Alertmanager.EnableAlerting.Value = true + config.Alertmanager.AlertEnabled_PortConnectivityCheck.Value = true + + ecChecked := false + ccChecked := false + + task := &CheckPortConnectivity{ + cfg: config, + log: log.NewColorLogger(0), + GetPublicIP: func() (string, error) { + return "1.2.3.4", nil + }, + IsPortReachableNATReflection: func(host string, port uint16) bool { + if port == config.ExecutionCommon.P2pPort.Value.(uint16) { + ecChecked = true + } + if port == config.ConsensusCommon.P2pPort.Value.(uint16) { + ccChecked = true + } + return true + }, + IsPortReachableExternalService: func(port uint16) (bool, string, error) { + return true, "", nil + }, + AlertEth1P2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, + AlertBeaconP2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, + } + + task.Run() + + if ecChecked { + t.Error("expected Execution client port check to be skipped for External mode") + } + if !ccChecked { + t.Error("expected Consensus client port check to be performed for Local mode") + } +} From 1310cbfc91c32d8483612a2f5041fad4c7314570 Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:12:39 -0500 Subject: [PATCH 4/9] formatting/naming tweaks --- .../connectivity/check-port-connectivity.go | 34 +++++++++---------- .../check-port-connectivity_test.go | 20 +++++------ 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/shared/services/connectivity/check-port-connectivity.go b/shared/services/connectivity/check-port-connectivity.go index cba37c23e..a99eb092e 100644 --- a/shared/services/connectivity/check-port-connectivity.go +++ b/shared/services/connectivity/check-port-connectivity.go @@ -46,11 +46,11 @@ type CheckPortConnectivity struct { wasBeaconP2POpen bool // Function pointers for network and alerting actions (to facilitate testing) - GetPublicIP func() (string, error) - IsPortReachableNATReflection func(string, uint16) bool - IsPortReachableExternalService func(uint16) (bool, string, error) - AlertEth1P2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error - AlertBeaconP2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error + getPublicIP func() (string, error) + isPortReachableNATReflection func(string, uint16) bool + isPortReachableExternalService func(uint16) (bool, string, error) + alertEth1P2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error + alertBeaconP2PPortNotOpen func(*cfg.RocketPoolConfig, uint16) error } // Create check port connectivity task @@ -64,11 +64,11 @@ func NewCheckPortConnectivity(c *cli.Context, config *cfg.RocketPoolConfig, logg wasBeaconP2POpen: true, // Default implementations - GetPublicIP: getPublicIP, - IsPortReachableNATReflection: isPortReachableNATReflection, - IsPortReachableExternalService: isPortReachableExternalService, - AlertEth1P2PPortNotOpen: alerting.AlertEth1P2PPortNotOpen, - AlertBeaconP2PPortNotOpen: alerting.AlertBeaconP2PPortNotOpen, + getPublicIP: getPublicIP, + isPortReachableNATReflection: isPortReachableNATReflection, + isPortReachableExternalService: isPortReachableExternalService, + alertEth1P2PPortNotOpen: alerting.AlertEth1P2PPortNotOpen, + alertBeaconP2PPortNotOpen: alerting.AlertBeaconP2PPortNotOpen, }, nil } @@ -94,19 +94,19 @@ func (t *CheckPortConnectivity) Run() error { ccOpen := false ecP2PPort := t.cfg.ExecutionCommon.P2pPort.Value.(uint16) ccP2PPort := t.cfg.ConsensusCommon.P2pPort.Value.(uint16) - publicIP, err := t.GetPublicIP() + publicIP, err := t.getPublicIP() if err == nil { if isLocalEc { - ecOpen = t.IsPortReachableNATReflection(publicIP, ecP2PPort) + ecOpen = t.isPortReachableNATReflection(publicIP, ecP2PPort) } if isLocalCc { - ccOpen = t.IsPortReachableNATReflection(publicIP, ccP2PPort) + ccOpen = t.isPortReachableNATReflection(publicIP, ccP2PPort) } } if isLocalEc && !ecOpen { // Fallback to using an external service - ecOpen, _, err = t.IsPortReachableExternalService(ecP2PPort) + ecOpen, _, err = t.isPortReachableExternalService(ecP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } @@ -121,7 +121,7 @@ func (t *CheckPortConnectivity) Run() error { if t.wasEth1PortOpen { t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) } - if err := t.AlertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { + if err := t.alertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) } } @@ -130,7 +130,7 @@ func (t *CheckPortConnectivity) Run() error { if isLocalCc && !ccOpen { // Fallback to using an external service - ccOpen, _, err = t.IsPortReachableExternalService(ccP2PPort) + ccOpen, _, err = t.isPortReachableExternalService(ccP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } @@ -145,7 +145,7 @@ func (t *CheckPortConnectivity) Run() error { if t.wasBeaconP2POpen { t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) } - if err := t.AlertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { + if err := t.alertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) } } diff --git a/shared/services/connectivity/check-port-connectivity_test.go b/shared/services/connectivity/check-port-connectivity_test.go index 5b8e40722..11aeec381 100644 --- a/shared/services/connectivity/check-port-connectivity_test.go +++ b/shared/services/connectivity/check-port-connectivity_test.go @@ -80,11 +80,11 @@ func TestCheckPortConnectivity_Run(t *testing.T) { task := &CheckPortConnectivity{ cfg: config, log: logger, - GetPublicIP: mockGetPublicIP, - IsPortReachableNATReflection: mockIsPortReachable, - IsPortReachableExternalService: mockExternalCheck, - AlertEth1P2PPortNotOpen: mockAlert, - AlertBeaconP2PPortNotOpen: mockAlert, + getPublicIP: mockGetPublicIP, + isPortReachableNATReflection: mockIsPortReachable, + isPortReachableExternalService: mockExternalCheck, + alertEth1P2PPortNotOpen: mockAlert, + alertBeaconP2PPortNotOpen: mockAlert, } err := task.Run() @@ -112,10 +112,10 @@ func TestCheckPortConnectivity_SkipSpecificPort(t *testing.T) { task := &CheckPortConnectivity{ cfg: config, log: log.NewColorLogger(0), - GetPublicIP: func() (string, error) { + getPublicIP: func() (string, error) { return "1.2.3.4", nil }, - IsPortReachableNATReflection: func(host string, port uint16) bool { + isPortReachableNATReflection: func(host string, port uint16) bool { if port == config.ExecutionCommon.P2pPort.Value.(uint16) { ecChecked = true } @@ -124,11 +124,11 @@ func TestCheckPortConnectivity_SkipSpecificPort(t *testing.T) { } return true }, - IsPortReachableExternalService: func(port uint16) (bool, string, error) { + isPortReachableExternalService: func(port uint16) (bool, string, error) { return true, "", nil }, - AlertEth1P2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, - AlertBeaconP2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, + alertEth1P2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, + alertBeaconP2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, } task.Run() From 1dd22670660fdf430c4c6b901e9f5b99b4e14c76 Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:32:48 -0500 Subject: [PATCH 5/9] comments --- shared/services/config/alertmanager-config.go | 3 +++ shared/services/connectivity/check-port-connectivity.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/shared/services/config/alertmanager-config.go b/shared/services/config/alertmanager-config.go index e687a73f5..5a5cabb69 100644 --- a/shared/services/config/alertmanager-config.go +++ b/shared/services/config/alertmanager-config.go @@ -380,6 +380,9 @@ func (cfg *AlertmanagerConfig) UpdateConfigurationFiles(configPath string) error isLocalEc := cfg.Parent.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local isLocalCc := cfg.Parent.ConsensusClientMode.Value.(config.Mode) == config.Mode_Local + // Only apply high-storage rules if at least one client is managed locally. + // We don't require much storage if the EC/CC are external. + // Note that the warning alert is still active in the default rules and this is just for critical low storage alerts. if isLocalEc || isLocalCc { err = cfg.processTemplate(configPath, HighStorageRulesConfigTemplate, HighStorageRulesConfigFile, "{{{", "}}}") if err != nil { diff --git a/shared/services/connectivity/check-port-connectivity.go b/shared/services/connectivity/check-port-connectivity.go index a99eb092e..309486b88 100644 --- a/shared/services/connectivity/check-port-connectivity.go +++ b/shared/services/connectivity/check-port-connectivity.go @@ -86,6 +86,9 @@ func (t *CheckPortConnectivity) Run() error { isLocalEc := t.cfg.ExecutionClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local isLocalCc := t.cfg.ConsensusClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local + // Only perform checks if at least one client is managed locally. + // We don't check external clients because the node operator is responsible for their + // connectivity, and they may be behind a different firewall or NAT entirely. if !isLocalEc && !isLocalCc { return nil } From 86c62df81c1e978fc1edd1fb20f0ee991ddbaa8e Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:38:45 -0500 Subject: [PATCH 6/9] connectivity UT descriptions --- shared/services/connectivity/check-port-connectivity_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/shared/services/connectivity/check-port-connectivity_test.go b/shared/services/connectivity/check-port-connectivity_test.go index 11aeec381..70172c04f 100644 --- a/shared/services/connectivity/check-port-connectivity_test.go +++ b/shared/services/connectivity/check-port-connectivity_test.go @@ -8,6 +8,9 @@ import ( log "github.com/rocket-pool/smartnode/shared/utils/log" ) +// TestCheckPortConnectivity_Run verifies that the port connectivity task correctly +// decides whether to perform network checks based on the client modes and global +// alerting settings. func TestCheckPortConnectivity_Run(t *testing.T) { logger := log.NewColorLogger(0) tests := []struct { @@ -99,6 +102,8 @@ func TestCheckPortConnectivity_Run(t *testing.T) { } } +// TestCheckPortConnectivity_SkipSpecificPort verifies that when one client is local +// and another is external, only the local client's P2P port is checked. func TestCheckPortConnectivity_SkipSpecificPort(t *testing.T) { config := cfg.NewRocketPoolConfig("", false) config.ExecutionClientMode.Value = cfgtypes.Mode_External From 48c9b5dcd1231819577eed97aee5fe011ca586aa Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Sat, 28 Feb 2026 13:45:18 -0500 Subject: [PATCH 7/9] expand connectivity UT --- .../connectivity/check-port-connectivity_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shared/services/connectivity/check-port-connectivity_test.go b/shared/services/connectivity/check-port-connectivity_test.go index 70172c04f..4dad5c089 100644 --- a/shared/services/connectivity/check-port-connectivity_test.go +++ b/shared/services/connectivity/check-port-connectivity_test.go @@ -38,13 +38,21 @@ func TestCheckPortConnectivity_Run(t *testing.T) { expectNetCalls: false, }, { - name: "local EC + external CC -> performs checks (for EC)", + name: "local EC + external CC -> performs checks", ecMode: cfgtypes.Mode_Local, ccMode: cfgtypes.Mode_External, enableAlerting: true, portAlertingEnabled: true, expectNetCalls: true, }, + { + name: "external EC + local CC -> performs checks", + ecMode: cfgtypes.Mode_External, + ccMode: cfgtypes.Mode_Local, + enableAlerting: true, + portAlertingEnabled: true, + expectNetCalls: true, + }, { name: "alerting disabled -> skips checks", ecMode: cfgtypes.Mode_Local, From d6b339860a31aa571a2cb7292f2004cb1a413b53 Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Tue, 3 Mar 2026 17:48:03 -0500 Subject: [PATCH 8/9] no mixed modes --- shared/services/config/alertmanager-config.go | 9 ++- .../config/alertmanager_config_test.go | 12 ---- .../connectivity/check-port-connectivity.go | 72 ++++++++----------- .../check-port-connectivity_test.go | 60 ---------------- 4 files changed, 35 insertions(+), 118 deletions(-) diff --git a/shared/services/config/alertmanager-config.go b/shared/services/config/alertmanager-config.go index 5a5cabb69..5fbb02cd5 100644 --- a/shared/services/config/alertmanager-config.go +++ b/shared/services/config/alertmanager-config.go @@ -377,13 +377,12 @@ func (cfg *AlertmanagerConfig) UpdateConfigurationFiles(configPath string) error return fmt.Errorf("error processing alerting rules template: %w", err) } - isLocalEc := cfg.Parent.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local - isLocalCc := cfg.Parent.ConsensusClientMode.Value.(config.Mode) == config.Mode_Local - - // Only apply high-storage rules if at least one client is managed locally. + // Only apply high-storage rules if clients are locally managed. // We don't require much storage if the EC/CC are external. // Note that the warning alert is still active in the default rules and this is just for critical low storage alerts. - if isLocalEc || isLocalCc { + // (EC and CC are always both local or both external - mixed configurations are rejected during validation.) + isLocalMode := cfg.Parent.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local + if isLocalMode { err = cfg.processTemplate(configPath, HighStorageRulesConfigTemplate, HighStorageRulesConfigFile, "{{{", "}}}") if err != nil { return fmt.Errorf("error processing high-storage alerting rules template: %w", err) diff --git a/shared/services/config/alertmanager_config_test.go b/shared/services/config/alertmanager_config_test.go index cc7d182f5..a8abe19f7 100644 --- a/shared/services/config/alertmanager_config_test.go +++ b/shared/services/config/alertmanager_config_test.go @@ -79,18 +79,6 @@ func TestUpdateConfigurationFiles_HighStorageRules(t *testing.T) { ccMode: cfgtypes.Mode_External, expectCreated: false, }, - { - name: "local EC + external CC creates high-storage.yml", - ecMode: cfgtypes.Mode_Local, - ccMode: cfgtypes.Mode_External, - expectCreated: true, - }, - { - name: "external EC + local CC creates high-storage.yml", - ecMode: cfgtypes.Mode_External, - ccMode: cfgtypes.Mode_Local, - expectCreated: true, - }, } for _, tc := range tests { diff --git a/shared/services/connectivity/check-port-connectivity.go b/shared/services/connectivity/check-port-connectivity.go index 309486b88..d9f9515a7 100644 --- a/shared/services/connectivity/check-port-connectivity.go +++ b/shared/services/connectivity/check-port-connectivity.go @@ -83,13 +83,11 @@ func (t *CheckPortConnectivity) Run() error { } t.log.Print("Checking port connectivity...") - isLocalEc := t.cfg.ExecutionClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local - isLocalCc := t.cfg.ConsensusClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local - - // Only perform checks if at least one client is managed locally. + // EC and CC are always both local or both external - mixed configurations are rejected during validation. // We don't check external clients because the node operator is responsible for their // connectivity, and they may be behind a different firewall or NAT entirely. - if !isLocalEc && !isLocalCc { + isLocalMode := t.cfg.ExecutionClientMode.Value.(cfgtypes.Mode) == cfgtypes.Mode_Local + if !isLocalMode { return nil } @@ -99,61 +97,53 @@ func (t *CheckPortConnectivity) Run() error { ccP2PPort := t.cfg.ConsensusCommon.P2pPort.Value.(uint16) publicIP, err := t.getPublicIP() if err == nil { - if isLocalEc { - ecOpen = t.isPortReachableNATReflection(publicIP, ecP2PPort) - } - if isLocalCc { - ccOpen = t.isPortReachableNATReflection(publicIP, ccP2PPort) - } + ecOpen = t.isPortReachableNATReflection(publicIP, ecP2PPort) + ccOpen = t.isPortReachableNATReflection(publicIP, ccP2PPort) } - if isLocalEc && !ecOpen { + if !ecOpen { // Fallback to using an external service ecOpen, _, err = t.isPortReachableExternalService(ecP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } } - if isLocalEc { - if ecOpen { - t.log.Printf("Port %d is OPEN.", ecP2PPort) - if !t.wasEth1PortOpen { - t.log.Printlnf("Execution client P2P port %d is now accessible from the internet.", ecP2PPort) - } - } else { - if t.wasEth1PortOpen { - t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) - } - if err := t.alertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { - t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) - } + if ecOpen { + t.log.Printf("Port %d is OPEN.", ecP2PPort) + if !t.wasEth1PortOpen { + t.log.Printlnf("Execution client P2P port %d is now accessible from the internet.", ecP2PPort) + } + } else { + if t.wasEth1PortOpen { + t.log.Printlnf("WARNING: Execution client P2P port %d is not accessible from the internet.", ecP2PPort) + } + if err := t.alertEth1P2PPortNotOpen(t.cfg, ecP2PPort); err != nil { + t.log.Printlnf("WARNING: Could not send Eth1P2PPortNotOpen alert: %s", err.Error()) } - t.wasEth1PortOpen = ecOpen } + t.wasEth1PortOpen = ecOpen - if isLocalCc && !ccOpen { + if !ccOpen { // Fallback to using an external service ccOpen, _, err = t.isPortReachableExternalService(ccP2PPort) if err != nil { return fmt.Errorf("error checking port connectivity: %w", err) } } - if isLocalCc { - if ccOpen { - t.log.Printf("Port %d is OPEN.", ccP2PPort) - if !t.wasBeaconP2POpen { - t.log.Printlnf("Consensus client P2P port %d is now accessible from the internet.", ccP2PPort) - } - } else { - if t.wasBeaconP2POpen { - t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) - } - if err := t.alertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { - t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) - } + if ccOpen { + t.log.Printf("Port %d is OPEN.", ccP2PPort) + if !t.wasBeaconP2POpen { + t.log.Printlnf("Consensus client P2P port %d is now accessible from the internet.", ccP2PPort) + } + } else { + if t.wasBeaconP2POpen { + t.log.Printlnf("WARNING: Consensus client P2P port %d is not accessible from the internet.", ccP2PPort) + } + if err := t.alertBeaconP2PPortNotOpen(t.cfg, ccP2PPort); err != nil { + t.log.Printlnf("WARNING: Could not send BeaconP2PPortNotOpen alert: %s", err.Error()) } - t.wasBeaconP2POpen = ccOpen } + t.wasBeaconP2POpen = ccOpen return nil } diff --git a/shared/services/connectivity/check-port-connectivity_test.go b/shared/services/connectivity/check-port-connectivity_test.go index 4dad5c089..5938dd7c2 100644 --- a/shared/services/connectivity/check-port-connectivity_test.go +++ b/shared/services/connectivity/check-port-connectivity_test.go @@ -37,22 +37,6 @@ func TestCheckPortConnectivity_Run(t *testing.T) { portAlertingEnabled: true, expectNetCalls: false, }, - { - name: "local EC + external CC -> performs checks", - ecMode: cfgtypes.Mode_Local, - ccMode: cfgtypes.Mode_External, - enableAlerting: true, - portAlertingEnabled: true, - expectNetCalls: true, - }, - { - name: "external EC + local CC -> performs checks", - ecMode: cfgtypes.Mode_External, - ccMode: cfgtypes.Mode_Local, - enableAlerting: true, - portAlertingEnabled: true, - expectNetCalls: true, - }, { name: "alerting disabled -> skips checks", ecMode: cfgtypes.Mode_Local, @@ -109,47 +93,3 @@ func TestCheckPortConnectivity_Run(t *testing.T) { }) } } - -// TestCheckPortConnectivity_SkipSpecificPort verifies that when one client is local -// and another is external, only the local client's P2P port is checked. -func TestCheckPortConnectivity_SkipSpecificPort(t *testing.T) { - config := cfg.NewRocketPoolConfig("", false) - config.ExecutionClientMode.Value = cfgtypes.Mode_External - config.ConsensusClientMode.Value = cfgtypes.Mode_Local - config.Alertmanager.EnableAlerting.Value = true - config.Alertmanager.AlertEnabled_PortConnectivityCheck.Value = true - - ecChecked := false - ccChecked := false - - task := &CheckPortConnectivity{ - cfg: config, - log: log.NewColorLogger(0), - getPublicIP: func() (string, error) { - return "1.2.3.4", nil - }, - isPortReachableNATReflection: func(host string, port uint16) bool { - if port == config.ExecutionCommon.P2pPort.Value.(uint16) { - ecChecked = true - } - if port == config.ConsensusCommon.P2pPort.Value.(uint16) { - ccChecked = true - } - return true - }, - isPortReachableExternalService: func(port uint16) (bool, string, error) { - return true, "", nil - }, - alertEth1P2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, - alertBeaconP2PPortNotOpen: func(*cfg.RocketPoolConfig, uint16) error { return nil }, - } - - task.Run() - - if ecChecked { - t.Error("expected Execution client port check to be skipped for External mode") - } - if !ccChecked { - t.Error("expected Consensus client port check to be performed for Local mode") - } -} From 5a58f1e6414b350125fd93743adbd87937d6ee3d Mon Sep 17 00:00:00 2001 From: b0a7 <127276179+b0a7@users.noreply.github.com> Date: Tue, 3 Mar 2026 17:56:05 -0500 Subject: [PATCH 9/9] revert disk space changes --- shared/services/config/alertmanager-config.go | 20 ---- .../config/alertmanager_config_test.go | 112 ------------------ .../install/alerting/rules/default.tmpl | 12 ++ .../install/alerting/rules/high-storage.tmpl | 22 ---- 4 files changed, 12 insertions(+), 154 deletions(-) delete mode 100644 shared/services/config/alertmanager_config_test.go delete mode 100644 shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl diff --git a/shared/services/config/alertmanager-config.go b/shared/services/config/alertmanager-config.go index 5fbb02cd5..17b9e4eb1 100644 --- a/shared/services/config/alertmanager-config.go +++ b/shared/services/config/alertmanager-config.go @@ -2,7 +2,6 @@ package config import ( "fmt" - "os" "github.com/mitchellh/go-homedir" "golang.org/x/text/cases" @@ -21,9 +20,6 @@ const AlertmanagerConfigFile string = "alerting/alertmanager.yml" const AlertingRulesConfigTemplate string = "alerting/rules/default.tmpl" const AlertingRulesConfigFile string = "alerting/rules/default.yml" -const HighStorageRulesConfigTemplate string = "alerting/rules/high-storage.tmpl" -const HighStorageRulesConfigFile string = "alerting/rules/high-storage.yml" - // Defaults const defaultAlertmanagerPort uint16 = 9093 const defaultAlertmanagerHost string = "localhost" @@ -377,22 +373,6 @@ func (cfg *AlertmanagerConfig) UpdateConfigurationFiles(configPath string) error return fmt.Errorf("error processing alerting rules template: %w", err) } - // Only apply high-storage rules if clients are locally managed. - // We don't require much storage if the EC/CC are external. - // Note that the warning alert is still active in the default rules and this is just for critical low storage alerts. - // (EC and CC are always both local or both external - mixed configurations are rejected during validation.) - isLocalMode := cfg.Parent.ExecutionClientMode.Value.(config.Mode) == config.Mode_Local - if isLocalMode { - err = cfg.processTemplate(configPath, HighStorageRulesConfigTemplate, HighStorageRulesConfigFile, "{{{", "}}}") - if err != nil { - return fmt.Errorf("error processing high-storage alerting rules template: %w", err) - } - } else { - if fileToRemove, err := homedir.Expand(fmt.Sprintf("%s/%s", configPath, HighStorageRulesConfigFile)); err == nil { - os.Remove(fileToRemove) - } - } - return nil } diff --git a/shared/services/config/alertmanager_config_test.go b/shared/services/config/alertmanager_config_test.go deleted file mode 100644 index a8abe19f7..000000000 --- a/shared/services/config/alertmanager_config_test.go +++ /dev/null @@ -1,112 +0,0 @@ -package config - -import ( - "io" - "os" - "path/filepath" - "testing" - - cfgtypes "github.com/rocket-pool/smartnode/shared/types/config" -) - -// installAlertingTemplates copies the alerting template files from the source -// tree into dst, mimicking what `rocketpool service install` does at runtime. -// When `go test` runs, the working directory is the package directory -// (shared/services/config), so we walk up three levels to reach the repo root. -func installAlertingTemplates(t *testing.T, dst string) { - t.Helper() - - repoRoot := filepath.Join("..", "..", "..") - srcBase := filepath.Join(repoRoot, "shared", "services", "rocketpool", "assets", "install", "alerting") - dstBase := filepath.Join(dst, "alerting") - - err := filepath.Walk(srcBase, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - rel, err := filepath.Rel(srcBase, path) - if err != nil { - return err - } - target := filepath.Join(dstBase, rel) - if info.IsDir() { - return os.MkdirAll(target, 0755) - } - return copyFile(target, path) - }) - if err != nil { - t.Fatalf("failed to install alerting templates: %v", err) - } -} - -func copyFile(dst, src string) error { - if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { - return err - } - in, err := os.Open(src) - if err != nil { - return err - } - defer in.Close() - out, err := os.Create(dst) - if err != nil { - return err - } - defer out.Close() - _, err = io.Copy(out, in) - return err -} - -// TestUpdateConfigurationFiles_HighStorageRules verifies that high-storage.yml -// is created when at least one client is locally managed, and deleted when -// both clients are externally managed. -func TestUpdateConfigurationFiles_HighStorageRules(t *testing.T) { - tests := []struct { - name string - ecMode cfgtypes.Mode - ccMode cfgtypes.Mode - expectCreated bool - }{ - { - name: "local EC + local CC creates high-storage.yml", - ecMode: cfgtypes.Mode_Local, - ccMode: cfgtypes.Mode_Local, - expectCreated: true, - }, - { - name: "external EC + external CC removes high-storage.yml", - ecMode: cfgtypes.Mode_External, - ccMode: cfgtypes.Mode_External, - expectCreated: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - // Use a fresh temp dir for each sub-test so they are independent. - testDir := t.TempDir() - installAlertingTemplates(t, testDir) - - cfg := NewRocketPoolConfig(testDir, false) - cfg.ExecutionClientMode.Value = tc.ecMode - cfg.ConsensusClientMode.Value = tc.ccMode - cfg.Alertmanager.EnableAlerting.Value = true - - if err := cfg.Alertmanager.UpdateConfigurationFiles(testDir); err != nil { - t.Fatalf("UpdateConfigurationFiles returned error: %v", err) - } - - highStorageFile := filepath.Join(testDir, HighStorageRulesConfigFile) - _, statErr := os.Stat(highStorageFile) - exists := !os.IsNotExist(statErr) - - if exists != tc.expectCreated { - if tc.expectCreated { - t.Errorf("expected high-storage.yml to be created, but it was not") - } else { - t.Errorf("expected high-storage.yml to be deleted, but it still exists") - } - } - }) - } -} diff --git a/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl b/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl index 0505ee76e..4fdd1ef65 100644 --- a/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl +++ b/shared/services/rocketpool/assets/install/alerting/rules/default.tmpl @@ -92,6 +92,18 @@ groups: description: "{{ $labels.instance }} has low disk space. Currently has {{ humanize $value }} GB free." {{{- end }}} + {{{- if .AlertEnabled_LowDiskSpaceCritical.Value }}} + - alert: LowDiskSpaceCritical + # NOTE: 50GB taken from PruneFreeSpaceRequired in rocketpool-cli's nethermind pruning (it won't prune below 50GB) + expr: node_filesystem_avail_bytes{job="node", mountpoint="/"} / 1024^3 < 50 + labels: + severity: critical + job: node + annotations: + summary: "Device {{ $labels.device }} on instance {{ $labels.instance }} has critically low disk space" + description: "{{ $labels.instance }} has critically low disk space. Currently has {{ humanize $value }} GB free." + {{{- end }}} + {{{- if .AlertEnabled_OSUpdatesAvailable.Value }}} - alert: OSUpdatesAvailable expr: max(os_upgrades_pending{job="node"}) > 0 diff --git a/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl b/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl deleted file mode 100644 index ba5a333b0..000000000 --- a/shared/services/rocketpool/assets/install/alerting/rules/high-storage.tmpl +++ /dev/null @@ -1,22 +0,0 @@ -# Autogenerated - DO NOT MODIFY THIS FILE DIRECTLY If you want to overwrite some -# of these values with your own customizations, please disable the alerts in the -# TUI and then add your own rule files to the /alerting/rules -# directory. -# -# NOTE: This file uses non-default go template delimiters (triple braces) to avoid -# conflicts with the default delimiters used in the alerting rules. - -groups: - - name: HighStorage - rules: - {{{- if .AlertEnabled_LowDiskSpaceCritical.Value }}} - - alert: LowDiskSpaceCritical - # NOTE: 50GB taken from PruneFreeSpaceRequired in rocketpool-cli's nethermind pruning (it won't prune below 50GB) - expr: node_filesystem_avail_bytes{job="node", mountpoint="/"} / 1024^3 < 50 - labels: - severity: critical - job: node - annotations: - summary: "Device {{ $labels.device }} on instance {{ $labels.instance }} has critically low disk space" - description: "{{ $labels.instance }} has critically low disk space. Currently has {{ humanize $value }} GB free." - {{{- end }}}