Merge pull request #2046 from tssurya/OCPBUGS-28819

OCPBUGS-28819: Support Permanent Session Affinity

go-controller/pkg/libovsdb/ops/loadbalancer.go

@@ -48,13 +48,14 @@ func getNonZeroLoadBalancerMutableFields(lb *nbdb.LoadBalancer) []interface{} {
 }
 
 // BuildLoadBalancer builds a load balancer
-func BuildLoadBalancer(name string, protocol nbdb.LoadBalancerProtocol, vips, options, externalIds map[string]string) *nbdb.LoadBalancer {
+func BuildLoadBalancer(name string, protocol nbdb.LoadBalancerProtocol, selectionFields []nbdb.LoadBalancerSelectionFields, vips, options, externalIds map[string]string) *nbdb.LoadBalancer {
 	return &nbdb.LoadBalancer{
-		Name:        name,
-		Protocol:    &protocol,
-		Vips:        vips,
-		Options:     options,
-		ExternalIDs: externalIds,
+		Name:            name,
+		Protocol:        &protocol,
+		Vips:            vips,
+		SelectionFields: selectionFields,
+		Options:         options,
+		ExternalIDs:     externalIds,
 	}
 }
 

go-controller/pkg/ovn/controller/services/lb_config.go

@@ -14,6 +14,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1"
 	"k8s.io/klog/v2"
+	"k8s.io/kubernetes/pkg/apis/core"
 	utilnet "k8s.io/utils/net"
 )
 
@@ -768,14 +769,14 @@ func getSessionAffinityTimeOut(service *v1.Service) int32 {
 	if service.Spec.SessionAffinityConfig == nil ||
 		service.Spec.SessionAffinityConfig.ClientIP == nil ||
 		service.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds == nil {
-		return 10800 // default value
+		return core.DefaultClientIPServiceAffinitySeconds // default value
 	}
 	return *service.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds
 }
 
 func hasSessionAffinityTimeOut(service *v1.Service) bool {
 	return service.Spec.SessionAffinity == v1.ServiceAffinityClientIP &&
-		getSessionAffinityTimeOut(service) > 0
+		getSessionAffinityTimeOut(service) != core.MaxClientIPServiceAffinitySeconds
 }
 
 // lbOpts generates the OVN load balancer options from the kubernetes Service.

go-controller/pkg/ovn/controller/services/loadbalancer.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/apis/core"
 
 	"k8s.io/klog/v2"
 )
@@ -362,8 +363,16 @@ func buildLB(lb *LB) *templateLoadBalancer {
 	// Session affinity
 	// If enabled, then bucket flows by 3-tuple (proto, srcip, dstip) for the specific timeout value
 	// otherwise, use default ovn value
+	selectionFields := []nbdb.LoadBalancerSelectionFields{}
 	if lb.Opts.AffinityTimeOut > 0 {
-		options["affinity_timeout"] = fmt.Sprintf("%d", lb.Opts.AffinityTimeOut)
+		if lb.Opts.AffinityTimeOut != core.MaxClientIPServiceAffinitySeconds {
+			options["affinity_timeout"] = fmt.Sprintf("%d", lb.Opts.AffinityTimeOut)
+		} else {
+			selectionFields = []string{
+				nbdb.LoadBalancerSelectionFieldsIPSrc,
+				nbdb.LoadBalancerSelectionFieldsIPDst,
+			}
+		}
 	}
 
 	if lb.Opts.Template {
@@ -377,7 +386,7 @@ func buildLB(lb *LB) *templateLoadBalancer {
 	}
 
 	return &templateLoadBalancer{
-		nbLB:      libovsdbops.BuildLoadBalancer(lb.Name, strings.ToLower(lb.Protocol), buildVipMap(lb.Rules), options, lb.ExternalIDs),
+		nbLB:      libovsdbops.BuildLoadBalancer(lb.Name, strings.ToLower(lb.Protocol), selectionFields, buildVipMap(lb.Rules), options, lb.ExternalIDs),
 		templates: lb.Templates,
 	}
 }

go-controller/pkg/ovn/controller/services/loadbalancer_test.go

@@ -4,13 +4,15 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
 	libovsdbtest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/libovsdb"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilpointer "k8s.io/utils/pointer"
 )
 
