Skip to content

Commit

Permalink
fix: Add annotation to unset LB Service session-affinity, fix l7mp/st…
Browse files Browse the repository at this point in the history
  • Loading branch information
rg0now committed Sep 20, 2024
1 parent 351016f commit 1f93864
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 11 deletions.
21 changes: 17 additions & 4 deletions internal/renderer/service_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,9 @@ func (r *Renderer) createLbService4Gateway(c *RenderContext, gw *gwapiv1.Gateway
Annotations: requestedAnnotations,
},
Spec: corev1.ServiceSpec{
Type: opdefault.DefaultServiceType,
Selector: map[string]string{},
Ports: []corev1.ServicePort{},
SessionAffinity: corev1.ServiceAffinityClientIP,
Type: opdefault.DefaultServiceType,
Selector: map[string]string{},
Ports: []corev1.ServicePort{},
},
}
} else {
Expand Down Expand Up @@ -502,6 +501,20 @@ func (r *Renderer) createLbService4Gateway(c *RenderContext, gw *gwapiv1.Gateway
}
}

// Set session affinity
disableSessionAffinity := false
if v, ok := annotations[opdefault.DisableSessionAffiffinityAnnotationKey]; ok &&
strings.ToLower(v) == opdefault.DisableSessionAffiffinityAnnotationValue {
disableSessionAffinity = true
}

if disableSessionAffinity {
svc.Spec.SessionAffinity = corev1.ServiceAffinityNone
svc.Spec.SessionAffinityConfig = nil
} else {
svc.Spec.SessionAffinity = corev1.ServiceAffinityClientIP
}

// Open the health-check port for LoadBalancer Services, and only if not disabled
healthCheckExposeDisabled := false
if v, ok := annotations[opdefault.DisableHealthCheckExposeAnnotationKey]; ok &&
Expand Down
34 changes: 34 additions & 0 deletions internal/renderer/service_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,40 @@ func TestRenderServiceUtil(t *testing.T) {
assert.False(t, found, "health-check port exists")
},
},
{
name: "lb service - disabling session affinity (Oracle Kubernetes)",
cls: []gwapiv1.GatewayClass{testutils.TestGwClass},
cfs: []stnrgwv1.GatewayConfig{testutils.TestGwConfig},
gws: []gwapiv1.Gateway{testutils.TestGw},
rs: []stnrgwv1.UDPRoute{},
svcs: []corev1.Service{testutils.TestSvc},
prep: func(c *renderTestConfig) {
w := testutils.TestGwConfig.DeepCopy()
w.Spec.LoadBalancerServiceAnnotations = make(map[string]string)
w.Spec.LoadBalancerServiceAnnotations[opdefault.DisableSessionAffiffinityAnnotationKey] =
opdefault.DisableSessionAffiffinityAnnotationValue
c.cfs = []stnrgwv1.GatewayConfig{*w}
},
tester: func(t *testing.T, r *Renderer) {
gc, err := r.getGatewayClass()
assert.NoError(t, err, "gw-class found")
c := &RenderContext{gc: gc, log: logr.Discard()}
c.gwConf, err = r.getGatewayConfig4Class(c)
assert.NoError(t, err, "gw-conf found")

gws := r.getGateways4Class(c)
assert.Len(t, gws, 1, "gateways for class")
gw := gws[0]

s, _ := r.createLbService4Gateway(c, gw)
assert.NotNil(t, s, "svc create")
assert.Equal(t, gw.GetNamespace(), s.GetNamespace(), "namespace ok")
assert.Equal(t, gw.GetName(), s.GetName(), "name ok")

assert.True(t, string(s.Spec.SessionAffinity) == "" ||
s.Spec.SessionAffinity == corev1.ServiceAffinityNone, "session affinity disabled")
},
},
{
name: "public address hint in Gateway Spec",
cls: []gwapiv1.GatewayClass{testutils.TestGwClass},
Expand Down
9 changes: 2 additions & 7 deletions internal/updater/client_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,8 @@ func (u *Updater) upsertConfigMap(cm *corev1.ConfigMap, gen int) (ctrlutil.Opera
}}

