From d0e4271dfb8c4d608b97581781f8a2c36f5ba5a1 Mon Sep 17 00:00:00 2001 From: saadali Date: Wed, 30 Aug 2017 17:48:50 -0700 Subject: [PATCH] GCE Cloud provider changes to enable RePD GCE cloud provider changes for enabling GCE Regional PDs. --- pkg/cloudprovider/providers/gce/BUILD | 1 + pkg/cloudprovider/providers/gce/gce.go | 363 +++++++++++++++-- pkg/cloudprovider/providers/gce/gce_disks.go | 323 +++++++++++---- .../providers/gce/gce_disks_test.go | 375 ++++++++++++++---- 4 files changed, 871 insertions(+), 191 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/BUILD b/pkg/cloudprovider/providers/gce/BUILD index 65f7a5e7d62..32cd13fd402 100644 --- a/pkg/cloudprovider/providers/gce/BUILD +++ b/pkg/cloudprovider/providers/gce/BUILD @@ -52,6 +52,7 @@ go_library( "//pkg/util/net/sets:go_default_library", "//pkg/util/version:go_default_library", "//pkg/volume:go_default_library", + "//pkg/volume/util:go_default_library", "//vendor/cloud.google.com/go/compute/metadata:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index b28f61c6bcc..2445d02170c 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -80,7 +80,8 @@ const ( // Defaults to 5 * 2 = 10 seconds before the LB will steer traffic away gceHcUnhealthyThreshold = int64(5) - gceComputeAPIEndpoint = "https://www.googleapis.com/compute/v1/" + gceComputeAPIEndpoint = "https://www.googleapis.com/compute/v1/" + gceComputeAPIEndpointAlpha = "https://www.googleapis.com/compute/alpha/" ) // gceObject is an abstraction of all GCE API object in go client @@ -135,16 +136,28 @@ type ServiceManager interface { name string, sizeGb int64, tagsStr string, - diskTypeURI string, + diskType string, zone string) (gceObject, error) - // Attach a persistent disk on GCE with the given disk spec to the specified instance. - AttachDisk(diskName string, - diskKind string, - diskZone string, - readWrite string, - source string, + // Creates a new regional persistent disk on GCE with the given disk spec. + CreateRegionalDisk( + name string, + sizeGb int64, + tagsStr string, diskType string, + zones sets.String) (gceObject, error) + + // Deletes the persistent disk from GCE with the given diskName. + DeleteDisk(zone string, disk string) (gceObject, error) + + // Deletes the regional persistent disk from GCE with the given diskName. + DeleteRegionalDisk(diskName string) (gceObject, error) + + // Attach a persistent disk on GCE with the given disk spec to the specified instance. + AttachDisk( + disk *GCEDisk, + readWrite string, + instanceZone string, instanceName string) (gceObject, error) // Detach a persistent disk on GCE with the given disk spec from the specified instance. @@ -154,13 +167,16 @@ type ServiceManager interface { devicePath string) (gceObject, error) // Gets the persistent disk from GCE with the given diskName. - GetDisk(project string, zone string, diskName string) (*GCEDisk, error) + GetDisk(zone string, diskName string) (*GCEDisk, error) - // Deletes the persistent disk from GCE with the given diskName. - DeleteDisk(project string, zone string, disk string) (gceObject, error) + // Gets the regional persistent disk from GCE with the given diskName. + GetRegionalDisk(diskName string) (*GCEDisk, error) // Waits until GCE reports the given operation in the given zone as done. WaitForZoneOp(op gceObject, zone string, mc *metricContext) error + + // Waits until GCE reports the given operation in the given region is done. + WaitForRegionalOp(op gceObject, mc *metricContext) error } type GCEServiceManager struct { @@ -740,8 +756,14 @@ func (manager *GCEServiceManager) CreateDisk( name string, sizeGb int64, tagsStr string, - diskTypeURI string, + diskType string, zone string) (gceObject, error) { + diskTypeURI, err := manager.getDiskTypeURI( + manager.gce.region /* diskRegion */, singleZone{zone}, diskType) + if err != nil { + return nil, err + } + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { diskToCreateAlpha := &computealpha.Disk{ Name: name, @@ -749,7 +771,9 @@ func (manager *GCEServiceManager) CreateDisk( Description: tagsStr, Type: diskTypeURI, } - return manager.gce.serviceAlpha.Disks.Insert(manager.gce.projectID, zone, diskToCreateAlpha).Do() + + return manager.gce.serviceAlpha.Disks.Insert( + manager.gce.projectID, zone, diskToCreateAlpha).Do() } diskToCreateV1 := &compute.Disk{ @@ -758,38 +782,72 @@ func (manager *GCEServiceManager) CreateDisk( Description: tagsStr, Type: diskTypeURI, } - return manager.gce.service.Disks.Insert(manager.gce.projectID, zone, diskToCreateV1).Do() + return manager.gce.service.Disks.Insert( + manager.gce.projectID, zone, diskToCreateV1).Do() +} + +func (manager *GCEServiceManager) CreateRegionalDisk( + name string, + sizeGb int64, + tagsStr string, + diskType string, + replicaZones sets.String) (gceObject, error) { + diskTypeURI, err := manager.getDiskTypeURI( + manager.gce.region /* diskRegion */, multiZone{replicaZones}, diskType) + if err != nil { + return nil, err + } + fullyQualifiedReplicaZones := []string{} + for _, replicaZone := range replicaZones.UnsortedList() { + fullyQualifiedReplicaZones = append( + fullyQualifiedReplicaZones, manager.getReplicaZoneURI(replicaZone)) + } + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + diskToCreateAlpha := &computealpha.Disk{ + Name: name, + SizeGb: sizeGb, + Description: tagsStr, + Type: diskTypeURI, + ReplicaZones: fullyQualifiedReplicaZones, + } + return manager.gce.serviceAlpha.RegionDisks.Insert( + manager.gce.projectID, manager.gce.region, diskToCreateAlpha).Do() + } + + return nil, fmt.Errorf("The regional PD feature is only available via the GCE Alpha API. Enable \"GCEDiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.") } func (manager *GCEServiceManager) AttachDisk( - diskName string, - diskKind string, - diskZone string, + disk *GCEDisk, readWrite string, - source string, - diskType string, + instanceZone string, instanceName string) (gceObject, error) { + source, err := manager.getDiskSourceURI(disk) + if err != nil { + return nil, err + } + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { attachedDiskAlpha := &computealpha.AttachedDisk{ - DeviceName: diskName, - Kind: diskKind, + DeviceName: disk.Name, + Kind: disk.Kind, Mode: readWrite, Source: source, - Type: diskType, + Type: diskTypePersistent, } return manager.gce.serviceAlpha.Instances.AttachDisk( - manager.gce.projectID, diskZone, instanceName, attachedDiskAlpha).Do() + manager.gce.projectID, instanceZone, instanceName, attachedDiskAlpha).Do() } attachedDiskV1 := &compute.AttachedDisk{ - DeviceName: diskName, - Kind: diskKind, + DeviceName: disk.Name, + Kind: disk.Kind, Mode: readWrite, Source: source, - Type: diskType, + Type: disk.Type, } return manager.gce.service.Instances.AttachDisk( - manager.gce.projectID, diskZone, instanceName, attachedDiskV1).Do() + manager.gce.projectID, instanceZone, instanceName, attachedDiskV1).Do() } func (manager *GCEServiceManager) DetachDisk( @@ -806,49 +864,270 @@ func (manager *GCEServiceManager) DetachDisk( } func (manager *GCEServiceManager) GetDisk( - project string, zone string, diskName string) (*GCEDisk, error) { + if zone == "" { + return nil, fmt.Errorf("Can not fetch disk %q. Zone is empty.", diskName) + } + + if diskName == "" { + return nil, fmt.Errorf("Can not fetch disk. Zone is specified (%q). But disk name is empty.", zone) + } if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { - diskAlpha, err := manager.gce.serviceAlpha.Disks.Get(project, zone, diskName).Do() + diskAlpha, err := manager.gce.serviceAlpha.Disks.Get( + manager.gce.projectID, zone, diskName).Do() if err != nil { return nil, err } + var zoneInfo zoneType + if len(diskAlpha.ReplicaZones) > 1 { + zones := sets.NewString() + for _, zoneURI := range diskAlpha.ReplicaZones { + zones.Insert(lastComponent(zoneURI)) + } + zoneInfo = multiZone{zones} + } else { + zoneInfo = singleZone{lastComponent(diskAlpha.Zone)} + if diskAlpha.Zone == "" { + zoneInfo = singleZone{lastComponent(zone)} + } + } + + region := strings.TrimSpace(lastComponent(diskAlpha.Region)) + if region == "" { + region, err = manager.getRegionFromZone(zoneInfo) + if err != nil { + return nil, fmt.Errorf("failed to extract region from zone for %q/%q err=%v", zone, diskName, err) + } + } + return &GCEDisk{ - Zone: lastComponent(diskAlpha.Zone), - Name: diskAlpha.Name, - Kind: diskAlpha.Kind, - Type: diskAlpha.Type, + ZoneInfo: zoneInfo, + Region: region, + Name: diskAlpha.Name, + Kind: diskAlpha.Kind, + Type: diskAlpha.Type, }, nil } - diskStable, err := manager.gce.service.Disks.Get(project, zone, diskName).Do() + diskStable, err := manager.gce.service.Disks.Get( + manager.gce.projectID, zone, diskName).Do() if err != nil { return nil, err } + zoneInfo := singleZone{strings.TrimSpace(lastComponent(diskStable.Zone))} + if zoneInfo.zone == "" { + zoneInfo = singleZone{zone} + } + + region, err := manager.getRegionFromZone(zoneInfo) + if err != nil { + return nil, fmt.Errorf("failed to extract region from zone for %q/%q err=%v", zone, diskName, err) + } + return &GCEDisk{ - Zone: lastComponent(diskStable.Zone), - Name: diskStable.Name, - Kind: diskStable.Kind, - Type: diskStable.Type, + ZoneInfo: zoneInfo, + Region: region, + Name: diskStable.Name, + Kind: diskStable.Kind, + Type: diskStable.Type, }, nil } +func (manager *GCEServiceManager) GetRegionalDisk( + diskName string) (*GCEDisk, error) { + + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + diskAlpha, err := manager.gce.serviceAlpha.RegionDisks.Get( + manager.gce.projectID, manager.gce.region, diskName).Do() + if err != nil { + return nil, err + } + + zones := sets.NewString() + for _, zoneURI := range diskAlpha.ReplicaZones { + zones.Insert(lastComponent(zoneURI)) + } + + return &GCEDisk{ + ZoneInfo: multiZone{zones}, + Region: lastComponent(diskAlpha.Region), + Name: diskAlpha.Name, + Kind: diskAlpha.Kind, + Type: diskAlpha.Type, + }, nil + } + + return nil, fmt.Errorf("The regional PD feature is only available via the GCE Alpha API. Enable \"GCEDiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.") +} + func (manager *GCEServiceManager) DeleteDisk( - project string, zone string, diskName string) (gceObject, error) { if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { - return manager.gce.serviceAlpha.Disks.Delete(project, zone, diskName).Do() + return manager.gce.serviceAlpha.Disks.Delete( + manager.gce.projectID, zone, diskName).Do() } - return manager.gce.service.Disks.Delete(project, zone, diskName).Do() + return manager.gce.service.Disks.Delete( + manager.gce.projectID, zone, diskName).Do() } -func (manager *GCEServiceManager) WaitForZoneOp(op gceObject, zone string, mc *metricContext) error { +func (manager *GCEServiceManager) DeleteRegionalDisk( + diskName string) (gceObject, error) { + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + return manager.gce.serviceAlpha.RegionDisks.Delete( + manager.gce.projectID, manager.gce.region, diskName).Do() + } + + return nil, fmt.Errorf("DeleteRegionalDisk is a regional PD feature and is only available via the GCE Alpha API. Enable \"GCEDiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.") +} + +func (manager *GCEServiceManager) WaitForZoneOp( + op gceObject, zone string, mc *metricContext) error { return manager.gce.waitForZoneOp(op, zone, mc) } + +func (manager *GCEServiceManager) WaitForRegionalOp( + op gceObject, mc *metricContext) error { + return manager.gce.waitForRegionOp(op, manager.gce.region, mc) +} + +func (manager *GCEServiceManager) getDiskSourceURI(disk *GCEDisk) (string, error) { + getProjectsAPIEndpoint := manager.getProjectsAPIEndpoint() + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + getProjectsAPIEndpoint = manager.getProjectsAPIEndpointAlpha() + } + + switch zoneInfo := disk.ZoneInfo.(type) { + case singleZone: + if zoneInfo.zone == "" || disk.Region == "" { + // Unexpected, but sanity-check + return "", fmt.Errorf("PD does not have zone/region information: %#v", disk) + } + + return getProjectsAPIEndpoint + fmt.Sprintf( + diskSourceURITemplateSingleZone, + manager.gce.projectID, + zoneInfo.zone, + disk.Name), nil + case multiZone: + if zoneInfo.replicaZones == nil || zoneInfo.replicaZones.Len() <= 0 { + // Unexpected, but sanity-check + return "", fmt.Errorf("PD is regional but does not have any replicaZones specified: %v", disk) + } + return getProjectsAPIEndpoint + fmt.Sprintf( + diskSourceURITemplateRegional, + manager.gce.projectID, + disk.Region, + disk.Name), nil + case nil: + // Unexpected, but sanity-check + return "", fmt.Errorf("PD did not have ZoneInfo: %v", disk) + default: + // Unexpected, but sanity-check + return "", fmt.Errorf("disk.ZoneInfo has unexpected type %T", zoneInfo) + } +} + +func (manager *GCEServiceManager) getDiskTypeURI( + diskRegion string, diskZoneInfo zoneType, diskType string) (string, error) { + getProjectsAPIEndpoint := manager.getProjectsAPIEndpoint() + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + getProjectsAPIEndpoint = manager.getProjectsAPIEndpointAlpha() + } + + switch zoneInfo := diskZoneInfo.(type) { + case singleZone: + if zoneInfo.zone == "" { + return "", fmt.Errorf("zone is empty: %v", zoneInfo) + } + + return getProjectsAPIEndpoint + fmt.Sprintf( + diskTypeURITemplateSingleZone, + manager.gce.projectID, + zoneInfo.zone, + diskType), nil + case multiZone: + if zoneInfo.replicaZones == nil || zoneInfo.replicaZones.Len() <= 0 { + return "", fmt.Errorf("zoneInfo is regional but does not have any replicaZones specified: %v", zoneInfo) + } + return getProjectsAPIEndpoint + fmt.Sprintf( + diskTypeURITemplateRegional, + manager.gce.projectID, + diskRegion, + diskType), nil + case nil: + return "", fmt.Errorf("zoneInfo nil") + default: + return "", fmt.Errorf("zoneInfo has unexpected type %T", zoneInfo) + } +} + +func (manager *GCEServiceManager) getReplicaZoneURI(zone string) string { + getProjectsAPIEndpoint := manager.getProjectsAPIEndpoint() + if manager.gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + getProjectsAPIEndpoint = manager.getProjectsAPIEndpointAlpha() + } + + return getProjectsAPIEndpoint + fmt.Sprintf( + replicaZoneURITemplateSingleZone, + manager.gce.projectID, + zone) +} + +func (manager *GCEServiceManager) getProjectsAPIEndpoint() string { + projectsApiEndpoint := gceComputeAPIEndpoint + "projects/" + if manager.gce.service != nil { + projectsApiEndpoint = manager.gce.service.BasePath + } + + return projectsApiEndpoint +} + +func (manager *GCEServiceManager) getProjectsAPIEndpointAlpha() string { + projectsApiEndpoint := gceComputeAPIEndpointAlpha + "projects/" + if manager.gce.service != nil { + projectsApiEndpoint = manager.gce.serviceAlpha.BasePath + } + + return projectsApiEndpoint +} + +func (manager *GCEServiceManager) getRegionFromZone(zoneInfo zoneType) (string, error) { + var zone string + switch zoneInfo := zoneInfo.(type) { + case singleZone: + if zoneInfo.zone == "" { + // Unexpected, but sanity-check + return "", fmt.Errorf("PD is single zone, but zone is not specified: %#v", zoneInfo) + } + + zone = zoneInfo.zone + case multiZone: + if zoneInfo.replicaZones == nil || zoneInfo.replicaZones.Len() <= 0 { + // Unexpected, but sanity-check + return "", fmt.Errorf("PD is regional but does not have any replicaZones specified: %v", zoneInfo) + } + + zone = zoneInfo.replicaZones.UnsortedList()[0] + case nil: + // Unexpected, but sanity-check + return "", fmt.Errorf("zoneInfo is nil") + default: + // Unexpected, but sanity-check + return "", fmt.Errorf("zoneInfo has unexpected type %T", zoneInfo) + } + + region, err := GetGCERegion(zone) + if err != nil { + glog.Warningf("failed to parse GCE region from zone %q: %v", zone, err) + region = manager.gce.region + } + + return region, nil +} diff --git a/pkg/cloudprovider/providers/gce/gce_disks.go b/pkg/cloudprovider/providers/gce/gce_disks.go index bdc126c3048..997b65d624c 100644 --- a/pkg/cloudprovider/providers/gce/gce_disks.go +++ b/pkg/cloudprovider/providers/gce/gce_disks.go @@ -23,9 +23,11 @@ import ( "strings" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/cloudprovider" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" "k8s.io/kubernetes/pkg/volume" + volumeutil "k8s.io/kubernetes/pkg/volume/util" "github.com/golang/glog" "google.golang.org/api/googleapi" @@ -37,9 +39,15 @@ const ( DiskTypeSSD = "pd-ssd" DiskTypeStandard = "pd-standard" - diskTypeDefault = DiskTypeStandard - diskTypeUriTemplate = "%s/zones/%s/diskTypes/%s" - diskTypePersistent = "PERSISTENT" + diskTypeDefault = DiskTypeStandard + diskTypeURITemplateSingleZone = "%s/zones/%s/diskTypes/%s" // {gce.projectID}/zones/{disk.Zone}/diskTypes/{disk.Type}" + diskTypeURITemplateRegional = "%s/regions/%s/diskTypes/%s" // {gce.projectID}/regions/{disk.Region}/diskTypes/{disk.Type}" + diskTypePersistent = "PERSISTENT" + + diskSourceURITemplateSingleZone = "%s/zones/%s/disks/%s" // {gce.projectID}/zones/{disk.Zone}/disks/{disk.Name}" + diskSourceURITemplateRegional = "%s/regions/%s/disks/%s" //{gce.projectID}/regions/{disk.Region}/disks/repd" + + replicaZoneURITemplateSingleZone = "%s/zones/%s" // {gce.projectID}/zones/{disk.Zone} ) // Disks is interface for manipulation with GCE PDs. @@ -63,6 +71,11 @@ type Disks interface { // as JSON into Description field. CreateDisk(name string, diskType string, zone string, sizeGb int64, tags map[string]string) error + // CreateRegionalDisk creates a new Regional Persistent Disk, with the + // specified properties, replicated to the specified zones. Tags are + // serialized as JSON into Description field. + CreateRegionalDisk(name string, diskType string, replicaZones sets.String, sizeGb int64, tags map[string]string) error + // DeleteDisk deletes PD. DeleteDisk(diskToDelete string) error @@ -77,14 +90,34 @@ type Disks interface { var _ Disks = (*GCECloud)(nil) type GCEDisk struct { - Zone string - Name string - Kind string - Type string + ZoneInfo zoneType + Region string + Name string + Kind string + Type string } -func newDiskMetricContext(request, zone string) *metricContext { - return newGenericMetricContext("disk", request, unusedMetricLabel, zone, computeV1Version) +type zoneType interface { + isZoneType() +} + +type multiZone struct { + replicaZones sets.String +} + +type singleZone struct { + zone string +} + +func (m multiZone) isZoneType() {} +func (s singleZone) isZoneType() {} + +func newDiskMetricContextZonal(request, region, zone string) *metricContext { + return newGenericMetricContext("disk", request, region, zone, computeV1Version) +} + +func newDiskMetricContextRegional(request, region string) *metricContext { + return newGenericMetricContext("disk", request, region, unusedMetricLabel, computeV1Version) } func (gce *GCECloud) AttachDisk(diskName string, nodeName types.NodeName, readOnly bool) error { @@ -93,26 +126,41 @@ func (gce *GCECloud) AttachDisk(diskName string, nodeName types.NodeName, readOn if err != nil { return fmt.Errorf("error getting instance %q", instanceName) } - disk, err := gce.getDiskByName(diskName, instance.Zone) - if err != nil { - return err + + // Try fetching as regional PD + var disk *GCEDisk + var mc *metricContext + if gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + disk, err = gce.getRegionalDiskByName(diskName) + if err != nil { + glog.V(5).Infof("Could not find regional PD named %q to Attach. Will look for a zonal PD", diskName) + err = nil + } else { + mc = newDiskMetricContextRegional("attach", gce.region) + } } + + if disk == nil { + disk, err = gce.getDiskByName(diskName, instance.Zone) + if err != nil { + return err + } + mc = newDiskMetricContextZonal("attach", gce.region, instance.Zone) + } + readWrite := "READ_WRITE" if readOnly { readWrite = "READ_ONLY" } - mc := newDiskMetricContext("attach", instance.Zone) - source := gce.service.BasePath + strings.Join([]string{ - gce.projectID, "zones", disk.Zone, "disks", disk.Name}, "/") attachOp, err := gce.manager.AttachDisk( - disk.Name, disk.Kind, disk.Zone, readWrite, source, diskTypePersistent, instance.Name) + disk, readWrite, instance.Zone, instance.Name) if err != nil { return mc.Observe(err) } - return gce.manager.WaitForZoneOp(attachOp, disk.Zone, mc) + return gce.manager.WaitForZoneOp(attachOp, instance.Zone, mc) } func (gce *GCECloud) DetachDisk(devicePath string, nodeName types.NodeName) error { @@ -131,7 +179,7 @@ func (gce *GCECloud) DetachDisk(devicePath string, nodeName types.NodeName) erro return fmt.Errorf("error getting instance %q", instanceName) } - mc := newDiskMetricContext("detach", inst.Zone) + mc := newDiskMetricContextZonal("detach", gce.region, inst.Zone) detachOp, err := gce.manager.DetachDisk(inst.Zone, inst.Name, devicePath) if err != nil { return mc.Observe(err) @@ -206,14 +254,7 @@ func (gce *GCECloud) CreateDisk( // Do not allow creation of PDs in zones that are not managed. Such PDs // then cannot be deleted by DeleteDisk. - isManaged := false - for _, managedZone := range gce.managedZones { - if zone == managedZone { - isManaged = true - break - } - } - if !isManaged { + if isManaged := gce.verifyZoneIsManaged(zone); !isManaged { return fmt.Errorf("kubernetes does not manage zone %q", zone) } @@ -222,25 +263,15 @@ func (gce *GCECloud) CreateDisk( return err } - switch diskType { - case DiskTypeSSD, DiskTypeStandard: - // noop - case "": - diskType = diskTypeDefault - default: - return fmt.Errorf("invalid GCE disk type %q", diskType) + diskType, err = getDiskType(diskType) + if err != nil { + return err } - projectsApiEndpoint := gceComputeAPIEndpoint + "projects/" - if gce.service != nil { - projectsApiEndpoint = gce.service.BasePath - } - diskTypeUri := projectsApiEndpoint + fmt.Sprintf(diskTypeUriTemplate, gce.projectID, zone, diskType) - - mc := newDiskMetricContext("create", zone) + mc := newDiskMetricContextZonal("create", gce.region, zone) createOp, err := gce.manager.CreateDisk( - name, sizeGb, tagsStr, diskTypeUri, zone) + name, sizeGb, tagsStr, diskType, zone) if isGCEError(err, "alreadyExists") { glog.Warningf("GCE PD %q already exists, reusing", name) @@ -257,6 +288,76 @@ func (gce *GCECloud) CreateDisk( return err } +// CreateRegionalDisk creates a new Regional Persistent Disk, with the specified +// name & size, replicated to the specified zones. It stores specified tags +// encoded in JSON in Description field. +func (gce *GCECloud) CreateRegionalDisk( + name string, diskType string, replicaZones sets.String, sizeGb int64, tags map[string]string) error { + + // Do not allow creation of PDs in zones that are not managed. Such PDs + // then cannot be deleted by DeleteDisk. + unmanagedZones := []string{} + for _, zone := range replicaZones.UnsortedList() { + if isManaged := gce.verifyZoneIsManaged(zone); !isManaged { + unmanagedZones = append(unmanagedZones, zone) + } + } + + if len(unmanagedZones) > 0 { + return fmt.Errorf("kubernetes does not manage specified zones: %q. Managed Zones: %q", unmanagedZones, gce.managedZones) + } + + tagsStr, err := gce.encodeDiskTags(tags) + if err != nil { + return err + } + + diskType, err = getDiskType(diskType) + if err != nil { + return err + } + + mc := newDiskMetricContextRegional("create", gce.region) + + createOp, err := gce.manager.CreateRegionalDisk( + name, sizeGb, tagsStr, diskType, replicaZones) + + if isGCEError(err, "alreadyExists") { + glog.Warningf("GCE PD %q already exists, reusing", name) + return nil + } else if err != nil { + return mc.Observe(err) + } + + err = gce.manager.WaitForRegionalOp(createOp, mc) + if isGCEError(err, "alreadyExists") { + glog.Warningf("GCE PD %q already exists, reusing", name) + return nil + } + return err +} + +func (gce *GCECloud) verifyZoneIsManaged(zone string) bool { + for _, managedZone := range gce.managedZones { + if zone == managedZone { + return true + } + } + + return false +} + +func getDiskType(diskType string) (string, error) { + switch diskType { + case DiskTypeSSD, DiskTypeStandard: + return diskType, nil + case "": + return diskTypeDefault, nil + default: + return "", fmt.Errorf("invalid GCE disk type %q", diskType) + } +} + func (gce *GCECloud) DeleteDisk(diskToDelete string) error { err := gce.doDeleteDisk(diskToDelete) if isGCEError(err, "resourceInUseByAnotherResource") { @@ -278,40 +379,66 @@ func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]st var disk *GCEDisk var err error if zone == "" { - // We would like as far as possible to avoid this case, - // because GCE doesn't guarantee that volumes are uniquely named per region, - // just per zone. However, creation of GCE PDs was originally done only - // by name, so we have to continue to support that. - // However, wherever possible the zone should be passed (and it is passed - // for most cases that we can control, e.g. dynamic volume provisioning) + // For regional PDs this is fine, but for zonal PDs we would like as far + // as possible to avoid this case, because GCE doesn't guarantee that + // volumes are uniquely named per region, just per zone. However, + // creation of GCE PDs was originally done only by name, so we have to + // continue to support that. + // However, wherever possible the zone should be passed (and it is + // passed for most cases that we can control, e.g. dynamic volume + // provisioning). disk, err = gce.GetDiskByNameUnknownZone(name) if err != nil { return nil, err } - zone = disk.Zone } else { // We could assume the disks exists; we have all the information we need // However it is more consistent to ensure the disk exists, // and in future we may gather addition information (e.g. disk type, IOPS etc) - disk, err = gce.getDiskByName(name, zone) + zoneSet, err := volumeutil.LabelZonesToSet(zone) if err != nil { - return nil, err + glog.Warningf("Failed to parse zone field: %q. Will use raw field.", zone) + } + + if len(zoneSet) > 1 { + // Regional PD + disk, err = gce.getRegionalDiskByName(name) + if err != nil { + return nil, err + } + } else { + // Zonal PD + disk, err = gce.getDiskByName(name, zone) + if err != nil { + return nil, err + } } } - region, err := GetGCERegion(zone) - if err != nil { - return nil, err - } - - if zone == "" || region == "" { - // Unexpected, but sanity-check - return nil, fmt.Errorf("PD did not have zone/region information: %q", disk.Name) - } - labels := make(map[string]string) - labels[kubeletapis.LabelZoneFailureDomain] = zone - labels[kubeletapis.LabelZoneRegion] = region + switch zoneInfo := disk.ZoneInfo.(type) { + case singleZone: + if zoneInfo.zone == "" || disk.Region == "" { + // Unexpected, but sanity-check + return nil, fmt.Errorf("PD did not have zone/region information: %v", disk) + } + labels[kubeletapis.LabelZoneFailureDomain] = zoneInfo.zone + labels[kubeletapis.LabelZoneRegion] = disk.Region + case multiZone: + if zoneInfo.replicaZones == nil || zoneInfo.replicaZones.Len() <= 0 { + // Unexpected, but sanity-check + return nil, fmt.Errorf("PD is regional but does not have any replicaZones specified: %v", disk) + } + labels[kubeletapis.LabelZoneFailureDomain] = + volumeutil.ZonesSetToLabelValue(zoneInfo.replicaZones) + labels[kubeletapis.LabelZoneRegion] = disk.Region + case nil: + // Unexpected, but sanity-check + return nil, fmt.Errorf("PD did not have ZoneInfo: %v", disk) + default: + // Unexpected, but sanity-check + return nil, fmt.Errorf("disk.ZoneInfo has unexpected type %T", zoneInfo) + } return labels, nil } @@ -319,8 +446,8 @@ func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]st // Returns a GCEDisk for the disk, if it is found in the specified zone. // If not found, returns (nil, nil) func (gce *GCECloud) findDiskByName(diskName string, zone string) (*GCEDisk, error) { - mc := newDiskMetricContext("get", zone) - disk, err := gce.manager.GetDisk(gce.projectID, zone, diskName) + mc := newDiskMetricContextZonal("get", gce.region, zone) + disk, err := gce.manager.GetDisk(zone, diskName) if err == nil { return disk, mc.Observe(nil) } @@ -339,10 +466,40 @@ func (gce *GCECloud) getDiskByName(diskName string, zone string) (*GCEDisk, erro return disk, err } +// Returns a GCEDisk for the regional disk, if it is found. +// If not found, returns (nil, nil) +func (gce *GCECloud) findRegionalDiskByName(diskName string) (*GCEDisk, error) { + mc := newDiskMetricContextRegional("get", gce.region) + disk, err := gce.manager.GetRegionalDisk(diskName) + if err == nil { + return disk, mc.Observe(nil) + } + if !isHTTPErrorCode(err, http.StatusNotFound) { + return nil, mc.Observe(err) + } + return nil, mc.Observe(nil) +} + +// Like findRegionalDiskByName, but returns an error if the disk is not found +func (gce *GCECloud) getRegionalDiskByName(diskName string) (*GCEDisk, error) { + disk, err := gce.findRegionalDiskByName(diskName) + if disk == nil && err == nil { + return nil, fmt.Errorf("GCE regional persistent disk not found: diskName=%q", diskName) + } + return disk, err +} + // Scans all managed zones to return the GCE PD // Prefer getDiskByName, if the zone can be established // Return cloudprovider.DiskNotFound if the given disk cannot be found in any zone func (gce *GCECloud) GetDiskByNameUnknownZone(diskName string) (*GCEDisk, error) { + if gce.AlphaFeatureGate.Enabled(GCEDiskAlphaFeatureGate) { + regionalDisk, err := gce.getRegionalDiskByName(diskName) + if err == nil { + return regionalDisk, err + } + } + // Note: this is the gotcha right now with GCE PD support: // disk names are not unique per-region. // (I can create two volumes with name "myvol" in e.g. us-central1-b & us-central1-f) @@ -365,7 +522,17 @@ func (gce *GCECloud) GetDiskByNameUnknownZone(diskName string) (*GCEDisk, error) continue } if found != nil { - return nil, fmt.Errorf("GCE persistent disk name was found in multiple zones: %q", diskName) + switch zoneInfo := disk.ZoneInfo.(type) { + case multiZone: + if zoneInfo.replicaZones.Has(zone) { + glog.Warningf("GCE PD name (%q) was found in multiple zones (%q), but ok because it is a RegionalDisk.", + diskName, zoneInfo.replicaZones) + continue + } + return nil, fmt.Errorf("GCE PD name was found in multiple zones: %q", diskName) + default: + return nil, fmt.Errorf("GCE PD name was found in multiple zones: %q", diskName) + } } found = disk } @@ -399,14 +566,28 @@ func (gce *GCECloud) doDeleteDisk(diskToDelete string) error { return err } - mc := newDiskMetricContext("delete", disk.Zone) + var mc *metricContext - deleteOp, err := gce.manager.DeleteDisk(gce.projectID, disk.Zone, disk.Name) - if err != nil { - return mc.Observe(err) + switch zoneInfo := disk.ZoneInfo.(type) { + case singleZone: + mc = newDiskMetricContextZonal("delete", disk.Region, zoneInfo.zone) + deleteOp, err := gce.manager.DeleteDisk(zoneInfo.zone, disk.Name) + if err != nil { + return mc.Observe(err) + } + return gce.manager.WaitForZoneOp(deleteOp, zoneInfo.zone, mc) + case multiZone: + mc = newDiskMetricContextRegional("delete", disk.Region) + deleteOp, err := gce.manager.DeleteRegionalDisk(disk.Name) + if err != nil { + return mc.Observe(err) + } + return gce.manager.WaitForRegionalOp(deleteOp, mc) + case nil: + return fmt.Errorf("PD has nil ZoneInfo: %v", disk) + default: + return fmt.Errorf("disk.ZoneInfo has unexpected type %T", zoneInfo) } - - return gce.manager.WaitForZoneOp(deleteOp, disk.Zone, mc) } // isGCEError returns true if given error is a googleapi.Error with given diff --git a/pkg/cloudprovider/providers/gce/gce_disks_test.go b/pkg/cloudprovider/providers/gce/gce_disks_test.go index 8eb960a1f72..358df9f3898 100644 --- a/pkg/cloudprovider/providers/gce/gce_disks_test.go +++ b/pkg/cloudprovider/providers/gce/gce_disks_test.go @@ -25,14 +25,19 @@ import ( computebeta "google.golang.org/api/compute/v0.beta" compute "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/cloudprovider" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" ) +// TODO TODO write a test for GetDiskByNameUnknownZone and make sure casting logic works +// TODO TODO verify that RegionDisks.Get does not return non-replica disks + func TestCreateDisk_Basic(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() - projectId := "test-project" + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -40,7 +45,7 @@ func TestCreateDisk_Basic(t *testing.T) { gce := GCECloud{ manager: fakeManager, managedZones: []string{"zone1"}, - projectID: projectId, + projectID: gceProjectId, AlphaFeatureGate: alphaFeatureGate, } @@ -51,7 +56,8 @@ func TestCreateDisk_Basic(t *testing.T) { tags := make(map[string]string) tags["test-tag"] = "test-value" - diskTypeUri := gceComputeAPIEndpoint + "projects/" + fmt.Sprintf(diskTypeUriTemplate, projectId, zone, diskType) + expectedDiskTypeURI := gceComputeAPIEndpoint + "projects/" + fmt.Sprintf( + diskTypeURITemplateSingleZone, gceProjectId, zone, diskType) expectedDescription := "{\"test-tag\":\"test-value\"}" /* Act */ @@ -74,8 +80,66 @@ func TestCreateDisk_Basic(t *testing.T) { t.Errorf("Expected disk name: %s; Actual: %s", diskName, diskToCreate.Name) } - if diskToCreate.Type != diskTypeUri { - t.Errorf("Expected disk type: %s; Actual: %s", diskTypeUri, diskToCreate.Type) + if diskToCreate.Type != expectedDiskTypeURI { + t.Errorf("Expected disk type: %s; Actual: %s", expectedDiskTypeURI, diskToCreate.Type) + } + if diskToCreate.SizeGb != sizeGb { + t.Errorf("Expected disk size: %d; Actual: %d", sizeGb, diskToCreate.SizeGb) + } + if diskToCreate.Description != expectedDescription { + t.Errorf("Expected tag string: %s; Actual: %s", expectedDescription, diskToCreate.Description) + } +} + +func TestCreateRegionalDisk_Basic(t *testing.T) { + /* Arrange */ + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) + alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{GCEDiskAlphaFeatureGate}) + if featureGateErr != nil { + t.Error(featureGateErr) + } + gce := GCECloud{ + manager: fakeManager, + managedZones: []string{"zone1", "zone3", "zone2"}, + projectID: gceProjectId, + AlphaFeatureGate: alphaFeatureGate, + } + + diskName := "disk" + diskType := DiskTypeSSD + replicaZones := sets.NewString("zone1", "zone2") + const sizeGb int64 = 128 + tags := make(map[string]string) + tags["test-tag"] = "test-value" + + expectedDiskTypeURI := gceComputeAPIEndpointAlpha + "projects/" + fmt.Sprintf( + diskTypeURITemplateRegional, gceProjectId, gceRegion, diskType) + expectedDescription := "{\"test-tag\":\"test-value\"}" + + /* Act */ + err := gce.CreateRegionalDisk(diskName, diskType, replicaZones, sizeGb, tags) + + /* Assert */ + if err != nil { + t.Error(err) + } + if !fakeManager.createDiskCalled { + t.Error("Never called GCE disk create.") + } + if !fakeManager.doesOpMatch { + t.Error("Ops used in WaitForZoneOp does not match what's returned by CreateDisk.") + } + + // Partial check of equality between disk description sent to GCE and parameters of method. + diskToCreate := fakeManager.diskToCreateStable + if diskToCreate.Name != diskName { + t.Errorf("Expected disk name: %s; Actual: %s", diskName, diskToCreate.Name) + } + + if diskToCreate.Type != expectedDiskTypeURI { + t.Errorf("Expected disk type: %s; Actual: %s", expectedDiskTypeURI, diskToCreate.Type) } if diskToCreate.SizeGb != sizeGb { t.Errorf("Expected disk size: %d; Actual: %d", sizeGb, diskToCreate.SizeGb) @@ -87,7 +151,9 @@ func TestCreateDisk_Basic(t *testing.T) { func TestCreateDisk_DiskAlreadyExists(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -100,7 +166,7 @@ func TestCreateDisk_DiskAlreadyExists(t *testing.T) { // Inject disk AlreadyExists error. alreadyExistsError := googleapi.ErrorItem{Reason: "alreadyExists"} - fakeManager.waitForZoneOpError = &googleapi.Error{ + fakeManager.waitForOpError = &googleapi.Error{ Errors: []googleapi.ErrorItem{alreadyExistsError}, } @@ -116,7 +182,9 @@ func TestCreateDisk_DiskAlreadyExists(t *testing.T) { func TestCreateDisk_WrongZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) gce := GCECloud{manager: fakeManager, managedZones: []string{"zone1"}} diskName := "disk" @@ -134,7 +202,9 @@ func TestCreateDisk_WrongZone(t *testing.T) { func TestCreateDisk_NoManagedZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) gce := GCECloud{manager: fakeManager, managedZones: []string{}} diskName := "disk" @@ -152,7 +222,9 @@ func TestCreateDisk_NoManagedZone(t *testing.T) { func TestCreateDisk_BadDiskType(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) gce := GCECloud{manager: fakeManager, managedZones: []string{"zone1"}} diskName := "disk" @@ -171,7 +243,9 @@ func TestCreateDisk_BadDiskType(t *testing.T) { func TestCreateDisk_MultiZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -198,7 +272,9 @@ func TestCreateDisk_MultiZone(t *testing.T) { func TestDeleteDisk_Basic(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -233,8 +309,18 @@ func TestDeleteDisk_Basic(t *testing.T) { func TestDeleteDisk_NotFound(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() - gce := GCECloud{manager: fakeManager, managedZones: []string{"zone1"}} + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) + alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) + if featureGateErr != nil { + t.Error(featureGateErr) + } + gce := GCECloud{ + manager: fakeManager, + managedZones: []string{"zone1"}, + AlphaFeatureGate: alphaFeatureGate, + } diskName := "disk" /* Act */ @@ -248,7 +334,9 @@ func TestDeleteDisk_NotFound(t *testing.T) { func TestDeleteDisk_ResourceBeingUsed(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -277,7 +365,9 @@ func TestDeleteDisk_ResourceBeingUsed(t *testing.T) { func TestDeleteDisk_SameDiskMultiZone(t *testing.T) { /* Assert */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -309,7 +399,9 @@ func TestDeleteDisk_SameDiskMultiZone(t *testing.T) { func TestDeleteDisk_DiffDiskMultiZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) if featureGateErr != nil { t.Error(featureGateErr) @@ -341,7 +433,9 @@ func TestDeleteDisk_DiffDiskMultiZone(t *testing.T) { func TestGetAutoLabelsForPD_Basic(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "us-central1" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" diskType := DiskTypeSSD zone := "us-central1-c" @@ -369,14 +463,16 @@ func TestGetAutoLabelsForPD_Basic(t *testing.T) { t.Errorf("Failure domain is '%v', but zone is '%v'", labels[kubeletapis.LabelZoneFailureDomain], zone) } - if labels[kubeletapis.LabelZoneRegion] != "us-central1" { - t.Errorf("Region is '%v', but zone is 'us-central1'", labels[kubeletapis.LabelZoneRegion]) + if labels[kubeletapis.LabelZoneRegion] != gceRegion { + t.Errorf("Region is '%v', but region is 'us-central1'", labels[kubeletapis.LabelZoneRegion]) } } func TestGetAutoLabelsForPD_NoZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "europe-west1" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" diskType := DiskTypeStandard zone := "europe-west1-d" @@ -403,14 +499,16 @@ func TestGetAutoLabelsForPD_NoZone(t *testing.T) { t.Errorf("Failure domain is '%v', but zone is '%v'", labels[kubeletapis.LabelZoneFailureDomain], zone) } - if labels[kubeletapis.LabelZoneRegion] != "europe-west1" { - t.Errorf("Region is '%v', but zone is 'europe-west1'", labels[kubeletapis.LabelZoneRegion]) + if labels[kubeletapis.LabelZoneRegion] != gceRegion { + t.Errorf("Region is '%v', but region is 'europe-west1'", labels[kubeletapis.LabelZoneRegion]) } } func TestGetAutoLabelsForPD_DiskNotFound(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" zone := "asia-northeast1-a" gce := GCECloud{manager: fakeManager, managedZones: []string{zone}} @@ -426,9 +524,19 @@ func TestGetAutoLabelsForPD_DiskNotFound(t *testing.T) { func TestGetAutoLabelsForPD_DiskNotFoundAndNoZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" - gce := GCECloud{manager: fakeManager, managedZones: []string{}} + alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{}) + if featureGateErr != nil { + t.Error(featureGateErr) + } + gce := GCECloud{ + manager: fakeManager, + managedZones: []string{}, + AlphaFeatureGate: alphaFeatureGate, + } /* Act */ _, err := gce.GetAutoLabelsForPD(diskName, "") @@ -441,7 +549,9 @@ func TestGetAutoLabelsForPD_DiskNotFoundAndNoZone(t *testing.T) { func TestGetAutoLabelsForPD_DupDisk(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "us-west1" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" diskType := DiskTypeStandard zone := "us-west1-b" @@ -471,14 +581,16 @@ func TestGetAutoLabelsForPD_DupDisk(t *testing.T) { t.Errorf("Failure domain is '%v', but zone is '%v'", labels[kubeletapis.LabelZoneFailureDomain], zone) } - if labels[kubeletapis.LabelZoneRegion] != "us-west1" { - t.Errorf("Region is '%v', but zone is 'us-west1'", labels[kubeletapis.LabelZoneRegion]) + if labels[kubeletapis.LabelZoneRegion] != gceRegion { + t.Errorf("Region is '%v', but region is 'us-west1'", labels[kubeletapis.LabelZoneRegion]) } } func TestGetAutoLabelsForPD_DupDiskNoZone(t *testing.T) { /* Arrange */ - fakeManager := newFakeManager() + gceProjectId := "test-project" + gceRegion := "fake-region" + fakeManager := newFakeManager(gceProjectId, gceRegion) diskName := "disk" diskType := DiskTypeStandard const sizeGb int64 = 128 @@ -515,13 +627,16 @@ const ( type FakeServiceManager struct { // Common fields shared among tests - targetAPI targetClientAPI - opAlpha *computealpha.Operation // Mocks an operation returned by GCE API calls - opBeta *computebeta.Operation // Mocks an operation returned by GCE API calls - opStable *compute.Operation // Mocks an operation returned by GCE API calls - doesOpMatch bool - disks map[string]string // zone: diskName - waitForZoneOpError error // Error to be returned by WaitForZoneOp + targetAPI targetClientAPI + gceProjectID string + gceRegion string + opAlpha *computealpha.Operation // Mocks an operation returned by GCE API calls + opBeta *computebeta.Operation // Mocks an operation returned by GCE API calls + opStable *compute.Operation // Mocks an operation returned by GCE API calls + doesOpMatch bool + zonalDisks map[string]string // zone: diskName + regionalDisks map[string]sets.String // diskName: zones + waitForOpError error // Error to be returned by WaitForZoneOp or WaitForRegionalOp // Fields for TestCreateDisk createDiskCalled bool @@ -534,8 +649,13 @@ type FakeServiceManager struct { resourceInUse bool // Marks the disk as in-use } -func newFakeManager() *FakeServiceManager { - return &FakeServiceManager{disks: make(map[string]string)} +func newFakeManager(gceProjectID string, gceRegion string) *FakeServiceManager { + return &FakeServiceManager{ + zonalDisks: make(map[string]string), + regionalDisks: make(map[string]sets.String), + gceProjectID: gceProjectID, + gceRegion: gceRegion, + } } /** @@ -546,10 +666,65 @@ func (manager *FakeServiceManager) CreateDisk( name string, sizeGb int64, tagsStr string, - diskTypeURI string, + diskType string, zone string) (gceObject, error) { manager.createDiskCalled = true + switch t := manager.targetAPI; t { + case targetStable: + manager.opStable = &compute.Operation{} + diskTypeURI := gceComputeAPIEndpoint + "projects/" + fmt.Sprintf(diskTypeURITemplateSingleZone, manager.gceProjectID, zone, diskType) + diskToCreateV1 := &compute.Disk{ + Name: name, + SizeGb: sizeGb, + Description: tagsStr, + Type: diskTypeURI, + } + manager.diskToCreateStable = diskToCreateV1 + manager.zonalDisks[zone] = diskToCreateV1.Name + return manager.opStable, nil + case targetBeta: + manager.opBeta = &computebeta.Operation{} + diskTypeURI := gceComputeAPIEndpoint + "projects/" + fmt.Sprintf(diskTypeURITemplateSingleZone, manager.gceProjectID, zone, diskType) + diskToCreateBeta := &computebeta.Disk{ + Name: name, + SizeGb: sizeGb, + Description: tagsStr, + Type: diskTypeURI, + } + manager.diskToCreateBeta = diskToCreateBeta + manager.zonalDisks[zone] = diskToCreateBeta.Name + return manager.opBeta, nil + case targetAlpha: + manager.opAlpha = &computealpha.Operation{} + diskTypeURI := gceComputeAPIEndpointAlpha + "projects/" + fmt.Sprintf(diskTypeURITemplateSingleZone, manager.gceProjectID, zone, diskType) + diskToCreateAlpha := &computealpha.Disk{ + Name: name, + SizeGb: sizeGb, + Description: tagsStr, + Type: diskTypeURI, + } + manager.diskToCreateAlpha = diskToCreateAlpha + manager.zonalDisks[zone] = diskToCreateAlpha.Name + return manager.opAlpha, nil + default: + return nil, fmt.Errorf("unexpected type: %T", t) + } +} + +/** + * Upon disk creation, disk info is stored in FakeServiceManager + * to be used by other tested methods. + */ +func (manager *FakeServiceManager) CreateRegionalDisk( + name string, + sizeGb int64, + tagsStr string, + diskType string, + zones sets.String) (gceObject, error) { + manager.createDiskCalled = true + diskTypeURI := gceComputeAPIEndpointAlpha + "projects/" + fmt.Sprintf(diskTypeURITemplateRegional, manager.gceProjectID, manager.gceRegion, diskType) + switch t := manager.targetAPI; t { case targetStable: manager.opStable = &compute.Operation{} @@ -560,42 +735,21 @@ func (manager *FakeServiceManager) CreateDisk( Type: diskTypeURI, } manager.diskToCreateStable = diskToCreateV1 - manager.disks[zone] = diskToCreateV1.Name + manager.regionalDisks[diskToCreateV1.Name] = zones return manager.opStable, nil case targetBeta: - manager.opBeta = &computebeta.Operation{} - diskToCreateBeta := &computebeta.Disk{ - Name: name, - SizeGb: sizeGb, - Description: tagsStr, - Type: diskTypeURI, - } - manager.diskToCreateBeta = diskToCreateBeta - manager.disks[zone] = diskToCreateBeta.Name - return manager.opBeta, nil + return nil, fmt.Errorf("RegionalDisk CreateDisk op not supported in beta.") case targetAlpha: - manager.opAlpha = &computealpha.Operation{} - diskToCreateAlpha := &computealpha.Disk{ - Name: name, - SizeGb: sizeGb, - Description: tagsStr, - Type: diskTypeURI, - } - manager.diskToCreateAlpha = diskToCreateAlpha - manager.disks[zone] = diskToCreateAlpha.Name - return manager.opAlpha, nil + return nil, fmt.Errorf("RegionalDisk CreateDisk op not supported in alpha.") default: return nil, fmt.Errorf("unexpected type: %T", t) } } func (manager *FakeServiceManager) AttachDisk( - diskName string, - diskKind string, - diskZone string, + disk *GCEDisk, readWrite string, - source string, - diskType string, + instanceZone string, instanceName string) (gceObject, error) { switch t := manager.targetAPI; t { @@ -636,11 +790,9 @@ func (manager *FakeServiceManager) DetachDisk( * Gets disk info stored in the FakeServiceManager. */ func (manager *FakeServiceManager) GetDisk( - project string, - zone string, - diskName string) (*GCEDisk, error) { + zone string, diskName string) (*GCEDisk, error) { - if manager.disks[zone] == "" { + if manager.zonalDisks[zone] == "" { return nil, cloudprovider.DiskNotFound } @@ -651,10 +803,36 @@ func (manager *FakeServiceManager) GetDisk( } return &GCEDisk{ - Zone: lastComponent(zone), - Name: diskName, - Kind: "compute#disk", - Type: "type", + Region: manager.gceRegion, + ZoneInfo: singleZone{lastComponent(zone)}, + Name: diskName, + Kind: "compute#disk", + Type: "type", + }, nil +} + +/** + * Gets disk info stored in the FakeServiceManager. + */ +func (manager *FakeServiceManager) GetRegionalDisk( + diskName string) (*GCEDisk, error) { + + if _, ok := manager.regionalDisks[diskName]; !ok { + return nil, cloudprovider.DiskNotFound + } + + if manager.resourceInUse { + errorItem := googleapi.ErrorItem{Reason: "resourceInUseByAnotherResource"} + err := &googleapi.Error{Errors: []googleapi.ErrorItem{errorItem}} + return nil, err + } + + return &GCEDisk{ + Region: manager.gceRegion, + ZoneInfo: multiZone{manager.regionalDisks[diskName]}, + Name: diskName, + Kind: "compute#disk", + Type: "type", }, nil } @@ -662,12 +840,32 @@ func (manager *FakeServiceManager) GetDisk( * Disk info is removed from the FakeServiceManager. */ func (manager *FakeServiceManager) DeleteDisk( - project string, zone string, disk string) (gceObject, error) { manager.deleteDiskCalled = true - manager.disks[zone] = "" + delete(manager.zonalDisks, zone) + + switch t := manager.targetAPI; t { + case targetStable: + manager.opStable = &compute.Operation{} + return manager.opStable, nil + case targetBeta: + manager.opBeta = &computebeta.Operation{} + return manager.opBeta, nil + case targetAlpha: + manager.opAlpha = &computealpha.Operation{} + return manager.opAlpha, nil + default: + return nil, fmt.Errorf("unexpected type: %T", t) + } +} + +func (manager *FakeServiceManager) DeleteRegionalDisk( + disk string) (gceObject, error) { + + manager.deleteDiskCalled = true + delete(manager.regionalDisks, disk) switch t := manager.targetAPI; t { case targetStable: @@ -704,5 +902,26 @@ func (manager *FakeServiceManager) WaitForZoneOp( default: return fmt.Errorf("unexpected type: %T", v) } - return manager.waitForZoneOpError + return manager.waitForOpError +} + +func (manager *FakeServiceManager) WaitForRegionalOp( + op gceObject, mc *metricContext) error { + switch v := op.(type) { + case *computealpha.Operation: + if op.(*computealpha.Operation) == manager.opAlpha { + manager.doesOpMatch = true + } + case *computebeta.Operation: + if op.(*computebeta.Operation) == manager.opBeta { + manager.doesOpMatch = true + } + case *compute.Operation: + if op.(*compute.Operation) == manager.opStable { + manager.doesOpMatch = true + } + default: + return fmt.Errorf("unexpected type: %T", v) + } + return manager.waitForOpError }