update podtolerations admission to mutate and validate separately

plugin/pkg/admission/podtolerationrestriction/admission.go

@@ -77,19 +77,7 @@ type podTolerationsPlugin struct {
 // scheduler.alpha.kubernetes.io/defaultTolerations and scheduler.alpha.kubernetes.io/tolerationsWhitelist
 // annotations keys.
 func (p *podTolerationsPlugin) Admit(a admission.Attributes) error {
-	resource := a.GetResource().GroupResource()
+	if shouldIgnore(a) {
-	if resource != api.Resource("pods") {
-		return nil
-	}
-	if a.GetSubresource() != "" {
-		// only run the checks below on pods proper and not subresources
-		return nil
-	}
-
-	obj := a.GetObject()
-	pod, ok := obj.(*api.Pod)
-	if !ok {
-		glog.Errorf("expected pod but got %s", a.GetKind().Kind)
 		return nil
 	}
 
@@ -97,28 +85,14 @@ func (p *podTolerationsPlugin) Admit(a admission.Attributes) error {
 		return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request"))
 	}
 
-	nsName := a.GetNamespace()
+	pod := a.GetObject().(*api.Pod)
-	namespace, err := p.namespaceLister.Get(nsName)
-	if errors.IsNotFound(err) {
-		// in case of latency in our caches, make a call direct to storage to verify that it truly exists or not
-		namespace, err = p.client.Core().Namespaces().Get(nsName, metav1.GetOptions{})
-		if err != nil {
-			if errors.IsNotFound(err) {
-				return err
-			}
-			return errors.NewInternalError(err)
-		}
-	} else if err != nil {
-		return errors.NewInternalError(err)
-	}
-
 	var finalTolerations []api.Toleration
 	updateUninitialized, err := util.IsUpdatingUninitializedObject(a)
 	if err != nil {
 		return err
 	}
 	if a.GetOperation() == admission.Create || updateUninitialized {
-		ts, err := p.getNamespaceDefaultTolerations(namespace)
+		ts, err := p.getNamespaceDefaultTolerations(a.GetNamespace())
 		if err != nil {
 			return err
 		}
@@ -148,9 +122,32 @@ func (p *podTolerationsPlugin) Admit(a admission.Attributes) error {
 		finalTolerations = pod.Spec.Tolerations
 	}
 
+	if qoshelper.GetPodQOS(pod) != api.PodQOSBestEffort {
+		finalTolerations = tolerations.MergeTolerations(finalTolerations, []api.Toleration{
+			{
+				Key:      algorithm.TaintNodeMemoryPressure,
+				Operator: api.TolerationOpExists,
+				Effect:   api.TaintEffectNoSchedule,
+			},
+		})
+	}
+	pod.Spec.Tolerations = finalTolerations
+
+	return p.Validate(a)
+}
+func (p *podTolerationsPlugin) Validate(a admission.Attributes) error {
+	if shouldIgnore(a) {
+		return nil
+	}
+
+	if !p.WaitForReady() {
+		return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request"))
+	}
+
 	// whitelist verification.
-	if len(finalTolerations) > 0 {
+	pod := a.GetObject().(*api.Pod)
-		whitelist, err := p.getNamespaceTolerationsWhitelist(namespace)
+	if len(pod.Spec.Tolerations) > 0 {
+		whitelist, err := p.getNamespaceTolerationsWhitelist(a.GetNamespace())
 		if err != nil {
 			return err
 		}
@@ -163,25 +160,33 @@ func (p *podTolerationsPlugin) Admit(a admission.Attributes) error {
 
 		if len(whitelist) > 0 {
 			// check if the merged pod tolerations satisfy its namespace whitelist
-			if !tolerations.VerifyAgainstWhitelist(finalTolerations, whitelist) {
+			if !tolerations.VerifyAgainstWhitelist(pod.Spec.Tolerations, whitelist) {
 				return fmt.Errorf("pod tolerations (possibly merged with namespace default tolerations) conflict with its namespace whitelist")
 			}
 		}
 	}
 
-	if qoshelper.GetPodQOS(pod) != api.PodQOSBestEffort {
+	return nil
-		finalTolerations = tolerations.MergeTolerations(finalTolerations, []api.Toleration{
+}
-			{
+
-				Key:      algorithm.TaintNodeMemoryPressure,
+func shouldIgnore(a admission.Attributes) bool {
-				Operator: api.TolerationOpExists,
+	resource := a.GetResource().GroupResource()
-				Effect:   api.TaintEffectNoSchedule,
+	if resource != api.Resource("pods") {
-			},
+		return true
-		})
+	}
+	if a.GetSubresource() != "" {
+		// only run the checks below on pods proper and not subresources
+		return true
 	}
 
-	pod.Spec.Tolerations = finalTolerations
+	obj := a.GetObject()
-	return nil
+	_, ok := obj.(*api.Pod)
+	if !ok {
+		glog.Errorf("expected pod but got %s", a.GetKind().Kind)
+		return true
+	}
 
+	return false
 }
 
 func NewPodTolerationsPlugin(pluginConfig *pluginapi.Configuration) *podTolerationsPlugin {
@@ -212,11 +217,38 @@ func (p *podTolerationsPlugin) ValidateInitialization() error {
 	return nil
 }
 
-func (p *podTolerationsPlugin) getNamespaceDefaultTolerations(ns *api.Namespace) ([]api.Toleration, error) {
+// in exceptional cases, this can result in two live calls, but once the cache catches up, that will stop.
+func (p *podTolerationsPlugin) getNamespace(nsName string) (*api.Namespace, error) {
+	namespace, err := p.namespaceLister.Get(nsName)
+	if errors.IsNotFound(err) {
+		// in case of latency in our caches, make a call direct to storage to verify that it truly exists or not
+		namespace, err = p.client.Core().Namespaces().Get(nsName, metav1.GetOptions{})
+		if err != nil {
+			if errors.IsNotFound(err) {
+				return nil, err
+			}
+			return nil, errors.NewInternalError(err)
+		}
+	} else if err != nil {
+		return nil, errors.NewInternalError(err)
+	}
+
+	return namespace, nil
+}
+
+func (p *podTolerationsPlugin) getNamespaceDefaultTolerations(nsName string) ([]api.Toleration, error) {
+	ns, err := p.getNamespace(nsName)
+	if err != nil {
+		return nil, err
+	}
 	return extractNSTolerations(ns, NSDefaultTolerations)
 }
 
-func (p *podTolerationsPlugin) getNamespaceTolerationsWhitelist(ns *api.Namespace) ([]api.Toleration, error) {
+func (p *podTolerationsPlugin) getNamespaceTolerationsWhitelist(nsName string) ([]api.Toleration, error) {
+	ns, err := p.getNamespace(nsName)
+	if err != nil {
+		return nil, err
+	}
 	return extractNSTolerations(ns, NSWLTolerations)
 }