Skip to content
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

VPA - Implementing automatic detection of injected sidecars #2726

Closed
krzysied opened this issue Jan 10, 2020 · 8 comments
Closed

VPA - Implementing automatic detection of injected sidecars #2726

krzysied opened this issue Jan 10, 2020 · 8 comments
Assignees
Labels
area/vertical-pod-autoscaler kind/feature Categorizes issue or PR as related to a new feature.

Comments

@krzysied
Copy link
Contributor

There will be new feature added to VPA dedicated to detecting injected sidecars and omitting them during making pod eviction decision.
This feature assumes that containers handled by Admission Controller will be listed in additional pod annotation. Containers not listed in this annotation will not be taken into account by the Updater.

@krzysied
Copy link
Contributor Author

/assign
/cc @bskiba
/cc @kgolab

@krzysied
Copy link
Contributor Author

krzysied commented Jan 10, 2020

Implementation plan:

Tests:

@bskiba
Copy link
Member

bskiba commented Jan 14, 2020

Please add an e2e test as a way of verifying the feature works.

@krzysied
Copy link
Contributor Author

I agree that an e2e test is something nice to have.
Assuming that we need a custom mutating webhook to test this change, I need to do some research about whether to implement a testing webbook or to use an available opensource one...
I will update the issue when I come up with the solution.

@krzysied
Copy link
Contributor Author

@aleksandra-malinowska suggest brilliant idea that webhooks should already be tested by k8s e2e tests.
https://github.com/kubernetes/kubernetes/blob/master/test/e2e/apimachinery/webhook.go
It looks like there is a pod mutation webhook test. I will try to re-use some of its components/code.

@bskiba bskiba added kind/feature Categorizes issue or PR as related to a new feature. area/vertical-pod-autoscaler labels Jan 17, 2020
@krzysied
Copy link
Contributor Author

The Agnhost webhook (used by k8s e2e tests) adds initContainer instead of standard container. -_-'
I will try add sidecar injection functionality to Agnhost. kubernetes/kubernetes#87383

@krzysied
Copy link
Contributor Author

krzysied commented Feb 6, 2020

The e2e test pr was merged. I will wait for two green runs of https://testgrid.k8s.io/sig-autoscaling-vpa#autoscaling-vpa-actuation and then I will close the issue.

@krzysied
Copy link
Contributor Author

krzysied commented Feb 7, 2020

The test is passing. Nothing more to do here.
I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertical-pod-autoscaler kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants