Add scaleset configurations#221
Open
yhaliaw wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds scaleset configuration support to the garm-configurator charm by introducing new charm config options, parsing/validating them into charm state, and extending unit/integration coverage to exercise the new validation paths.
Changes:
- Added scaleset-related config options to
charmcraft.yamland corresponding config key constants/state parsing incharm_state.py. - Implemented
ScalesetConfigvalidation (required fields, non-negative numeric fields, repo/org mutual exclusivity). - Updated unit and integration tests plus changelog to reflect the new required configuration.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/changelog.md | Adds a dated changelog entry for scaleset configuration support. |
| charms/tests/integration/test_garm_configurator.py | Updates deployment config in integration tests to include required scaleset settings. |
| charms/garm-configurator/tests/unit/test_charm.py | Adds unit tests covering new scaleset validation cases (missing/empty fields, negative values, repo/org constraints). |
| charms/garm-configurator/src/charm_state.py | Introduces ScalesetConfig and wires it into CharmState.from_charm() validation/parsing. |
| charms/garm-configurator/charmcraft.yaml | Adds new user-facing config options for scaleset configuration. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
florentianayuwono
approved these changes
Jun 3, 2026
Collaborator
florentianayuwono
left a comment
There was a problem hiding this comment.
lgtm thanks andrew! im not sure if this is needed now, but there r terraform module tests as well that we can add on :)
| f"{SCALESET_MIN_IDLE_RUNNER_CONFIG_NAME} must be non-negative" | ||
| ) | ||
|
|
||
| max_runner = int(charm.config.get(SCALESET_MAX_RUNNER_CONFIG_NAME, 0)) |
Collaborator
There was a problem hiding this comment.
can we add a check that max >= min?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Add the scaleset configurations for GARM configurator charm.
AI summary
This pull request adds comprehensive support for configuring GitHub runner scalesets in the
garm-configuratorcharm. It introduces new configuration options for scaleset management, validates these options, and extends both the charm's logic and its tests to ensure robust handling of various configuration scenarios.Key changes:
Configuration and validation for scalesets:
name,flavor,os-arch,min-idle-runner,max-runner,labels,repo,org,runner-group,pre-install-scripts) tocharmcraft.yamlfor flexible scaleset configuration.charm_state.pyfor maintainability.ScalesetConfigclass with validation logic, ensuring required fields are present, numeric fields are non-negative, and thatrepoandorgare mutually exclusive but at least one is set.Charm state and integration:
CharmStateclass and itsfrom_charmmethod to include and initialize the newScalesetConfig. [1] [2]Testing enhancements:
Why we need it
Needed to support the scaleset features of GARM charm.
Checklist
CONTRIBUTING.mdhas been updated upon changes to the contribution/development process (e.g. changes to the way tests are run)docs/changelog.mdwith user-relevant changes(e.g., in
.github/workflows/integration_tests.yaml, ensure themoduleslist is correct)terraform fmtpasses andtflintreports no errors