Skip to content

Commit

Permalink
Merge pull request #1223 from spencerhance/static-ip-followup-2
Browse files Browse the repository at this point in the history
Fix internal ingress static IP bug
  • Loading branch information
k8s-ci-robot authored Aug 24, 2020
2 parents 2dc9fb9 + 0afca04 commit cfec2cb
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 4 deletions.
3 changes: 2 additions & 1 deletion pkg/loadbalancers/forwarding_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink,
isL7ILB := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress)
tr := translator.NewTranslator(isL7ILB, l.namer)
env := &translator.Env{VIP: ip, Network: l.cloud.NetworkURL(), Subnetwork: l.cloud.SubnetworkURL()}
fr := tr.ToCompositeForwardingRule(env, protocol, version, proxyLink, description)
fr := tr.ToCompositeForwardingRule(env, protocol, version, proxyLink, description, l.runtimeInfo.StaticIPSubnet)

existing, _ = composite.GetForwardingRule(l.cloud, key, version)
if existing != nil && (fr.IPAddress != "" && existing.IPAddress != fr.IPAddress || existing.PortRange != fr.PortRange) {
Expand Down Expand Up @@ -180,6 +180,7 @@ func (l *L7) getEffectiveIP() (string, bool, error) {
return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing static IP.",
l.runtimeInfo.StaticIPName)
} else {
l.runtimeInfo.StaticIPSubnet = ip.Subnetwork
return ip.Address, false, nil
}
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/loadbalancers/l7.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ type L7RuntimeInfo struct {
// this name is used in the Forwarding Rules for this loadbalancer.
// If this is an l7-ILB ingress, the static IP is assumed to be internal
StaticIPName string
// The name of the static IP subnet, this is only used for L7-ILB Ingress static IPs
StaticIPSubnet string
// UrlMap is our internal representation of a url map.
UrlMap *utils.GCEURLMap
// FrontendConfig is the type which encapsulates features for the load balancer.
Expand Down
8 changes: 6 additions & 2 deletions pkg/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ const (
)

// ToCompositeForwardingRule returns a composite.ForwardingRule of type HTTP or HTTPS.
func (t *Translator) ToCompositeForwardingRule(env *Env, protocol namer.NamerProtocol, version meta.Version, proxyLink, description string) *composite.ForwardingRule {
func (t *Translator) ToCompositeForwardingRule(env *Env, protocol namer.NamerProtocol, version meta.Version, proxyLink, description, fwSubnet string) *composite.ForwardingRule {
var portRange string
if protocol == namer.HTTPProtocol {
portRange = httpDefaultPortRange
Expand All @@ -265,7 +265,11 @@ func (t *Translator) ToCompositeForwardingRule(env *Env, protocol namer.NamerPro
if t.IsL7ILB {
fr.LoadBalancingScheme = "INTERNAL_MANAGED"
fr.Network = env.Network
fr.Subnetwork = env.Subnetwork
if fwSubnet != "" {
fr.Subnetwork = fwSubnet
} else {
fr.Subnetwork = env.Subnetwork
}
}

return fr
Expand Down
21 changes: 20 additions & 1 deletion pkg/translator/translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ func TestToForwardingRule(t *testing.T) {
desc string
isL7ILB bool
protocol namer_util.NamerProtocol
ipSubnet string
want *composite.ForwardingRule
}{
{
Expand Down Expand Up @@ -428,13 +429,31 @@ func TestToForwardingRule(t *testing.T) {
Subnetwork: subnetwork,
},
},
{
desc: "http-ilb with different subnet for ip",
isL7ILB: true,
protocol: namer_util.HTTPProtocol,
ipSubnet: "different-subnet",
want: &composite.ForwardingRule{
Name: "foo-fr",
IPAddress: vip,
Target: proxyLink,
PortRange: httpDefaultPortRange,
IPProtocol: "TCP",
Description: description,
Version: version,
LoadBalancingScheme: "INTERNAL_MANAGED",
Network: network,
Subnetwork: "different-subnet",
},
},
}

for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
tr := NewTranslator(tc.isL7ILB, &testNamer{"foo"})
env := &Env{VIP: vip, Network: network, Subnetwork: subnetwork}
got := tr.ToCompositeForwardingRule(env, tc.protocol, version, proxyLink, description)
got := tr.ToCompositeForwardingRule(env, tc.protocol, version, proxyLink, description, tc.ipSubnet)
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Fatalf("Got diff for ForwardingRule (-want +got):\n%s", diff)
}
Expand Down

0 comments on commit cfec2cb

Please sign in to comment.