Merge pull request #64338 from agau4779/deprecate-gce-addr-fakes

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

[GCE] use fakeGCECloud instead of gce address fakes

**What this PR does / why we need it**:
Use the fakeGCECloud mock instead of FakeCloudAddressService.

**Release note**:
```release-note
NONE
```

pkg/cloudprovider/providers/gce/BUILD

@@ -13,7 +13,6 @@ go_library(
         "gce.go",
         "gce_address_manager.go",
         "gce_addresses.go",
-        "gce_addresses_fakes.go",
         "gce_alpha.go",
         "gce_annotations.go",
         "gce_backendservice.go",

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

@@ -94,7 +94,7 @@ func RemoveInstanceHook(ctx context.Context, key *meta.Key, req *ga.TargetPoolsR
 
 func convertAndInsertAlphaForwardingRule(key *meta.Key, obj gceObject, mRules map[meta.Key]*cloud.MockForwardingRulesObj, version meta.Version, projectID string) (bool, error) {
 	if !key.Valid() {
-		return false, fmt.Errorf("invalid GCE key (%+v)", key)
+		return true, fmt.Errorf("invalid GCE key (%+v)", key)
 	}
 
 	if _, ok := mRules[*key]; ok {
@@ -102,16 +102,16 @@ func convertAndInsertAlphaForwardingRule(key *meta.Key, obj gceObject, mRules ma
 			Code:    http.StatusConflict,
 			Message: fmt.Sprintf("MockForwardingRule %v exists", key),
 		}
-		return false, err
+		return true, err
 	}
 
 	enc, err := obj.MarshalJSON()
 	if err != nil {
-		return false, err
+		return true, err
 	}
 	var fwdRule alpha.ForwardingRule
 	if err := json.Unmarshal(enc, &fwdRule); err != nil {
-		return false, err
+		return true, err
 	}
 	// Set the default values for the Alpha fields.
 	if fwdRule.NetworkTier == "" {
@@ -162,7 +162,7 @@ type AddressAttributes struct {
 
 func convertAndInsertAlphaAddress(key *meta.Key, obj gceObject, mAddrs map[meta.Key]*cloud.MockAddressesObj, version meta.Version, projectID string, addressAttrs AddressAttributes) (bool, error) {
 	if !key.Valid() {
-		return false, fmt.Errorf("invalid GCE key (%+v)", key)
+		return true, fmt.Errorf("invalid GCE key (%+v)", key)
 	}
 
 	if _, ok := mAddrs[*key]; ok {
@@ -170,16 +170,16 @@ func convertAndInsertAlphaAddress(key *meta.Key, obj gceObject, mAddrs map[meta.
 			Code:    http.StatusConflict,
 			Message: fmt.Sprintf("MockAddresses %v exists", key),
 		}
-		return false, err
+		return true, err
 	}
 
 	enc, err := obj.MarshalJSON()
 	if err != nil {
-		return false, err
+		return true, err
 	}
 	var addr alpha.Address
 	if err := json.Unmarshal(enc, &addr); err != nil {
-		return false, err
+		return true, err
 	}
 
 	// Set default address type if not present.
@@ -204,7 +204,7 @@ func convertAndInsertAlphaAddress(key *meta.Key, obj gceObject, mAddrs map[meta.
 				errorCode = http.StatusBadRequest
 			}
 
-			return false, &googleapi.Error{Code: errorCode, Message: msg}
+			return true, &googleapi.Error{Code: errorCode, Message: msg}
 		}
 	}
 

pkg/cloudprovider/providers/gce/gce_address_manager_test.go

@@ -26,95 +26,104 @@ import (
 )
 
 const testSvcName = "my-service"
-const testRegion = "us-central1"
 const testSubnet = "/projects/x/testRegions/us-central1/testSubnetworks/customsub"
 const testLBName = "a111111111111111"
 
+var vals = DefaultTestClusterValues()
+
 // TestAddressManagerNoRequestedIP tests the typical case of passing in no requested IP
 func TestAddressManagerNoRequestedIP(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	targetIP := ""
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
-	testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal))
+	testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal))
-	testReleaseAddress(t, mgr, svc, testLBName, testRegion)
+	testReleaseAddress(t, mgr, svc, testLBName, vals.Region)
 }
 
 // TestAddressManagerBasic tests the typical case of reserving and unreserving an address.
 func TestAddressManagerBasic(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	targetIP := "1.1.1.1"
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
-	testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal))
+	testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal))
-	testReleaseAddress(t, mgr, svc, testLBName, testRegion)
+	testReleaseAddress(t, mgr, svc, testLBName, vals.Region)
 }
 
 // TestAddressManagerOrphaned tests the case where the address exists with the IP being equal
 // to the requested address (forwarding rule or loadbalancer IP).
 func TestAddressManagerOrphaned(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	targetIP := "1.1.1.1"
 
 	addr := &compute.Address{Name: testLBName, Address: targetIP, AddressType: string(cloud.SchemeInternal)}
-	err := svc.ReserveRegionAddress(addr, testRegion)
+	err = svc.ReserveRegionAddress(addr, vals.Region)
 	require.NoError(t, err)
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
-	testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal))
+	testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal))
-	testReleaseAddress(t, mgr, svc, testLBName, testRegion)
+	testReleaseAddress(t, mgr, svc, testLBName, vals.Region)
 }
 
 // TestAddressManagerOutdatedOrphan tests the case where an address exists but points to
 // an IP other than the forwarding rule or loadbalancer IP.
 func TestAddressManagerOutdatedOrphan(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	previousAddress := "1.1.0.0"
 	targetIP := "1.1.1.1"
 
 	addr := &compute.Address{Name: testLBName, Address: previousAddress, AddressType: string(cloud.SchemeExternal)}
-	err := svc.ReserveRegionAddress(addr, testRegion)
+	err = svc.ReserveRegionAddress(addr, vals.Region)
 	require.NoError(t, err)
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
-	testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal))
+	testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal))
-	testReleaseAddress(t, mgr, svc, testLBName, testRegion)
+	testReleaseAddress(t, mgr, svc, testLBName, vals.Region)
 }
 
 // TestAddressManagerExternallyOwned tests the case where the address exists but isn't
 // owned by the controller.
 func TestAddressManagerExternallyOwned(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	targetIP := "1.1.1.1"
 
 	addr := &compute.Address{Name: "my-important-address", Address: targetIP, AddressType: string(cloud.SchemeInternal)}
-	err := svc.ReserveRegionAddress(addr, testRegion)
+	err = svc.ReserveRegionAddress(addr, vals.Region)
 	require.NoError(t, err)
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
 	ipToUse, err := mgr.HoldAddress()
 	require.NoError(t, err)
 	assert.NotEmpty(t, ipToUse)
 
-	_, err = svc.GetRegionAddress(testLBName, testRegion)
+	ad, err := svc.GetRegionAddress(testLBName, vals.Region)
 	assert.True(t, isNotFound(err))
+	require.Nil(t, ad)
 
-	testReleaseAddress(t, mgr, svc, testLBName, testRegion)
+	testReleaseAddress(t, mgr, svc, testLBName, vals.Region)
 }
 
 // TestAddressManagerExternallyOwned tests the case where the address exists but isn't
 // owned by the controller. However, this address has the wrong type.
 func TestAddressManagerBadExternallyOwned(t *testing.T) {
-	svc := NewFakeCloudAddressService()
+	svc, err := fakeGCECloud(vals)
+	require.NoError(t, err)
 	targetIP := "1.1.1.1"
 
 	addr := &compute.Address{Name: "my-important-address", Address: targetIP, AddressType: string(cloud.SchemeExternal)}
-	err := svc.ReserveRegionAddress(addr, testRegion)
+	err = svc.ReserveRegionAddress(addr, vals.Region)
 	require.NoError(t, err)
 
-	mgr := newAddressManager(svc, testSvcName, testRegion, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
+	mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal)
-	_, err = mgr.HoldAddress()
+	ad, err := mgr.HoldAddress()
-	assert.NotNil(t, err)
+	assert.NotNil(t, err) // FIXME
+	require.Equal(t, ad, "")
 }
 
 func testHoldAddress(t *testing.T, mgr *addressManager, svc CloudAddressService, name, region, targetIP, scheme string) {

pkg/cloudprovider/providers/gce/gce_addresses_fakes.go

@@ -1,239 +0,0 @@
-/*
-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 (
-	"bytes"
-	"encoding/json"
-	"fmt"
-	"net/http"
-
-	computealpha "google.golang.org/api/compute/v0.alpha"
-	computebeta "google.golang.org/api/compute/v0.beta"
-	compute "google.golang.org/api/compute/v1"
-
-	"k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud"
-)
-
-// test
-
-type FakeCloudAddressService struct {
-	count int
-	// reservedAddrs tracks usage of IP addresses
-	// Key is the IP address as a string
-	reservedAddrs map[string]bool
-	// addrsByRegionAndName
-	// Outer key is for region string; inner key is for address name.
-	addrsByRegionAndName map[string]map[string]*computealpha.Address
-}
-
-// FakeCloudAddressService Implements CloudAddressService
-var _ CloudAddressService = &FakeCloudAddressService{}
-
-func NewFakeCloudAddressService() *FakeCloudAddressService {
-	return &FakeCloudAddressService{
-		reservedAddrs:        make(map[string]bool),
-		addrsByRegionAndName: make(map[string]map[string]*computealpha.Address),
-	}
-}
-
-// SetRegionalAddresses sets the addresses of there region. This is used for
-// setting the test environment.
-func (cas *FakeCloudAddressService) SetRegionalAddresses(region string, addrs []*computealpha.Address) {
-	// Reset addresses in the region.
-	cas.addrsByRegionAndName[region] = make(map[string]*computealpha.Address)
-
-	for _, addr := range addrs {
-		cas.reservedAddrs[addr.Address] = true
-		cas.addrsByRegionAndName[region][addr.Name] = addr
-	}
-}
-
-func (cas *FakeCloudAddressService) ReserveAlphaRegionAddress(addr *computealpha.Address, region string) error {
-	if addr.Address == "" {
-		addr.Address = fmt.Sprintf("1.2.3.%d", cas.count)
-		cas.count++
-	}
-
-	if addr.AddressType == "" {
-		addr.AddressType = string(cloud.SchemeExternal)
-	}
-
-	if cas.reservedAddrs[addr.Address] {
-		msg := "IP in use"
-		// When the IP is already in use, this call returns an error code based
-		// on the type (internal vs external) of the address. This is to be
-		// consistent with actual GCE API.
-		switch cloud.LbScheme(addr.AddressType) {
-		case cloud.SchemeExternal:
-			return makeGoogleAPIError(http.StatusBadRequest, msg)
-		default:
-			return makeGoogleAPIError(http.StatusConflict, msg)
-		}
-	}
-
-	if _, exists := cas.addrsByRegionAndName[region]; !exists {
-		cas.addrsByRegionAndName[region] = make(map[string]*computealpha.Address)
-	}
-
-	if _, exists := cas.addrsByRegionAndName[region][addr.Name]; exists {
-		return makeGoogleAPIError(http.StatusConflict, "name in use")
-	}
-
-	cas.addrsByRegionAndName[region][addr.Name] = addr
-	cas.reservedAddrs[addr.Address] = true
-	return nil
-}
-
-func (cas *FakeCloudAddressService) ReserveBetaRegionAddress(addr *computebeta.Address, region string) error {
-	alphaAddr := convertToAlphaAddress(addr)
-	return cas.ReserveAlphaRegionAddress(alphaAddr, region)
-}
-
-func (cas *FakeCloudAddressService) ReserveRegionAddress(addr *compute.Address, region string) error {
-	alphaAddr := convertToAlphaAddress(addr)
-	return cas.ReserveAlphaRegionAddress(alphaAddr, region)
-}
-
-func (cas *FakeCloudAddressService) GetAlphaRegionAddress(name, region string) (*computealpha.Address, error) {
-	if _, exists := cas.addrsByRegionAndName[region]; !exists {
-		return nil, makeGoogleAPINotFoundError("")
-	}
-
-	if addr, exists := cas.addrsByRegionAndName[region][name]; !exists {
-		return nil, makeGoogleAPINotFoundError("")
-	} else {
-		return addr, nil
-	}
-}
-
-func (cas *FakeCloudAddressService) GetBetaRegionAddress(name, region string) (*computebeta.Address, error) {
-	addr, err := cas.GetAlphaRegionAddress(name, region)
-	if addr != nil {
-		return convertToBetaAddress(addr), err
-	}
-	return nil, err
-}
-
-func (cas *FakeCloudAddressService) GetRegionAddress(name, region string) (*compute.Address, error) {
-	addr, err := cas.GetAlphaRegionAddress(name, region)
-	if addr != nil {
-		return convertToV1Address(addr), err
-	}
-	return nil, err
-}
-
-func (cas *FakeCloudAddressService) DeleteRegionAddress(name, region string) error {
-	if _, exists := cas.addrsByRegionAndName[region]; !exists {
-		return makeGoogleAPINotFoundError("")
-	}
-
-	addr, exists := cas.addrsByRegionAndName[region][name]
-	if !exists {
-		return makeGoogleAPINotFoundError("")
-	}
-
-	delete(cas.reservedAddrs, addr.Address)
-	delete(cas.addrsByRegionAndName[region], name)
-	return nil
-}
-
-func (cas *FakeCloudAddressService) GetAlphaRegionAddressByIP(region, ipAddress string) (*computealpha.Address, error) {
-	if _, exists := cas.addrsByRegionAndName[region]; !exists {
-		return nil, makeGoogleAPINotFoundError("")
-	}
-
-	for _, addr := range cas.addrsByRegionAndName[region] {
-		if addr.Address == ipAddress {
-			return addr, nil
-		}
-	}
-	return nil, makeGoogleAPINotFoundError("")
-}
-
-func (cas *FakeCloudAddressService) GetBetaRegionAddressByIP(name, region string) (*computebeta.Address, error) {
-	addr, err := cas.GetAlphaRegionAddressByIP(name, region)
-	if addr != nil {
-		return convertToBetaAddress(addr), nil
-	}
-	return nil, err
-}
-
-func (cas *FakeCloudAddressService) GetRegionAddressByIP(name, region string) (*compute.Address, error) {
-	addr, err := cas.GetAlphaRegionAddressByIP(name, region)
-	if addr != nil {
-		return convertToV1Address(addr), nil
-	}
-	return nil, err
-}
-
-func (cas *FakeCloudAddressService) getNetworkTierFromAddress(name, region string) (string, error) {
-	addr, err := cas.GetAlphaRegionAddress(name, region)
-	if err != nil {
-		return "", err
-	}
-	return addr.NetworkTier, nil
-}
-
-func convertToV1Address(object gceObject) *compute.Address {
-	enc, err := object.MarshalJSON()
-	if err != nil {
-		panic(fmt.Sprintf("Failed to encode to json: %v", err))
-	}
-	var addr compute.Address
-	if err := json.Unmarshal(enc, &addr); err != nil {
-		panic(fmt.Sprintf("Failed to convert GCE apiObject %v to v1 address: %v", object, err))
-	}
-	return &addr
-}
-
-func convertToAlphaAddress(object gceObject) *computealpha.Address {
-	enc, err := object.MarshalJSON()
-	if err != nil {
-		panic(fmt.Sprintf("Failed to encode to json: %v", err))
-	}
-	var addr computealpha.Address
-	if err := json.Unmarshal(enc, &addr); err != nil {
-		panic(fmt.Sprintf("Failed to convert GCE apiObject %v to alpha address: %v", object, err))
-	}
-	// Set the default values for the Alpha fields.
-	addr.NetworkTier = cloud.NetworkTierDefault.ToGCEValue()
-	return &addr
-}
-
-func convertToBetaAddress(object gceObject) *computebeta.Address {
-	enc, err := object.MarshalJSON()
-	if err != nil {
-		panic(fmt.Sprintf("Failed to encode to json: %v", err))
-	}
-	var addr computebeta.Address
-	if err := json.Unmarshal(enc, &addr); err != nil {
-		panic(fmt.Sprintf("Failed to convert GCE apiObject %v to beta address: %v", object, err))
-	}
-	return &addr
-}
-
-func (cas *FakeCloudAddressService) String() string {
-	var b bytes.Buffer
-	for region, regAddresses := range cas.addrsByRegionAndName {
-		b.WriteString(fmt.Sprintf("%v:\n", region))
-		for name, addr := range regAddresses {
-			b.WriteString(fmt.Sprintf(" %v:  %v\n", name, addr.Address))
-		}
-	}
-	return b.String()
-}