Skip to content

Commit

Permalink
RegionalGCLBForVIP() Fix
Browse files Browse the repository at this point in the history
Fixes a bug where the ilb e2e test helper function may confuse multiple forwarding rules
from different VPCs with the same VIP
  • Loading branch information
spencerhance committed Dec 7, 2019
1 parent e3fe2c5 commit c3eb7c1
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
8 changes: 4 additions & 4 deletions cmd/e2e-test/ilb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestILB(t *testing.T) {
t.Fatalf("got %v, want RFC1918 address, ing: %v", vip, ing)
}

params := &fuzz.GCLBForVIPParams{VIP: vip, Validators: fuzz.FeatureValidators(features.All), Region: Framework.Region}
params := &fuzz.GCLBForVIPParams{VIP: vip, Validators: fuzz.FeatureValidators(features.All), Region: Framework.Region, Network: Framework.Network}
gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params)
if err != nil {
t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err)
Expand Down Expand Up @@ -263,7 +263,7 @@ func TestILBHttps(t *testing.T) {
t.Fatalf("got %v, want RFC1918 address, ing: %v", vip, ing)
}

params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Validators: fuzz.FeatureValidators(features.All)}
params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Network: Framework.Network, Validators: fuzz.FeatureValidators(features.All)}
gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params)
if err != nil {
t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err)
Expand Down Expand Up @@ -404,7 +404,7 @@ func TestILBUpdate(t *testing.T) {
t.Fatalf("got %v, want RFC1918 address, ing: %v", vip, ing)
}

params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Validators: fuzz.FeatureValidators(features.All)}
params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Network: Framework.Network, Validators: fuzz.FeatureValidators(features.All)}
gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params)
if err != nil {
t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err)
Expand Down Expand Up @@ -599,7 +599,7 @@ func TestILBShared(t *testing.T) {
t.Fatalf("got %v, want RFC1918 address, ing: %v", vip, ing)
}

params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Validators: fuzz.FeatureValidators(features.All)}
params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Network: Framework.Network, Validators: fuzz.FeatureValidators(features.All)}
gclb, err = fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params)
if err != nil {
t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err)
Expand Down
11 changes: 6 additions & 5 deletions pkg/fuzz/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ func hasBetaResource(resourceType string, validators []FeatureValidator) bool {
type GCLBForVIPParams struct {
VIP string
Region string
Network string
Validators []FeatureValidator
}

Expand All @@ -398,6 +399,7 @@ func GCLBForVIP(ctx context.Context, c cloud.Cloud, params *GCLBForVIPParams) (*
if err := RegionalGCLBForVIP(ctx, c, gclb, params); err != nil {
return nil, err
}
return gclb, nil
}

allGFRs, err := c.GlobalForwardingRules().List(ctx, filter.None)
Expand Down Expand Up @@ -628,22 +630,21 @@ func GCLBForVIP(ctx context.Context, c cloud.Cloud, params *GCLBForVIPParams) (*

// GCLBForVIP retrieves all of the resources associated with the GCLB for a given VIP.
func RegionalGCLBForVIP(ctx context.Context, c cloud.Cloud, gclb *GCLB, params *GCLBForVIPParams) error {

allRFRs, err := c.ForwardingRules().List(ctx, params.Region, filter.None)
if err != nil {
klog.Warningf("Error listing forwarding rules: %v", err)
return err
}

var rfrs []*compute.ForwardingRule
for _, gfr := range allRFRs {
if gfr.IPAddress == params.VIP {
rfrs = append(rfrs, gfr)
for _, rfr := range allRFRs {
if rfr.IPAddress == params.VIP && rfr.Network == params.Network {
rfrs = append(rfrs, rfr)
}
}

if len(rfrs) == 0 {
klog.Warningf("No params.Regional forwarding rules found, can't get all GCLB resources")
klog.Warningf("No regional forwarding rules found, can't get all GCLB resources")
return nil
}

Expand Down

0 comments on commit c3eb7c1

Please sign in to comment.