op, err := ctrlutil.CreateOrUpdate(u.ctx, client, current, func() error {
// thing that might have been changed by the controler: the owner ref, annotations
// and the data

// u.log.Info("before", "cm", fmt.Sprintf("%#v\n", current))

// settings that might have been changed by the controler: the owner ref,
// annotations and the data
if err := setMetadata(current, cm); err != nil {
return nil
}
Expand All @@ -178,8 +175,6 @@ func (u *Updater) upsertConfigMap(cm *corev1.ConfigMap, gen int) (ctrlutil.Opera
current.Data[k] = v
}

// u.log.Info("after", "cm", fmt.Sprintf("%#v\n", current))

return nil
})

Expand Down
15 changes: 15 additions & 0 deletions pkg/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,21 @@ const (
// DisableHealthCheckExposeAnnotationValue is the value that can be used to disable the
// exposing the health-check port.
DisableHealthCheckExposeAnnotationValue = "true"

// DisableSessionAffiffinityAnnotationKey is a Gateway annotation to prevent STUNner from
// applying the sessionAffinity=client setting in the LB service. Normally this setting
// improves stability by ensuring that TURN sessions are pinned to the right dataplane
// pod. However, certain ingress controllers (in particular, Oracle Kubernetes) reject UDP
// LB services that have this setting on, breaking STUNner installations on these systems,
// see https://github.com/l7mp/stunner/issues/155. Setting this annotation to "true" for a
// Gateway will remove this setting from the LB Service created STUNner for the Gateway in
// order to improve compatibility with Kubernetes deployments that reject it. Default is to
// apply the session affinity setting.
DisableSessionAffiffinityAnnotationKey = "stunner.l7mp.io/disable-session-affinity"

// DisableSessionAffiffinityAnnotationValue is the value that can be used to remove
// session-affinity settings from the LB service.
DisableSessionAffiffinityAnnotationValue = "true"
)

var (
Expand Down
33 changes: 33 additions & 0 deletions test/managed_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,39 @@ func testManagedMode() {
Expect(svc.Spec.ExternalTrafficPolicy).Should(Equal(corev1.ServiceExternalTrafficPolicyType("")))
})

It("should handle session-affinity", func() {
svc := &corev1.Service{}

// default is SessionAffinity=clientIP
Eventually(func() bool {
lookupKey := store.GetNamespacedName(testGw)
if err := k8sClient.Get(ctx, lookupKey, svc); err != nil {
return false
}
return svc.Spec.SessionAffinity == corev1.ServiceAffinityClientIP
}, timeout, interval).Should(BeTrue())
Expect(svc.Spec.SessionAffinity).Should(Equal(corev1.ServiceAffinityClientIP))

// disable session-affinity
ctrl.Log.Info("re-loading gateway with annotation: stunner.l7mp.io/disable-session-affinity=true")
createOrUpdateGateway(testGw, func(current *gwapiv1.Gateway) {
current.SetAnnotations(map[string]string{
opdefault.DisableSessionAffiffinityAnnotationKey: // newline
opdefault.DisableSessionAffiffinityAnnotationValue,
})
})

Eventually(func() bool {
lookupKey := store.GetNamespacedName(testGw)
if err := k8sClient.Get(ctx, lookupKey, svc); err != nil {
return false
}

return svc.Spec.SessionAffinity == corev1.ServiceAffinityNone ||
string(svc.Spec.SessionAffinity) == ""
}, timeout, interval).Should(BeTrue())
})

It("should restore the public IP/port when the exposed LoadBalancer service type changes to NodePort", func() {
ctrl.Log.Info("re-loading gateway-config with annotation: service-type: ClusterIP")
createOrUpdateGatewayConfig(testGwConfig, func(current *stnrgwv1.GatewayConfig) {
Expand Down

0 comments on commit 1f93864

Please sign in to comment.