Skip to content

Conversation

@BradSwain
Copy link
Contributor

Related #169

This changes the minikube deployment to use more resources, if available.

It also allows the deployment to run on systems with less than the previously hardcoded resources (8cpus, 32g mem).

It warns if resources are below some threshold, and errors if below a minimum threshold.

I took a best guess at what good values would be, but they can be changed/updated.

Copy link
Collaborator

@ret2libc ret2libc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about this for now. The request resources depends heavily on the number of pods you want to spawn, so maybe on lower systems you also want to reduce the number of build-bot, etc and maybe reduce the resource constraints of some of those pods.

@BradSwain
Copy link
Contributor Author

@claude use docker system info data and get the cpus/memory from there

@claude
Copy link
Contributor

claude bot commented Jul 30, 2025

Claude finished @BradSwain's task —— View job


Task List

  • Read and understand current resource detection implementation
  • Research docker system info output format
  • Modify script to use Docker system info for CPU/memory detection
  • Test the changes locally if possible
  • Commit and push changes to PR branch

Changes Made

Replaced OS-specific resource detection with docker system info --format "{{.NCPU}},{{.MemTotal}}"
Added Docker availability check with proper error handling
Updated output message to clarify "Docker-available resources"
Tested parsing logic - works correctly (CPUs: 2, Memory: 7944MB)

Summary

The script now uses docker system info to get Docker-available resources instead of system-wide resources. This ensures minikube gets accurate resource limits, especially important on macOS with Docker Desktop/Colima where Docker resources may be constrained compared to system resources.

Key improvements:

  • Consistent cross-platform resource detection
  • Uses actual Docker-available resources, not system-wide
  • Added error handling for Docker not running
  • Clearer output messaging

…commands

This ensures minikube gets accurate Docker-available resources rather than
system-wide resources, which is especially important on macOS with Docker
Desktop/Colima where Docker resources may be constrained.

Co-authored-by: Brad Swain <BradSwain@users.noreply.github.com>
@ret2libc
Copy link
Collaborator

@claude address the review

@claude
Copy link
Contributor

claude bot commented Jul 31, 2025

Claude finished @ret2libc's task —— View job


Task List

  • Read current script to understand existing thresholds
  • Update CPU minimum threshold to 5 (per review feedback)
  • Update memory minimum threshold to 10GB (per review feedback)
  • Test changes and run linting if available
  • Commit and push changes

Changes Made

Updated minimum CPU requirement from 2 to 5 cores (deployment/crs-architecture.sh:116)
Updated minimum memory requirement from 16GB to 10GB (deployment/crs-architecture.sh:123)
Fixed shellcheck warnings by adding proper variable quoting
Validated script syntax with bash -n

Summary

Successfully addressed both review comments:

  1. CPU threshold: Increased from 2 to 5 CPUs based on reviewer's observation that systems with one build-bot use more than 4 CPUs
  2. Memory threshold: Reduced from 16GB to 10GB as reviewer noted that 9-10GB is sufficient for the k8s cluster specifically

The changes ensure that minikube deployments will only proceed on systems that meet the actual resource requirements for running Buttercup effectively.

Commit: 4cc838a - Address review feedback: update minimum resource thresholds
| Branch: brad/scale-minikube-deployment

- Increase minimum CPU requirement from 2 to 5 cores
- Decrease minimum memory requirement from 16GB to 10GB for k8s cluster
- Add proper variable quoting to fix shellcheck warnings

Co-authored-by: Riccardo Schirone <ret2libc@users.noreply.github.com>
MINIKUBE_CPUS=$((AVAILABLE_CPUS > 7 ? 8 : AVAILABLE_CPUS > 4 ? AVAILABLE_CPUS - 1 : AVAILABLE_CPUS))
MINIKUBE_MEMORY_MB=$((AVAILABLE_MEMORY_MB * 75 / 100))

# Check if system meets minimum requirements
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README should document the minimum requirements

Copy link
Contributor

@reytchison reytchison Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also go in the #214 PR

@BradSwain
Copy link
Contributor Author

Got pulled to some other work, but I plan to make the resources configurable during setup instead of during deployment.

I made env vars to configure minikube resources, pod count, and individual pod resource, and moved the "detect resources" logic to a script that runs during local setup

@BradSwain
Copy link
Contributor Author

Closed in favor of listing the minimum required resources in #214

@dguido dguido deleted the brad/scale-minikube-deployment branch August 8, 2025 16:56
dguido pushed a commit that referenced this pull request Aug 18, 2025
* Add retrieving types from treesitter

* Update preprocessor types. Update fuzz-tooling directory layout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants