Merge pull request #53581 from Random-Liu/add-containerd-validation-node-e2e

Automatic merge from submit-queue (batch tested with PRs 53668, 53624, 52639, 53581, 51215). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add extra log and node env metadata support.

This PR:
1) Make log collection logic extensible via flags, so that we could collect more daemon logs in this PR. (e.g. `containerd.log` and `cri-containerd.log`)
2) Add extra node metadata from specified environment variable. (e.g. `PULL_REFS` in prow).

@krzyzacy I'll change the test-infra side soon. Let's discuss whether we should move/copy this code to test infra in your refactoring.

/cc @dchen1107 @yujuhong @abhi @mikebrow 

```release-note
NONE
```

test/e2e_node/remote/node_e2e.go

@@ -185,6 +185,7 @@ func updateOSSpecificKubeletFlags(args, host, workspace string) (string, error)
 // RunTest runs test on the node.
 func (n *NodeE2ERemote) RunTest(host, workspace, results, imageDesc, junitFilePrefix, testArgs, ginkgoArgs, systemSpecName string, timeout time.Duration) (string, error) {
 	// Install the cni plugins and add a basic CNI configuration.
+	// TODO(random-liu): Do this in cloud init after we remove containervm test.
 	if err := setupCNI(host, workspace); err != nil {
 		return "", err
 	}

test/e2e_node/runner/remote/run_remote.go

@@ -61,6 +61,33 @@ var gubernator = flag.Bool("gubernator", false, "If true, output Gubernator link
 var ginkgoFlags = flag.String("ginkgo-flags", "", "Passed to ginkgo to specify additional flags such as --skip=.")
 var systemSpecName = flag.String("system-spec-name", "", "The name of the system spec used for validating the image in the node conformance test. The specs are at test/e2e_node/system/specs/. If unspecified, the default built-in spec (system.DefaultSpec) will be used.")
 
+// envs is the type used to collect all node envs. The key is the env name,
+// and the value is the env value
+type envs map[string]string
+
+// String function of flag.Value
+func (e *envs) String() string {
+	return fmt.Sprint(*e)
+}
+
+// Set function of flag.Value
+func (e *envs) Set(value string) error {
+	kv := strings.SplitN(value, "=", 2)
+	if len(kv) != 2 {
+		return fmt.Errorf("invalid env string")
+	}
+	emap := *e
+	emap[kv[0]] = kv[1]
+	return nil
+}
+
+// nodeEnvs is the node envs from the flag `node-env`.
+var nodeEnvs = make(envs)
+
+func init() {
+	flag.Var(&nodeEnvs, "node-env", "An environment variable passed to instance as metadata, e.g. when '--node-env=PATH=/usr/bin' is specified, there will be an extra instance metadata 'PATH=/usr/bin'.")
+}
+
 const (
 	defaultMachine                = "n1-standard-1"
 	acceleratorTypeResourceFormat = "https://www.googleapis.com/compute/beta/projects/%s/zones/%s/acceleratorTypes/%s"
@@ -584,6 +611,8 @@ func createInstance(imageConfig *internalGCEImage) (string, error) {
 		if len(externalIp) > 0 {
 			remote.AddHostnameIp(name, externalIp)
 		}
+		// TODO(random-liu): Remove the docker version check. Use some other command to check
+		// instance readiness.
 		var output string
 		output, err = remote.SSH(name, "docker", "version")
 		if err != nil {
@@ -701,6 +730,9 @@ func parseInstanceMetadata(str string) map[string]string {
 		}
 		metadata[kp[0]] = string(v)
 	}
+	for k, v := range nodeEnvs {
+		metadata[k] = v
+	}
 	return metadata
 }
 

test/e2e_node/services/BUILD

@@ -12,6 +12,7 @@ go_library(
         "etcd.go",
         "internal_services.go",
         "kubelet.go",
+        "logs.go",
         "namespace_controller.go",
         "server.go",
         "services.go",

test/e2e_node/services/kubelet.go

@@ -128,8 +128,9 @@ func (e *E2EServices) startKubelet() (*server, error) {
 		cmdArgs = append(cmdArgs, systemdRun, "--unit="+unitName, "--slice=runtime.slice", "--remain-after-exit", builder.GetKubeletServerBin())
 		killCommand = exec.Command("systemctl", "kill", unitName)
 		restartCommand = exec.Command("systemctl", "restart", unitName)
-		e.logFiles["kubelet.log"] = logFileData{
+		e.logs["kubelet.log"] = LogFileData{
-			journalctlCommand: []string{"-u", unitName},
+			Name:              "kubelet.log",
+			JournalctlCommand: []string{"-u", unitName},
 		}
 		cmdArgs = append(cmdArgs,
 			"--kubelet-cgroups=/kubelet.slice",
@@ -138,6 +139,7 @@ func (e *E2EServices) startKubelet() (*server, error) {
 	} else {
 		cmdArgs = append(cmdArgs, builder.GetKubeletServerBin())
 		cmdArgs = append(cmdArgs,
+			// TODO(random-liu): Get rid of this docker specific thing.
 			"--runtime-cgroups=/docker-daemon",
 			"--kubelet-cgroups=/kubelet",
 			"--cgroup-root=/",

test/e2e_node/services/logs.go

@@ -0,0 +1,100 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package services
+
+import (
+	"encoding/json"
+	"flag"
+	"fmt"
+)
+
+// LogFileData holds data about logfiles to fetch with a journalctl command or
+// file from a node's file system.
+type LogFileData struct {
+	// Name of the log file.
+	Name string `json:"name"`
+	// Files are possible absolute paths of the log file.
+	Files []string `json:"files"`
+	// JournalctlCommand is the journalctl command to get log.
+	JournalctlCommand []string `json:"journalctl"`
+}
+
+// logFiles are the type used to collect all log files. The key is the expected
+// name of the log file after collected.
+type logFiles map[string]LogFileData
+
+// String function of flag.Value
+func (l *logFiles) String() string {
+	return fmt.Sprint(*l)
+}
+
+// Set function of flag.Value
+func (l *logFiles) Set(value string) error {
+	// Someone else is calling flag.Parse after the flags are parsed in the
+	// test framework. Use this to avoid the flag being parsed twice.
+	// TODO(random-liu): Figure out who is parsing the flags.
+	if flag.Parsed() {
+		return nil
+	}
+	var log LogFileData
+	if err := json.Unmarshal([]byte(value), &log); err != nil {
+		return err
+	}
+	// Note that we assume all white space in flag string is separating fields
+	logs := *l
+	logs[log.Name] = log
+	return nil
+}
+
+// extraLogs is the extra logs specified by the test runner.
+var extraLogs = make(logFiles)
+
+func init() {
+	flag.Var(&extraLogs, "extra-log", "Extra log to collect after test in the json format of LogFile.")
+}
+
+// requiredLogs is the required logs to collect after the test.
+var requiredLogs = []LogFileData{
+	{
+		Name:              "kern.log",
+		Files:             []string{"/var/log/kern.log"},
+		JournalctlCommand: []string{"-k"},
+	},
+	{
+		Name:              "cloud-init.log",
+		Files:             []string{"/var/log/cloud-init.log"},
+		JournalctlCommand: []string{"-u", "cloud*"},
+	},
+	// TODO(random-liu): Make docker.log non-required.
+	{
+		Name:              "docker.log",
+		Files:             []string{"/var/log/docker.log", "/var/log/upstart/docker.log"},
+		JournalctlCommand: []string{"-u", "docker"},
+	},
+}
+
+// getLogFiles get all logs to collect after the test.
+func getLogFiles() logFiles {
+	logs := make(logFiles)
+	for _, l := range requiredLogs {
+		logs[l.Name] = l
+	}
+	for _, l := range extraLogs {
+		logs[l.Name] = l
+	}
+	return logs
+}

test/e2e_node/services/services.go

@@ -39,14 +39,7 @@ type E2EServices struct {
 	monitorParent bool
 	services      *server
 	kubelet       *server
-	logFiles      map[string]logFileData
+	logs          logFiles
-}
-
-// logFileData holds data about logfiles to fetch with a journalctl command or
-// symlink from a node's file system.
-type logFileData struct {
-	files             []string
-	journalctlCommand []string
 }
 
 // NewE2EServices returns a new E2EServices instance.
@@ -54,11 +47,7 @@ func NewE2EServices(monitorParent bool) *E2EServices {
 	return &E2EServices{
 		monitorParent: monitorParent,
 		// Special log files that need to be collected for additional debugging.
-		logFiles: map[string]logFileData{
+		logs: getLogFiles(),
-			"kern.log":       {[]string{"/var/log/kern.log"}, []string{"-k"}},
-			"docker.log":     {[]string{"/var/log/docker.log", "/var/log/upstart/docker.log"}, []string{"-u", "docker"}},
-			"cloud-init.log": {[]string{"/var/log/cloud-init.log"}, []string{"-u", "cloud*"}},
-		},
 	}
 }
 
@@ -91,7 +80,7 @@ func (e *E2EServices) Stop() {
 	defer func() {
 		if !framework.TestContext.NodeConformance {
 			// Collect log files.
-			e.getLogFiles()
+			e.collectLogFiles()
 		}
 	}()
 	if e.services != nil {
@@ -145,25 +134,25 @@ func (e *E2EServices) startInternalServices() (*server, error) {
 	return server, server.start()
 }
 
-// getLogFiles gets logs of interest either via journalctl or by creating sym
+// collectLogFiles collects logs of interest either via journalctl or by creating sym
 // links. Since we scp files from the remote directory, symlinks will be
 // treated as normal files and file contents will be copied over.
-func (e *E2EServices) getLogFiles() {
+func (e *E2EServices) collectLogFiles() {
 	// Nothing to do if report dir is not specified.
 	if framework.TestContext.ReportDir == "" {
 		return
 	}
 	glog.Info("Fetching log files...")
 	journaldFound := isJournaldAvailable()
-	for targetFileName, logFileData := range e.logFiles {
+	for targetFileName, log := range e.logs {
 		targetLink := path.Join(framework.TestContext.ReportDir, targetFileName)
 		if journaldFound {
 			// Skip log files that do not have an equivalent in journald-based machines.
-			if len(logFileData.journalctlCommand) == 0 {
+			if len(log.JournalctlCommand) == 0 {
 				continue
 			}
-			glog.Infof("Get log file %q with journalctl command %v.", targetFileName, logFileData.journalctlCommand)
+			glog.Infof("Get log file %q with journalctl command %v.", targetFileName, log.JournalctlCommand)
-			out, err := exec.Command("journalctl", logFileData.journalctlCommand...).CombinedOutput()
+			out, err := exec.Command("journalctl", log.JournalctlCommand...).CombinedOutput()
 			if err != nil {
 				glog.Errorf("failed to get %q from journald: %v, %v", targetFileName, string(out), err)
 			} else {
@@ -173,7 +162,7 @@ func (e *E2EServices) getLogFiles() {
 			}
 			continue
 		}
-		for _, file := range logFileData.files {
+		for _, file := range log.Files {
 			if _, err := os.Stat(file); err != nil {
 				// Expected file not found on this distro.
 				continue