Skip to content

Commit

Permalink
Reserve Static IPv6 address before syncing L4 NetLB
Browse files Browse the repository at this point in the history 
This will prevent losing address while recreating forwarding rule
  • Loading branch information
@panslava
panslava committed Jun 22, 2023
1 parent aba04e2 commit a57af6b
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 15 deletions.
54 changes: 40 additions & 14 deletions pkg/loadbalancers/forwarding_rules_ipv6.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/cloud-provider-gcp/providers/gce"
"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/composite"
Expand Down Expand Up @@ -134,19 +135,47 @@ func (l4 *L4) deleteChangedIPv6ForwardingRule(existingFwdRule *composite.Forward
func (l4netlb *L4NetLB) ensureIPv6ForwardingRule(bsLink string) (*composite.ForwardingRule, error) {
start := time.Now()

expectedIPv6FwdRule, err := l4netlb.buildExpectedIPv6ForwardingRule(bsLink)
klog.V(2).Infof("Ensuring external ipv6 forwarding rule for L4 NetLB Service %s/%s, backend service link: %s", l4netlb.Service.Namespace, l4netlb.Service.Name, bsLink)
defer func() {
klog.V(2).Infof("Finished ensuring external ipv6 forwarding rule for L4 NetLB Service %s/%s, time taken: %v", l4netlb.Service.Namespace, l4netlb.Service.Name, time.Since(start))
}()

expectedIPv6FrName := l4netlb.ipv6FRName()
existingIPv6FwdRule, err := l4netlb.forwardingRules.Get(expectedIPv6FrName)
if err != nil {
return nil, fmt.Errorf("l4netlb.buildExpectedIPv6ForwardingRule(%s) returned error %w, want nil", bsLink, err)
klog.Errorf("l4netlb.forwardingRules.Get(%s) returned error %v", expectedIPv6FrName, err)
return nil, err
}

klog.V(2).Infof("Ensuring external ipv6 forwarding rule %s for L4 NetLB Service %s/%s, backend service link: %s", expectedIPv6FwdRule.Name, l4netlb.Service.Namespace, l4netlb.Service.Name, bsLink)
defer func() {
klog.V(2).Infof("Finished ensuring external ipv6 forwarding rule %s for L4 NetLB Service %s/%s, time taken: %v", expectedIPv6FwdRule.Name, l4netlb.Service.Namespace, l4netlb.Service.Name, time.Since(start))
}()
subnetworkURL, err := l4netlb.IPv6ForwardingRuleSubnet()
if err != nil {
return nil, fmt.Errorf("error getting ipv6 forwarding rule subnet: %w", err)
}

// Determine IP which will be used for this LB. If no forwarding rule has been established
// or specified in the Service spec, then requestedIP = "".
ipv6AddrToUse := ipv6AddressToUse(existingIPv6FwdRule, subnetworkURL)
if !l4netlb.cloud.IsLegacyNetwork() {
nm := types.NamespacedName{Namespace: l4netlb.Service.Namespace, Name: l4netlb.Service.Name}.String()
addrMgr := newAddressManager(l4netlb.cloud, nm, l4netlb.cloud.Region(), subnetworkURL, expectedIPv6FrName, ipv6AddrToUse, cloud.SchemeExternal, cloud.NetworkTierPremium, IPv6Version)

existingIPv6FwdRule, err := l4netlb.forwardingRules.Get(expectedIPv6FwdRule.Name)
ipv6AddrToUse, _, err = addrMgr.HoldAddress()
if err != nil {
return nil, err
}
klog.V(2).Infof("ensureIPv6ForwardingRule(%v): reserved IP %q for the forwarding rule %s", nm, ipv6AddrToUse, expectedIPv6FrName)
defer func() {
// Release the address that was reserved, in all cases. If the forwarding rule was successfully created,
// the ephemeral IP is not needed anymore. If it was not created, the address should be released to prevent leaks.
if err := addrMgr.ReleaseAddress(); err != nil {
klog.Errorf("ensureIPv6ForwardingRule: failed to release address reservation, possibly causing an orphan: %v", err)
}
}()
}

expectedIPv6FwdRule, err := l4netlb.buildExpectedIPv6ForwardingRule(bsLink, ipv6AddrToUse, subnetworkURL)
if err != nil {
return nil, fmt.Errorf("l4netlb.forwardingRules.GetForwardingRule(%s) returned error %w, want nil", expectedIPv6FwdRule.Name, err)
return nil, fmt.Errorf("l4netlb.buildExpectedIPv6ForwardingRule(%s) returned error %w, want nil", bsLink, err)
}

if existingIPv6FwdRule != nil {
Expand All @@ -173,7 +202,7 @@ func (l4netlb *L4NetLB) ensureIPv6ForwardingRule(bsLink string) (*composite.Forw
return createdFr, err
}

func (l4netlb *L4NetLB) buildExpectedIPv6ForwardingRule(bsLink string) (*composite.ForwardingRule, error) {
func (l4netlb *L4NetLB) buildExpectedIPv6ForwardingRule(bsLink, ipv6AddressToUse, subnetworkURL string) (*composite.ForwardingRule, error) {
frName := l4netlb.ipv6FRName()

frDesc, err := utils.MakeL4IPv6ForwardingRuleDescription(l4netlb.Service)
Expand All @@ -183,20 +212,17 @@ func (l4netlb *L4NetLB) buildExpectedIPv6ForwardingRule(bsLink string) (*composi

svcPorts := l4netlb.Service.Spec.Ports
portRange, protocol := utils.MinMaxPortRangeAndProtocol(svcPorts)
subnetURL, err := l4netlb.IPv6ForwardingRuleSubnet()
if err != nil {
return nil, fmt.Errorf("error getting ipv6 forwarding rule subnet: %w", err)
}
fr := &composite.ForwardingRule{
Name: frName,
Description: frDesc,
IPAddress: ipv6AddressToUse,
IPProtocol: protocol,
PortRange: portRange,
LoadBalancingScheme: string(cloud.SchemeExternal),
BackendService: bsLink,
IpVersion: IPVersionIPv6,
NetworkTier: cloud.NetworkTierPremium.ToGCEValue(),
Subnetwork: subnetURL,
Subnetwork: subnetworkURL,
}

return fr, nil
Expand Down
96 changes: 95 additions & 1 deletion pkg/loadbalancers/l4netlb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package loadbalancers

import (
"context"
"fmt"
"reflect"
"strings"
Expand Down Expand Up @@ -399,7 +400,7 @@ func TestEnsureExternalDualStackLoadBalancer(t *testing.T) {
// - ServiceExternalTrafficPolicy
// - Protocol
// - IPFamilies
// for dual-stack service. In total 400 combinations
// for dual-stack service. In total 400 combinations.
func TestDualStackNetLBTransitions(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -457,6 +458,7 @@ func TestDualStackNetLBTransitions(t *testing.T) {

namer := namer_util.NewL4Namer(kubeSystemUID, nil)
fakeGCE := getFakeGCECloud(vals)
c := fakeGCE.Compute().(*cloud.MockGCE)

svc := test.NewL4NetLBRBSDualStackService(tc.initialProtocol, tc.initialIPFamily, tc.initialTrafficPolicy)
l4NetLBParams := &L4NetLBParams{
Expand All @@ -469,6 +471,28 @@ func TestDualStackNetLBTransitions(t *testing.T) {
l4NetLB := NewL4NetLB(l4NetLBParams)
l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder)

// Before deleting forwarding rule, check, that the address was reserved
c.MockForwardingRules.DeleteHook = func(ctx context.Context, key *meta.Key, m *cloud.MockForwardingRules) (bool, error) {
fr, err := c.MockForwardingRules.Get(ctx, key)
// if forwarding rule not exists, don't need to check if address reserved
if utils.IsNotFoundError(err) {
return false, nil
}
if err != nil {
return false, err
}

addr, err := l4NetLB.cloud.GetRegionAddressByIP(fr.Region, fr.IPAddress)
if utils.IgnoreHTTPNotFound(err) != nil {
return true, err
}
if addr == nil || utils.IsNotFoundError(err) {
t.Errorf("Address not reserved before deleting forwarding rule +%v", fr)
}

return false, nil
}

if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil {
t.Errorf(
"Unexpected error when adding nodes %v", err)
Expand All @@ -486,6 +510,7 @@ func TestDualStackNetLBTransitions(t *testing.T) {
finalSvc.Annotations = result.Annotations
assertDualStackNetLBResources(t, l4NetLB, nodeNames)

c.MockForwardingRules.DeleteHook = nil
l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service)
assertDualStackNetLBResourcesDeleted(t, l4NetLB)
})
Expand Down Expand Up @@ -697,6 +722,75 @@ func TestEnsureIPv6ExternalLoadBalancerCustomSubnet(t *testing.T) {
assertDualStackNetLBResourcesDeleted(t, l4NetLB)
}

// TestDualStackNetLBModifyProtocol verifies modifying protocol for DualStack NetLB Service.
// This is specifically useful to verify that address was reserved before recreating forwarding rule.
func TestDualStackNetLBModifyProtocol(t *testing.T) {
t.Parallel()

initialProtocol := v1.ProtocolTCP
finalProtocol := v1.ProtocolUDP
nodeNames := []string{"test-node-1"}
vals := gce.DefaultTestClusterValues()

namer := namer_util.NewL4Namer(kubeSystemUID, nil)
fakeGCE := getFakeGCECloud(vals)
c := fakeGCE.Compute().(*cloud.MockGCE)

svc := test.NewL4NetLBRBSDualStackService(initialProtocol, []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, v1.ServiceExternalTrafficPolicyTypeCluster)
l4NetLBParams := &L4NetLBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
DualStackEnabled: true,
}
l4NetLB := NewL4NetLB(l4NetLBParams)
l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder)

// Before deleting forwarding rule, check, that the address was reserved
c.MockForwardingRules.DeleteHook = func(ctx context.Context, key *meta.Key, m *cloud.MockForwardingRules) (bool, error) {
fr, err := c.MockForwardingRules.Get(ctx, key)
// if forwarding rule not exists, don't need to check if address reserved
if utils.IsNotFoundError(err) {
return false, nil
}
if err != nil {
return false, err
}

addr, err := l4NetLB.cloud.GetRegionAddressByIP(fr.Region, fr.IPAddress)
if utils.IgnoreHTTPNotFound(err) != nil {
return true, err
}
if addr == nil || utils.IsNotFoundError(err) {
t.Errorf("Address not reserved before deleting forwarding rule +%v", fr)
}

return false, nil
}

if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil {
t.Errorf(
"Unexpected error when adding nodes %v", err)
}

result := l4NetLB.EnsureFrontend(nodeNames, svc)
svc.Annotations = result.Annotations
assertDualStackNetLBResources(t, l4NetLB, nodeNames)

finalSvc := test.NewL4NetLBRBSDualStackService(finalProtocol, []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, v1.ServiceExternalTrafficPolicyTypeCluster)
finalSvc.Annotations = svc.Annotations
l4NetLB.Service = finalSvc

result = l4NetLB.EnsureFrontend(nodeNames, svc)
finalSvc.Annotations = result.Annotations
assertDualStackNetLBResources(t, l4NetLB, nodeNames)

c.MockForwardingRules.DeleteHook = nil
l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service)
assertDualStackNetLBResourcesDeleted(t, l4NetLB)
}

func ensureLoadBalancer(port int, vals gce.TestClusterValues, fakeGCE *gce.Cloud, t *testing.T) (*v1.Service, *L4NetLB) {
svc := test.NewL4NetLBRBSService(port)
namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw"))
Expand Down

0 comments on commit a57af6b

Please sign in to comment.