Merge pull request #38255 from bprashanth/svc_cleanup

Automatic merge from submit-queue

Delete regional static-ip instead of global for type=lb

Global vs region is the difference between 
```
$ gcloud compute addresses delete foo --global
$ gcloud compute addresses delete foo --region us-central1
```

Type=LoadBalancer users the second type and were were doing the first. 
Also adds some logging.

test/e2e/framework/BUILD

@@ -17,6 +17,7 @@ go_library(
         "exec_util.go",
         "federation_util.go",
         "framework.go",
+        "google_compute.go",
         "kubelet_stats.go",
         "log_size_monitoring.go",
         "metrics_util.go",
@@ -99,6 +100,7 @@ go_library(
         "//vendor:github.com/spf13/viper",
         "//vendor:golang.org/x/crypto/ssh",
         "//vendor:golang.org/x/net/websocket",
+        "//vendor:google.golang.org/api/googleapi",
         "//vendor:gopkg.in/yaml.v2",
         "//vendor:k8s.io/client-go/kubernetes",
         "//vendor:k8s.io/client-go/pkg/util/sets",

test/e2e/framework/google_compute.go

@@ -0,0 +1,98 @@
+/*
+Copyright 2014 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 framework
+
+import (
+	"fmt"
+	"os/exec"
+	"regexp"
+	"strings"
+	"time"
+
+	"github.com/golang/glog"
+
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
+)
+
+// 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"
+	// Created [https://www.googleapis.com/compute/v1/projects/abshah-kubernetes-001/regions/us-central1/addresses/test-static-ip].
+	// NAME           REGION      ADDRESS       STATUS
+	// test-static-ip us-central1 104.197.143.7 RESERVED
+
+	var outputBytes []byte
+	var err error
+	region, err := gce.GetGCERegion(TestContext.CloudConfig.Zone)
+	if err != nil {
+		return "", fmt.Errorf("failed to convert zone to region: %v", err)
+	}
+	glog.Infof("Creating static IP with name %q in project %q in region %q", name, TestContext.CloudConfig.ProjectID, region)
+	for attempts := 0; attempts < 4; attempts++ {
+		outputBytes, err = exec.Command("gcloud", "compute", "addresses", "create",
+			name, "--project", TestContext.CloudConfig.ProjectID,
+			"--region", region, "-q").CombinedOutput()
+		if err == nil {
+			break
+		}
+		glog.Errorf("output from failed attempt to create static IP: %s", outputBytes)
+		time.Sleep(time.Duration(5*attempts) * time.Second)
+	}
+	if err != nil {
+		// 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)
+	}
+	output := string(outputBytes)
+	if strings.Contains(output, "RESERVED") {
+		r, _ := regexp.Compile("[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+")
+		staticIP := r.FindString(output)
+		if staticIP == "" {
+			return "", fmt.Errorf("static IP not found in gcloud command output: %v", output)
+		} else {
+			return staticIP, nil
+		}
+	} else {
+		return "", fmt.Errorf("static IP %q could not be reserved: %v", name, output)
+	}
+}
+
+func DeleteGCEStaticIP(name 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"
+	// Created [https://www.googleapis.com/compute/v1/projects/abshah-kubernetes-001/regions/us-central1/addresses/test-static-ip].
+	// NAME           REGION      ADDRESS       STATUS
+	// test-static-ip us-central1 104.197.143.7 RESERVED
+
+	region, err := gce.GetGCERegion(TestContext.CloudConfig.Zone)
+	if err != nil {
+		return fmt.Errorf("failed to convert zone to region: %v", err)
+	}
+	glog.Infof("Deleting static IP with name %q in project %q in region %q", name, TestContext.CloudConfig.ProjectID, region)
+	outputBytes, err := exec.Command("gcloud", "compute", "addresses", "delete",
+		name, "--project", TestContext.CloudConfig.ProjectID,
+		"--region", region, "-q").CombinedOutput()
+	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
+}

test/e2e/framework/util.go

@@ -41,6 +41,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"google.golang.org/api/googleapi"
 	"k8s.io/kubernetes/federation/client/clientset_generated/federation_release_1_5"
 	"k8s.io/kubernetes/pkg/api"
 	apierrs "k8s.io/kubernetes/pkg/api/errors"
@@ -5000,17 +5001,49 @@ func (p *E2ETestNodePreparer) CleanupNodes() error {
 	return encounteredError
 }
 
-func CleanupGCEResources(loadBalancerName string) (err error) {
+// CleanupGCEResources cleans up GCE Service Type=LoadBalancer resources with
+// the given name. The name is usually the UUID of the Service prefixed with an
+// alpha-numeric character ('a') to work around cloudprovider rules.
+func CleanupGCEResources(loadBalancerName string) (retErr error) {
 	gceCloud, ok := TestContext.CloudConfig.Provider.(*gcecloud.GCECloud)
 	if !ok {
 		return fmt.Errorf("failed to convert CloudConfig.Provider to GCECloud: %#v", TestContext.CloudConfig.Provider)
 	}
-	gceCloud.DeleteFirewall(loadBalancerName)
-	gceCloud.DeleteForwardingRule(loadBalancerName)
-	gceCloud.DeleteGlobalStaticIP(loadBalancerName)
-	hc, _ := gceCloud.GetHttpHealthCheck(loadBalancerName)
-	gceCloud.DeleteTargetPool(loadBalancerName, hc)
-	return nil
+	if err := gceCloud.DeleteFirewall(loadBalancerName); err != nil &&
+		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
+		retErr = err
+	}
+	if err := gceCloud.DeleteForwardingRule(loadBalancerName); err != nil &&
+		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
+		retErr = fmt.Errorf("%v\n%v", retErr, err)
+
+	}
+	if err := gceCloud.DeleteGlobalStaticIP(loadBalancerName); err != nil &&
+		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
+		retErr = fmt.Errorf("%v\n%v", retErr, err)
+	}
+	// This function shells out to gcloud, so we can't compare for NotFound errors.
+	// TODO: Invoke cloudprovider method directly instead.
+	if err := DeleteGCEStaticIP(loadBalancerName); err != nil {
+		Logf("%v", err)
+	}
+	hc, getErr := gceCloud.GetHttpHealthCheck(loadBalancerName)
+	if getErr != nil && !IsGoogleAPIHTTPErrorCode(getErr, http.StatusNotFound) {
+		retErr = fmt.Errorf("%v\n%v", retErr, getErr)
+		return
+	}
+	if err := gceCloud.DeleteTargetPool(loadBalancerName, hc); err != nil &&
+		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
+		retErr = fmt.Errorf("%v\n%v", retErr, err)
+	}
+	return
+}
+
+// IsHTTPErrorCode returns true if the error is a google api
+// error matching the corresponding HTTP error code.
+func IsGoogleAPIHTTPErrorCode(err error, code int) bool {
+	apiErr, ok := err.(*googleapi.Error)
+	return ok && apiErr.Code == code
 }
 
 // getMaster populates the externalIP, internalIP and hostname fields of the master.