Admission metrics

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")

plugin/pkg/admission/persistentvolume/label/admission_test.go

@@ -77,8 +77,7 @@ func mockVolumeLabels(labels map[string]string) *mockVolumes {
 
 // TestAdmission
 func TestAdmission(t *testing.T) {
-	pvHandler := NewPersistentVolumeLabel()
-	handler := admission.NewChainHandler(pvHandler)
+	handler := NewPersistentVolumeLabel()
 	ignoredPV := api.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"},
 		Spec: api.PersistentVolumeSpec{
@@ -107,21 +106,20 @@ func TestAdmission(t *testing.T) {
 	}
 
 	// We only add labels on creation
-	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Delete, nil))
-	if err != nil {
-		t.Errorf("Unexpected error returned from admission handler (when deleting aws pv):  %v", err)
+	if handler.Handles(admission.Delete) {
+		t.Errorf("Expected to only handle create")
 	}
 
 	// Errors from the cloudprovider block creation of the volume
-	pvHandler.ebsVolumes = mockVolumeFailure(fmt.Errorf("invalid volume"))
+	handler.ebsVolumes = mockVolumeFailure(fmt.Errorf("invalid volume"))
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, nil))
 	if err == nil {
 		t.Errorf("Expected error when aws pv info fails")
 	}
 
 	// Don't add labels if the cloudprovider doesn't return any
 	labels := make(map[string]string)
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	handler.ebsVolumes = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")
@@ -131,7 +129,7 @@ func TestAdmission(t *testing.T) {
 	}
 
 	// Don't panic if the cloudprovider returns nil, nil
