Skip to content

Commit

Permalink
Merge pull request kubernetes#64338 from agau4779/deprecate-gce-addr-…
Browse files Browse the repository at this point in the history 
…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
```
  • Loading branch information
Kubernetes Submit Queue authored May 31, 2018
2 parents a1c8d3f + cf393d7 commit 887f8ec
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 278 deletions.
1 change: 0 additions & 1 deletion pkg/cloudprovider/providers/gce/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 9 additions & 9 deletions pkg/cloudprovider/providers/gce/cloud/mock/mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,24 @@ 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 {
err := &googleapi.Error{
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 == "" {
Expand Down Expand Up @@ -162,24 +162,24 @@ 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 {
err := &googleapi.Error{
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.
Expand All @@ -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}
}
}

Expand Down
67 changes: 38 additions & 29 deletions pkg/cloudprovider/providers/gce/gce_address_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 887f8ec

Please sign in to comment.