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

BackendConfig support for session affinity #526

Merged
merged 4 commits into from
Oct 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 7 additions & 5 deletions pkg/apis/backendconfig/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ type BackendConfig struct {
// BackendConfigSpec is the spec for a BackendConfig resource
// +k8s:openapi-gen=true
type BackendConfigSpec struct {
Iap *IAPConfig `json:"iap,omitempty"`
Cdn *CDNConfig `json:"cdn,omitempty"`
SecurityPolicy *SecurityPolicyConfig `json:"securityPolicy,omitempty"`
TimeoutSec *int64 `json:"timeoutSec,omitempty"`
ConnectionDraining *ConnectionDrainingConfig `json:"connectionDraining,omitempty"`
Iap *IAPConfig `json:"iap,omitempty"`
Cdn *CDNConfig `json:"cdn,omitempty"`
SecurityPolicy *SecurityPolicyConfig `json:"securityPolicy,omitempty"`
TimeoutSec *int64 `json:"timeoutSec,omitempty"`
ConnectionDraining *ConnectionDrainingConfig `json:"connectionDraining,omitempty"`
AffinityCookieTtlSec *int64 `json:"affinityCookieTtlSec,omitempty"`
SessionAffinity string `json:"sessionAffinity,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we encapsulate these settings in a struct? Something like this:

type SessionAffinityConfig struct {
SessionAffinityType string
AffinityCookieTtlSec *int64
}

}

// BackendConfigStatus is the status for a BackendConfig resource
bpineau marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/backendconfig/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions pkg/backends/features/affinity.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package features

import (
"strings"

"github.com/golang/glog"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/utils"
)

// EnsureAffinity reads the sessionAffinity and AffinityCookieTtlSec configuration
bpineau marked this conversation as resolved.
Show resolved Hide resolved
// specified in the ServicePort.BackendConfig and applies it to the BackendService.
// It returns true if there were existing settings on the BackendService
// that were overwritten.
func EnsureAffinity(sp utils.ServicePort, be *composite.BackendService) bool {
changed := false

// Should we check if specified SessionAffinity is among the currently GCP supported values?
// For now let's forward as is to GCP, to inherit API error message (and evolutions).
// Same for TTL (should be in 0-86400 range).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this validation to pkg/backendconfig/validation.go?

That way, we can give feedback to the user before the LB starts to get created.

if sp.BackendConfig.Spec.SessionAffinity != "" && strings.Compare(sp.BackendConfig.Spec.SessionAffinity, be.SessionAffinity) != 0 {
be.SessionAffinity = sp.BackendConfig.Spec.SessionAffinity
glog.V(2).Infof("Updated SessionAffinity settings for service %v/%v.", sp.ID.Service.Namespace, sp.ID.Service.Name)
changed = true
}

if sp.BackendConfig.Spec.AffinityCookieTtlSec != nil && *sp.BackendConfig.Spec.AffinityCookieTtlSec != be.AffinityCookieTtlSec {
be.AffinityCookieTtlSec = *sp.BackendConfig.Spec.AffinityCookieTtlSec
glog.V(2).Infof("Updated AffinityCookieTtlSec settings for service %v/%v.", sp.ID.Service.Namespace, sp.ID.Service.Name)
changed = true
}

return changed
}
119 changes: 119 additions & 0 deletions pkg/backends/features/affinity_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
Copyright 2015 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package features

import (
"testing"

backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/utils"
)

var testTTL = int64(10)

func TestEnsureAffinity(t *testing.T) {
testCases := []struct {
desc string
sp utils.ServicePort
be *composite.BackendService
updateExpected bool
}{
{
desc: "affinity settings missing from spec, no update needed",
sp: utils.ServicePort{
BackendConfig: &backendconfigv1beta1.BackendConfig{
Spec: backendconfigv1beta1.BackendConfigSpec{},
},
},
be: &composite.BackendService{
SessionAffinity: "GENERATED_COOKIE",
AffinityCookieTtlSec: 10,
},
updateExpected: false,
},
{
desc: "sessionaffinity setting differing, update needed",
sp: utils.ServicePort{
BackendConfig: &backendconfigv1beta1.BackendConfig{
Spec: backendconfigv1beta1.BackendConfigSpec{
SessionAffinity: "CLIENT_IP",
},
},
},
be: &composite.BackendService{
SessionAffinity: "NONE",
},
updateExpected: true,
},
{
desc: "affinity ttl setting differing, update needed",
sp: utils.ServicePort{
BackendConfig: &backendconfigv1beta1.BackendConfig{
Spec: backendconfigv1beta1.BackendConfigSpec{
AffinityCookieTtlSec: &testTTL,
},
},
},
be: &composite.BackendService{
AffinityCookieTtlSec: 20,
},
updateExpected: true,
},
{
desc: "sessionaffinity and ttl settings differing, update needed",
sp: utils.ServicePort{
BackendConfig: &backendconfigv1beta1.BackendConfig{
Spec: backendconfigv1beta1.BackendConfigSpec{
SessionAffinity: "CLIENT_IP",
AffinityCookieTtlSec: &testTTL,
},
},
},
be: &composite.BackendService{
SessionAffinity: "NONE",
AffinityCookieTtlSec: 20,
},
updateExpected: true,
},
{
desc: "affinity settings identical, no updated needed",
sp: utils.ServicePort{
BackendConfig: &backendconfigv1beta1.BackendConfig{
Spec: backendconfigv1beta1.BackendConfigSpec{
SessionAffinity: "CLIENT_IP",
AffinityCookieTtlSec: &testTTL,
},
},
},
be: &composite.BackendService{
SessionAffinity: "CLIENT_IP",
AffinityCookieTtlSec: testTTL,
},
updateExpected: false,
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
result := EnsureAffinity(tc.sp, tc.be)
if result != tc.updateExpected {
t.Errorf("%v: expected %v but got %v", tc.desc, tc.updateExpected, result)
}
})
}
}
1 change: 1 addition & 0 deletions pkg/backends/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func (s *backendSyncer) ensureBackendService(sp utils.ServicePort) error {
needUpdate = features.EnsureIAP(sp, be) || needUpdate
needUpdate = features.EnsureTimeout(sp, be) || needUpdate
needUpdate = features.EnsureDraining(sp, be) || needUpdate
needUpdate = features.EnsureAffinity(sp, be) || needUpdate
}

if needUpdate {
Expand Down