Merge pull request #62297 from verult/gce-disks-refactor

pkg/cloudprovider/providers/gce/BUILD

@@ -31,7 +31,6 @@ go_library(
         "gce_loadbalancer_internal.go",
         "gce_loadbalancer_naming.go",
         "gce_networkendpointgroup.go",
-        "gce_op.go",
         "gce_routes.go",
         "gce_securitypolicy.go",
         "gce_targetpool.go",

pkg/cloudprovider/providers/gce/cloud/BUILD

@@ -4,6 +4,7 @@ go_library(
     name = "go_default_library",
     srcs = [
         "constants.go",
+        "context.go",
         "doc.go",
         "gce_projects.go",
         "gen.go",

pkg/cloudprovider/providers/gce/cloud/context.go

+/*
+Copyright 2018 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 cloud
+
+import (
+	"context"
+	"time"
+)
+
+const (
+	defaultCallTimeout = 1 * time.Hour
+)
+
+// ContextWithCallTimeout returns a context with a default timeout, used for generated client calls.
+func ContextWithCallTimeout() (context.Context, context.CancelFunc) {
+	return context.WithTimeout(context.Background(), defaultCallTimeout)
+}

pkg/cloudprovider/providers/gce/cloud/gen_test.go

@@ -312,29 +312,18 @@ func TestDisksGroup(t *testing.T) {
 	mock := NewMockGCE(pr)
 
 	var key *meta.Key
-	keyAlpha := meta.ZonalKey("key-alpha", "location")
-	key = keyAlpha
 	keyGA := meta.ZonalKey("key-ga", "location")
 	key = keyGA
 	// Ignore unused variables.
 	_, _, _ = ctx, mock, key
 
 	// Get not found.
-	if _, err := mock.AlphaDisks().Get(ctx, key); err == nil {
-		t.Errorf("AlphaDisks().Get(%v, %v) = _, nil; want error", ctx, key)
-	}
 	if _, err := mock.Disks().Get(ctx, key); err == nil {
 		t.Errorf("Disks().Get(%v, %v) = _, nil; want error", ctx, key)
 	}
 
 	// Insert.
 	{
-		obj := &alpha.Disk{}
-		if err := mock.AlphaDisks().Insert(ctx, keyAlpha, obj); err != nil {
-			t.Errorf("AlphaDisks().Insert(%v, %v, %v) = %v; want nil", ctx, keyAlpha, obj, err)
-		}
-	}
-	{
 		obj := &ga.Disk{}
 		if err := mock.Disks().Insert(ctx, keyGA, obj); err != nil {
 			t.Errorf("Disks().Insert(%v, %v, %v) = %v; want nil", ctx, keyGA, obj, err)
@@ -342,36 +331,17 @@ func TestDisksGroup(t *testing.T) {
 	}
 
 	// Get across versions.
-	if obj, err := mock.AlphaDisks().Get(ctx, key); err != nil {
-		t.Errorf("AlphaDisks().Get(%v, %v) = %v, %v; want nil", ctx, key, obj, err)
-	}
 	if obj, err := mock.Disks().Get(ctx, key); err != nil {
 		t.Errorf("Disks().Get(%v, %v) = %v, %v; want nil", ctx, key, obj, err)
 	}
 
 	// List.
-	mock.MockAlphaDisks.Objects[*keyAlpha] = mock.MockAlphaDisks.Obj(&alpha.Disk{Name: keyAlpha.Name})
 	mock.MockDisks.Objects[*keyGA] = mock.MockDisks.Obj(&ga.Disk{Name: keyGA.Name})
 	want := map[string]bool{
-		"key-alpha": true,
-		"key-ga":    true,
+		"key-ga": true,
 	}
 	_ = want // ignore unused variables.
 	{
-		objs, err := mock.AlphaDisks().List(ctx, location, filter.None)
-		if err != nil {
-			t.Errorf("AlphaDisks().List(%v, %v, %v) = %v, %v; want _, nil", ctx, location, filter.None, objs, err)
-		} else {
-			got := map[string]bool{}
-			for _, obj := range objs {
-				got[obj.Name] = true
-			}
-			if !reflect.DeepEqual(got, want) {
-				t.Errorf("AlphaDisks().List(); got %+v, want %+v", got, want)
-			}
-		}
-	}
-	{
 		objs, err := mock.Disks().List(ctx, location, filter.None)
 		if err != nil {
 			t.Errorf("Disks().List(%v, %v, %v) = %v, %v; want _, nil", ctx, location, filter.None, objs, err)
@@ -387,17 +357,11 @@ func TestDisksGroup(t *testing.T) {
 	}
 
 	// Delete across versions.
-	if err := mock.AlphaDisks().Delete(ctx, keyAlpha); err != nil {
-		t.Errorf("AlphaDisks().Delete(%v, %v) = %v; want nil", ctx, keyAlpha, err)
-	}
 	if err := mock.Disks().Delete(ctx, keyGA); err != nil {
 		t.Errorf("Disks().Delete(%v, %v) = %v; want nil", ctx, keyGA, err)
 	}
 
 	// Delete not found.
-	if err := mock.AlphaDisks().Delete(ctx, keyAlpha); err == nil {
-		t.Errorf("AlphaDisks().Delete(%v, %v) = nil; want error", ctx, keyAlpha)
-	}
 	if err := mock.Disks().Delete(ctx, keyGA); err == nil {
 		t.Errorf("Disks().Delete(%v, %v) = nil; want error", ctx, keyGA)
 	}
@@ -1315,39 +1279,39 @@ func TestRegionDisksGroup(t *testing.T) {
 	mock := NewMockGCE(pr)
 
 	var key *meta.Key
-	keyAlpha := meta.RegionalKey("key-alpha", "location")
-	key = keyAlpha
+	keyBeta := meta.RegionalKey("key-beta", "location")
+	key = keyBeta
 	// Ignore unused variables.
 	_, _, _ = ctx, mock, key
 
 	// Get not found.
-	if _, err := mock.AlphaRegionDisks().Get(ctx, key); err == nil {
-		t.Errorf("AlphaRegionDisks().Get(%v, %v) = _, nil; want error", ctx, key)
+	if _, err := mock.BetaRegionDisks().Get(ctx, key); err == nil {
+		t.Errorf("BetaRegionDisks().Get(%v, %v) = _, nil; want error", ctx, key)
 	}
 
 	// Insert.
 	{
-		obj := &alpha.Disk{}
-		if err := mock.AlphaRegionDisks().Insert(ctx, keyAlpha, obj); err != nil {
-			t.Errorf("AlphaRegionDisks().Insert(%v, %v, %v) = %v; want nil", ctx, keyAlpha, obj, err)
+		obj := &beta.Disk{}
+		if err := mock.BetaRegionDisks().Insert(ctx, keyBeta, obj); err != nil {
+			t.Errorf("BetaRegionDisks().Insert(%v, %v, %v) = %v; want nil", ctx, keyBeta, obj, err)
 		}
 	}
 
 	// Get across versions.
-	if obj, err := mock.AlphaRegionDisks().Get(ctx, key); err != nil {
-		t.Errorf("AlphaRegionDisks().Get(%v, %v) = %v, %v; want nil", ctx, key, obj, err)
+	if obj, err := mock.BetaRegionDisks().Get(ctx, key); err != nil {
+		t.Errorf("BetaRegionDisks().Get(%v, %v) = %v, %v; want nil", ctx, key, obj, err)
 	}
 
 	// List.
-	mock.MockAlphaRegionDisks.Objects[*keyAlpha] = mock.MockAlphaRegionDisks.Obj(&alpha.Disk{Name: keyAlpha.Name})
+	mock.MockBetaRegionDisks.Objects[*keyBeta] = mock.MockBetaRegionDisks.Obj(&beta.Disk{Name: keyBeta.Name})
 	want := map[string]bool{
-		"key-alpha": true,
+		"key-beta": true,
 	}
 	_ = want // ignore unused variables.
 	{
-		objs, err := mock.AlphaRegionDisks().List(ctx, location, filter.None)
+		objs, err := mock.BetaRegionDisks().List(ctx, location, filter.None)
 		if err != nil {
-			t.Errorf("AlphaRegionDisks().List(%v, %v, %v) = %v, %v; want _, nil", ctx, location, filter.None, objs, err)
+			t.Errorf("BetaRegionDisks().List(%v, %v, %v) = %v, %v; want _, nil", ctx, location, filter.None, objs, err)
 		} else {
 			got := map[string]bool{}
 			for _, obj := range objs {
@@ -1360,13 +1324,13 @@ func TestRegionDisksGroup(t *testing.T) {
 	}
 
 	// Delete across versions.
-	if err := mock.AlphaRegionDisks().Delete(ctx, keyAlpha); err != nil {
-		t.Errorf("AlphaRegionDisks().Delete(%v, %v) = %v; want nil", ctx, keyAlpha, err)
+	if err := mock.BetaRegionDisks().Delete(ctx, keyBeta); err != nil {
+		t.Errorf("BetaRegionDisks().Delete(%v, %v) = %v; want nil", ctx, keyBeta, err)
 	}
 
 	// Delete not found.
-	if err := mock.AlphaRegionDisks().Delete(ctx, keyAlpha); err == nil {
-		t.Errorf("AlphaRegionDisks().Delete(%v, %v) = nil; want error", ctx, keyAlpha)
+	if err := mock.BetaRegionDisks().Delete(ctx, keyBeta); err == nil {
+		t.Errorf("BetaRegionDisks().Delete(%v, %v) = nil; want error", ctx, keyBeta)
 	}
 }
 

pkg/cloudprovider/providers/gce/cloud/meta/meta.go

@@ -160,22 +160,20 @@ var AllServices = []*ServiceInfo{
 		Resource:    "disks",
 		keyType:     Zonal,
 		serviceType: reflect.TypeOf(&ga.DisksService{}),
-	},
-	{
-		Object:      "Disk",
-		Service:     "Disks",
-		Resource:    "disks",
-		version:     VersionAlpha,
-		keyType:     Zonal,
-		serviceType: reflect.TypeOf(&alpha.DisksService{}),
+		additionalMethods: []string{
+			"Resize",
+		},
 	},
 	{
 		Object:      "Disk",
 		Service:     "RegionDisks",
 		Resource:    "disks",
-		version:     VersionAlpha,
+		version:     VersionBeta,
 		keyType:     Regional,
-		serviceType: reflect.TypeOf(&alpha.DisksService{}),
+		serviceType: reflect.TypeOf(&beta.RegionDisksService{}),
+		additionalMethods: []string{
+			"Resize",
+		},
 	},
 	{
 		Object:      "Firewall",

pkg/cloudprovider/providers/gce/gce_op.go

-/*
-Copyright 2017 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 gce
-
-import (
-	"encoding/json"
-	"fmt"
-	"time"
-
-	"k8s.io/apimachinery/pkg/util/wait"
-
-	"github.com/golang/glog"
-	computealpha "google.golang.org/api/compute/v0.alpha"
-	computebeta "google.golang.org/api/compute/v0.beta"
-	computev1 "google.golang.org/api/compute/v1"
-	"google.golang.org/api/googleapi"
-)
-
-func (gce *GCECloud) waitForOp(op *computev1.Operation, getOperation func(operationName string) (*computev1.Operation, error), mc *metricContext) error {
-	if op == nil {
-		return mc.Observe(fmt.Errorf("operation must not be nil"))
-	}
-
-	if opIsDone(op) {
-		return getErrorFromOp(op)
-	}
-
-	opStart := time.Now()
-	opName := op.Name
-
-	return wait.Poll(operationPollInterval, operationPollTimeoutDuration, func() (bool, error) {
-		start := time.Now()
-		gce.operationPollRateLimiter.Accept()
-		duration := time.Since(start)
-		if duration > 5*time.Second {
-			glog.V(2).Infof("pollOperation: throttled %v for %v", duration, opName)
-		}
-		pollOp, err := getOperation(opName)
-		if err != nil {
-			glog.Warningf("GCE poll operation %s failed: pollOp: [%v] err: [%v] getErrorFromOp: [%v]",
-				opName, pollOp, err, getErrorFromOp(pollOp))
-		}
-
-		done := opIsDone(pollOp)
-		if done {
-			duration := time.Since(opStart)
-			if duration > 1*time.Minute {
-				// Log the JSON. It's cleaner than the %v structure.
-				enc, err := pollOp.MarshalJSON()
-				if err != nil {
-					glog.Warningf("waitForOperation: long operation (%v): %v (failed to encode to JSON: %v)",
-						duration, pollOp, err)
-				} else {
-					glog.V(2).Infof("waitForOperation: long operation (%v): %v",
-						duration, string(enc))
-				}
-			}
-		}
-
-		return done, mc.Observe(getErrorFromOp(pollOp))
-	})
-}
-
-func opIsDone(op *computev1.Operation) bool {
-	return op != nil && op.Status == "DONE"
-}
-
-func getErrorFromOp(op *computev1.Operation) error {
-	if op != nil && op.Error != nil && len(op.Error.Errors) > 0 {
-		err := &googleapi.Error{
-			Code:    int(op.HttpErrorStatusCode),
-			Message: op.Error.Errors[0].Message,
-		}
-		glog.Errorf("GCE operation failed: %v", err)
-		return err
-	}
-
-	return nil
-}
-
-func (gce *GCECloud) waitForGlobalOp(op gceObject, mc *metricContext) error {
-	return gce.waitForGlobalOpInProject(op, gce.ProjectID(), mc)
-}
-
-func (gce *GCECloud) waitForRegionOp(op gceObject, region string, mc *metricContext) error {
-	return gce.waitForRegionOpInProject(op, gce.ProjectID(), region, mc)
-}
-
-func (gce *GCECloud) waitForZoneOp(op gceObject, zone string, mc *metricContext) error {
-	return gce.waitForZoneOpInProject(op, gce.ProjectID(), zone, mc)
-}
-
-func (gce *GCECloud) waitForGlobalOpInProject(op gceObject, projectID string, mc *metricContext) error {
-	switch v := op.(type) {
-	case *computealpha.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceAlpha.GlobalOperations.Get(projectID, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computebeta.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceBeta.GlobalOperations.Get(projectID, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computev1.Operation:
-		return gce.waitForOp(op.(*computev1.Operation), func(operationName string) (*computev1.Operation, error) {
-			return gce.service.GlobalOperations.Get(projectID, operationName).Do()
-		}, mc)
-	default:
-		return fmt.Errorf("unexpected type: %T", v)
-	}
-}
-
-func (gce *GCECloud) waitForRegionOpInProject(op gceObject, projectID, region string, mc *metricContext) error {
-	switch v := op.(type) {
-	case *computealpha.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceAlpha.RegionOperations.Get(projectID, region, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computebeta.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceBeta.RegionOperations.Get(projectID, region, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computev1.Operation:
-		return gce.waitForOp(op.(*computev1.Operation), func(operationName string) (*computev1.Operation, error) {
-			return gce.service.RegionOperations.Get(projectID, region, operationName).Do()
-		}, mc)
-	default:
-		return fmt.Errorf("unexpected type: %T", v)
-	}
-}
-
-func (gce *GCECloud) waitForZoneOpInProject(op gceObject, projectID, zone string, mc *metricContext) error {
-	switch v := op.(type) {
-	case *computealpha.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceAlpha.ZoneOperations.Get(projectID, zone, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computebeta.Operation:
-		return gce.waitForOp(convertToV1Operation(op), func(operationName string) (*computev1.Operation, error) {
-			op, err := gce.serviceBeta.ZoneOperations.Get(projectID, zone, operationName).Do()
-			return convertToV1Operation(op), err
-		}, mc)
-	case *computev1.Operation:
-		return gce.waitForOp(op.(*computev1.Operation), func(operationName string) (*computev1.Operation, error) {
-			return gce.service.ZoneOperations.Get(projectID, zone, operationName).Do()
-		}, mc)
-	default:
-		return fmt.Errorf("unexpected type: %T", v)
-	}
-}
-
-func convertToV1Operation(object gceObject) *computev1.Operation {
-	enc, err := object.MarshalJSON()
-	if err != nil {
-		panic(fmt.Sprintf("Failed to encode to json: %v", err))
-	}
-	var op computev1.Operation
-	if err := json.Unmarshal(enc, &op); err != nil {
-		panic(fmt.Sprintf("Failed to convert GCE apiObject %v to v1 operation: %v", object, err))
-	}
-	return &op
-}

pkg/cloudprovider/providers/gce/gce_test.go

@@ -18,16 +18,12 @@ package gce
 
 import (
 	"context"
-	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"
 
 	"golang.org/x/oauth2/google"
 
-	computealpha "google.golang.org/api/compute/v0.alpha"
-	computebeta "google.golang.org/api/compute/v0.beta"
-	computev1 "google.golang.org/api/compute/v1"
 	"k8s.io/kubernetes/pkg/cloudprovider"
 )
 
@@ -480,56 +476,6 @@ func TestGenerateCloudConfigs(t *testing.T) {
 	}
 }
 
-func TestConvertToV1Operation(t *testing.T) {
-	v1Op := getTestOperation()
-	enc, _ := v1Op.MarshalJSON()
-	var op interface{}
-	var alphaOp computealpha.Operation
-	var betaOp computebeta.Operation
-
-	if err := json.Unmarshal(enc, &alphaOp); err != nil {
-		t.Errorf("Failed to unmarshal operation: %v", err)
-	}
-
-	if err := json.Unmarshal(enc, &betaOp); err != nil {
-		t.Errorf("Failed to unmarshal operation: %v", err)
-	}
-
-	op = convertToV1Operation(&alphaOp)
-	if _, ok := op.(*computev1.Operation); ok {
-		if !reflect.DeepEqual(op, v1Op) {
-			t.Errorf("Failed to maintain consistency across conversion")
-		}
-	} else {
-		t.Errorf("Expect output to be type v1 operation, but got %v", op)
-	}
-
-	op = convertToV1Operation(&betaOp)
-	if _, ok := op.(*computev1.Operation); ok {
-		if !reflect.DeepEqual(op, v1Op) {
-			t.Errorf("Failed to maintain consistency across conversion")
-		}
-	} else {
-		t.Errorf("Expect output to be type v1 operation, but got %v", op)
-	}
-}
-
-func getTestOperation() *computev1.Operation {
-	return &computev1.Operation{
-		Name:        "test",
-		Description: "test",
-		Id:          uint64(12345),
-		Error: &computev1.OperationError{
-			Errors: []*computev1.OperationErrorErrors{
-				{
-					Code:    "555",
-					Message: "error",
-				},
-			},
-		},
-	}
-}
-
 func TestNewAlphaFeatureGate(t *testing.T) {
 	testCases := []struct {
 		alphaFeatures  []string