Refactor GCE wrapper library to allow execution from E2E test suite

This reverts commit 147b691, reversing changes made to 6fd9860.
nikhiljindal · Nov 25, 2015 · 42b200a · 42b200a
1 parent 981b5a6
commit 42b200a
Show file tree

Hide file tree

Showing 11 changed files with 255 additions and 103 deletions.
diff --git a/hack/ginkgo-e2e.sh b/hack/ginkgo-e2e.sh
@@ -97,6 +97,7 @@ export PATH=$(dirname "${e2e_test}"):"${PATH}"
   --provider="${KUBERNETES_PROVIDER}" \
   --gce-project="${PROJECT:-}" \
   --gce-zone="${ZONE:-}" \
+  --gce-service-account="${GCE_SERVICE_ACCOUNT:-}" \
   --gke-cluster="${CLUSTER_NAME:-}" \
   --kube-master="${KUBE_MASTER:-}" \
   --cluster-tag="${CLUSTER_ID:-}" \

diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt
@@ -105,6 +105,7 @@ func-dest
 fuzz-iters
 gather-resource-usage
 gce-project
+gce-service-account
 gce-zone
 gke-cluster
 go-header-file

diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go
@@ -62,13 +62,12 @@ const (
 
 // GCECloud is an implementation of Interface, TCPLoadBalancer and Instances for Google Compute Engine.
 type GCECloud struct {
-	service          *compute.Service
-	containerService *container.Service
-	projectID        string
-	zone             string
-	instanceID       string
-	externalID       string
-	networkURL       string
+	service           *compute.Service
+	containerService  *container.Service
+	projectID         string
+	zone              string
+	networkURL        string
+	useMetadataServer bool
 }
 
 type Config struct {
@@ -101,7 +100,7 @@ func getProjectAndZone() (string, string, error) {
 	return projectID, zone, nil
 }
 
-func getInstanceID() (string, error) {
+func getInstanceIDViaMetadata() (string, error) {
 	result, err := metadata.Get("instance/hostname")
 	if err != nil {
 		return "", err
@@ -113,15 +112,15 @@ func getInstanceID() (string, error) {
 	return parts[0], nil
 }
 
-func getCurrentExternalID() (string, error) {
+func getCurrentExternalIDViaMetadata() (string, error) {
 	externalID, err := metadata.Get("instance/id")
 	if err != nil {
 		return "", fmt.Errorf("couldn't get external ID: %v", err)
 	}
 	return externalID, nil
 }
 
-func getNetworkName() (string, error) {
+func getNetworkNameViaMetadata() (string, error) {
 	result, err := metadata.Get("instance/network-interfaces/0/network")
 	if err != nil {
 		return "", err
@@ -133,28 +132,32 @@ func getNetworkName() (string, error) {
 	return parts[3], nil
 }
 
-// newGCECloud creates a new instance of GCECloud.
-func newGCECloud(config io.Reader) (*GCECloud, error) {
-	projectID, zone, err := getProjectAndZone()
+func getNetworkNameViaAPICall(svc *compute.Service, projectID string) (string, error) {
+	networkList, err := svc.Networks.List(projectID).Do()
 	if err != nil {
-		return nil, err
+		return "", err
 	}
-	// TODO: if we want to use this on a machine that doesn't have the http://metadata server
-	// e.g. on a user's machine (not VM) somewhere, we need to have an alternative for
-	// instance id lookup.
-	instanceID, err := getInstanceID()
-	if err != nil {
-		return nil, err
+
+	if networkList == nil || len(networkList.Items) <= 0 {
+		return "", fmt.Errorf("GCE Network List call returned no networks for project %q.", projectID)
 	}
-	externalID, err := getCurrentExternalID()
+
+	return networkList.Items[0].Name, nil
+}
+
+// newGCECloud creates a new instance of GCECloud.
+func newGCECloud(config io.Reader) (*GCECloud, error) {
+	projectID, zone, err := getProjectAndZone()
 	if err != nil {
 		return nil, err
 	}
-	networkName, err := getNetworkName()
+
+	networkName, err := getNetworkNameViaMetadata()
 	if err != nil {
 		return nil, err
 	}
 	networkURL := gceNetworkURL(projectID, networkName)
+
 	tokenSource := google.ComputeTokenSource("")
 	if config != nil {
 		var cfg Config
@@ -176,23 +179,54 @@ func newGCECloud(config io.Reader) (*GCECloud, error) {
 			tokenSource = newAltTokenSource(cfg.Global.TokenURL, cfg.Global.TokenBody)
 		}
 	}
+
+	return CreateGCECloud(projectID, zone, networkURL, tokenSource, true /* useMetadataServer */)
+}
+
+// Creates a GCECloud object using the specified parameters.
+// If no networkUrl is specified, loads networkName via rest call.
+// If no tokenSource is specified, uses oauth2.DefaultTokenSource.
+func CreateGCECloud(projectID, zone, networkURL string, tokenSource oauth2.TokenSource, useMetadataServer bool) (*GCECloud, error) {
+	if tokenSource == nil {
+		var err error
+		tokenSource, err = google.DefaultTokenSource(
+			oauth2.NoContext,
+			compute.CloudPlatformScope,
+			compute.ComputeScope)
+		glog.Infof("Using DefaultTokenSource %#v", tokenSource)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		glog.Infof("Using existing Token Source %#v", tokenSource)
+	}
+
 	client := oauth2.NewClient(oauth2.NoContext, tokenSource)
 	svc, err := compute.New(client)
 	if err != nil {
 		return nil, err
 	}
+
 	containerSvc, err := container.New(client)
 	if err != nil {
 		return nil, err
 	}
+
+	if networkURL == "" {
+		networkName, err := getNetworkNameViaAPICall(svc, projectID)
+		if err != nil {
+			return nil, err
+		}
+		networkURL = gceNetworkURL(projectID, networkName)
+	}
+
 	return &GCECloud{
-		service:          svc,
-		containerService: containerSvc,
-		projectID:        projectID,
-		zone:             zone,
-		instanceID:       instanceID,
-		externalID:       externalID,
-		networkURL:       networkURL,
+		service:           svc,
+		containerService:  containerSvc,
+		projectID:         projectID,
+		zone:              zone,
+		networkURL:        networkURL,
+		useMetadataServer: useMetadataServer,
 	}, nil
 }
 
@@ -1368,16 +1402,31 @@ func (gce *GCECloud) NodeAddresses(_ string) ([]api.NodeAddress, error) {
 	}, nil
 }
 
-func (gce *GCECloud) isCurrentInstance(instance string) bool {
-	return gce.instanceID == canonicalizeInstanceName(instance)
+// isCurrentInstance uses metadata server to check if specified instanceID matches current machine's instanceID
+func (gce *GCECloud) isCurrentInstance(instanceID string) bool {
+	currentInstanceID, err := getInstanceIDViaMetadata()
+	if err != nil {
+		// Log and swallow error
+		glog.Errorf("Failed to fetch instanceID via Metadata: %v", err)
+		return false
+	}
+
+	return currentInstanceID == canonicalizeInstanceName(instanceID)
 }
 
 // ExternalID returns the cloud provider ID of the specified instance (deprecated).
 func (gce *GCECloud) ExternalID(instance string) (string, error) {
-	// if we are asking about the current instance, just go to metadata
-	if gce.isCurrentInstance(instance) {
-		return gce.externalID, nil
+	if gce.useMetadataServer {
+		// Use metadata, if possible, to fetch ID. See issue #12000
+		if gce.isCurrentInstance(instance) {
+			externalInstanceID, err := getCurrentExternalIDViaMetadata()
+			if err == nil {
+				return externalInstanceID, nil
+			}
+		}
 	}
+
+	// Fallback to GCE API call if metadata server fails to retrieve ID
 	inst, err := gce.getInstanceByName(instance)
 	if err != nil {
 		return "", err
@@ -1494,7 +1543,29 @@ func (gce *GCECloud) GetZone() (cloudprovider.Zone, error) {
 	}, nil
 }
 
-func (gce *GCECloud) AttachDisk(diskName string, readOnly bool) error {
+func (gce *GCECloud) CreateDisk(name string, sizeGb int64) error {
+	diskToCreate := &compute.Disk{
+		Name:   name,
+		SizeGb: sizeGb,
+	}
+	createOp, err := gce.service.Disks.Insert(gce.projectID, gce.zone, diskToCreate).Do()
+	if err != nil {
+		return err
+	}
+
+	return gce.waitForZoneOp(createOp)
+}
+
+func (gce *GCECloud) DeleteDisk(diskToDelete string) error {
+	deleteOp, err := gce.service.Disks.Delete(gce.projectID, gce.zone, diskToDelete).Do()
+	if err != nil {
+		return err
+	}
+
+	return gce.waitForZoneOp(deleteOp)
+}
+
+func (gce *GCECloud) AttachDisk(diskName, instanceID string, readOnly bool) error {
 	disk, err := gce.getDisk(diskName)
 	if err != nil {
 		return err
@@ -1505,23 +1576,39 @@ func (gce *GCECloud) AttachDisk(diskName string, readOnly bool) error {
 	}
 	attachedDisk := gce.convertDiskToAttachedDisk(disk, readWrite)
 
-	attachOp, err := gce.service.Instances.AttachDisk(gce.projectID, gce.zone, gce.instanceID, attachedDisk).Do()
+	attachOp, err := gce.service.Instances.AttachDisk(gce.projectID, gce.zone, instanceID, attachedDisk).Do()
 	if err != nil {
 		return err
 	}
 
 	return gce.waitForZoneOp(attachOp)
 }
 
-func (gce *GCECloud) DetachDisk(devicePath string) error {
-	detachOp, err := gce.service.Instances.DetachDisk(gce.projectID, gce.zone, gce.instanceID, devicePath).Do()
+func (gce *GCECloud) DetachDisk(devicePath, instanceID string) error {
+	detachOp, err := gce.service.Instances.DetachDisk(gce.projectID, gce.zone, instanceID, devicePath).Do()
 	if err != nil {
 		return err
 	}
 
 	return gce.waitForZoneOp(detachOp)
 }
 
+func (gce *GCECloud) DiskIsAttached(diskName, instanceID string) (bool, error) {
+	instance, err := gce.service.Instances.Get(gce.projectID, gce.zone, instanceID).Do()
+	if err != nil {
+		return false, err
+	}
+
+	for _, disk := range instance.Disks {
+		if disk.DeviceName == diskName {
+			// Disk is still attached to node
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
 func (gce *GCECloud) getDisk(diskName string) (*compute.Disk, error) {
 	return gce.service.Disks.Get(gce.projectID, gce.zone, diskName).Do()
 }

diff --git a/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go b/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go
@@ -294,3 +294,7 @@ func (f *PersistentVolumeRecycler) GetMounter() mount.Interface {
 func (f *PersistentVolumeRecycler) GetWriter() ioutil.Writer {
 	return nil
 }
+
+func (f *PersistentVolumeRecycler) GetHostName() string {
+	return ""
+}
diff --git a/pkg/kubelet/volumes.go b/pkg/kubelet/volumes.go
@@ -93,6 +93,11 @@ func (vh *volumeHost) GetWriter() io.Writer {
 	return vh.kubelet.writer
 }
 
+// Returns the hostname of the host kubelet is running on
+func (vh *volumeHost) GetHostName() string {
+	return vh.kubelet.hostname
+}
+
 func (kl *Kubelet) newVolumeBuilderFromPlugins(spec *volume.Spec, pod *api.Pod, opts volume.VolumeOptions) (volume.Builder, error) {
 	plugin, err := kl.volumePluginMgr.FindPluginBySpec(spec)
 	if err != nil {

diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go
@@ -132,7 +132,7 @@ func attachDiskAndVerify(b *gcePersistentDiskBuilder, sdBeforeSet sets.String) (
 			glog.Warningf("Retrying attach for GCE PD %q (retry count=%v).", b.pdName, numRetries)
 		}
 
-		if err := gceCloud.AttachDisk(b.pdName, b.readOnly); err != nil {
+		if err := gceCloud.AttachDisk(b.pdName, b.plugin.host.GetHostName(), b.readOnly); err != nil {
 			glog.Errorf("Error attaching PD %q: %v", b.pdName, err)
 			time.Sleep(errorSleepDuration)
 			continue
@@ -206,7 +206,7 @@ func detachDiskAndVerify(c *gcePersistentDiskCleaner) {
 			glog.Warningf("Retrying detach for GCE PD %q (retry count=%v).", c.pdName, numRetries)
 		}
 
-		if err := gceCloud.DetachDisk(c.pdName); err != nil {
+		if err := gceCloud.DetachDisk(c.pdName, c.plugin.host.GetHostName()); err != nil {
 			glog.Errorf("Error detaching PD %q: %v", c.pdName, err)
 			time.Sleep(errorSleepDuration)
 			continue

diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go
@@ -156,6 +156,9 @@ type VolumeHost interface {
 
 	// Get writer interface for writing data to disk.
 	GetWriter() io.Writer
+
+	// Returns the hostname of the host kubelet is running on
+	GetHostName() string
 }
 
 // VolumePluginMgr tracks registered plugins.

diff --git a/pkg/volume/testing.go b/pkg/volume/testing.go
@@ -92,6 +92,11 @@ func (f *fakeVolumeHost) NewWrapperCleaner(spec *Spec, podUID types.UID) (Cleane
 	return plug.NewCleaner(spec.Name(), podUID)
 }
 
+// Returns the hostname of the host kubelet is running on
+func (f *fakeVolumeHost) GetHostName() string {
+	return "fakeHostName"
+}
+
 func ProbeVolumePlugins(config VolumeConfig) []VolumePlugin {
 	if _, ok := config.OtherAttributes["fake-property"]; ok {
 		return []VolumePlugin{

diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
@@ -30,9 +30,12 @@ import (
 	"github.com/onsi/ginkgo/config"
 	"github.com/onsi/ginkgo/reporters"
 	"github.com/onsi/gomega"
+	"golang.org/x/oauth2"
+	"golang.org/x/oauth2/google"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	"k8s.io/kubernetes/pkg/cloudprovider"
+	gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
 	"k8s.io/kubernetes/pkg/util"
 )
 
@@ -73,6 +76,7 @@ func init() {
 	flag.StringVar(&cloudConfig.MasterName, "kube-master", "", "Name of the kubernetes master. Only required if provider is gce or gke")
 	flag.StringVar(&cloudConfig.ProjectID, "gce-project", "", "The GCE project being used, if applicable")
 	flag.StringVar(&cloudConfig.Zone, "gce-zone", "", "GCE zone being used, if applicable")
+	flag.StringVar(&cloudConfig.ServiceAccount, "gce-service-account", "", "GCE service account to use for GCE API calls, if applicable")
 	flag.StringVar(&cloudConfig.Cluster, "gke-cluster", "", "GKE name of cluster being used, if applicable")
 	flag.StringVar(&cloudConfig.NodeInstanceGroup, "node-instance-group", "", "Name of the managed instance group for nodes. Valid only for gce, gke or aws")
 	flag.IntVar(&cloudConfig.NumNodes, "num-nodes", -1, "Number of nodes in the cluster")
@@ -102,6 +106,23 @@ func TestE2E(t *testing.T) {
 		glog.Info("The --provider flag is not set.  Treating as a conformance test.  Some tests may not be run.")
 	}
 
+	if testContext.Provider == "gce" || testContext.Provider == "gke" {
+		var err error
+		Logf("Fetching cloud provider for %q\r\n", testContext.Provider)
+		var tokenSource oauth2.TokenSource
+		tokenSource = nil
+		if cloudConfig.ServiceAccount != "" {
+			// Use specified service account for auth
+			Logf("Using service account %q as token source.", cloudConfig.ServiceAccount)
+			tokenSource = google.ComputeTokenSource(cloudConfig.ServiceAccount)
+		}
+		cloudConfig.Provider, err = gcecloud.CreateGCECloud(testContext.CloudConfig.ProjectID, testContext.CloudConfig.Zone, "" /* networkUrl */, tokenSource, false /* useMetadataServer */)
+		if err != nil {
+			glog.Fatal("Error building GCE provider: ", err)
+		}
+
+	}
+
 	if testContext.Provider == "aws" {
 		awsConfig := "[Global]\n"
 		if cloudConfig.Zone == "" {