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 committed Jun 22, 2023
1 parent aba04e2 commit a48e2f0
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 15 deletions.
54 changes: 40 additions & 14 deletions pkg/loadbalancers/forwarding_rules_ipv6.go
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
27 changes: 26 additions & 1 deletion pkg/loadbalancers/l4netlb_test.go
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

0 comments on commit a48e2f0

Please sign in to comment.