Windows Support: Fix Build Regressions, Tests Pass

pkg/custompluginmonitor/plugin/plugin.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/golang/glog"
 	cpmtypes "k8s.io/node-problem-detector/pkg/custompluginmonitor/types"
+	"k8s.io/node-problem-detector/pkg/util"
 	"k8s.io/node-problem-detector/pkg/util/tomb"
 )
 
@@ -147,12 +148,7 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 	}
 	defer cancel()
 
-	// create a process group
-	sysProcAttr := &syscall.SysProcAttr{
-		Setpgid: true,
-	}
-	cmd := exec.Command(rule.Path, rule.Args...)
-	cmd.SysProcAttr = sysProcAttr
+	cmd := util.Exec(rule.Path, rule.Args...)
 
 	stdoutPipe, err := cmd.StdoutPipe()
 	if err != nil {
@@ -172,6 +168,9 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 	waitChan := make(chan struct{})
 	defer close(waitChan)
 
+	var m sync.Mutex
+	timeout := false
+
 	go func() {
 		select {
 		case <-ctx.Done():
@@ -183,7 +182,12 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 				glog.Errorf("Error in cmd.Process check %q", rule.Path)
 				break
 			}
-			err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+
+			m.Lock()
+			timeout = true
+			m.Unlock()
+
+			err := util.Kill(cmd)
 			if err != nil {
 				glog.Errorf("Error in kill process %d, %v", cmd.Process.Pid, err)
 			}
@@ -202,12 +206,12 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 
 	wg.Add(2)
 	go func() {
+		defer wg.Done()
 		stdout, stdoutErr = readFromReader(stdoutPipe, maxCustomPluginBufferBytes)
-		wg.Done()
 	}()
 	go func() {
+		defer wg.Done()
 		stderr, stderrErr = readFromReader(stderrPipe, maxCustomPluginBufferBytes)
-		wg.Done()
 	}()
 	// This will wait for the reads to complete. If the execution times out, the pipes
 	// will be closed and the wait group unblocks.
@@ -234,7 +238,11 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 	output = string(stdout)
 	output = strings.TrimSpace(output)
 
-	if cmd.ProcessState.Sys().(syscall.WaitStatus).Signaled() {
+	m.Lock()
+	cmdKilled := timeout
+	m.Unlock()
+
+	if cmdKilled {
 		output = fmt.Sprintf("Timeout when running plugin %q: state - %s. output - %q", rule.Path, cmd.ProcessState.String(), output)
 	}
 
@@ -257,6 +265,7 @@ func (p *Plugin) run(rule cpmtypes.CustomRule) (exitStatus cpmtypes.Status, outp
 	}
 }
 
+// Stop the plugin.
 func (p *Plugin) Stop() {
 	p.tomb.Stop()
 	glog.Info("Stop plugin execution")

pkg/custompluginmonitor/plugin/plugin_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package plugin
 
 import (
+	"runtime"
 	"testing"
 	"time"
 
@@ -25,6 +26,13 @@ import (
 
 func TestNewPluginRun(t *testing.T) {
 	ruleTimeout := 1 * time.Second
+	timeoutExitStatus := cpmtypes.Unknown
+	ext := "sh"
+
+	if runtime.GOOS == "windows" {
+		ext = "cmd"
+		timeoutExitStatus = cpmtypes.NonOK
+	}
 
 	utMetas := map[string]struct {
 		Rule       cpmtypes.CustomRule
@@ -33,7 +41,7 @@ func TestNewPluginRun(t *testing.T) {
 	}{
 		"ok": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/ok.sh",
+				Path:    "./test-data/ok." + ext,
 				Timeout: &ruleTimeout,
 			},
 			ExitStatus: cpmtypes.OK,
@@ -41,7 +49,7 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"non-ok": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/non-ok.sh",
+				Path:    "./test-data/non-ok." + ext,
 				Timeout: &ruleTimeout,
 			},
 			ExitStatus: cpmtypes.NonOK,
@@ -49,7 +57,7 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"unknown": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/unknown.sh",
+				Path:    "./test-data/unknown." + ext,
 				Timeout: &ruleTimeout,
 			},
 			ExitStatus: cpmtypes.Unknown,
@@ -57,6 +65,7 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"non executable": {
 			Rule: cpmtypes.CustomRule{
+				// Intentionally run .sh for Windows, this is meant to be not executable.
 				Path:    "./test-data/non-executable.sh",
 				Timeout: &ruleTimeout,
 			},
@@ -65,7 +74,7 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"longer than 80 stdout with ok exit status": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/longer-than-80-stdout-with-ok-exit-status.sh",
+				Path:    "./test-data/longer-than-80-stdout-with-ok-exit-status." + ext,
 				Timeout: &ruleTimeout,
 			},
 			ExitStatus: cpmtypes.OK,
@@ -73,7 +82,7 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"non defined exit status": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/non-defined-exit-status.sh",
+				Path:    "./test-data/non-defined-exit-status." + ext,
 				Timeout: &ruleTimeout,
 			},
 			ExitStatus: cpmtypes.Unknown,
@@ -81,29 +90,32 @@ func TestNewPluginRun(t *testing.T) {
 		},
 		"sleep 3 second with ok exit status": {
 			Rule: cpmtypes.CustomRule{
-				Path:    "./test-data/sleep-3-second-with-ok-exit-status.sh",
+				Path:    "./test-data/sleep-3-second-with-ok-exit-status." + ext,
 				Timeout: &ruleTimeout,
 			},
-			ExitStatus: cpmtypes.Unknown,
-			Output:     `Timeout when running plugin "./test-data/sleep-3-second-with-ok-exit-status.sh": state - signal: killed. output - ""`,
+			ExitStatus: timeoutExitStatus,
+			Output:     `Timeout when running plugin "./test-data/sleep-3-second-with-ok-exit-status.` + ext + `": state - signal: killed. output - ""`,
 		},
 	}
 
-	conf := cpmtypes.CustomPluginConfig{}
-	(&conf).ApplyConfiguration()
-	p := Plugin{config: conf}
-	for desp, utMeta := range utMetas {
-		gotExitStatus, gotOutput := p.run(utMeta.Rule)
-		// cut at position max_output_length if expected output is longer than max_output_length bytes
-		if len(utMeta.Output) > *p.config.PluginGlobalConfig.MaxOutputLength {
-			utMeta.Output = utMeta.Output[:*p.config.PluginGlobalConfig.MaxOutputLength]
-		}
-		if gotExitStatus != utMeta.ExitStatus || gotOutput != utMeta.Output {
-			t.Errorf("%s", desp)
-			t.Errorf("Error in run plugin and get exit status and output for %q. "+
-				"Got exit status: %v, Expected exit status: %v. "+
-				"Got output: %q, Expected output: %q",
-				utMeta.Rule.Path, gotExitStatus, utMeta.ExitStatus, gotOutput, utMeta.Output)
-		}
+	for k, v := range utMetas {
+		desp := k
+		utMeta := v
+		t.Run(desp, func(t *testing.T) {
+			conf := cpmtypes.CustomPluginConfig{}
+			(&conf).ApplyConfiguration()
+			p := Plugin{config: conf}
+			gotExitStatus, gotOutput := p.run(utMeta.Rule)
+			// cut at position max_output_length if expected output is longer than max_output_length bytes
+			if len(utMeta.Output) > *p.config.PluginGlobalConfig.MaxOutputLength {
+				utMeta.Output = utMeta.Output[:*p.config.PluginGlobalConfig.MaxOutputLength]
+			}
+			if gotExitStatus != utMeta.ExitStatus || gotOutput != utMeta.Output {
+				t.Errorf("Error in run plugin and get exit status and output for %q. "+
+					"Got exit status: %v, Expected exit status: %v. "+
+					"Got output: %q, Expected output: %q",
+					utMeta.Rule.Path, gotExitStatus, utMeta.ExitStatus, gotOutput, utMeta.Output)
+			}
+		})
 	}
 }

pkg/custompluginmonitor/plugin/test-data/longer-than-80-stdout-with-ok-exit-status.cmd

@@ -0,0 +1,4 @@
+@echo off
+
+echo 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+exit 0

pkg/custompluginmonitor/plugin/test-data/non-defined-exit-status.cmd

@@ -0,0 +1,4 @@
+@echo off
+
+echo NON-DEFINED-EXIT-STATUS
+exit 100

pkg/custompluginmonitor/plugin/test-data/non-ok.cmd

@@ -0,0 +1,4 @@
+@echo off
+
+echo NonOK
+exit 1

pkg/custompluginmonitor/plugin/test-data/ok.cmd

@@ -0,0 +1,4 @@
+@echo off
+
+echo OK
+exit 0

pkg/custompluginmonitor/plugin/test-data/sleep-3-second-with-ok-exit-status.cmd

@@ -0,0 +1,5 @@
+@echo off
+
+ping 127.0.0.1 -n 3 > nul
+echo SLEEP 3S SECOND
+exit 0

pkg/custompluginmonitor/plugin/test-data/unknown.cmd

@@ -0,0 +1,4 @@
+@echo off
+
+echo UNKNOWN
+exit 3