diff --git a/plugin/pkg/admission/namespace/autoprovision/admission_test.go b/plugin/pkg/admission/namespace/autoprovision/admission_test.go index 755906d560d..57e444b6946 100644 --- a/plugin/pkg/admission/namespace/autoprovision/admission_test.go +++ b/plugin/pkg/admission/namespace/autoprovision/admission_test.go @@ -129,19 +129,15 @@ func TestAdmissionNamespaceExists(t *testing.T) { // TestIgnoreAdmission validates that a request is ignored if its not a create func TestIgnoreAdmission(t *testing.T) { - namespace := "test" mockClient := newMockClientForTest([]string{}) handler, informerFactory, err := newHandlerForTest(mockClient) if err != nil { t.Errorf("unexpected error initializing handler: %v", err) } informerFactory.Start(wait.NeverStop) - chainHandler := admission.NewChainHandler(handler) - pod := newPod(namespace) - err = chainHandler.Admit(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil)) - if err != nil { - t.Errorf("unexpected error returned from admission handler") + if handler.Handles(admission.Update) { + t.Errorf("expected not to handle Update") } if hasCreateNamespaceAction(mockClient) { t.Errorf("unexpected create namespace action") diff --git a/plugin/pkg/admission/persistentvolume/label/admission_test.go b/plugin/pkg/admission/persistentvolume/label/admission_test.go index d13dcc30384..ddf02e69e6d 100644 --- a/plugin/pkg/admission/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/persistentvolume/label/admission_test.go @@ -78,7 +78,7 @@ func mockVolumeLabels(labels map[string]string) *mockVolumes { // TestAdmission func TestAdmission(t *testing.T) { pvHandler := NewPersistentVolumeLabel() - handler := admission.NewChainHandler(pvHandler) + handler := admission.NewChainHandler(admission.NewNamedHandler("pv", pvHandler)) ignoredPV := api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"}, Spec: api.PersistentVolumeSpec{ diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 55ca3d0e8e4..d5167718647 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -35,13 +35,10 @@ import ( ) func TestIgnoresNonCreate(t *testing.T) { - pod := &api.Pod{} for _, op := range []admission.Operation{admission.Delete, admission.Connect} { - attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "myns", "myname", api.Resource("pods").WithVersion("version"), "", op, nil) - handler := admission.NewChainHandler(NewServiceAccount()) - err := handler.Admit(attrs) - if err != nil { - t.Errorf("Expected %s operation allowed, got err: %v", op, err) + handler := NewServiceAccount() + if handler.Handles(op) { + t.Errorf("Expected not to handle operation %s", op) } } } @@ -50,7 +47,7 @@ func TestIgnoresUpdateOfInitializedPod(t *testing.T) { pod := &api.Pod{} oldPod := &api.Pod{} attrs := admission.NewAttributesRecord(pod, oldPod, api.Kind("Pod").WithVersion("version"), "myns", "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, nil) - handler := admission.NewChainHandler(NewServiceAccount()) + handler := NewServiceAccount() err := handler.Admit(attrs) if err != nil { t.Errorf("Expected update of initialized pod allowed, got err: %v", err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/BUILD index 3d8927eb3d9..0102e5b38c7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/BUILD @@ -13,10 +13,16 @@ go_test( "config_test.go", "errors_test.go", "handler_test.go", + "metrics_test.go", + "testutil_test.go", ], importpath = "k8s.io/apiserver/pkg/admission", library = ":go_default_library", deps = [ + "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", + "//vendor/github.com/prometheus/client_model/go:go_default_library", + "//vendor/k8s.io/api/admissionregistration/v1alpha1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/apiserver:go_default_library", @@ -32,12 +38,15 @@ go_library( "errors.go", "handler.go", "interfaces.go", + "metrics.go", "plugins.go", ], importpath = "k8s.io/apiserver/pkg/admission", deps = [ "//vendor/github.com/ghodss/yaml:go_default_library", "//vendor/github.com/golang/glog:go_default_library", + "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", + "//vendor/k8s.io/api/admissionregistration/v1alpha1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apimachinery/announced:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/chain.go b/staging/src/k8s.io/apiserver/pkg/admission/chain.go index 65c7a526187..49bccc55467 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain.go @@ -16,22 +16,46 @@ limitations under the License. package admission -// chainAdmissionHandler is an instance of admission.Interface that performs admission control using a chain of admission handlers -type chainAdmissionHandler []Interface +import "time" + +// chainAdmissionHandler is an instance of admission.NamedHandler that performs admission control using +// a chain of admission handlers +type chainAdmissionHandler []NamedHandler // NewChainHandler creates a new chain handler from an array of handlers. Used for testing. -func NewChainHandler(handlers ...Interface) chainAdmissionHandler { +func NewChainHandler(handlers ...NamedHandler) chainAdmissionHandler { return chainAdmissionHandler(handlers) } +func NewNamedHandler(name string, i Interface) NamedHandler { + return &pluginHandler{ + i: i, + name: name, + } +} + +const ( + stepValidate = "validate" + stepAdmit = "admit" +) + // Admit performs an admission control check using a chain of handlers, and returns immediately on first error func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error { + start := time.Now() + err := admissionHandler.admit(a) + Metrics.ObserveAdmissionStep(time.Since(start), err != nil, a, stepAdmit) + return err +} + +func (admissionHandler chainAdmissionHandler) admit(a Attributes) error { for _, handler := range admissionHandler { - if !handler.Handles(a.GetOperation()) { + if !handler.Interface().Handles(a.GetOperation()) { continue } - if mutator, ok := handler.(MutationInterface); ok { + if mutator, ok := handler.Interface().(MutationInterface); ok { + t := time.Now() err := mutator.Admit(a) + Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepAdmit) if err != nil { return err } @@ -42,12 +66,21 @@ func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error { // Validate performs an admission control check using a chain of handlers, and returns immediately on first error func (admissionHandler chainAdmissionHandler) Validate(a Attributes) error { + start := time.Now() + err := admissionHandler.validate(a) + Metrics.ObserveAdmissionStep(time.Since(start), err != nil, a, stepValidate) + return err +} + +func (admissionHandler chainAdmissionHandler) validate(a Attributes) (err error) { for _, handler := range admissionHandler { - if !handler.Handles(a.GetOperation()) { + if !handler.Interface().Handles(a.GetOperation()) { continue } - if validator, ok := handler.(ValidationInterface); ok { + if validator, ok := handler.Interface().(ValidationInterface); ok { + t := time.Now() err := validator.Validate(a) + Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepValidate) if err != nil { return err } @@ -59,7 +92,7 @@ func (admissionHandler chainAdmissionHandler) Validate(a Attributes) error { // Handles will return true if any of the handlers handles the given operation func (admissionHandler chainAdmissionHandler) Handles(operation Operation) bool { for _, handler := range admissionHandler { - if handler.Handles(operation) { + if handler.Interface().Handles(operation) { return true } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go b/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go index faab4c87517..622bd1e9e10 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go @@ -17,47 +17,18 @@ limitations under the License. package admission import ( - "fmt" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) -type FakeHandler struct { - *Handler - name string - admit, admitCalled bool - validate, validateCalled bool -} - -func (h *FakeHandler) Admit(a Attributes) (err error) { - h.admitCalled = true - if h.admit { - return nil - } - return fmt.Errorf("Don't admit") -} - -func (h *FakeHandler) Validate(a Attributes) (err error) { - h.validateCalled = true - if h.validate { - return nil - } - return fmt.Errorf("Don't validate") -} - -func makeHandler(name string, accept bool, ops ...Operation) Interface { - return &FakeHandler{ - name: name, - admit: accept, - validate: accept, - Handler: NewHandler(ops...), - } -} - func TestAdmitAndValidate(t *testing.T) { + sysns := metav1.NamespaceSystem + otherns := "default" tests := []struct { name string + ns string operation Operation chain chainAdmissionHandler accept bool @@ -65,97 +36,149 @@ func TestAdmitAndValidate(t *testing.T) { }{ { name: "all accept", + ns: sysns, operation: Create, - chain: []Interface{ - makeHandler("a", true, Update, Delete, Create), - makeHandler("b", true, Delete, Create), - makeHandler("c", true, Create), + chain: []NamedHandler{ + makeNamedHandler("a", true, Update, Delete, Create), + makeNamedHandler("b", true, Delete, Create), + makeNamedHandler("c", true, Create), }, calls: map[string]bool{"a": true, "b": true, "c": true}, accept: true, }, { name: "ignore handler", + ns: otherns, operation: Create, - chain: []Interface{ - makeHandler("a", true, Update, Delete, Create), - makeHandler("b", false, Delete), - makeHandler("c", true, Create), + chain: []NamedHandler{ + makeNamedHandler("a", true, Update, Delete, Create), + makeNamedHandler("b", false, Delete), + makeNamedHandler("c", true, Create), }, calls: map[string]bool{"a": true, "c": true}, accept: true, }, { name: "ignore all", + ns: sysns, operation: Connect, - chain: []Interface{ - makeHandler("a", true, Update, Delete, Create), - makeHandler("b", false, Delete), - makeHandler("c", true, Create), + chain: []NamedHandler{ + makeNamedHandler("a", true, Update, Delete, Create), + makeNamedHandler("b", false, Delete), + makeNamedHandler("c", true, Create), }, calls: map[string]bool{}, accept: true, }, { name: "reject one", + ns: otherns, operation: Delete, - chain: []Interface{ - makeHandler("a", true, Update, Delete, Create), - makeHandler("b", false, Delete), - makeHandler("c", true, Create), + chain: []NamedHandler{ + makeNamedHandler("a", true, Update, Delete, Create), + makeNamedHandler("b", false, Delete), + makeNamedHandler("c", true, Create), }, calls: map[string]bool{"a": true, "b": true}, accept: false, }, } for _, test := range tests { + Metrics.reset() + t.Logf("testcase = %s", test.name) // call admit and check that validate was not called at all - err := test.chain.Admit(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "", schema.GroupVersionResource{}, "", test.operation, nil)) + err := test.chain.Admit(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) accepted := (err == nil) if accepted != test.accept { - t.Errorf("%s: unexpected result of admit call: %v\n", test.name, accepted) + t.Errorf("unexpected result of admit call: %v", accepted) } for _, h := range test.chain { - fake := h.(*FakeHandler) - _, shouldBeCalled := test.calls[fake.name] + fake := h.Interface().(*FakeHandler) + _, shouldBeCalled := test.calls[h.Name()] if shouldBeCalled != fake.admitCalled { - t.Errorf("%s: admit handler %s not called as expected: %v", test.name, fake.name, fake.admitCalled) + t.Errorf("admit handler %s not called as expected: %v", h.Name(), fake.admitCalled) continue } if fake.validateCalled { - t.Errorf("%s: validate handler %s called during admit", test.name, fake.name) + t.Errorf("validate handler %s called during admit", h.Name()) } // reset value for validation test fake.admitCalled = false } + labelFilter := map[string]string{ + "type": "admit", + } + + checkAdmitAndValidateMetrics(t, labelFilter, test.accept, test.calls) + Metrics.reset() // call validate and check that admit was not called at all - err = test.chain.Validate(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "", schema.GroupVersionResource{}, "", test.operation, nil)) + err = test.chain.Validate(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) accepted = (err == nil) if accepted != test.accept { - t.Errorf("%s: unexpected result of validate call: %v\n", test.name, accepted) + t.Errorf("unexpected result of validate call: %v\n", accepted) } for _, h := range test.chain { - fake := h.(*FakeHandler) - _, shouldBeCalled := test.calls[fake.name] + fake := h.Interface().(*FakeHandler) + + _, shouldBeCalled := test.calls[h.Name()] if shouldBeCalled != fake.validateCalled { - t.Errorf("%s: validate handler %s not called as expected: %v", test.name, fake.name, fake.validateCalled) + t.Errorf("validate handler %s not called as expected: %v", h.Name(), fake.validateCalled) continue } if fake.admitCalled { - t.Errorf("%s: admit handler %s called during admit", test.name, fake.name) + t.Errorf("mutating handler unexpectedly called: %s", h.Name()) } } + + labelFilter = map[string]string{ + "type": "validate", + } + + checkAdmitAndValidateMetrics(t, labelFilter, test.accept, test.calls) + } +} + +func checkAdmitAndValidateMetrics(t *testing.T, labelFilter map[string]string, accept bool, calls map[string]bool) { + acceptFilter := map[string]string{"rejected": "false"} + for k, v := range labelFilter { + acceptFilter[k] = v + } + + rejectFilter := map[string]string{"rejected": "true"} + for k, v := range labelFilter { + rejectFilter[k] = v + } + + if accept { + // Ensure exactly one admission end-to-end admission accept should have been recorded. + expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", acceptFilter, 1) + + // Ensure the expected count of admission controllers have been executed. + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", acceptFilter, len(calls)) + } else { + // When not accepted, ensure exactly one end-to-end rejection has been recorded. + expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", rejectFilter, 1) + if len(calls) > 0 { + if len(calls) > 1 { + // When not accepted, ensure that all but the last controller had been accepted, since + // the chain stops execution at the first rejection. + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", acceptFilter, len(calls)-1) + } + + // When not accepted, ensure exactly one controller has been rejected. + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", rejectFilter, 1) + } } } func TestHandles(t *testing.T) { chain := chainAdmissionHandler{ - makeHandler("a", true, Update, Delete, Create), - makeHandler("b", true, Delete, Create), - makeHandler("c", true, Create), + makeNamedHandler("a", true, Update, Delete, Create), + makeNamedHandler("b", true, Delete, Create), + makeNamedHandler("c", true, Create), } tests := []struct { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics.go new file mode 100644 index 00000000000..22c0cef130d --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics.go @@ -0,0 +1,151 @@ +/* +Copyright 2017 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 admission + +import ( + "fmt" + "strconv" + "time" + + "k8s.io/api/admissionregistration/v1alpha1" + + "github.com/prometheus/client_golang/prometheus" +) + +const ( + namespace = "apiserver" + subsystem = "admission" +) + +var ( + latencyBuckets = prometheus.ExponentialBuckets(25000, 2.0, 7) + latencySummaryMaxAge = 5 * time.Hour + + // Metrics provides access to all admission metrics. + Metrics = newAdmissionMetrics() +) + +// NamedHandler requires each admission.Interface be named, primarly for metrics tracking purposes. +type NamedHandler interface { + Interface() Interface + Name() string +} + +// AdmissionMetrics instruments admission with prometheus metrics. +type AdmissionMetrics struct { + step *metricSet + controller *metricSet + webhook *metricSet +} + +// newAdmissionMetrics create a new AdmissionMetrics, configured with default metric names. +func newAdmissionMetrics() *AdmissionMetrics { + // Admission metrics for a step of the admission flow. The entire admission flow is broken down into a series of steps + // Each step is identified by a distinct type label value. + step := newMetricSet("step", + []string{"type", "operation", "group", "version", "resource", "subresource", "rejected"}, + "Admission sub-step %s, broken out for each operation and API resource and step type (validate or admit).") + + // Built-in admission controller metrics. Each admission controller is identified by name. + controller := newMetricSet("controller", + []string{"name", "type", "operation", "group", "version", "resource", "subresource", "rejected"}, + "Admission controller %s, identified by name and broken out for each operation and API resource and type (validate or admit).") + + // Admission webhook metrics. Each webhook is identified by name. + webhook := newMetricSet("webhook", + []string{"name", "type", "operation", "group", "version", "resource", "subresource", "rejected"}, + "Admission webhook %s, identified by name and broken out for each operation and API resource and type (validate or admit).") + + step.mustRegister() + controller.mustRegister() + webhook.mustRegister() + return &AdmissionMetrics{step: step, controller: controller, webhook: webhook} +} + +func (m *AdmissionMetrics) reset() { + m.step.reset() + m.controller.reset() + m.webhook.reset() +} + +// ObserveAdmissionStep records admission related metrics for a admission step, identified by step type. +func (m *AdmissionMetrics) ObserveAdmissionStep(elapsed time.Duration, rejected bool, attr Attributes, stepType string) { + gvr := attr.GetResource() + m.step.observe(elapsed, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected)) +} + +// ObserveAdmissionController records admission related metrics for a built-in admission controller, identified by it's plugin handler name. +func (m *AdmissionMetrics) ObserveAdmissionController(elapsed time.Duration, rejected bool, handler NamedHandler, attr Attributes, stepType string) { + gvr := attr.GetResource() + m.controller.observe(elapsed, handler.Name(), stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected)) +} + +// ObserveWebhook records admission related metrics for a admission webhook. +func (m *AdmissionMetrics) ObserveWebhook(elapsed time.Duration, rejected bool, hook *v1alpha1.Webhook, attr Attributes) { + t := "admit" // TODO: pass in type (validate|admit) once mutating webhook functionality has been implemented + gvr := attr.GetResource() + m.webhook.observe(elapsed, hook.Name, t, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected)) +} + +type metricSet struct { + latencies *prometheus.HistogramVec + latenciesSummary *prometheus.SummaryVec +} + +func newMetricSet(name string, labels []string, helpTemplate string) *metricSet { + return &metricSet{ + latencies: prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: fmt.Sprintf("%s_admission_latencies_seconds", name), + Help: fmt.Sprintf(helpTemplate, "latency histogram"), + Buckets: latencyBuckets, + }, + labels, + ), + latenciesSummary: prometheus.NewSummaryVec( + prometheus.SummaryOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: fmt.Sprintf("%s_admission_latencies_seconds_summary", name), + Help: fmt.Sprintf(helpTemplate, "latency summary"), + MaxAge: latencySummaryMaxAge, + }, + labels, + ), + } +} + +// MustRegister registers all the prometheus metrics in the metricSet. +func (m *metricSet) mustRegister() { + prometheus.MustRegister(m.latencies) + prometheus.MustRegister(m.latenciesSummary) +} + +// Reset resets all the prometheus metrics in the metricSet. +func (m *metricSet) reset() { + m.latencies.Reset() + m.latenciesSummary.Reset() +} + +// Observe records an observed admission event to all metrics in the metricSet. +func (m *metricSet) observe(elapsed time.Duration, labels ...string) { + elapsedMicroseconds := float64(elapsed / time.Microsecond) + m.latencies.WithLabelValues(labels...).Observe(elapsedMicroseconds) + m.latenciesSummary.WithLabelValues(labels...).Observe(elapsedMicroseconds) +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go new file mode 100644 index 00000000000..264d3adb765 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go @@ -0,0 +1,84 @@ +/* +Copyright 2017 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 admission + +import ( + "testing" + "time" + + "k8s.io/api/admissionregistration/v1alpha1" + + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var ( + kind = schema.GroupVersionKind{Group: "kgroup", Version: "kversion", Kind: "kind"} + resource = schema.GroupVersionResource{Group: "rgroup", Version: "rversion", Resource: "resource"} + attr = NewAttributesRecord(nil, nil, kind, "ns", "name", resource, "subresource", Create, nil) +) + +func TestObserveAdmissionStep(t *testing.T) { + Metrics.reset() + Metrics.ObserveAdmissionStep(2*time.Second, false, attr, "admit") + wantLabels := map[string]string{ + "operation": string(Create), + "group": resource.Group, + "version": resource.Version, + "resource": resource.Resource, + "subresource": "subresource", + "type": "admit", + "rejected": "false", + } + expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", wantLabels, 1) + expectFindMetric(t, "apiserver_admission_step_admission_latencies_seconds_summary", wantLabels) +} + +func TestObserveAdmissionController(t *testing.T) { + Metrics.reset() + handler := makeValidatingNamedHandler("a", true, Create) + Metrics.ObserveAdmissionController(2*time.Second, false, handler, attr, "validate") + wantLabels := map[string]string{ + "name": "a", + "operation": string(Create), + "group": resource.Group, + "version": resource.Version, + "resource": resource.Resource, + "subresource": "subresource", + "type": "validate", + "rejected": "false", + } + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", wantLabels, 1) + expectFindMetric(t, "apiserver_admission_controller_admission_latencies_seconds_summary", wantLabels) +} + +func TestObserveWebhook(t *testing.T) { + Metrics.reset() + hook := &v1alpha1.Webhook{Name: "x"} + Metrics.ObserveWebhook(2*time.Second, false, hook, attr) + wantLabels := map[string]string{ + "name": "x", + "operation": string(Create), + "group": resource.Group, + "version": resource.Version, + "resource": resource.Resource, + "subresource": "subresource", + "type": "admit", + "rejected": "false", + } + expectHistogramCountTotal(t, "apiserver_admission_webhook_admission_latencies_seconds", wantLabels, 1) + expectFindMetric(t, "apiserver_admission_webhook_admission_latencies_seconds_summary", wantLabels) +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go index 737009068c1..4461372188d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go @@ -26,6 +26,7 @@ import ( "net" "net/url" "sync" + "time" "github.com/golang/glog" lru "github.com/hashicorp/golang-lru" @@ -306,7 +307,9 @@ func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error { go func(hook *v1alpha1.Webhook) { defer wg.Done() + t := time.Now() err := a.callHook(ctx, hook, versionedAttr) + admission.Metrics.ObserveWebhook(time.Since(t), err != nil, hook, attr) if err == nil { return } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go index 172ac337b34..db1add66a63 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go @@ -39,6 +39,20 @@ type Plugins struct { registry map[string]Factory } +// pluginHandler associates name with a admission.Interface handler. +type pluginHandler struct { + i Interface + name string +} + +func (h *pluginHandler) Interface() Interface { + return h.i +} + +func (h *pluginHandler) Name() string { + return h.name +} + // All registered admission options. var ( // PluginEnabledFn checks whether a plugin is enabled. By default, if you ask about it, it's enabled. @@ -121,7 +135,7 @@ func splitStream(config io.Reader) (io.Reader, io.Reader, error) { // NewFromPlugins returns an admission.Interface that will enforce admission control decisions of all // the given plugins. func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigProvider, pluginInitializer PluginInitializer) (Interface, error) { - plugins := []Interface{} + handlers := []NamedHandler{} for _, pluginName := range pluginNames { pluginConfig, err := configProvider.ConfigFor(pluginName) if err != nil { @@ -133,10 +147,11 @@ func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigPro return nil, err } if plugin != nil { - plugins = append(plugins, plugin) + handler := &pluginHandler{i: plugin, name: pluginName} + handlers = append(handlers, handler) } } - return chainAdmissionHandler(plugins), nil + return chainAdmissionHandler(handlers), nil } // InitPlugin creates an instance of the named interface. diff --git a/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go b/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go new file mode 100644 index 00000000000..a2f1cc980e2 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2017 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 admission + +import ( + "fmt" + "testing" + + "github.com/prometheus/client_golang/prometheus" + ptype "github.com/prometheus/client_model/go" +) + +// FakeHandler provide a mock implement both MutationInterface and ValidationInterface that tracks which +// methods have been called and always returns an error if admit is false. +type FakeHandler struct { + *Handler + admit bool + admitCalled bool + validateCalled bool +} + +func (h *FakeHandler) Admit(a Attributes) (err error) { + h.admitCalled = true + if h.admit { + return nil + } + return fmt.Errorf("Don't admit") +} + +func (h *FakeHandler) Validate(a Attributes) (err error) { + h.validateCalled = true + if h.admit { + return nil + } + return fmt.Errorf("Don't admit") +} + +func makeHandler(admit bool, ops ...Operation) *FakeHandler { + return &FakeHandler{ + admit: admit, + Handler: NewHandler(ops...), + } +} + +func makeNamedHandler(name string, admit bool, ops ...Operation) NamedHandler { + return &pluginHandler{ + i: &FakeHandler{ + admit: admit, + Handler: NewHandler(ops...), + }, + name: name, + } +} + +// FakeValidatingHandler provide a mock of ValidationInterface that tracks which +// methods have been called and always returns an error if validate is false. +type FakeValidatingHandler struct { + *Handler + validate, validateCalled bool +} + +func (h *FakeValidatingHandler) Validate(a Attributes) (err error) { + h.validateCalled = true + if h.validate { + return nil + } + return fmt.Errorf("Don't validate") +} + +func makeValidatingHandler(validate bool, ops ...Operation) *FakeValidatingHandler { + return &FakeValidatingHandler{ + validate: validate, + Handler: NewHandler(ops...), + } +} + +func makeValidatingNamedHandler(name string, validate bool, ops ...Operation) NamedHandler { + return &pluginHandler{ + i: &FakeValidatingHandler{ + validate: validate, + Handler: NewHandler(ops...), + }, + name: name, + } +} + +func labelsMatch(metric *ptype.Metric, labelFilter map[string]string) bool { + for _, lp := range metric.GetLabel() { + if value, ok := labelFilter[lp.GetName()]; ok && lp.GetValue() != value { + return false + } + } + return true +} + +// expectFindMetric find a metric with the given name nad labels or reports a fatal test error. +func expectFindMetric(t *testing.T, name string, expectedLabels map[string]string) *ptype.Metric { + metrics, err := prometheus.DefaultGatherer.Gather() + if err != nil { + t.Fatalf("Failed to gather metrics: %s", err) + } + + for _, mf := range metrics { + if mf.GetName() == name { + for _, metric := range mf.GetMetric() { + if labelsMatch(metric, expectedLabels) { + gotLabelCount := len(metric.GetLabel()) + wantLabelCount := len(expectedLabels) + if wantLabelCount != gotLabelCount { + t.Errorf("Got metric with %d labels, but wanted %d labels. Wanted %#+v for %s", + gotLabelCount, wantLabelCount, expectedLabels, metric.String()) + } + return metric + } + } + } + } + t.Fatalf("No metric found with name %s and labels %#+v", name, expectedLabels) + return nil +} + +// expectHistogramCountTotal ensures that the sum of counts of metrics matching the labelFilter is as +// expected. +func expectHistogramCountTotal(t *testing.T, name string, labelFilter map[string]string, wantCount int) { + metrics, err := prometheus.DefaultGatherer.Gather() + if err != nil { + t.Fatalf("Failed to gather metrics: %s", err) + } + + counterSum := 0 + for _, mf := range metrics { + if mf.GetName() != name { + continue // Ignore other metrics. + } + for _, metric := range mf.GetMetric() { + if !labelsMatch(metric, labelFilter) { + continue + } + counterSum += int(metric.GetHistogram().GetSampleCount()) + } + } + if wantCount != counterSum { + t.Errorf("Wanted count %d, got %d for metric %s with labels %#+v", wantCount, counterSum, name, labelFilter) + for _, mf := range metrics { + if mf.GetName() == name { + for _, metric := range mf.GetMetric() { + t.Logf("\tnear match: %s", metric.String()) + } + } + } + } +}