Merge pull request #30321 from fejta/check-resources

Automatic merge from submit-queue

Convert bool to error, helper func for cd to skew

Switch from various functions returning `False` when something bad happens to returning an `error`
Encapsulate logic to switch to the skew directory inside chdirSkew

Also add a TODO for using `hyphen-flags` instead of `underscore_flags`

docs/devel/e2e-tests.md

@@ -124,9 +124,6 @@ go run hack/e2e.go -v --build
 # Create a fresh cluster.  Deletes a cluster first, if it exists
 go run hack/e2e.go -v --up
 
-# Test if a cluster is up.
-go run hack/e2e.go -v --isup
-
 # Push code to an existing cluster
 go run hack/e2e.go -v --push
 

hack/e2e.go

@@ -31,6 +31,7 @@ import (
 )
 
 var (
+	// TODO(fejta): change all these _ flags to -
 	build          = flag.Bool("build", false, "If true, build a new release. Otherwise, use whatever is there.")
 	checkNodeCount = flag.Bool("check_node_count", true, ""+
 		"By default, verify that the cluster has at least two nodes."+
@@ -45,7 +46,6 @@ var (
 	down                 = flag.Bool("down", false, "If true, tear down the cluster before exiting.")
 	dump                 = flag.String("dump", "", "If set, dump cluster logs to this location on test or cluster-up failure")
 	kubemark             = flag.Bool("kubemark", false, "If true, run kubemark tests.")
-	isup                 = flag.Bool("isup", false, "Check to see if the e2e cluster is up, then exit.")
 	push                 = flag.Bool("push", false, "If true, push to e2e cluster. Has no effect if -up is true.")
 	pushup               = flag.Bool("pushup", false, "If true, push to e2e cluster if it's up, otherwise start the e2e cluster.")
 	skewTests            = flag.Bool("skew", false, "If true, run tests in another version at ../kubernetes/hack/e2e.go")
@@ -60,6 +60,13 @@ const (
 	minNodeCount = 2
 )
 
+func appendError(errs []error, err error) []error {
+	if err != nil {
+		return append(errs, err)
+	}
+	return errs
+}
+
 func main() {
 	log.SetFlags(log.LstdFlags | log.Lshortfile)
 	flag.Parse()
@@ -77,41 +84,35 @@ func main() {
 		log.Fatalf("Must run from kubernetes directory: %v", cwd)
 	}
 
-	if *isup {
-		status := 1
-		if IsUp() {
-			status = 0
-			log.Printf("Cluster is UP")
-		} else {
-			log.Printf("Cluster is DOWN")
-		}
-		os.Exit(status)
-	}
-
 	if *build {
 		// The build-release script needs stdin to ask the user whether
 		// it's OK to download the docker image.
 		cmd := exec.Command("make", "quick-release")
 		cmd.Stdin = os.Stdin
-		if !finishRunning("build-release", cmd) {
+		if err := finishRunning("build-release", cmd); err != nil {
-			log.Fatal("Error building. Aborting.")
+			log.Fatalf("error building kubernetes: %v", err)
 		}
 	}
 
-	if *up && !TearDown() {
+	if *up {
-		log.Fatal("Could not tear down previous cluster")
+		if err := TearDown(); err != nil {
+			log.Fatalf("error tearing down previous cluster: %v", err)
+		}
 	}
 
-	beforeResources := ""
+	var errs []error
+
+	var beforeResources []byte
 	if *checkLeakedResources {
-		beforeResources = ListResources()
+		beforeResources, err = ListResources()
+		errs = appendError(errs, err)
 	}
 
 	os.Setenv("KUBECTL", strings.Join(append([]string{"./cluster/kubectl.sh"}, kubectlArgs()...), " "))
 
 	if *upgradeArgs != "" { // Start the cluster using a previous version.
-		if !UpgradeUp() {
+		if err := UpgradeUp(); err != nil {
-			log.Fatal("Failed to start cluster to upgrade. Aborting.")
+			log.Fatalf("error starting cluster to upgrade: %v", err)
 		}
 	} else { // Start the cluster using this version.
 		if *pushup {
@@ -126,112 +127,106 @@ func main() {
 			}
 		}
 		if *up {
-			if !Up() {
+			if err := Up(); err != nil {
-				DumpClusterLogs(*dump)
+				log.Fatalf("starting e2e cluster: %v", err)
-				log.Fatal("Error starting e2e cluster. Aborting.")
 			}
 		} else if *push {
-			if !finishRunning("push", exec.Command("./hack/e2e-internal/e2e-push.sh")) {
+			if err := finishRunning("push", exec.Command("./hack/e2e-internal/e2e-push.sh")); err != nil {
-				DumpClusterLogs(*dump)
+				log.Fatalf("error pushing e2e clsuter: %v", err)
-				log.Fatal("Error pushing e2e cluster. Aborting.")
 			}
 		}
 	}
 
-	upResources := ""
+	var upResources []byte
 	if *checkLeakedResources {
-		upResources = ListResources()
+		upResources, err = ListResources()
+		errs = appendError(errs, err)
 	}
 
-	success := true
-
 	if *ctlCmd != "" {
 		ctlArgs := strings.Fields(*ctlCmd)
 		os.Setenv("KUBE_CONFIG_FILE", "config-test.sh")
-		ctlSuccess := finishRunning("'kubectl "+*ctlCmd+"'", exec.Command("./cluster/kubectl.sh", ctlArgs...))
+		errs = appendError(errs, finishRunning("'kubectl "+*ctlCmd+"'", exec.Command("./cluster/kubectl.sh", ctlArgs...)))
-		success = success && ctlSuccess
 	}
 
 	if *upgradeArgs != "" {
-		upgradeSuccess := UpgradeTest(*upgradeArgs)
+		errs = appendError(errs, UpgradeTest(*upgradeArgs))
-		success = success && upgradeSuccess
 	}
 
 	if *test {
 		if *skewTests {
-			skewSuccess := SkewTest()
+			errs = appendError(errs, SkewTest())
-			success = success && skewSuccess
 		} else {
-			testSuccess := Test()
+			errs = appendError(errs, Test())
-			success = success && testSuccess
 		}
 	}
 
 	if *kubemark {
-		kubeSuccess := KubemarkTest()
+		errs = appendError(errs, KubemarkTest())
-		success = success && kubeSuccess
-	}
-
-	if !success {
-		DumpClusterLogs(*dump)
 	}
 
 	if *down {
-		tearSuccess := TearDown()
+		if errs != nil && *dump != "" {
-		success = success && tearSuccess
+			DumpClusterLogs(*dump)
+		}
+		errs = appendError(errs, TearDown())
 	}
 
 	if *checkLeakedResources {
 		log.Print("Sleeping for 30 seconds...") // Wait for eventually consistent listing
 		time.Sleep(30 * time.Second)
-		DiffResources(beforeResources, upResources, ListResources(), *dump)
+		afterResources, err := ListResources()
+		if err != nil {
+			errs = append(errs, err)
+		} else {
+			errs = appendError(errs, DiffResources(beforeResources, upResources, afterResources, *dump))
+		}
 	}
 
-	if !success {
+	if len(errs) != 0 {
-		os.Exit(1)
+		log.Fatalf("Encountered %d errors: %v", len(errs), errs)
 	}
 }
 
-func writeOrDie(dir, name, data string) string {
+func DiffResources(before, clusterUp, after []byte, location string) error {
-	f, err := os.Create(filepath.Join(dir, name))
-	if err != nil {
-		log.Fatal(err)
-	}
-	if _, err := f.WriteString(data); err != nil {
-		log.Fatal(err)
-	}
-	if err := f.Close(); err != nil {
-		log.Fatal(err)
-	}
-	log.Printf("Created file: %s", f.Name())
-	return f.Name()
-}
-
-func DiffResources(before, clusterUp, after, location string) {
 	if location == "" {
 		var err error
 		location, err = ioutil.TempDir("", "e2e-check-resources")
 		if err != nil {
-			log.Fatal(err)
+			return fmt.Errorf("Could not create e2e-check-resources temp dir: %s", err)
 		}
 	}
-	bp := writeOrDie(location, "gcp-resources-before.txt", before)
+
-	writeOrDie(location, "gcp-resources-cluster-up.txt", clusterUp)
+	var mode os.FileMode = 0664
-	ap := writeOrDie(location, "gcp-resources-after.txt", after)
+	bp := filepath.Join(location, "gcp-resources-before.txt")
+	up := filepath.Join(location, "gcp-resources-cluster-up.txt")
+	ap := filepath.Join(location, "gcp-resources-after.txt")
+	dp := filepath.Join(location, "gcp-resources-diff.txt")
+
+	if err := ioutil.WriteFile(bp, before, mode); err != nil {
+		return err
+	}
+	if err := ioutil.WriteFile(up, clusterUp, mode); err != nil {
+		return err
+	}
+	if err := ioutil.WriteFile(ap, after, mode); err != nil {
+		return err
+	}
 
 	cmd := exec.Command("diff", "-sw", "-U0", "-F^\\[.*\\]$", bp, ap)
 	if *verbose {
 		cmd.Stderr = os.Stderr
 	}
-	o, err := cmd.Output()
+	stdout, cerr := cmd.Output()
-	stdout := string(o)
+	if err := ioutil.WriteFile(dp, stdout, mode); err != nil {
-	writeOrDie(location, "gcp-resources-diff.txt", stdout)
+		return err
-	if err == nil {
-		return
 	}
-	lines := strings.Split(stdout, "\n")
+	if cerr == nil { // No diffs
+		return nil
+	}
+	lines := strings.Split(string(stdout), "\n")
 	if len(lines) < 3 { // Ignore the +++ and --- header lines
-		return
+		return nil
 	}
 	lines = lines[2:]
 
@@ -242,12 +237,12 @@ func DiffResources(before, clusterUp, after, location string) {
 		}
 	}
 	if len(added) > 0 {
-		log.Printf("Error: %d leaked resources", len(added))
+		return fmt.Errorf("Error: %d leaked resources\n%v", len(added), strings.Join(added, "\n"))
-		log.Fatal(strings.Join(added, "\n"))
 	}
+	return nil
 }
 
-func ListResources() string {
+func ListResources() ([]byte, error) {
 	log.Printf("Listing resources...")
 	cmd := exec.Command("./cluster/gce/list-resources.sh")
 	if *verbose {
@@ -255,24 +250,24 @@ func ListResources() string {
 	}
 	stdout, err := cmd.Output()
 	if err != nil {
-		log.Fatalf("Failed to list resources (%s):\n%s", err, stdout)
+		return nil, fmt.Errorf("Failed to list resources (%s):\n%s", err, string(stdout))
 	}
-	return string(stdout)
+	return stdout, nil
 }
 
-func TearDown() bool {
+func TearDown() error {
 	return finishRunning("teardown", exec.Command("./hack/e2e-internal/e2e-down.sh"))
 }
 
 // Up brings an e2e cluster up, recreating it if one is already running.
-func Up() bool {
+func Up() error {
 	// force having batch/v2alpha1 always on for e2e tests
 	os.Setenv("KUBE_RUNTIME_CONFIG", "batch/v2alpha1=true")
 	return finishRunning("up", exec.Command("./hack/e2e-internal/e2e-up.sh"))
 }
 
 // Ensure that the cluster is large engough to run the e2e tests.
-func ValidateClusterSize() {
+func ValidateClusterSize() error {
 	// Check that there are at least minNodeCount nodes running
 	cmd := exec.Command("./hack/e2e-internal/e2e-cluster-size.sh")
 	if *verbose {
@@ -280,37 +275,35 @@ func ValidateClusterSize() {
 	}
 	stdout, err := cmd.Output()
 	if err != nil {
-		log.Fatalf("Could not get nodes to validate cluster size (%s)", err)
+		return fmt.Errorf("Could not get nodes to validate cluster size (%s)", err)
 	}
 
 	numNodes, err := strconv.Atoi(strings.TrimSpace(string(stdout)))
 	if err != nil {
-		log.Fatalf("Could not count number of nodes to validate cluster size (%s)", err)
+		return fmt.Errorf("Could not count number of nodes to validate cluster size (%s)", err)
 	}
 
 	if numNodes < minNodeCount {
-		log.Fatalf("Cluster size (%d) is too small to run e2e tests.  %d Nodes are required.", numNodes, minNodeCount)
+		return fmt.Errorf("Cluster size (%d) is too small to run e2e tests.  %d Nodes are required.", numNodes, minNodeCount)
 	}
+	return nil
 }
 
 // Is the e2e cluster up?
 func IsUp() bool {
-	return finishRunning("get status", exec.Command("./hack/e2e-internal/e2e-status.sh"))
+	return finishRunning("get status", exec.Command("./hack/e2e-internal/e2e-status.sh")) == nil
 }
 
-func DumpClusterLogs(location string) {
+func DumpClusterLogs(location string) error {
-	if location == "" {
-		return
-	}
 	log.Printf("Dumping cluster logs to: %v", location)
-	finishRunning("dump cluster logs", exec.Command("./cluster/log-dump.sh", location))
+	return finishRunning("dump cluster logs", exec.Command("./cluster/log-dump.sh", location))
 }
 
-func KubemarkTest() bool {
+func KubemarkTest() error {
 	// Stop previous run
-	if !finishRunning("Stop kubemark", exec.Command("./test/kubemark/stop-kubemark.sh")) {
+	err := finishRunning("Stop kubemark", exec.Command("./test/kubemark/stop-kubemark.sh"))
-		log.Print("stop kubemark failed")
+	if err != nil {
-		return false
+		return err
 	}
 
 	// Start new run
@@ -325,9 +318,9 @@ func KubemarkTest() bool {
 	}
 	os.Setenv("NUM_NODES", os.Getenv("KUBEMARK_NUM_NODES"))
 	os.Setenv("MASTER_SIZE", os.Getenv("KUBEMARK_MASTER_SIZE"))
-	if !finishRunning("Start Kubemark", exec.Command("./test/kubemark/start-kubemark.sh")) {
+	err = finishRunning("Start Kubemark", exec.Command("./test/kubemark/start-kubemark.sh"))
-		log.Print("Error: start kubemark failed")
+	if err != nil {
-		return false
+		return err
 	}
 
 	// Run kubemark tests
@@ -337,31 +330,36 @@ func KubemarkTest() bool {
 	}
 	test_args := os.Getenv("KUBEMARK_TEST_ARGS")
 
-	if !finishRunning("Run kubemark tests", exec.Command("./test/kubemark/run-e2e-tests.sh", "--ginkgo.focus="+focus, test_args)) {
+	err = finishRunning("Run kubemark tests", exec.Command("./test/kubemark/run-e2e-tests.sh", "--ginkgo.focus="+focus, test_args))
-		log.Print("Error: run kubemark tests failed")
+	if err != nil {
-		return false
+		return err
 	}
 
-	// Stop kubemark
+	err = finishRunning("Stop kubemark", exec.Command("./test/kubemark/stop-kubemark.sh"))
-	if !finishRunning("Stop kubemark", exec.Command("./test/kubemark/stop-kubemark.sh")) {
+	if err != nil {
-		log.Print("Error: stop kubemark failed")
+		return err
-		return false
 	}
-	return true
+	return nil
 }
 
-func UpgradeUp() bool {
+func chdirSkew() (string, error) {
 	old, err := os.Getwd()
 	if err != nil {
-		log.Printf("Failed to os.Getwd(): %v", err)
+		return "", fmt.Errorf("Failed to os.Getwd(): %v", err)
-		return false
 	}
-	defer os.Chdir(old)
 	err = os.Chdir("../kubernetes_skew")
 	if err != nil {
-		log.Printf("Failed to cd ../kubernetes_skew: %v", err)
+		return "", fmt.Errorf("Failed to cd ../kubernetes_skew: %v", err)
-		return false
 	}
+	return old, nil
+}
+
+func UpgradeUp() error {
+	old, err := chdirSkew()
+	if err != nil {
+		return err
+	}
+	defer os.Chdir(old)
 	return finishRunning("UpgradeUp",
 		exec.Command(
 			"go", "run", "./hack/e2e.go",
@@ -373,18 +371,12 @@ func UpgradeUp() bool {
 		))
 }
 
-func UpgradeTest(args string) bool {
+func UpgradeTest(args string) error {
-	old, err := os.Getwd()
+	old, err := chdirSkew()
 	if err != nil {
-		log.Printf("Failed to os.Getwd(): %v", err)
+		return err
-		return false
 	}
 	defer os.Chdir(old)
-	err = os.Chdir("../kubernetes_skew")
-	if err != nil {
-		log.Printf("Failed to cd ../kubernetes_skew: %v", err)
-		return false
-	}
 	previous, present := os.LookupEnv("E2E_REPORT_PREFIX")
 	if present {
 		defer os.Setenv("E2E_REPORT_PREFIX", previous)
@@ -401,18 +393,12 @@ func UpgradeTest(args string) bool {
 			fmt.Sprintf("--check_version_skew=%t", *checkVersionSkew)))
 }
 
-func SkewTest() bool {
+func SkewTest() error {
-	old, err := os.Getwd()
+	old, err := chdirSkew()
 	if err != nil {
-		log.Printf("Failed to Getwd: %v", err)
+		return err
-		return false
 	}
 	defer os.Chdir(old)
-	err = os.Chdir("../kubernetes_skew")
-	if err != nil {
-		log.Printf("Failed to cd ../kubernetes_skew: %v", err)
-		return false
-	}
 	return finishRunning("Skewed Ginkgo tests",
 		exec.Command(
 			"go", "run", "./hack/e2e.go",
@@ -422,7 +408,7 @@ func SkewTest() bool {
 			fmt.Sprintf("--check_version_skew=%t", *checkVersionSkew)))
 }
 
-func Test() bool {
+func Test() error {
 	if !IsUp() {
 		log.Fatal("Testing requested, but e2e cluster not up!")
 	}
@@ -430,7 +416,9 @@ func Test() bool {
 	// TODO(fejta): add a --federated or something similar
 	if os.Getenv("FEDERATION") != "true" {
 		if *checkNodeCount {
-			ValidateClusterSize()
+			if err := ValidateClusterSize(); err != nil {
+				return err
+			}
 		}
 		return finishRunning("Ginkgo tests", exec.Command("./hack/ginkgo-e2e.sh", strings.Fields(*testArgs)...))
 	}
@@ -441,7 +429,7 @@ func Test() bool {
 	return finishRunning("Federated Ginkgo tests", exec.Command("./hack/federated-ginkgo-e2e.sh", strings.Fields(*testArgs)...))
 }
 
-func finishRunning(stepName string, cmd *exec.Cmd) bool {
+func finishRunning(stepName string, cmd *exec.Cmd) error {
 	if *verbose {
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
@@ -452,10 +440,9 @@ func finishRunning(stepName string, cmd *exec.Cmd) bool {
 	}(time.Now())
 
 	if err := cmd.Run(); err != nil {
-		log.Printf("Error running %v: %v", stepName, err)
+		return fmt.Errorf("error running %v: %v", stepName, err)
-		return false
 	}
-	return true
+	return nil
 }
 
 // returns either "", or a list of args intended for appending with the