diff --git a/pkg/backendconfig/validation.go b/pkg/backendconfig/validation.go index f711380477..f5a0d63d6b 100644 --- a/pkg/backendconfig/validation.go +++ b/pkg/backendconfig/validation.go @@ -48,6 +48,10 @@ func Validate(kubeClient kubernetes.Interface, beConfig *backendconfigv1.Backend return err } + if err := validateLogging(beConfig); err != nil { + return err + } + return nil } @@ -104,3 +108,16 @@ func validateSessionAffinity(kubeClient kubernetes.Interface, beConfig *backendc return nil } + +func validateLogging(beConfig *backendconfigv1.BackendConfig) error { + if beConfig.Spec.Logging == nil || beConfig.Spec.Logging.SampleRate == nil { + return nil + } + + if *beConfig.Spec.Logging.SampleRate < 0.0 || *beConfig.Spec.Logging.SampleRate > 1.0 { + return fmt.Errorf("unsupported SampleRate: %f, should be between 0.0 and 1.0", + *beConfig.Spec.Logging.SampleRate) + } + + return nil +} diff --git a/pkg/backendconfig/validation_test.go b/pkg/backendconfig/validation_test.go index ab1792f076..ce569f1c84 100644 --- a/pkg/backendconfig/validation_test.go +++ b/pkg/backendconfig/validation_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" + testutils "k8s.io/ingress-gce/pkg/test" ) var ( @@ -239,3 +240,90 @@ func TestValidateSessionAffinity(t *testing.T) { } } } + +func TestValidateLogging(t *testing.T) { + for _, tc := range []struct { + desc string + beConfig *backendconfigv1.BackendConfig + expectError bool + }{ + { + desc: "nil access log config", + beConfig: &backendconfigv1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1.BackendConfigSpec{}, + }, + expectError: false, + }, + { + desc: "empty access log config", + beConfig: &backendconfigv1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{}, + }, + }, + expectError: false, + }, + { + desc: "invalid sample rate", + beConfig: &backendconfigv1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: false, + SampleRate: testutils.Float64ToPtr(1.01), + }, + }, + }, + expectError: true, + }, + { + desc: "valid sample rate", + beConfig: &backendconfigv1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + expectError: false, + }, + { + desc: "valid integer sample rate", + beConfig: &backendconfigv1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(1), + }, + }, + }, + expectError: false, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + err := Validate(kubeClient, tc.beConfig) + if tc.expectError && err == nil { + t.Errorf("Expected error but got nil") + } + if !tc.expectError && err != nil { + t.Errorf("Did not expect error but got: %v", err) + } + }) + } +} diff --git a/pkg/backends/features/logging.go b/pkg/backends/features/logging.go new file mode 100644 index 0000000000..2886ce1023 --- /dev/null +++ b/pkg/backends/features/logging.go @@ -0,0 +1,61 @@ +/* +Copyright 2020 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 ( + "fmt" + + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/klog" +) + +// EnsureLogging reads the log configurations 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 EnsureLogging(sp utils.ServicePort, be *composite.BackendService) bool { + if sp.BackendConfig.Spec.Logging == nil { + return false + } + svcKey := fmt.Sprintf("%s/%s", sp.ID.Service.Namespace, sp.ID.Service.Name) + expectedLogConfig := expectedBackendServiceLogConfig(sp) + if !expectedLogConfig.Enable && !be.LogConfig.Enable { + klog.V(3).Infof("Logging continues to stay disabled for service %s, skipping update", svcKey) + return false + } + if be.LogConfig == nil || expectedLogConfig.Enable != be.LogConfig.Enable || + expectedLogConfig.SampleRate != be.LogConfig.SampleRate { + be.LogConfig = expectedLogConfig + klog.V(2).Infof("Updated Logging settings for service %s.", svcKey) + return true + } + return false +} + +// expectedBackendServiceLogConfig returns composite.BackendServiceLogConfig for +// the access log settings specified in the BackendConfig to the passed in +// composite.BackendService. +func expectedBackendServiceLogConfig(sp utils.ServicePort) *composite.BackendServiceLogConfig { + logConfig := &composite.BackendServiceLogConfig{ + Enable: sp.BackendConfig.Spec.Logging.Enable, + } + if !sp.BackendConfig.Spec.Logging.Enable || sp.BackendConfig.Spec.Logging.SampleRate == nil { + return logConfig + } + logConfig.SampleRate = *sp.BackendConfig.Spec.Logging.SampleRate + return logConfig +} diff --git a/pkg/backends/features/logging_test.go b/pkg/backends/features/logging_test.go new file mode 100644 index 0000000000..5c7c002a19 --- /dev/null +++ b/pkg/backends/features/logging_test.go @@ -0,0 +1,272 @@ +/* +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" + + "github.com/google/go-cmp/cmp" + backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" + "k8s.io/ingress-gce/pkg/composite" + testutils "k8s.io/ingress-gce/pkg/test" + "k8s.io/ingress-gce/pkg/utils" +) + +func TestEnsureLogging(t *testing.T) { + for _, tc := range []struct { + desc string + sp utils.ServicePort + be *composite.BackendService + expectUpdate bool + }{ + { + desc: "logging configurations are missing from spec, no update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: nil, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 0.5, + }, + }, + expectUpdate: false, + }, + { + desc: "empty log config, updated to disable logging", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{}, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 0.5, + }, + }, + expectUpdate: true, + }, + { + desc: "settings are identical, no update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 0.5, + }, + }, + expectUpdate: false, + }, + { + desc: "logging is disabled but config is different, no update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: false, + SampleRate: testutils.Float64ToPtr(0.4), + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: false, + SampleRate: 1.0, + }, + }, + expectUpdate: false, + }, + { + desc: "no existing settings, update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: nil, + }, + expectUpdate: true, + }, + { + desc: "sample rate is different, update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 0.2, + }, + }, + expectUpdate: true, + }, + { + desc: "enable logging, update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: false, + }, + }, + expectUpdate: true, + }, + { + desc: "disable logging, update needed", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: false, + }, + }, + }, + }, + be: &composite.BackendService{ + LogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 1.0, + }, + }, + expectUpdate: true, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + hasUpdated := EnsureLogging(tc.sp, tc.be) + if hasUpdated != tc.expectUpdate { + t.Errorf("%v: expected %v but got %v", tc.desc, tc.expectUpdate, hasUpdated) + } + }) + } +} + +func TestExpectedBackendServiceLogConfig(t *testing.T) { + for _, tc := range []struct { + desc string + sp utils.ServicePort + expectLogConfig *composite.BackendServiceLogConfig + }{ + { + desc: "empty log config", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{}, + }, + }, + }, + expectLogConfig: &composite.BackendServiceLogConfig{ + Enable: false, + }, + }, + { + desc: "logging disabled", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: false, + SampleRate: testutils.Float64ToPtr(0.5), + }, + }, + }, + }, + expectLogConfig: &composite.BackendServiceLogConfig{ + Enable: false, + }, + }, + { + desc: "logging enabled", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(0.4), + }, + }, + }, + }, + expectLogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 0.4, + }, + }, + { + desc: "logging enabled, invalid sample rate", + sp: utils.ServicePort{ + BackendConfig: &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{ + Logging: &backendconfigv1.LogConfig{ + Enable: true, + SampleRate: testutils.Float64ToPtr(1.4), + }, + }, + }, + }, + expectLogConfig: &composite.BackendServiceLogConfig{ + Enable: true, + SampleRate: 1.4, + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + gotLogConfig := expectedBackendServiceLogConfig(tc.sp) + if diff := cmp.Diff(tc.expectLogConfig, gotLogConfig); diff != "" { + t.Errorf("expectedBackendServiceLogConfig(%#v) mismatch (-want +got):\n%s", tc.sp, diff) + } + }) + } +} diff --git a/pkg/backends/syncer.go b/pkg/backends/syncer.go index 2c980e25fc..d5e01108fd 100644 --- a/pkg/backends/syncer.go +++ b/pkg/backends/syncer.go @@ -109,6 +109,7 @@ func (s *backendSyncer) ensureBackendService(sp utils.ServicePort) error { needUpdate = features.EnsureDraining(sp, be) || needUpdate needUpdate = features.EnsureAffinity(sp, be) || needUpdate needUpdate = features.EnsureCustomRequestHeaders(sp, be) || needUpdate + needUpdate = features.EnsureLogging(sp, be) || needUpdate } if needUpdate { diff --git a/pkg/test/utils.go b/pkg/test/utils.go index d82298a331..f09448dda1 100644 --- a/pkg/test/utils.go +++ b/pkg/test/utils.go @@ -236,3 +236,8 @@ func CheckEvent(recorder *record.FakeRecorder, expected string, shouldMatch bool return nil } } + +// Float64ToPtr returns float ptr for given float. +func Float64ToPtr(val float64) *float64 { + return &val +}