-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(backend): Caching - Only send cache-enabled pods to the caching webhook #4429
fix(backend): Caching - Only send cache-enabled pods to the caching webhook #4429
Conversation
/lgtm |
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.
b6cc5dc
to
b12a34b
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bobgy, rmgogogo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/test kubeflow-pipeline-backend-test |
/retest |
The change seems to be working. The KFP pods reach cache-server (logs), but non-KFP pods no longer reach it. |
/unhold |
/test kubeflow-pipeline-backend-test |
@rmgogogo Can you re-LGTM please? |
/lgtm |
/retest |
1 similar comment
/retest |
…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
…ebhook (#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
…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
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.