From 811447ea0a9928c326965f3bbd8195826b20daf8 Mon Sep 17 00:00:00 2001 From: Di Xu Date: Thu, 28 Sep 2017 17:14:29 +0800 Subject: [PATCH] avoid kubelet converts and validates pods multiple times --- pkg/kubelet/config/BUILD | 1 - pkg/kubelet/config/common.go | 7 +++--- pkg/kubelet/config/config.go | 37 +++++++------------------------ pkg/kubelet/config/config_test.go | 4 +++- 4 files changed, 14 insertions(+), 35 deletions(-) diff --git a/pkg/kubelet/config/BUILD b/pkg/kubelet/config/BUILD index defc45f72d3..1456828cc1b 100644 --- a/pkg/kubelet/config/BUILD +++ b/pkg/kubelet/config/BUILD @@ -42,7 +42,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/yaml:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", diff --git a/pkg/kubelet/config/common.go b/pkg/kubelet/config/common.go index 8203efa57ce..afe17622f45 100644 --- a/pkg/kubelet/config/common.go +++ b/pkg/kubelet/config/common.go @@ -118,8 +118,7 @@ func tryDecodeSinglePod(data []byte, defaultFn defaultFunc) (parsed bool, pod *v newPod, ok := obj.(*api.Pod) // Check whether the object could be converted to single pod. if !ok { - err = fmt.Errorf("invalid pod: %#v", obj) - return false, pod, err + return false, pod, fmt.Errorf("invalid pod: %#v", obj) } // Apply default values and validate the pod. @@ -127,11 +126,11 @@ func tryDecodeSinglePod(data []byte, defaultFn defaultFunc) (parsed bool, pod *v return true, pod, err } if errs := validation.ValidatePod(newPod); len(errs) > 0 { - err = fmt.Errorf("invalid pod: %v", errs) - return true, pod, err + return true, pod, fmt.Errorf("invalid pod: %v", errs) } v1Pod := &v1.Pod{} if err := k8s_api_v1.Convert_api_Pod_To_v1_Pod(newPod, v1Pod, nil); err != nil { + glog.Errorf("Pod %q failed to convert to v1", newPod.Name) return true, nil, err } return true, v1Pod, nil diff --git a/pkg/kubelet/config/config.go b/pkg/kubelet/config/config.go index ed7869dfb01..ca322d45f3c 100644 --- a/pkg/kubelet/config/config.go +++ b/pkg/kubelet/config/config.go @@ -25,11 +25,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/tools/record" - "k8s.io/kubernetes/pkg/api" - k8s_api_v1 "k8s.io/kubernetes/pkg/api/v1" - "k8s.io/kubernetes/pkg/api/validation" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/events" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" @@ -323,34 +319,17 @@ func (s *podStorage) seenSources(sources ...string) bool { func filterInvalidPods(pods []*v1.Pod, source string, recorder record.EventRecorder) (filtered []*v1.Pod) { names := sets.String{} for i, pod := range pods { - var errlist field.ErrorList - // TODO: remove the conversion when validation is performed on versioned objects. - internalPod := &api.Pod{} - if err := k8s_api_v1.Convert_v1_Pod_To_api_Pod(pod, internalPod, nil); err != nil { - glog.Warningf("Pod[%d] (%s) from %s failed to convert to v1, ignoring: %v", i+1, format.Pod(pod), source, err) - recorder.Eventf(pod, v1.EventTypeWarning, "FailedConversion", "Error converting pod %s from %s, ignoring: %v", format.Pod(pod), source, err) + // Pods from each source are assumed to have passed validation individually. + // This function only checks if there is any naming conflict. + name := kubecontainer.GetPodFullName(pod) + if names.Has(name) { + glog.Warningf("Pod[%d] (%s) from %s failed validation due to duplicate pod name %q, ignoring", i+1, format.Pod(pod), source, pod.Name) + recorder.Eventf(pod, v1.EventTypeWarning, events.FailedValidation, "Error validating pod %s from %s due to duplicate pod name %q, ignoring", format.Pod(pod), source, pod.Name) continue - } - if errs := validation.ValidatePod(internalPod); len(errs) != 0 { - errlist = append(errlist, errs...) - // If validation fails, don't trust it any further - - // even Name could be bad. } else { - name := kubecontainer.GetPodFullName(pod) - if names.Has(name) { - // TODO: when validation becomes versioned, this gets a bit - // more complicated. - errlist = append(errlist, field.Duplicate(field.NewPath("metadata", "name"), pod.Name)) - } else { - names.Insert(name) - } - } - if len(errlist) > 0 { - err := errlist.ToAggregate() - glog.Warningf("Pod[%d] (%s) from %s failed validation, ignoring: %v", i+1, format.Pod(pod), source, err) - recorder.Eventf(pod, v1.EventTypeWarning, events.FailedValidation, "Error validating pod %s from %s, ignoring: %v", format.Pod(pod), source, err) - continue + names.Insert(name) } + filtered = append(filtered, pod) } return diff --git a/pkg/kubelet/config/config_test.go b/pkg/kubelet/config/config_test.go index 1e93e5f2ab0..deb1f4dcaf3 100644 --- a/pkg/kubelet/config/config_test.go +++ b/pkg/kubelet/config/config_test.go @@ -146,8 +146,10 @@ func TestNewPodAddedInvalidNamespace(t *testing.T) { // see an update podUpdate := CreatePodUpdate(kubetypes.ADD, TestSource, CreateValidPod("foo", "")) channel <- podUpdate + expectPodUpdate(t, ch, CreatePodUpdate(kubetypes.ADD, TestSource, CreateValidPod("foo", ""))) + config.Sync() - expectPodUpdate(t, ch, CreatePodUpdate(kubetypes.SET, kubetypes.AllSource)) + expectPodUpdate(t, ch, CreatePodUpdate(kubetypes.SET, kubetypes.AllSource, CreateValidPod("foo", ""))) } func TestNewPodAddedDefaultNamespace(t *testing.T) {