Skip to content

Commit

Permalink
fix(backend): Caching - Only send cache-enabled pods to the caching w…
Browse files Browse the repository at this point in the history
…ebhook (kubeflow#4429)

* Backend - Caching - Only send cache-enabled pods to the caching webhook

The caching webhook already checks whether the pod is cache-enabled, but this change makes the check happen sooner - even before calling the webhook.
This way the webhook cannot possibly affect any non-KFP pods.

This feature requires API v1 and Kubernetes v1.15, so we use it conditionally.

* Support filtering on Kubernetes v1.15 as well
  • Loading branch information
Ark-kun authored and Bobgy committed Sep 4, 2020
1 parent 9c0bacb commit fbd3f48
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 1 deletion.
22 changes: 22 additions & 0 deletions backend/src/cache/deployer/cache-webhook-config.v1.yaml.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: cache-webhook-${NAMESPACE}
webhooks:
- name: cache-server.${NAMESPACE}.svc
clientConfig:
service:
name: cache-server
namespace: ${NAMESPACE}
path: "/mutate"
caBundle: ${CA_BUNDLE}
rules:
- operations: [ "CREATE" ]
apiGroups: [""]
apiVersions: ["v1"]
resources: ["pods"]
timeoutSeconds: 5
objectSelector:
matchLabels:
pipelines.kubeflow.org/cache_enabled: "true"

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: admissionregistration.k8s.io/v1beta1
kind: MutatingWebhookConfiguration
metadata:
name: cache-webhook-${NAMESPACE}
webhooks:
- name: cache-server.${NAMESPACE}.svc
clientConfig:
service:
name: cache-server
namespace: ${NAMESPACE}
path: "/mutate"
caBundle: ${CA_BUNDLE}
rules:
- operations: [ "CREATE" ]
apiGroups: [""]
apiVersions: ["v1"]
resources: ["pods"]
timeoutSeconds: 5
objectSelector:
matchLabels:
pipelines.kubeflow.org/cache_enabled: "true"

13 changes: 12 additions & 1 deletion backend/src/cache/deployer/deploy-cache-service.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,18 @@ touch ${CA_FILE}
echo "Signed certificate generated for cache server"

# Patch CA_BUNDLE for MutatingWebhookConfiguration
NAMESPACE="$NAMESPACE" ./webhook-patch-ca-bundle.sh --cert_input_path "${CA_FILE}" <./cache-configmap.yaml.template >./cache-configmap-ca-bundle.yaml
# Choosing the correct API version.
# Kubernetes v1.15+ supports better filtering, but it's not trivial to detect since the API version was only bumped to v1 in v1.16.
# Kubernetes has broken it's versioning policy here. https://github.com/kubernetes/kubernetes/pull/78505#commitcomment-41870735
# We still want to support filtering on v1.15, so we need to detect it.
if kubectl api-versions | grep --word-regexp 'admissionregistration.k8s.io/v1'; then
cache_webhook_config_template="cache-webhook-config.v1.yaml.template"
elif kubectl version | grep 'Server Version: version.Info{Major:"1", Minor:"15'; then
cache_webhook_config_template="cache-webhook-config.v1beta1.v1.15.yaml.template"
else
cache_webhook_config_template="cache-webhook-config.v1beta1.yaml.template"
fi
NAMESPACE="$NAMESPACE" ./webhook-patch-ca-bundle.sh --cert_input_path "${CA_FILE}" <./"$cache_webhook_config_template" >./cache-configmap-ca-bundle.yaml
echo "CA_BUNDLE patched successfully"

# Create MutatingWebhookConfiguration
Expand Down

0 comments on commit fbd3f48

Please sign in to comment.