-	pvHandler.ebsVolumes = mockVolumeFailure(nil)
+	handler.ebsVolumes = mockVolumeFailure(nil)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, nil))
 	if err != nil {
 		t.Errorf("Expected no error when cloud provider returns empty labels")
@@ -141,7 +139,7 @@ func TestAdmission(t *testing.T) {
 	labels = make(map[string]string)
 	labels["a"] = "1"
 	labels["b"] = "2"
-	pvHandler.ebsVolumes = mockVolumeLabels(labels)
+	handler.ebsVolumes = mockVolumeLabels(labels)
 	err = handler.Admit(admission.NewAttributesRecord(&awsPV, nil, api.Kind("PersistentVolume").WithVersion("version"), awsPV.Namespace, awsPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, nil))
 	if err != nil {
 		t.Errorf("Expected no error when creating aws pv")

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)

staging/src/k8s.io/apiserver/pkg/admission/BUILD

@@ -16,6 +16,9 @@ go_test(
     importpath = "k8s.io/apiserver/pkg/admission",
     library = ":go_default_library",
     deps = [
+        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/github.com/stretchr/testify/require: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",
@@ -37,10 +40,12 @@ go_library(
     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/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",
         "//vendor/k8s.io/apimachinery/pkg/apimachinery/registered: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/apimachinery/pkg/runtime/serializer:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/chain.go

@@ -16,23 +16,66 @@ limitations under the License.
 
 package admission
 
+import (
+	"strconv"
+
+	"github.com/prometheus/client_golang/prometheus"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+const (
+	metricNamespace = "apiserver"
+	metricSubsystem = "admission"
+)
+
+var (
+	handleCounter = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: metricNamespace,
+			Subsystem: metricSubsystem,
+			Name:      "handle_total",
+			Help:      "Counter of all calls to Admit.",
+		},
+		[]string{"is_system_ns"})
+	rejectCounter = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: metricNamespace,
+			Subsystem: metricSubsystem,
+			Name:      "reject_total",
+			Help:      "Counter of all errors returned during admission.",
+		},
+		[]string{"plugin", "is_system_ns"})
+)
+
+func init() {
+	prometheus.MustRegister(handleCounter)
+	prometheus.MustRegister(rejectCounter)
+}
+
 // chainAdmissionHandler is an instance of admission.Interface that performs admission control using a chain of admission handlers
-type chainAdmissionHandler []Interface
+type chainAdmissionHandler []namedAdmissionHandler
+
+type namedAdmissionHandler struct {
+	name string
+	Interface
+}
 
-// NewChainHandler creates a new chain handler from an array of handlers. Used for testing.
-func NewChainHandler(handlers ...Interface) chainAdmissionHandler {
-	return chainAdmissionHandler(handlers)
+func (admissionHandler chainAdmissionHandler) Append(name string, handler Interface) chainAdmissionHandler {
+	return append(admissionHandler, namedAdmissionHandler{name, handler})
 }
 
 // Admit performs an admission control check using a chain of handlers, and returns immediately on first error
 func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error {
+	handleCounter.WithLabelValues(isSystemNsLabel(a)).Inc()
 	for _, handler := range admissionHandler {
 		if !handler.Handles(a.GetOperation()) {
 			continue
 		}
-		if mutator, ok := handler.(MutationInterface); ok {
+		if mutator, ok := handler.Interface.(MutationInterface); ok {
 			err := mutator.Admit(a)
 			if err != nil {
+				rejectCounter.WithLabelValues(handler.name, isSystemNsLabel(a)).Inc()
 				return err
 			}
 		}
@@ -46,7 +89,7 @@ func (admissionHandler chainAdmissionHandler) Validate(a Attributes) error {
 		if !handler.Handles(a.GetOperation()) {
 			continue
 		}
-		if validator, ok := handler.(ValidationInterface); ok {
+		if validator, ok := handler.Interface.(ValidationInterface); ok {
 			err := validator.Validate(a)
 			if err != nil {
 				return err
@@ -65,3 +108,8 @@ func (admissionHandler chainAdmissionHandler) Handles(operation Operation) bool
 	}
 	return false
 }
+
+// Returns the value to use for the `is_system_ns` metric label.
+func isSystemNsLabel(a Attributes) string {
+	return strconv.FormatBool(a.GetNamespace() == metav1.NamespaceSystem)
+}

staging/src/k8s.io/apiserver/pkg/admission/chain_test.go

@@ -18,8 +18,14 @@ package admission
 
 import (
 	"fmt"
+	"strconv"
+	"strings"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
@@ -46,26 +52,39 @@ func (h *FakeHandler) Validate(a Attributes) (err error) {
 	return fmt.Errorf("Don't admit")
 }
 
-func makeHandler(name string, admit bool, ops ...Operation) Interface {
+func makeHandler(name string, admit bool, ops ...Operation) *FakeHandler {
 	return &FakeHandler{
 		name:    name,
 		admit:   admit,
 		Handler: NewHandler(ops...),
 	}
 }
 
+func makeChain(handlers ...*FakeHandler) chainAdmissionHandler {
+	chain := chainAdmissionHandler{}
+	for _, fh := range handlers {
+		chain = chain.Append(fh.name, fh)
+	}
+	return chain
+}
+
 func TestAdmit(t *testing.T) {
+	sysns := "kube-system"
+	otherns := "default"
 	tests := []struct {
 		name      string
+		ns        string
 		operation Operation
-		chain     chainAdmissionHandler
+		chain     []*FakeHandler
 		accept    bool
+		reject    string
 		calls     map[string]bool
 	}{
 		{
 			name:      "all accept",
+			ns:        sysns,
 			operation: Create,
-			chain: []Interface{
+			chain: []*FakeHandler{
 				makeHandler("a", true, Update, Delete, Create),
 				makeHandler("b", true, Delete, Create),
 				makeHandler("c", true, Create),
@@ -75,8 +94,9 @@ func TestAdmit(t *testing.T) {
 		},
 		{
 			name:      "ignore handler",
+			ns:        otherns,
 			operation: Create,
-			chain: []Interface{
+			chain: []*FakeHandler{
 				makeHandler("a", true, Update, Delete, Create),
 				makeHandler("b", false, Delete),
 				makeHandler("c", true, Create),
@@ -86,8 +106,9 @@ func TestAdmit(t *testing.T) {
 		},
 		{
 			name:      "ignore all",
+			ns:        sysns,
 			operation: Connect,
-			chain: []Interface{
+			chain: []*FakeHandler{
 				makeHandler("a", true, Update, Delete, Create),
 				makeHandler("b", false, Delete),
 				makeHandler("c", true, Create),
@@ -97,39 +118,44 @@ func TestAdmit(t *testing.T) {
 		},
 		{
 			name:      "reject one",
+			ns:        otherns,
 			operation: Delete,
-			chain: []Interface{
+			chain: []*FakeHandler{
 				makeHandler("a", true, Update, Delete, Create),
 				makeHandler("b", false, Delete),
 				makeHandler("c", true, Create),
 			},
 			calls:  map[string]bool{"a": true, "b": true},
 			accept: false,
+			reject: "b",
 		},
 	}
 	for _, test := range tests {
-		err := test.chain.Admit(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "", schema.GroupVersionResource{}, "", test.operation, nil))
+		t.Logf("testcase = %s", test.name)
+		chain := makeChain(test.chain...)
+		resetMetrics()
+		err := 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)
+		for _, fake := range test.chain {
 			_, shouldBeCalled := test.calls[fake.name]
 			if shouldBeCalled != fake.admitCalled {
-				t.Errorf("%s: handler %s not called as expected: %v", test.name, fake.name, fake.admitCalled)
+				t.Errorf("handler %s not called as expected: %v", fake.name, fake.admitCalled)
 				continue
 			}
 		}
+		expectMetrics(t, test.reject, test.ns == sysns)
 	}
 }
 
 func TestHandles(t *testing.T) {
-	chain := chainAdmissionHandler{
+	chain := makeChain(
 		makeHandler("a", true, Update, Delete, Create),
 		makeHandler("b", true, Delete, Create),
 		makeHandler("c", true, Create),
-	}
+	)
 
 	tests := []struct {
 		name      string
@@ -160,3 +186,44 @@ func TestHandles(t *testing.T) {
 		}
 	}
 }
+
+// Expect these metrics to be incremented after a single call to Admit.
+func expectMetrics(t *testing.T, reject string, systemNs bool) {
+	metrics, err := prometheus.DefaultGatherer.Gather()
+	require.NoError(t, err)
+
+	for _, mf := range metrics {
+		if !strings.HasPrefix(mf.GetName(), "apiserver_admission_") {
+			continue // Ignore other metrics.
+		}
+		for _, metric := range mf.GetMetric() {
+			for _, lp := range metric.GetLabel() {
+				switch lp.GetName() {
+				case "is_system_ns":
+					assert.Equal(t, strconv.FormatBool(systemNs), lp.GetValue(), "metric=%s", mf.GetName())
+				case "plugin":
+					assert.Equal(t, reject, lp.GetValue(), "metric=%s", mf.GetName())
+				default:
+					t.Errorf("Unexpected metric label %s on %s", lp.GetName(), mf.GetName())
+				}
+			}
+			switch mf.GetName() {
+			case "apiserver_admission_handle_total":
+				assert.EqualValues(t, 1, metric.GetCounter().GetValue())
+			case "apiserver_admission_reject_total":
+				if reject == "" {
+					t.Errorf("Unexpected reject")
+				}
+				assert.EqualValues(t, 1, metric.GetCounter().GetValue())
+			default:
+				t.Errorf("Unexpected metric: %s", mf.GetName())
+				continue
+			}
+		}
+	}
+}
+
+func resetMetrics() {
+	handleCounter.Reset()
+	rejectCounter.Reset()
+}