Azure Pipelines Parent Scaler not scaling (2.10)

[Eldarrin]

Report

Just upgraded from 2.9 to 2.10 and scaling has stopped working. Will work it out and update but putting a pin here for now.

Expected Behavior

Pipelines to Scale

Actual Behavior

Pipelines don't scale

Steps to Reproduce the Problem

1. Setup AzDO Scaler using parent method on keda v2.9.1
2. Check scaledjobs scale based on parent
3. Upgrade keda to v2.10.0
4. Jobs using parent method will not scale

Logs from KEDA operator

Logs have no difference between version

KEDA Version

2.10.0

Kubernetes Version

1.25.5

Platform

Microsoft Azure

Scaler Details

azure-pipelines

Anything else?

When calling */jobrequests?top=nnn returns a different api result to */jobrequests. For instance, different number of returns and no matchedAgent field. But json is sufficiently similar to not fail marshalling.

[JorTurFer]

Hi,
This is weird because we have the scaler covered with e2e tests. Maybe is a not covered case? How are you using it? Maybe we could add the use case to the e2e test to cover better the scaler

[Eldarrin]

Well, I know what it is, I just don't know why it is :) It's adding $top to the url query. Will investigate further as it only seems to be affected the matchedAgents field which is used by parent method only

Its an interesting one. If you have access to AzDO can you just execute the jobrequests api with and without the $top query. Even the count changes (and not in line with top). I'd be interested if you see it too. Ideally if you can have a job pending with no agent to run it also shows the matchedAgent difference.

[Eldarrin]

@JorTurFer not sure what we want to do here. It seems to me like the quick and dirty fix is to exclude top when using parent metadata and add an e2e test to it. Would need to dig into API versions I suspect to work out a true fix,

Any thoughts? Basically without the patch 2.10 is unusable if using parent

[shayki5]

Hi, I added the top the API in order to solve another bug...
It looks like you're right, in this case the matchedAgents doesn't exist in the response (Microsoft 😡).
@JorTurFer Should I just update it what @Eldarrin suggested? to exclude top when using parent?

[Eldarrin]

I'm writing a bunch of e2e tests at the moment to handle the more extreme cases using demands and parent type scalers. We should probably aim to coincide with that

[shayki5]

Ok great, so can you add the condition?

[Eldarrin]

Ok great, so can you add the condition?

Sure, I can put the hotfix in. @JorTurFer are you ok with that as an interim?

[tomkerkhove]

Related: #4374

[EugeneLugovtsov]

@Eldarrin @shayki5 hi, I have the following error. can it be related?

[Eldarrin]

What meta type are you using? demands, parent or none? I haven't done any volume testing on it myself as I use parent and it won't load any jobs with that

[EugeneLugovtsov]

I use demands type

[EugeneLugovtsov]

In one agent pool, I have several agent types. so I use the introduced in the 2.10 feature requireAllDemands: "true"

[EugeneLugovtsov]

for some reason there is no Agent.Version demand for yaml pipelines (_apis/distributedtask/pools/{agent-pool}/jobrequests response). this demandis presents only in releases.

But it's weird, it stopped working a few hours ago, it worked like a charm before

[Eldarrin]

I wonder if it’s the top query flipping the response like with parent. I’ll look further into it tomorrow

[JorTurFer]

Let me know @Eldarrin if you need anything from our side to enable the new e2e scenarios

[JorTurFer]

Sure, I can put the hotfix in. @JorTurFer are you ok with that as an interim?

I think so, the top is a new feature for huge cases, but if the API isn't compatible we cannot do anything... Ideally, we could check the API and somehow we could solve this gap, but IDK if we can do it. @tomkerkhove @v-shenoy , could you ask internally if there is any way to achieve this?

[Eldarrin]

The current e2e only deals with the base scale scenario so I’m rewriting it to a more comprehensive suite. The problem is around ADO slowness when testing /sigh . I’m hoping to get the new suite in the next day of so and retest against 2.9 and 2.10 to check for other scenarios that may need remediation. The only additions should be another pipeline and env which I’ll stick into test-tools and a new image type for the ado agent.

[shayki5]

@JorTurFer If the $top cause issues, maybe we can disable it by default and only if explicitly we pass a parameter what we wand it it will be added to the api call?

[JorTurFer]

@JorTurFer If the $top cause issues, maybe we can disable it by default and only if explicitly we pass a parameter what we wand it it will be added to the api call?

I understood that @Eldarrin wanted to add the $top in case of not using demands. Let's give some time 😄

[Eldarrin]

I've got a commit in, needs more work which I'll do tomorrow, but at least you have a start point if I drop off the face of the earth :) luckily, the tests proved it was only parent which is affected

[Eldarrin]

Should be ready to run e2e on azure_pipelines and azure_pipelines_adv. Just need to mod your ado setup as per test-tools and build an adv agent or just use mine for now.

[JorTurFer]

Hi @Eldarrin
One question about this, I have noticed that we generate this image e2e/images/azure-pipelines/advagent as part of kedacore/test-tools#118 but we are not using it at all.
Can we delete it?

[Eldarrin]

HI @JorTurFer
Actually we should be using it, when I set it up I set it to use my image of this on docker:

keda/tests/scalers/azure/azure_pipelines_adv/azure_pipelines_adv_test.go

Line 100 in b5de66f

image: eldarrin/azure:main

This should be set to use your version of the image.

[JorTurFer]

Would you open a PR with the change? (as you have done all the work and maybe you want to get the commit ownership)
I can open it if you don't have time :)

[Eldarrin]

Submitted #4641 :)

[JorTurFer]

Thanks a lot! 🙇

[Eldarrin]

No problem, time for a beer 🍺

[zroubalik]

No problem, time for a beer 🍺

Well deserved, thanks!