Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reserve Static IPv6 address before syncing L4 NetLB #2165

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
31 changes: 30 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,9 @@ 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 = assertAddressOldReservedHook(t, c, fakeGCE)

if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil {
t.Errorf(
"Unexpected error when adding nodes %v", err)
Expand All @@ -486,6 +491,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 @@ -1216,3 +1222,26 @@ func checkMetrics(m metrics.L4NetLBServiceState, isManaged, isPremium, isUserErr
}
return nil
}

func assertAddressOldReservedHook(t *testing.T, mockGCE *cloud.MockGCE, gceCloud *gce.Cloud) func(ctx context.Context, key *meta.Key, m *cloud.MockForwardingRules) (bool, error) {
return func(ctx context.Context, key *meta.Key, _ *cloud.MockForwardingRules) (bool, error) {
fr, err := mockGCE.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 := gceCloud.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
}
}