WIP: Add Capability info to Provider interface and schedule based on label prediction#581
Open
6543 wants to merge 77 commits into
Open
WIP: Add Capability info to Provider interface and schedule based on label prediction#5816543 wants to merge 77 commits into
6543 wants to merge 77 commits into
Conversation
Member
|
Can you explain the bugs and limitations first? |
Member
Author
Problems
|
Member
Author
|
missed one: You can not have different scheduling strategies |
Member
Author
|
followup that adds an scheduler interface will be #582 ... |
Member
|
Can you split the PRs? Its pretty big now and adds new logig mixed with an overall refactoring. |
xoxys
reviewed
Apr 14, 2026
Member
Author
|
well i have more code planed and thought i already splitted it enouth but I'll break it down even more |
Member
|
@anbraten would you add your thougts as well? You had strong opinions about the autoscaler in the past :) |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bfb091b to
216cd18
Compare
6543
commented
Apr 26, 2026
6543
commented
Apr 28, 2026
6543
commented
Apr 28, 2026
Co-authored-by: 6543 <6543@obermui.de>
6543
added a commit
that referenced
this pull request
May 21, 2026
not here jet with this pull but ... with #581 we will calc what labels an agent will have based on platform, backend and WOODPECKER_AGENT_LABELS. based on that we will look into the queue tasks and see if we should deploy. using filter_labels is just a hacky way to achive the same but it works not always, so with v2.0 we should just get rid of it.
Member
Author
|
next split: #632 |
Member
Author
|
next split: #636 |
6543
commented
Jun 2, 2026
Bring back the per-provider changes from the previous PR head (ff38c21) that were dropped by the rebase/merge: the Capability parameter on DeployAgent, the Capabilities() implementations and the capability derivation helpers for aws, hetznercloud, linode, scaleway and vultr. Conflicts with the BillingModel() methods that landed on main in the meantime were resolved by keeping both.
DeployAgent built the "only support docker backend" error but never returned it, so a request for an unsupported backend silently deployed anyway (and go vet flags the unused fmt.Errorf result).
The merge of main into the branch left drainAgents with the old per-agent signature and body, plus an orphaned tail of the bucket-aware version referencing undefined agent/bucket/drained, while the planScaling dispatcher already calls it with a bucket. Restore the bucket-aware signature and capability filter and fold in the BillingModel teardown-window logic that landed on main, so hourly-round-up agents are still only drained inside their teardown window.
The Equinix Metal provider landed on main after the interface change and still implemented the old Provider interface, breaking the build. Derive Capabilities from the architecture suffix of the configured plan slugs (.x86 -> linux/amd64, .arm/.arm64 -> linux/arm64) and make DeployAgent pick the first configured plan matching the requested capability instead of always using the primary plan. Tests adjusted for the new signature.
Repo convention: don't name the parameter cb (or cap, which shadows the builtin).
Same bug as in the scaleway provider: DeployAgent built the "only support docker backend" error but never returned it, so unsupported backends deployed anyway (flagged by go vet).
The BillingModel tests that came in with the main merge still built Autoscaler.agents as a slice and called drainAgents without a bucket. Convert them to the name-keyed agents map and pass a zero-value bucket, which matches the test agents since they report no platform/backend.
The subtest mocked GetByNameAndArchitecture, but the provider calls Image().GetForArchitecture, and it returned an image without an architecture, which the new architecture consistency check in resolveServerConfigs rejects. Mock the method that is actually called and return an image matching the server type architecture.
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.
the current architecture limits us in some ways ... so i refactor it to solve bugs and add features afterwards...
this pull here aims to not change the current behavior at all but prepare for it.
Problems
Solve
TODO: