Merge pull request #9299 from hashicorp/b-detect-unloaded-kmod

client/fingerprint: detect unloaded dynamic bridge kernel module

CHANGELOG.md

@@ -42,6 +42,7 @@ BUG FIXES:
  * core: Fixed a bug where a request to scale a job would fail if the job was not in the default namespace. [[GH-9296](https://github.com/hashicorp/nomad/pull/9296)]
  * config (Enterprise): Fixed default enterprise config merging. [[GH-9083](https://github.com/hashicorp/nomad/pull/9083)]
  * client: Fixed an issue with the Java fingerprinter on macOS causing pop-up notifications when no JVM installed. [[GH-9225](https://github.com/hashicorp/nomad/pull/9225)] 
+ * client: Fixed an fingerprinter issue detecting bridge kernel module [[GH-9299](https://github.com/hashicorp/nomad/pull/9299)]
  * consul: Fixed a bug to correctly validate task when using script-checks in group-level services [[GH-8952](https://github.com/hashicorp/nomad/issues/8952)]
  * consul: Fixed a bug where canary_meta was not being interpolated with environment variables [[GH-9096](https://github.com/hashicorp/nomad/pull/9096)]
  * consul/connect: Fixed a bug to correctly trigger updates on jobspec changes [[GH-9029](https://github.com/hashicorp/nomad/pull/9029)]

client/fingerprint/bridge.go

@@ -3,8 +3,9 @@ package fingerprint
 import log "github.com/hashicorp/go-hclog"
 
 type BridgeFingerprint struct {
-	logger log.Logger
 	StaticFingerprinter
+
+	logger log.Logger
 }
 
 func NewBridgeFingerprint(logger log.Logger) Fingerprint {

client/fingerprint/bridge_linux.go

@@ -6,71 +6,98 @@ import (
 	"os"
 	"regexp"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/shirou/gopsutil/host"
 )
 
 const bridgeKernelModuleName = "bridge"
 
+const (
+	dynamicModuleRe = `%s\s+.*$`
+	builtinModuleRe = `.+/%s.ko$`
+	dependsModuleRe = `.+/%s.ko:.*$`
+)
+
 func (f *BridgeFingerprint) Fingerprint(req *FingerprintRequest, resp *FingerprintResponse) error {
-	if err := f.checkKMod(bridgeKernelModuleName); err != nil {
+	if err := f.detect(bridgeKernelModuleName); err != nil {
 		f.logger.Warn("failed to detect bridge kernel module, bridge network mode disabled", "error", err)
 		return nil
 	}
 
 	resp.NodeResources = &structs.NodeResources{
-		Networks: []*structs.NetworkResource{
+		Networks: []*structs.NetworkResource{{
-			{
+			Mode: "bridge",
-				Mode: "bridge",
+		}},
-			},
+		NodeNetworks: []*structs.NodeNetworkResource{{
-		},
+			Mode:   "bridge",
-		NodeNetworks: []*structs.NodeNetworkResource{
+			Device: req.Config.BridgeNetworkName,
-			{
+		}},
-				Mode:   "bridge",
-				Device: req.Config.BridgeNetworkName,
-			},
-		},
 	}
+
 	resp.Detected = true
 	return nil
 }
 
-func (f *BridgeFingerprint) checkKMod(mod string) error {
+func (f *BridgeFingerprint) regexp(pattern, module string) *regexp.Regexp {
+	return regexp.MustCompile(fmt.Sprintf(pattern, module))
+}
+
+func (f *BridgeFingerprint) detect(module string) error {
+	// accumulate errors from every place we might find the module
+	var errs error
+
+	// check if the module has been dynamically loaded
+	dynamicPath := "/proc/modules"
+	if err := f.searchFile(module, dynamicPath, f.regexp(dynamicModuleRe, module)); err != nil {
+		errs = multierror.Append(errs, err)
+	} else {
+		return nil
+	}
+
+	// will need kernel info to look for builtin and unloaded modules
 	hostInfo, err := host.Info()
 	if err != nil {
 		return err
 	}
 
-	dynErr := f.checkKModFile(mod, "/proc/modules", fmt.Sprintf("%s\\s+.*$", mod))
+	// check if the module is builtin to the kernel
-	if dynErr == nil {
+	builtinPath := fmt.Sprintf("/lib/modules/%s/modules.builtin", hostInfo.KernelVersion)
+	if err := f.searchFile(module, builtinPath, f.regexp(builtinModuleRe, module)); err != nil {
+		errs = multierror.Append(errs, err)
+	} else {
 		return nil
 	}
 
-	builtinErr := f.checkKModFile(mod,
+	// check if the module is dynamic but unloaded (will have a dep entry)
-		fmt.Sprintf("/lib/modules/%s/modules.builtin", hostInfo.KernelVersion),
+	dependsPath := fmt.Sprintf("/lib/modules/%s/modules.dep", hostInfo.KernelVersion)
-		fmt.Sprintf(".+\\/%s.ko$", mod))
+	if err := f.searchFile(module, dependsPath, f.regexp(dependsModuleRe, module)); err != nil {
-	if builtinErr == nil {
+		errs = multierror.Append(errs, err)
+	} else {
 		return nil
 	}
 
-	return fmt.Errorf("%v, %v", dynErr, builtinErr)
+	return errs
 }
 
-func (f *BridgeFingerprint) checkKModFile(mod, fileName, pattern string) error {
+func (f *BridgeFingerprint) searchFile(module, filename string, re *regexp.Regexp) error {
-	file, err := os.Open(fileName)
+	file, err := os.Open(filename)
 	if err != nil {
-		return fmt.Errorf("could not read %s: %v", fileName, err)
+		return fmt.Errorf("failed to open %s: %v", filename, err)
 	}
-	defer file.Close()
+	defer func() {
+		_ = file.Close()
+	}()
 
 	scanner := bufio.NewScanner(file)
 	for scanner.Scan() {
-		if matched, err := regexp.MatchString(pattern, scanner.Text()); matched {
+		if re.MatchString(scanner.Text()) {
-			return nil
+			return nil // found the module!
-		} else if err != nil {
-			return fmt.Errorf("could not parse %s: %v", fileName, err)
 		}
 	}
+	if err := scanner.Err(); err != nil {
+		return fmt.Errorf("failed to scan %s: %v", filename, err)
+	}
 
-	return fmt.Errorf("could not detect kernel module %s", mod)
+	return fmt.Errorf("module %s not in %s", module, filename)
 }

client/fingerprint/bridge_linux_test.go

@@ -1,14 +1,129 @@
 package fingerprint
 
 import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"strings"
 	"testing"
 
+	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/stretchr/testify/require"
 )
 
-func TestBridgeFingerprint_checkKMod(t *testing.T) {
+func TestBridgeFingerprint_detect(t *testing.T) {
-	require := require.New(t)
+	f := &BridgeFingerprint{logger: testlog.HCLogger(t)}
-	f := &BridgeFingerprint{}
+	require.NoError(t, f.detect("ip_tables"))
-	require.NoError(f.checkKMod("ip_tables"))
+
-	require.Error(f.checkKMod("nonexistentmodule"))
+	err := f.detect("nonexistentmodule")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "3 errors occurred")
+}
+
+func writeFile(t *testing.T, prefix, content string) string {
+	f, err := ioutil.TempFile("", "bridge-fp-")
+	require.NoError(t, err)
+
+	_, err = io.Copy(f, strings.NewReader(content))
+	require.NoError(t, err)
+
+	err = f.Close()
+	require.NoError(t, err)
+
+	return f.Name()
+}
+
+func cleanupFile(t *testing.T, name string) {
+	err := os.Remove(name)
+	require.NoError(t, err)
+}
+
+const (
+	dynamicModuleContent = `
+ip_tables 32768 0 - Live 0xffffffffc03ee000
+x_tables 40960 1 ip_tables, Live 0xffffffffc03e3000
+autofs4 45056 2 - Live 0xffffffffc03d7000
+bpfilter 32768 0 - Live 0x0000000000000000
+br_netfilter 28672 0 - Live 0x0000000000000000
+bridge 176128 1 br_netfilter, Live 0x0000000000000000
+btrfs 1253376 0 - Live 0xffffffffc02a4000
+`
+
+	builtinModuleContent = `
+kernel/drivers/mfd/max14577.ko
+kernel/drivers/mfd/max77693.ko
+kernel/drivers/mfd/sec-core.ko
+kernel/drivers/mfd/sec-irq.ko
+kernel/drivers/net/bridge.ko
+kernel/drivers/net/tun.ko
+kernel/drivers/net/xen-netfront.k
+`
+
+	dependsModuleContent = `
+kernel/net/bridge/netfilter/ebt_log.ko: kernel/net/netfilter/x_tables.ko
+kernel/net/bridge/netfilter/ebt_nflog.ko: kernel/net/netfilter/x_tables.ko
+kernel/net/bridge/bridge.ko: kernel/net/802/stp.ko kernel/net/llc/llc.ko
+kernel/net/bridge/br_netfilter.ko: kernel/net/bridge/bridge.ko kernel/net/802/stp.ko kernel/net/llc/llc.ko
+kernel/net/appletalk/appletalk.ko: kernel/net/802/psnap.ko kernel/net/llc/llc.ko
+kernel/net/x25/x25.ko:
+`
+)
+
+func TestBridgeFingerprint_search(t *testing.T) {
+	f := &BridgeFingerprint{logger: testlog.HCLogger(t)}
+
+	t.Run("dynamic loaded module", func(t *testing.T) {
+		t.Run("present", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", dynamicModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("bridge", file, f.regexp(dynamicModuleRe, "bridge"))
+			require.NoError(t, err)
+		})
+
+		t.Run("absent", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", dynamicModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("nonexistent", file, f.regexp(dynamicModuleRe, "nonexistent"))
+			require.EqualError(t, err, fmt.Sprintf("module nonexistent not in %s", file))
+		})
+	})
+
+	t.Run("builtin module", func(t *testing.T) {
+		t.Run("present", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", builtinModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("bridge", file, f.regexp(builtinModuleRe, "bridge"))
+			require.NoError(t, err)
+		})
+
+		t.Run("absent", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", builtinModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("nonexistent", file, f.regexp(builtinModuleRe, "nonexistent"))
+			require.EqualError(t, err, fmt.Sprintf("module nonexistent not in %s", file))
+		})
+	})
+
+	t.Run("dynamic unloaded module", func(t *testing.T) {
+		t.Run("present", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", dependsModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("bridge", file, f.regexp(dependsModuleRe, "bridge"))
+			require.NoError(t, err)
+		})
+
+		t.Run("absent", func(t *testing.T) {
+			file := writeFile(t, "bridge-fp-", dependsModuleContent)
+			defer cleanupFile(t, file)
+
+			err := f.searchFile("nonexistent", file, f.regexp(dependsModuleRe, "nonexistent"))
+			require.EqualError(t, err, fmt.Sprintf("module nonexistent not in %s", file))
+		})
+	})
 }