diff --git a/pkg/cloudprovider/providers/gce/BUILD b/pkg/cloudprovider/providers/gce/BUILD index 05a1f345e57..a3315db2fbd 100644 --- a/pkg/cloudprovider/providers/gce/BUILD +++ b/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", diff --git a/pkg/cloudprovider/providers/gce/cloud/mock/mock.go b/pkg/cloudprovider/providers/gce/cloud/mock/mock.go index f0e87cf0ce7..6a28c9f8e4b 100644 --- a/pkg/cloudprovider/providers/gce/cloud/mock/mock.go +++ b/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} } } diff --git a/pkg/cloudprovider/providers/gce/gce_address_manager_test.go b/pkg/cloudprovider/providers/gce/gce_address_manager_test.go index 3c7d60dc564..3b7409c6c19 100644 --- a/pkg/cloudprovider/providers/gce/gce_address_manager_test.go +++ b/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) - testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal)) - testReleaseAddress(t, mgr, svc, testLBName, testRegion) + mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal) + testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal)) + 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) - testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal)) - testReleaseAddress(t, mgr, svc, testLBName, testRegion) + mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal) + testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal)) + 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) - testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal)) - testReleaseAddress(t, mgr, svc, testLBName, testRegion) + mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal) + testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal)) + 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) - testHoldAddress(t, mgr, svc, testLBName, testRegion, targetIP, string(cloud.SchemeInternal)) - testReleaseAddress(t, mgr, svc, testLBName, testRegion) + mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal) + testHoldAddress(t, mgr, svc, testLBName, vals.Region, targetIP, string(cloud.SchemeInternal)) + 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) - _, err = mgr.HoldAddress() - assert.NotNil(t, err) + mgr := newAddressManager(svc, testSvcName, vals.Region, testSubnet, testLBName, targetIP, cloud.SchemeInternal) + ad, err := mgr.HoldAddress() + assert.NotNil(t, err) // FIXME + require.Equal(t, ad, "") } func testHoldAddress(t *testing.T, mgr *addressManager, svc CloudAddressService, name, region, targetIP, scheme string) { diff --git a/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go b/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go deleted file mode 100644 index 75dfa571c9c..00000000000 --- a/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go +++ /dev/null @@ -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() -}