-func TestEnsureLBs(t *testing.T) {
+func TestEnsureStaleLBs(t *testing.T) {
 	nbClient, cleanup, err := libovsdbtest.NewNBTestHarness(libovsdbtest.TestSetup{}, nil)
 	if err != nil {
 		t.Fatalf("Error creating NB: %v", err)
@@ -88,3 +90,142 @@ func TestEnsureLBs(t *testing.T) {
 		t.Fatalf("EnsureLBs did not set UUID of cached LB as is should")
 	}
 }
+
+func TestEnsureLBs(t *testing.T) {
+	tests := []struct {
+		desc    string
+		service *v1.Service
+		LBs     []LB
+		finalLB *nbdb.LoadBalancer
+	}{
+		{
+			desc: "create service with permanent session affinity",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "testns"},
+				Spec: v1.ServiceSpec{
+					Type:            v1.ServiceTypeClusterIP,
+					SessionAffinity: v1.ServiceAffinityClientIP,
+					SessionAffinityConfig: &v1.SessionAffinityConfig{
+						ClientIP: &v1.ClientIPConfig{
+							TimeoutSeconds: utilpointer.Int32(86400),
+						},
+					},
+				},
+			},
+			LBs: []LB{
+				{
+					Name: "Service_foo/testns_TCP_cluster",
+					ExternalIDs: map[string]string{
+						types.LoadBalancerKindExternalID:  "Service",
+						types.LoadBalancerOwnerExternalID: fmt.Sprintf("%s/%s", "foo", "testns"),
+					},
+					Routers:  []string{"gr-node-a"},
+					Protocol: "TCP",
+					Rules: []LBRule{
+						{
+							Source:  Addr{IP: "192.168.1.1", Port: 80},
+							Targets: []Addr{{IP: "10.0.244.3", Port: 8080}},
+						},
+					},
+					UUID: "test-UUID",
+					Opts: LBOpts{
+						Reject:          true,
+						AffinityTimeOut: 86400,
+					},
+				},
+			},
+			finalLB: &nbdb.LoadBalancer{
+				UUID:     loadBalancerClusterWideTCPServiceName("foo", "testns"),
+				Name:     loadBalancerClusterWideTCPServiceName("foo", "testns"),
+				Options:  servicesOptions(),
+				Protocol: &nbdb.LoadBalancerProtocolTCP,
+				Vips: map[string]string{
+					"192.168.1.1:80": "10.0.244.3:8080",
+				},
+				ExternalIDs:     serviceExternalIDs(namespacedServiceName("foo", "testns")),
+				SelectionFields: []string{"ip_src", "ip_dst"}, // permanent session affinity, no learn flows
+			},
+		},
+		{
+			desc: "create service with default session affinity timeout",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "testns"},
+				Spec: v1.ServiceSpec{
+					Type:            v1.ServiceTypeClusterIP,
+					SessionAffinity: v1.ServiceAffinityClientIP,
+					SessionAffinityConfig: &v1.SessionAffinityConfig{
+						ClientIP: &v1.ClientIPConfig{
+							TimeoutSeconds: utilpointer.Int32(10800),
+						},
+					},
+				},
+			},
+			LBs: []LB{
+				{
+					Name: "Service_foo/testns_TCP_cluster",
+					ExternalIDs: map[string]string{
+						types.LoadBalancerKindExternalID:  "Service",
+						types.LoadBalancerOwnerExternalID: fmt.Sprintf("%s/%s", "foo", "testns"),
+					},
+					Routers:  []string{"gr-node-a"},
+					Protocol: "TCP",
+					Rules: []LBRule{
+						{
+							Source:  Addr{IP: "192.168.1.1", Port: 80},
+							Targets: []Addr{{IP: "10.0.244.3", Port: 8080}},
+						},
+					},
+					UUID: "test-UUID",
+					Opts: LBOpts{
+						Reject:          true,
+						AffinityTimeOut: 10800,
+					},
+				},
+			},
+			finalLB: &nbdb.LoadBalancer{
+				UUID:     loadBalancerClusterWideTCPServiceName("foo", "testns"),
+				Name:     loadBalancerClusterWideTCPServiceName("foo", "testns"),
+				Options:  servicesOptionsWithAffinityTimeout(), // timeout set in the options
+				Protocol: &nbdb.LoadBalancerProtocolTCP,
+				Vips: map[string]string{
+					"192.168.1.1:80": "10.0.244.3:8080",
+				},
+				ExternalIDs: serviceExternalIDs(namespacedServiceName("foo", "testns")),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			nbClient, cleanup, err := libovsdbtest.NewNBTestHarness(libovsdbtest.TestSetup{
+				NBData: []libovsdbtest.TestData{
+					&nbdb.LogicalRouter{
+						Name: "gr-node-a",
+					},
+				},
+			}, nil)
+			if err != nil {
+				t.Fatalf("test: \"%s\" failed to set up test harness: %v", tt.desc, err)
+			}
+			t.Cleanup(cleanup.Cleanup)
+
+			err = EnsureLBs(nbClient, tt.service, []LB{}, tt.LBs)
+			if err != nil {
+				t.Fatalf("Error EnsureLBs: %v", err)
+			}
+			matcher := libovsdbtest.HaveDataIgnoringUUIDs([]libovsdbtest.TestData{
+				tt.finalLB,
+				&nbdb.LogicalRouter{
+					Name:         "gr-node-a",
+					LoadBalancer: []string{loadBalancerClusterWideTCPServiceName("foo", "testns")},
+				},
+			})
+			success, err := matcher.Match(nbClient)
+			if !success {
+				t.Fatal(fmt.Errorf("test: \"%s\" didn't match expected with actual, err: %v", tt.desc, matcher.FailureMessage(nbClient)))
+			}
+			if err != nil {
+				t.Fatal(fmt.Errorf("test: \"%s\" encountered error: %v", tt.desc, err))
+			}
+		})
+	}
+}

go-controller/pkg/ovn/controller/services/services_controller_test.go

@@ -748,6 +748,12 @@ func servicesOptions() map[string]string {
 	}
 }
 
+func servicesOptionsWithAffinityTimeout() map[string]string {
+	options := servicesOptions()
+	options["affinity_timeout"] = "10800"
+	return options
+}
+
 func templateServicesOptions() map[string]string {
 	// Template LBs need "options:template=true" and "options:address-family" set.
 	opts := servicesOptions()