Feature/cpu advertisement #18
Conversation
There was a problem hiding this comment.
Hi David, thank you for putting this together. I think this is a great improvement, however, I left quite some comments mostly around organization of these changes. Right now this series of commits puts a lot of back and forth in that ultimately does not add any value, especially given the minimal commit messages. As far as I can tell the functional aspects are sound, but the back and forth makes it harder to review.
I am not sure, if this is on me and how I view git history, but I would like some more context as to the whats and whys on changes. This can either be in the PR overview or in the commit messages (preferably in the commit messages, because it works for both types of folks - those that go through github PRs and those that go through git log on their local check-out).
Let me know what you think.
|
thanks @foersleo , considered squashing the commits first and re-write history however reading your fb think it makes sense to keep refactoring separated from feature commit and version bumps - will update PR |
dpdornseifer
force-pushed
the
feature/cpu_advertisement
branch
from
f179f3f to
96b43a6
Compare
dpdornseifer
force-pushed
the
feature/cpu_advertisement
branch
2 times, most recently
from
7ccc7c2 to
cbd0a3e
Compare
dpdornseifer
force-pushed
the
feature/cpu_advertisement
branch
from
cbd0a3e to
4c7325c
Compare
* Moved device-plugin and device monitor to separate packages * Moved main.go to dedicated cmd folder * Updated docker build container
* Added separate config package including validation function
* Added separate device plugin to advise available vCPUs for enclaves which can be turned on via feature flag. K8s scheduler can pickup available vCPUs and schedule workloads based on vCPU availability. * Added config options to Readme and extended deployment manifest. * Added unit tests for config and extended unit tests for previous device plugin to test new functionality.
* Moved IBasicDevicePlugin to monitoring package and added ResourceName() so that monitor is aware of device-plugin it is running * Reused device monitor package for Nitro Enclaves CPU plugin to avoid code duplication
* Updated Helm chart to expose new config interface for vCPU advertisement device-plugin
* Added GitHub workflow to run unit tests and build flow on pull requests
dpdornseifer
force-pushed
the
feature/cpu_advertisement
branch
from
4c7325c to
3171b29
Compare
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.