Extending the Azure DevOps Agent scaling - possibility to "ignore" demands
Proposal
I would like the possibility within the azure-pipelines trigger of a ScaledObject and ScaledJob to differentiate between demands which are required for scaling up and demands which are ignore/accepted but not important for actually triggering the scaling.
Idea:
demands: 'cap-8gb'
requireAllDemands: true
demandsToIgnore: 'npm' # new configuration
This is because we are not fully in control of the demands which are requested by an Azure DevOps pipeline. Demands can be defined in the pipeline definition. But demands are also just added automatically when using certain tasks.
Use-Case
We currently use Keda to scale our Azure DevOps agents using the azure-pipelines trigger, with the following setup:
- We have two kinds of agents which differ in the resources (more kinds or other differentiation could come). Lets say the two kinds are 4GB and 8GB. So as demand something like
cap-4gbandcap-8gb. - We set
requireAllDemands: falsefor the 4GB agent, so that it would be the default, since a pipeline job with no demands at all will trigger this agent (and it is also triggered when a pipeline has the demandcap-4gb).
demands: 'cap-4gb'
requireAllDemands: false
- We set
requireAllDemands: truefor the 8GB agent, because we want to avoid scaling both agents when a pipeline job with no demands is issued
demands: 'cap-8gb'
requireAllDemands: true
This worked out for us until we investigated a scaling issue where a job with demand cap-8gb would not trigger Keda to scale. It turned out the pipeline job uses a Npm@1 task which as stated at the bottom under Requirements will add the demand npm to the pipeline just through using that task.
This does conflict with requireAllDemands: true since it checks for the exact match (in this case the job will have demands cap-8gb and npm).
Is this a feature you are interested in implementing yourself?
Yes
Anything else?
Quick fixes are
- adding a third agent with demands
cap-8gb,npmandrequireAllDemands: true - adding another trigger for both kinds of agents which account for
npm
Both solutions work for now, but I'm not sure if there are more pipeline task that add demands because the possibilities that would be needed to be covered by trigger combinations would grow fast.
Would love some feedback about how you feel about adding something like demandsToIgnore.
hey, any discussion or feedback on this? 🙂
I missed this issue, sorry for that :( I guess that it can make sense but I can me missing something. @tomkerkhove @Eldarrin , WDYT about this option?
IT looks fine to me, there is an existing function that already strips the agent version from the demand array so that could be extended. Although I would suggest looking at the parent matching system rather than the demand matching system as although that is more complex to initially set up that passes the scaling problem back to ADO to handle the agent matching paradigms so that keda doesn't have to do it.
is the parent approach worth it for your use case @jan-mrm ?
@JorTurFer I have looked at it a while ago but I didn't really understand how to use it I guess. I can take another look at it
If you are struggling reach out and I can help. If the documentation is insufficient I probably need to improve it :)
I'm also facing this. Random azure pipeline tasks like CMake adds "invisible" demands to a job, release pipelines also has demands that's added based on what tasks are being used that can't be removed.
A workaround for this would be to not use the requireAllDemands and just generate unique permutation of all demands.
An example of this would be turning the following separate demands cpu-16, cpu-32, mem-8, mem-32, into cpu-16-mem-16, cpu-16-mem-32 etc..
That will avoid the issue where a demand matches multiple scale definitions, as mentioned in the docs:
Note: If more than one scaling definition is able to fulfill the demands of the job then they will both spin up an agent.
I don't think the suggestion of adding a demandsToIgnore would scale well, because a list like this would need to be constantly updated. Using a "parent" would also not solve my use-case, I think.
In my opinion, this could be solved in a few ways:
- Change the scaling behavior to only dispatch a single job even if multiple demands match.
- Add a
demandApproveListthat can include all the demands you want to consider when matching.
Example using approve list:
demands: mem-8,cpu-16
requireAllDemands: true
demandApproveList: cpu-16,cpu-32,mem-8,mem-32
So even if "CMake" is part of the demands, it will not be considered. The downside of this is that all definitions need to know about all the demands, but that could be easily automated using helm.
Edit: If possible, making the demandApproveList a "global" KEDA configuration would probably be better.
I don't think the suggestion of adding a
demandsToIgnorewould scale well, because a list like this would need to be constantly updated. Using a "parent" would also not solve my use-case, I think.
I see your point. If there are new demands, which are added by task, encountered, then the list would need to be updated. Not sure how often that happens and for my use case I'd think that's okay. Also since you would make sure that the self-hosted agent actually fulfils the demand. How many are known to you at this point, besides cmake?
- Add a
demandApproveListthat can include all the demands you want to consider when matching.
demands: mem-8,cpu-16 requireAllDemands: true demandApproveList: cpu-16,cpu-32,mem-8,mem-32
I think I do not fully understand your demandApproveList idea and the example yet. Could you explain it again?
I see you set the two demands and requireAllDemands. What is the effect of the list in the trigger definition. For example what does the cpu-32 demand in the list do?
To be triggered a pipeline would have set the two required demands.
Just another idea:
Add a flag like allowAdditionalDemands as a boolean. Since requireAllDemands exactly matches the given demands and does not allow 'more' / 'additional' demands. (That it does not allow less demands is the expected behavior but an idea would be to allow any additional)
That would not need any lists to be updated or demands to be known.
What do you think about that?
Edit: Or a change of the requireAllDemands behavior to allow additional demands in general (without a new flag), but that would mean a behavior change of a current feature.
As I mentioned in my edit, the demandApproveList would make more sense as a "global" config and would tell KEDA what demands to consider when matching and ignore anything else, but I think KEDA is designed in a way that makes that hard, because each scale definition seems very "standalone".
The main issue now, is that demands: mem-8,cpu-16 without requireAllDemands would mach any subset of that list, as far as I understand? So a job with only mem-8 demand would match.
Adding a allowAdditionalDemands would make little sense as you would need the following config that would look a bit contradicting:
demands: mem-8,cpu-16
requireAllDemands: true
allowAdditionalDemands: true
What we need is a flag that conveys "require at least all specified demands, ignore anything else", but coming up with a user friendly name for it seems difficult. Here's a few suggestions from chatgpt:
-
requireExactOrSupersetDemands -
ignoreUnspecifiedDemands -
exactOrSupersetDemandsOnly -
strictDemandMatching -
minimumRequiredDemands -
matchAtLeastSpecifiedDemands -
enforceSpecifiedDemands -
mandatoryDemandsOnly -
requireAndIgnoreOthers -
demandSubsetRestriction
As I mentioned in my edit, the
demandApproveListwould make more sense as a "global" config and would tell KEDA what demands to consider when matching and ignore anything else, but I think KEDA is designed in a way that makes that hard, because each scale definition seems very "standalone".
Alright, kind of got it.
The main issue now, is that
demands: mem-8,cpu-16withoutrequireAllDemandswould mach any subset of that list, as far as I understand? So a job with onlymem-8demand would match. Adding aallowAdditionalDemandswould make little sense as you would need the following config that would look a bit contradicting:demands: mem-8,cpu-16 requireAllDemands: true allowAdditionalDemands: true
Yes that is true. Having to set both flags seems ugly and also the allowAdditionalDemands would seem to have no effect when not also setting requireAllDemands.
What we need is a flag that conveys "require at least all specified demands, ignore anything else"
So just so sum up the idea:
We should not change the behavior of the current requireAllDemands? So if not then the idea is to add one additional flag besides requireAllDemands.
requireAllDemands would stay as it is right now, as the exact match.
The new flag - something like requireAllDemandsAndIgnoreOthers (just something to convey the meaning here, the actual user friendly name would need to be decided) - would check that all given demands from field demands are present but any additional ones defined in the pipeline would be ignored/accepted.
Is that right?
How should we handle setting both flags to true since they conflict?
How should we handle setting both flags to true since they conflict?
Because requireAllDemands is a "stronger" rule, I'm thinking it can take precedence.
I opened a draft PR just to get the ball rolling on this.
maybe @JorTurFer and/or @Eldarrin have feedback to this idea Nohac an I discussed (see draft PR of Nohac):
not having a list of demands to ignore anymore (original idea of this issue) but a less strict version of requireAllDemands which does allow additional demands
I think that the approach proposed by @Nohac of having different "matching" rules can bring a lot of flexibility. @tomkerkhove @Eldarrin ?
I feel like this is a relatively low risk feature to add, and shouldn't need to much consideration apart from the naming. If my changes looks OK and we can agree on the name, I will publish the PR I made and open a PR to update the docs as well.
Ping @tomkerkhove @Eldarrin ?
PR looks fine to me, its non breaking :)
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
so we just need to agree on a name (in the already opened draft pr) and update the docs?
What about the currently proposed name 'requireAllDemandsAndIgnoreOthers'? I feel like it does match the functionality. Feels a bit clunky too but yea
I'm sorry about the delay, the PR should be ready now.
Let's just keep the name as is. It's definitely a bit clunky, but at the same time also the most descriptive name.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
I still think this feature is helpful. @Nohac do you have some time to take a look at the feedback in the pull request you opened to implement this feature? 🙂
Sorry about this. They have so many requirements to the commits that's hard to follow and I can't understand being necessary, but I'll try to find some time this week to finish it.