From 54dc6bb6c92e0244e0a4665817953dab130723d6 Mon Sep 17 00:00:00 2001
From: Alex Robinson <arob@google.com>
Date: Fri, 9 Oct 2015 22:29:42 +0000
Subject: [PATCH] Improve the error handling when creating/delete GCE IPs in
 e2e tests.

Prior to this, test failures caused by errors in these helpers printed
out long exec.ExitErrors structs that didn't contain any useful info,
for example:
```
Expected error:
      <*exec.ExitError | 0xc20804e518>: {
          ProcessState: {
              pid: 22855,
              status: 256,
              rusage: {
                  Utime: {Sec: 0, Usec: 312000},
                  Stime: {Sec: 0, Usec: 96000},
                  Maxrss: 42912,
                  Ixrss: 0,
                  Idrss: 0,
                  Isrss: 0,
                  Minflt: 21667,
                  Majflt: 0,
                  Nswap: 0,
                  Inblock: 0,
                  Oublock: 40,
                  Msgsnd: 0,
                  Msgrcv: 0,
                  Nsignals: 0,
                  Nvcsw: 52,
                  Nivcsw: 34,
              },
          },
      }
      exit status 1
  not to have occurred
```
---
 test/e2e/google_compute.go | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/test/e2e/google_compute.go b/test/e2e/google_compute.go
index f33a78a5725..17fcd1c149b 100644
--- a/test/e2e/google_compute.go
+++ b/test/e2e/google_compute.go
@@ -26,6 +26,9 @@ import (
 	"github.com/golang/glog"
 )
 
+// TODO: These should really just use the GCE API client library or at least use
+// better formatted output from the --format flag.
+
 func createGCEStaticIP(name string) (string, error) {
 	// gcloud compute --project "abshah-kubernetes-001" addresses create "test-static-ip" --region "us-central1"
 	// abshah@abhidesk:~/go/src/code.google.com/p/google-api-go-client/compute/v1$ gcloud compute --project "abshah-kubernetes-001" addresses create "test-static-ip" --region "us-central1"
@@ -33,34 +36,35 @@ func createGCEStaticIP(name string) (string, error) {
 	// NAME           REGION      ADDRESS       STATUS
 	// test-static-ip us-central1 104.197.143.7 RESERVED
 
-	var output []byte
+	glog.Infof("Creating static IP with name %q in project %q", name, testContext.CloudConfig.ProjectID)
+	var outputBytes []byte
 	var err error
 	for attempts := 0; attempts < 4; attempts++ {
-		output, err = exec.Command("gcloud", "compute", "addresses", "create",
+		outputBytes, err = exec.Command("gcloud", "compute", "addresses", "create",
 			name, "--project", testContext.CloudConfig.ProjectID,
 			"--region", "us-central1", "-q").CombinedOutput()
 		if err == nil {
 			break
 		}
-		glog.Errorf("Creating static IP with name:%s in project: %s", name, testContext.CloudConfig.ProjectID)
-		glog.Errorf("output: %s", output)
+		glog.Errorf("output from failed attempt to create static IP: %s", outputBytes)
 		time.Sleep(time.Duration(5*attempts) * time.Second)
 	}
 	if err != nil {
-		return "", err
+		// Ditch the error, since the stderr in the output is what actually contains
+		// any useful info.
+		return "", fmt.Errorf("failed to create static IP: %s", outputBytes)
 	}
-	text := string(output)
-	if strings.Contains(text, "RESERVED") {
+	output := string(outputBytes)
+	if strings.Contains(output, "RESERVED") {
 		r, _ := regexp.Compile("[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+")
-		staticIP := r.FindString(text)
+		staticIP := r.FindString(output)
 		if staticIP == "" {
-			glog.Errorf("Static IP creation output is \n %s", text)
-			return "", fmt.Errorf("Static IP not found in gcloud compute command output")
+			return "", fmt.Errorf("static IP not found in gcloud command output: %v", output)
 		} else {
 			return staticIP, nil
 		}
 	} else {
-		return "", fmt.Errorf("Static IP Could not be reserved.")
+		return "", fmt.Errorf("static IP %q could not be reserved: %v", name, output)
 	}
 }
 
@@ -71,8 +75,13 @@ func deleteGCEStaticIP(name string) error {
 	// NAME           REGION      ADDRESS       STATUS
 	// test-static-ip us-central1 104.197.143.7 RESERVED
 
-	_, err := exec.Command("gcloud", "compute", "addresses", "delete",
+	outputBytes, err := exec.Command("gcloud", "compute", "addresses", "delete",
 		name, "--project", testContext.CloudConfig.ProjectID,
 		"--region", "us-central1", "-q").CombinedOutput()
-	return err
+	if err != nil {
+		// Ditch the error, since the stderr in the output is what actually contains
+		// any useful info.
+		return fmt.Errorf("failed to delete static IP %q: %v", name, string(outputBytes))
+	}
+	return nil
 }