From d82ae45a4cf7e34cf02755b7eaa6e040da590d67 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 16 Nov 2017 21:20:14 -0800 Subject: [PATCH 1/2] #55183 follow up: Reinstate admission chain composition and ns test --- .../namespace/autoprovision/admission_test.go | 8 ++++++-- .../src/k8s.io/apiserver/pkg/admission/chain.go | 14 +++++++------- .../apiserver/pkg/admission/chain_test.go | 4 ++-- .../k8s.io/apiserver/pkg/admission/metrics.go | 2 +- .../k8s.io/apiserver/pkg/admission/plugins.go | 8 ++------ .../apiserver/pkg/admission/testutil_test.go | 17 ++++++++++------- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/plugin/pkg/admission/namespace/autoprovision/admission_test.go b/plugin/pkg/admission/namespace/autoprovision/admission_test.go index 57e444b694..b0c52f63d1 100644 --- a/plugin/pkg/admission/namespace/autoprovision/admission_test.go +++ b/plugin/pkg/admission/namespace/autoprovision/admission_test.go @@ -129,15 +129,19 @@ 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(admission.NewNamedHandler("ns", handler)) - if handler.Handles(admission.Update) { - t.Errorf("expected not to handle Update") + 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 hasCreateNamespaceAction(mockClient) { t.Errorf("unexpected create namespace action") diff --git a/staging/src/k8s.io/apiserver/pkg/admission/chain.go b/staging/src/k8s.io/apiserver/pkg/admission/chain.go index 49bccc5546..ea9a43ff63 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain.go @@ -29,8 +29,8 @@ func NewChainHandler(handlers ...NamedHandler) chainAdmissionHandler { func NewNamedHandler(name string, i Interface) NamedHandler { return &pluginHandler{ - i: i, - name: name, + Interface: i, + name: name, } } @@ -49,10 +49,10 @@ func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error { func (admissionHandler chainAdmissionHandler) admit(a Attributes) error { for _, handler := range admissionHandler { - if !handler.Interface().Handles(a.GetOperation()) { + if !handler.Handles(a.GetOperation()) { continue } - if mutator, ok := handler.Interface().(MutationInterface); ok { + if mutator, ok := handler.(MutationInterface); ok { t := time.Now() err := mutator.Admit(a) Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepAdmit) @@ -74,10 +74,10 @@ func (admissionHandler chainAdmissionHandler) Validate(a Attributes) error { func (admissionHandler chainAdmissionHandler) validate(a Attributes) (err error) { for _, handler := range admissionHandler { - if !handler.Interface().Handles(a.GetOperation()) { + if !handler.Handles(a.GetOperation()) { continue } - if validator, ok := handler.Interface().(ValidationInterface); ok { + if validator, ok := handler.(ValidationInterface); ok { t := time.Now() err := validator.Validate(a) Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepValidate) @@ -92,7 +92,7 @@ func (admissionHandler chainAdmissionHandler) validate(a Attributes) (err 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.Interface().Handles(operation) { + if handler.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 622bd1e9e1..4ef9154fae 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go @@ -93,7 +93,7 @@ func TestAdmitAndValidate(t *testing.T) { t.Errorf("unexpected result of admit call: %v", accepted) } for _, h := range test.chain { - fake := h.Interface().(*FakeHandler) + fake := h.(*FakeHandler) _, shouldBeCalled := test.calls[h.Name()] if shouldBeCalled != fake.admitCalled { t.Errorf("admit handler %s not called as expected: %v", h.Name(), fake.admitCalled) @@ -120,7 +120,7 @@ func TestAdmitAndValidate(t *testing.T) { t.Errorf("unexpected result of validate call: %v\n", accepted) } for _, h := range test.chain { - fake := h.Interface().(*FakeHandler) + fake := h.(*FakeHandler) _, shouldBeCalled := test.calls[h.Name()] if shouldBeCalled != fake.validateCalled { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics.go index 22c0cef130..a42f0ed50f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics.go @@ -41,7 +41,7 @@ var ( // NamedHandler requires each admission.Interface be named, primarly for metrics tracking purposes. type NamedHandler interface { - Interface() Interface + Interface Name() string } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go index db1add66a6..36dba5b2d3 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go @@ -41,14 +41,10 @@ type Plugins struct { // pluginHandler associates name with a admission.Interface handler. type pluginHandler struct { - i Interface + Interface name string } -func (h *pluginHandler) Interface() Interface { - return h.i -} - func (h *pluginHandler) Name() string { return h.name } @@ -147,7 +143,7 @@ func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigPro return nil, err } if plugin != nil { - handler := &pluginHandler{i: plugin, name: pluginName} + handler := &pluginHandler{Interface: plugin, name: pluginName} handlers = append(handlers, handler) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go b/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go index a2f1cc980e..90e81f0538 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go @@ -28,11 +28,16 @@ import ( // methods have been called and always returns an error if admit is false. type FakeHandler struct { *Handler + name string admit bool admitCalled bool validateCalled bool } +func (h *FakeHandler) Name() string { + return h.name +} + func (h *FakeHandler) Admit(a Attributes) (err error) { h.admitCalled = true if h.admit { @@ -57,12 +62,10 @@ func makeHandler(admit bool, ops ...Operation) *FakeHandler { } func makeNamedHandler(name string, admit bool, ops ...Operation) NamedHandler { - return &pluginHandler{ - i: &FakeHandler{ - admit: admit, - Handler: NewHandler(ops...), - }, - name: name, + return &FakeHandler{ + name: name, + admit: admit, + Handler: NewHandler(ops...), } } @@ -90,7 +93,7 @@ func makeValidatingHandler(validate bool, ops ...Operation) *FakeValidatingHandl func makeValidatingNamedHandler(name string, validate bool, ops ...Operation) NamedHandler { return &pluginHandler{ - i: &FakeValidatingHandler{ + Interface: &FakeValidatingHandler{ validate: validate, Handler: NewHandler(ops...), }, From baba0c827bfddfdc56b69c88e19406966ef900a2 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 17 Nov 2017 11:49:55 +0100 Subject: [PATCH 2/2] admission: make metrics compositional and move to metrics sub-package --- .../namespace/autoprovision/admission_test.go | 2 +- .../persistentvolume/label/admission_test.go | 2 +- .../Godeps/Godeps.json | 4 + .../src/k8s.io/apiserver/pkg/admission/BUILD | 9 +- .../k8s.io/apiserver/pkg/admission/chain.go | 36 +-- .../apiserver/pkg/admission/chain_test.go | 129 ++++----- .../apiserver/pkg/admission/metrics/BUILD | 42 +++ .../pkg/admission/{ => metrics}/metrics.go | 81 +++++- .../pkg/admission/metrics/metrics_test.go | 250 ++++++++++++++++++ .../admission/{ => metrics}/testutil_test.go | 80 +----- .../apiserver/pkg/admission/metrics_test.go | 84 ------ .../admission/plugin/webhook/mutating/BUILD | 1 + .../plugin/webhook/mutating/admission.go | 3 +- .../admission/plugin/webhook/validating/BUILD | 1 + .../plugin/webhook/validating/admission.go | 3 +- .../k8s.io/apiserver/pkg/admission/plugins.go | 23 +- .../k8s.io/apiserver/pkg/server/options/BUILD | 1 + .../apiserver/pkg/server/options/admission.go | 5 +- .../k8s.io/kube-aggregator/Godeps/Godeps.json | 4 + .../sample-apiserver/Godeps/Godeps.json | 4 + 20 files changed, 454 insertions(+), 310 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/admission/metrics/BUILD rename staging/src/k8s.io/apiserver/pkg/admission/{ => metrics}/metrics.go (62%) create mode 100644 staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go rename staging/src/k8s.io/apiserver/pkg/admission/{ => metrics}/testutil_test.go (60%) delete mode 100644 staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go diff --git a/plugin/pkg/admission/namespace/autoprovision/admission_test.go b/plugin/pkg/admission/namespace/autoprovision/admission_test.go index b0c52f63d1..755906d560 100644 --- a/plugin/pkg/admission/namespace/autoprovision/admission_test.go +++ b/plugin/pkg/admission/namespace/autoprovision/admission_test.go @@ -136,7 +136,7 @@ func TestIgnoreAdmission(t *testing.T) { t.Errorf("unexpected error initializing handler: %v", err) } informerFactory.Start(wait.NeverStop) - chainHandler := admission.NewChainHandler(admission.NewNamedHandler("ns", handler)) + 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)) diff --git a/plugin/pkg/admission/persistentvolume/label/admission_test.go b/plugin/pkg/admission/persistentvolume/label/admission_test.go index ddf02e69e6..d13dcc3038 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(admission.NewNamedHandler("pv", pvHandler)) + handler := admission.NewChainHandler(pvHandler) ignoredPV := api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"}, Spec: api.PersistentVolumeSpec{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json b/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json index 03af06beb9..695ce94530 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json @@ -866,6 +866,10 @@ "ImportPath": "k8s.io/apiserver/pkg/admission/initializer", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/apiserver/pkg/admission/metrics", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/apiserver/pkg/admission/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/BUILD index 90b3fd250e..9e57234bbb 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/BUILD @@ -13,15 +13,10 @@ 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", @@ -38,15 +33,12 @@ 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", @@ -76,6 +68,7 @@ filegroup( ":package-srcs", "//staging/src/k8s.io/apiserver/pkg/admission/configuration:all-srcs", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:all-srcs", + "//staging/src/k8s.io/apiserver/pkg/admission/metrics:all-srcs", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/initialization:all-srcs", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle:all-srcs", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config:all-srcs", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/chain.go b/staging/src/k8s.io/apiserver/pkg/admission/chain.go index ea9a43ff63..011641ff06 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain.go @@ -16,46 +16,23 @@ limitations under the License. package admission -import "time" - // chainAdmissionHandler is an instance of admission.NamedHandler that performs admission control using // a chain of admission handlers -type chainAdmissionHandler []NamedHandler +type chainAdmissionHandler []Interface // NewChainHandler creates a new chain handler from an array of handlers. Used for testing. -func NewChainHandler(handlers ...NamedHandler) chainAdmissionHandler { +func NewChainHandler(handlers ...Interface) chainAdmissionHandler { return chainAdmissionHandler(handlers) } -func NewNamedHandler(name string, i Interface) NamedHandler { - return &pluginHandler{ - Interface: 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()) { continue } if mutator, ok := handler.(MutationInterface); ok { - t := time.Now() err := mutator.Admit(a) - Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepAdmit) if err != nil { return err } @@ -66,21 +43,12 @@ 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()) { continue } if validator, ok := handler.(ValidationInterface); ok { - t := time.Now() err := validator.Validate(a) - Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepValidate) if err != nil { return err } 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 4ef9154fae..e3821e731d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/chain_test.go @@ -17,12 +17,45 @@ 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) *FakeHandler { + return &FakeHandler{ + name: name, + admit: accept, + validate: accept, + Handler: NewHandler(ops...), + } +} + func TestAdmitAndValidate(t *testing.T) { sysns := metav1.NamespaceSystem otherns := "default" @@ -38,10 +71,10 @@ func TestAdmitAndValidate(t *testing.T) { name: "all accept", ns: sysns, operation: Create, - chain: []NamedHandler{ - makeNamedHandler("a", true, Update, Delete, Create), - makeNamedHandler("b", true, Delete, Create), - makeNamedHandler("c", true, Create), + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", true, Delete, Create), + makeHandler("c", true, Create), }, calls: map[string]bool{"a": true, "b": true, "c": true}, accept: true, @@ -50,10 +83,10 @@ func TestAdmitAndValidate(t *testing.T) { name: "ignore handler", ns: otherns, operation: Create, - chain: []NamedHandler{ - makeNamedHandler("a", true, Update, Delete, Create), - makeNamedHandler("b", false, Delete), - makeNamedHandler("c", true, Create), + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("c", true, Create), }, calls: map[string]bool{"a": true, "c": true}, accept: true, @@ -62,10 +95,10 @@ func TestAdmitAndValidate(t *testing.T) { name: "ignore all", ns: sysns, operation: Connect, - chain: []NamedHandler{ - makeNamedHandler("a", true, Update, Delete, Create), - makeNamedHandler("b", false, Delete), - makeNamedHandler("c", true, Create), + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("c", true, Create), }, calls: map[string]bool{}, accept: true, @@ -74,17 +107,16 @@ func TestAdmitAndValidate(t *testing.T) { name: "reject one", ns: otherns, operation: Delete, - chain: []NamedHandler{ - makeNamedHandler("a", true, Update, Delete, Create), - makeNamedHandler("b", false, Delete), - makeNamedHandler("c", true, Create), + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("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{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) @@ -94,25 +126,19 @@ func TestAdmitAndValidate(t *testing.T) { } for _, h := range test.chain { fake := h.(*FakeHandler) - _, shouldBeCalled := test.calls[h.Name()] + _, shouldBeCalled := test.calls[fake.name] if shouldBeCalled != fake.admitCalled { - t.Errorf("admit handler %s not called as expected: %v", h.Name(), fake.admitCalled) + t.Errorf("admit handler %s not called as expected: %v", fake.name, fake.admitCalled) continue } if fake.validateCalled { - t.Errorf("validate handler %s called during admit", h.Name()) + t.Errorf("validate handler %s called during admit", fake.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{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) accepted = (err == nil) @@ -122,63 +148,24 @@ func TestAdmitAndValidate(t *testing.T) { for _, h := range test.chain { fake := h.(*FakeHandler) - _, shouldBeCalled := test.calls[h.Name()] + _, shouldBeCalled := test.calls[fake.name] if shouldBeCalled != fake.validateCalled { - t.Errorf("validate handler %s not called as expected: %v", h.Name(), fake.validateCalled) + t.Errorf("validate handler %s not called as expected: %v", fake.name, fake.validateCalled) continue } if fake.admitCalled { - t.Errorf("mutating handler unexpectedly called: %s", h.Name()) + t.Errorf("mutating handler unexpectedly called: %s", fake.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{ - makeNamedHandler("a", true, Update, Delete, Create), - makeNamedHandler("b", true, Delete, Create), - makeNamedHandler("c", true, Create), + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", true, Delete, Create), + makeHandler("c", true, Create), } tests := []struct { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/metrics/BUILD new file mode 100644 index 0000000000..32feb2f9f2 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/BUILD @@ -0,0 +1,42 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["metrics.go"], + importpath = "k8s.io/apiserver/pkg/admission/metrics", + visibility = ["//visibility:public"], + deps = [ + "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = [ + "metrics_test.go", + "testutil_test.go", + ], + importpath = "k8s.io/apiserver/pkg/admission/metrics", + 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/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go similarity index 62% rename from staging/src/k8s.io/apiserver/pkg/admission/metrics.go rename to staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go index a42f0ed50f..f50dc18ab1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go @@ -14,16 +14,16 @@ See the License for the specific language governing permissions and limitations under the License. */ -package admission +package metrics import ( "fmt" "strconv" "time" - "k8s.io/api/admissionregistration/v1alpha1" - "github.com/prometheus/client_golang/prometheus" + + "k8s.io/apiserver/pkg/admission" ) const ( @@ -39,10 +39,64 @@ var ( Metrics = newAdmissionMetrics() ) -// NamedHandler requires each admission.Interface be named, primarly for metrics tracking purposes. -type NamedHandler interface { - Interface - Name() string +// ObserverFunc is a func that emits metrics. +type ObserverFunc func(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) + +const ( + stepValidate = "validate" + stepAdmit = "admit" +) + +// WithControllerMetrics is a decorator for named admission handlers. +func WithControllerMetrics(i admission.Interface, name string) admission.Interface { + return WithMetrics(i, Metrics.ObserveAdmissionController, name) +} + +// WithStepMetrics is a decorator for a whole admission phase, i.e. admit or validation.admission step. +func WithStepMetrics(i admission.Interface) admission.Interface { + return WithMetrics(i, Metrics.ObserveAdmissionStep) +} + +// WithMetrics is a decorator for admission handlers with a generic observer func. +func WithMetrics(i admission.Interface, observer ObserverFunc, extraLabels ...string) admission.Interface { + return &pluginHandlerWithMetrics{ + Interface: i, + observer: observer, + extraLabels: extraLabels, + } +} + +// pluginHandlerWithMetrics decorates a admission handler with metrics. +type pluginHandlerWithMetrics struct { + admission.Interface + observer ObserverFunc + extraLabels []string +} + +// Admit performs a mutating admission control check and emit metrics. +func (p pluginHandlerWithMetrics) Admit(a admission.Attributes) error { + mutatingHandler, ok := p.Interface.(admission.MutationInterface) + if !ok { + return nil + } + + start := time.Now() + err := mutatingHandler.Admit(a) + p.observer(time.Since(start), err != nil, a, stepAdmit, p.extraLabels...) + return err +} + +// Validate performs a non-mutating admission control check and emits metrics. +func (p pluginHandlerWithMetrics) Validate(a admission.Attributes) error { + validatingHandler, ok := p.Interface.(admission.ValidationInterface) + if !ok { + return nil + } + + start := time.Now() + err := validatingHandler.Validate(a) + p.observer(time.Since(start), err != nil, a, stepValidate, p.extraLabels...) + return err } // AdmissionMetrics instruments admission with prometheus metrics. @@ -83,22 +137,21 @@ func (m *AdmissionMetrics) 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) { +func (m *AdmissionMetrics) ObserveAdmissionStep(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) { gvr := attr.GetResource() - m.step.observe(elapsed, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected)) + m.step.observe(elapsed, append(extraLabels, 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) { +func (m *AdmissionMetrics) ObserveAdmissionController(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...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)) + m.controller.observe(elapsed, append(extraLabels, 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 +func (m *AdmissionMetrics) ObserveWebhook(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) { gvr := attr.GetResource() - m.webhook.observe(elapsed, hook.Name, t, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected)) + m.webhook.observe(elapsed, append(extraLabels, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))...) } type metricSet struct { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go new file mode 100644 index 0000000000..d2f7fae411 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go @@ -0,0 +1,250 @@ +/* +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 metrics + +import ( + "fmt" + "testing" + "time" + + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" +) + +var ( + kind = schema.GroupVersionKind{Group: "kgroup", Version: "kversion", Kind: "kind"} + resource = schema.GroupVersionResource{Group: "rgroup", Version: "rversion", Resource: "resource"} + attr = admission.NewAttributesRecord(nil, nil, kind, "ns", "name", resource, "subresource", admission.Create, nil) +) + +func TestObserveAdmissionStep(t *testing.T) { + Metrics.reset() + handler := WithStepMetrics(&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create), true, true}) + handler.(admission.MutationInterface).Admit(attr) + handler.(admission.ValidationInterface).Validate(attr) + wantLabels := map[string]string{ + "operation": string(admission.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) + + wantLabels["type"] = "validate" + 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 := WithControllerMetrics(&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create), true, true}, "a") + handler.(admission.MutationInterface).Admit(attr) + handler.(admission.ValidationInterface).Validate(attr) + wantLabels := map[string]string{ + "name": "a", + "operation": string(admission.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) + + wantLabels["type"] = "validate" + 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() + Metrics.ObserveWebhook(2*time.Second, false, attr, stepAdmit, "x") + wantLabels := map[string]string{ + "name": "x", + "operation": string(admission.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) +} + +func TestWithMetrics(t *testing.T) { + Metrics.reset() + + type Test struct { + name string + ns string + operation admission.Operation + handler admission.Interface + admit, validate bool + } + for _, test := range []Test{ + { + "both-interfaces-admit-and-validate", + "some-ns", + admission.Create, + &mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true, true}, + true, true, + }, + { + "both-interfaces-dont-admit", + "some-ns", + admission.Create, + &mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), false, true}, + false, true, + }, + { + "both-interfaces-admit-dont-validate", + "some-ns", + admission.Create, + &mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true, false}, + true, false, + }, + { + "validate-interfaces-validate", + "some-ns", + admission.Create, + &validatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true}, + true, true, + }, + { + "validate-interfaces-dont-validate", + "some-ns", + admission.Create, + &validatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true}, + true, false, + }, + { + "mutating-interfaces-admit", + "some-ns", + admission.Create, + &mutatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true}, + true, true, + }, + { + "mutating-interfaces-dont-admit", + "some-ns", + admission.Create, + &mutatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), false}, + true, false, + }, + } { + Metrics.reset() + + h := WithMetrics(test.handler, Metrics.ObserveAdmissionController, test.name) + + // test mutation + err := h.(admission.MutationInterface).Admit(admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) + if test.admit && err != nil { + t.Errorf("expected admit to succeed, but failed: %v", err) + continue + } else if !test.admit && err == nil { + t.Errorf("expected admit to fail, but it succeeded") + continue + } + + filter := map[string]string{"rejected": "false"} + if !test.admit { + filter["rejected"] = "true" + } + if _, mutating := test.handler.(admission.MutationInterface); mutating { + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 1) + } else { + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 0) + } + + if err == nil { + // skip validation step if mutation failed + continue + } + + // test validation + err = h.(admission.ValidationInterface).Validate(admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil)) + if test.validate && err != nil { + t.Errorf("expected admit to succeed, but failed: %v", err) + continue + } else if !test.validate && err == nil { + t.Errorf("expected validation to fail, but it succeeded") + continue + } + + filter = map[string]string{"rejected": "false"} + if !test.admit { + filter["rejected"] = "true" + } + if _, validating := test.handler.(admission.ValidationInterface); validating { + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 1) + } else { + expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 0) + } + } +} + +type mutatingAndValidatingFakeHandler struct { + *admission.Handler + admit bool + validate bool +} + +func (h *mutatingAndValidatingFakeHandler) Admit(a admission.Attributes) (err error) { + if h.admit { + return nil + } + return fmt.Errorf("don't admit") +} + +func (h *mutatingAndValidatingFakeHandler) Validate(a admission.Attributes) (err error) { + if h.validate { + return nil + } + return fmt.Errorf("don't validate") +} + +type validatingFakeHandler struct { + *admission.Handler + validate bool +} + +func (h *validatingFakeHandler) Validate(a admission.Attributes) (err error) { + if h.validate { + return nil + } + return fmt.Errorf("don't validate") +} + +type mutatingFakeHandler struct { + *admission.Handler + admit bool +} + +func (h *mutatingFakeHandler) Amit(a admission.Attributes) (err error) { + if h.admit { + return nil + } + return fmt.Errorf("don't admit") +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/testutil_test.go similarity index 60% rename from staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go rename to staging/src/k8s.io/apiserver/pkg/admission/metrics/testutil_test.go index 90e81f0538..af5ee79a8b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/testutil_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/testutil_test.go @@ -14,93 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package admission +package metrics 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 - name string - admit bool - admitCalled bool - validateCalled bool -} - -func (h *FakeHandler) Name() string { - return h.name -} - -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 &FakeHandler{ - name: name, - admit: admit, - Handler: NewHandler(ops...), - } -} - -// 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{ - Interface: &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 { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go deleted file mode 100644 index 264d3adb76..0000000000 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go +++ /dev/null @@ -1,84 +0,0 @@ -/* -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/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index d31e651152..440e23b1b7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD @@ -23,6 +23,7 @@ go_library( "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go index 64527b4963..e8e1f2dbb0 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go @@ -39,6 +39,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/configuration" genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" + admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" "k8s.io/apiserver/pkg/admission/plugin/webhook/config" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" "k8s.io/apiserver/pkg/admission/plugin/webhook/namespace" @@ -238,7 +239,7 @@ func (a *MutatingWebhook) Admit(attr admission.Attributes) error { for _, hook := range relevantHooks { t := time.Now() err := a.callAttrMutatingHook(ctx, hook, versionedAttr) - admission.Metrics.ObserveWebhook(time.Since(t), err != nil, hook, attr) + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr, "admit", hook.Name) if err == nil { continue } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index 111242176a..cf2890efd4 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -21,6 +21,7 @@ go_library( "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go index 4911777c64..53aacdef1c 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go @@ -38,6 +38,7 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/configuration" genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" + admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" "k8s.io/apiserver/pkg/admission/plugin/webhook/config" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" "k8s.io/apiserver/pkg/admission/plugin/webhook/namespace" @@ -238,7 +239,7 @@ func (a *ValidatingAdmissionWebhook) Admit(attr admission.Attributes) error { t := time.Now() err := a.callHook(ctx, hook, versionedAttr) - admission.Metrics.ObserveWebhook(time.Since(t), err != nil, hook, attr) + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr, "validating", hook.Name) 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 36dba5b2d3..3639df3da6 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go @@ -39,16 +39,6 @@ type Plugins struct { registry map[string]Factory } -// pluginHandler associates name with a admission.Interface handler. -type pluginHandler struct { - Interface - name string -} - -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. @@ -128,10 +118,12 @@ func splitStream(config io.Reader) (io.Reader, io.Reader, error) { return bytes.NewBuffer(configBytes), bytes.NewBuffer(configBytes), nil } +type Decorator func(handler Interface, name string) Interface + // 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) { - handlers := []NamedHandler{} +func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigProvider, pluginInitializer PluginInitializer, decorator Decorator) (Interface, error) { + handlers := []Interface{} for _, pluginName := range pluginNames { pluginConfig, err := configProvider.ConfigFor(pluginName) if err != nil { @@ -143,8 +135,11 @@ func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigPro return nil, err } if plugin != nil { - handler := &pluginHandler{Interface: plugin, name: pluginName} - handlers = append(handlers, handler) + if decorator != nil { + handlers = append(handlers, decorator(plugin, pluginName)) + } else { + handlers = append(handlers, plugin) + } } } return chainAdmissionHandler(handlers), nil diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/BUILD b/staging/src/k8s.io/apiserver/pkg/server/options/BUILD index 33ae317972..3fc9df50f2 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/options/BUILD @@ -30,6 +30,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/initialization:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go index b6abf70c46..97764cf0f5 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/initializer" + admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" "k8s.io/apiserver/pkg/admission/plugin/initialization" "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" @@ -105,12 +106,12 @@ func (a *AdmissionOptions) ApplyTo( pluginInitializers = append(pluginInitializers, genericInitializer) initializersChain = append(initializersChain, pluginInitializers...) - admissionChain, err := a.Plugins.NewFromPlugins(pluginNames, pluginsConfigProvider, initializersChain) + admissionChain, err := a.Plugins.NewFromPlugins(pluginNames, pluginsConfigProvider, initializersChain, admissionmetrics.WithControllerMetrics) if err != nil { return err } - c.AdmissionControl = admissionChain + c.AdmissionControl = admissionmetrics.WithStepMetrics(admissionChain) return nil } diff --git a/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json b/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json index a8c6facc23..f247c14d01 100644 --- a/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json +++ b/staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json @@ -834,6 +834,10 @@ "ImportPath": "k8s.io/apiserver/pkg/admission/initializer", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/apiserver/pkg/admission/metrics", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json b/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json index 361834574e..84cb089a44 100644 --- a/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json @@ -830,6 +830,10 @@ "ImportPath": "k8s.io/apiserver/pkg/admission/initializer", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/apiserver/pkg/admission/metrics", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"