From 389dd0a499e4fa79d3d2ef4261aa9f25aa94e6b0 Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Wed, 26 Feb 2020 15:58:57 -0800 Subject: [PATCH] Don't log "SHOULD NOT HAPPEN" errors more than once per second --- .../handlers/fieldmanager/internal/BUILD | 2 + .../fieldmanager/internal/atmostevery.go | 60 +++++++++++++++++++ .../fieldmanager/internal/atmostevery_test.go | 51 ++++++++++++++++ .../handlers/fieldmanager/structuredmerge.go | 9 ++- 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery.go create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/BUILD index 74d40c4f1a2..313a92f2b7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/BUILD @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "atmostevery.go", "conflict.go", "fields.go", "gvkparser.go", @@ -33,6 +34,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "atmostevery_test.go", "conflict_test.go", "fields_test.go", "managedfields_test.go", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery.go new file mode 100644 index 00000000000..b75ef7416e7 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery.go @@ -0,0 +1,60 @@ +/* +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 internal + +import ( + "sync" + "time" +) + +// AtMostEvery will never run the method more than once every specified +// duration. +type AtMostEvery struct { + delay time.Duration + lastCall time.Time + mutex sync.Mutex +} + +// NewAtMostEvery creates a new AtMostEvery, that will run the method at +// most every given duration. +func NewAtMostEvery(delay time.Duration) *AtMostEvery { + return &AtMostEvery{ + delay: delay, + } +} + +// updateLastCall returns true if the lastCall time has been updated, +// false if it was too early. +func (s *AtMostEvery) updateLastCall() bool { + s.mutex.Lock() + defer s.mutex.Unlock() + if time.Since(s.lastCall) < s.delay { + return false + } + s.lastCall = time.Now() + return true +} + +// Do will run the method if enough time has passed, and return true. +// Otherwise, it does nothing and returns false. +func (s *AtMostEvery) Do(fn func()) bool { + if !s.updateLastCall() { + return false + } + fn() + return true +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery_test.go new file mode 100644 index 00000000000..46fe4f7e867 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/atmostevery_test.go @@ -0,0 +1,51 @@ +/* +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 internal_test + +import ( + "testing" + "time" + + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal" +) + +func TestAtMostEvery(t *testing.T) { + duration := time.Second + delay := 179 * time.Millisecond + atMostEvery := internal.NewAtMostEvery(delay) + count := 0 + exit := time.NewTicker(duration) + tick := time.NewTicker(2 * time.Millisecond) + defer exit.Stop() + defer tick.Stop() + + done := false + for !done { + select { + case <-exit.C: + done = true + case <-tick.C: + atMostEvery.Do(func() { + count++ + }) + } + } + + if expected := int(duration/delay) + 1; count != expected { + t.Fatalf("Function called %d times, should have been called exactly %d times", count, expected) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/structuredmerge.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/structuredmerge.go index 4f61099039e..5cb4a723e32 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/structuredmerge.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/structuredmerge.go @@ -42,6 +42,7 @@ type structuredMergeManager struct { } var _ Manager = &structuredMergeManager{} +var atMostEverySecond = internal.NewAtMostEvery(time.Second) // NewStructuredMergeManager creates a new Manager that merges apply requests // and update managed fields for other types of requests. @@ -99,13 +100,17 @@ func (f *structuredMergeManager) Update(liveObj, newObj runtime.Object, managed newObjTyped, err := f.typeConverter.ObjectToTyped(newObjVersioned) if err != nil { // Return newObj and just by-pass fields update. This really shouldn't happen. - klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object of type %v: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err) + atMostEverySecond.Do(func() { + klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object of type %v: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err) + }) return newObj, managed, nil } liveObjTyped, err := f.typeConverter.ObjectToTyped(liveObjVersioned) if err != nil { // Return newObj and just by-pass fields update. This really shouldn't happen. - klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object of type %v: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err) + atMostEverySecond.Do(func() { + klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object of type %v: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err) + }) return newObj, managed, nil } apiVersion := fieldpath.APIVersion(f.groupVersion.String())