-
Notifications
You must be signed in to change notification settings - Fork 159
Auto set resources on make setup-local
#218
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall I think it's a nice idea, however I'm not convinced all this logic is really needed. We are talking anyway about a very test-based deployment, just for showing the program. For any real usage, people will have to play a bit with the pods allocation themselves and give the right resources. For example, just having one seed-gen and one fuzzer-bot is not going to work very well. Mid/long-term i think we are better of implementing auto-scaling within k8s so that we can start everything with just 1 pod per component and scale up/down automatically as needed.
The reason why I think this might be a bit over-engineered is that given it's just for testing/showing the tool, ensuring the system has a minimum of X cpu/Y mem is enough to deploy a basic system that works. Any other use case above that will require some thinking about resources allocated to each component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set defaults to MINIKUBE_* vars if not present. On a production deployment you probably don't evan care about setting those vars in the env file
| # Docker build arguments, useful for local deployment | ||
| export FUZZER_BASE_IMAGE="gcr.io/oss-fuzz-base/base-runner" | ||
|
|
||
| # Minikube cluster resource allocation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain a bit more how/when these are used, saying they are for the minikube environment only. Also, maybe make these commented out by default and enable them in the detect-resources script?! WDYT?
| # Convert memory from bytes to MiB | ||
| local total_memory_mib=$((total_memory_bytes / 1048576)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be done somehow by the convert_to_mib function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and actually that fnction is never used, I think
| [ "$total_cpus" = "0" ] && total_cpus=4 | ||
| [ "$total_memory_mib" = "0" ] && total_memory_mib=4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [ "$total_cpus" = "0" ] && total_cpus=4 | |
| [ "$total_memory_mib" = "0" ] && total_memory_mib=4096 | |
| [ "$total_cpus" = "0" ] && total_cpus=5 | |
| [ "$total_memory_mib" = "0" ] && total_memory_mib=9216 |
I agree here, this is nice to have but it's probably not the most pressing priority. |
Potentially replaces #201
Makes the resources configurable values set in the
envfile. The values are automatically set duringmake setup-localbased on available system resources.TODOS:
make setup-local@ret2libc What do you think of this approach in general compared to #201 ?