search

kedacore / keda

KEDA is a Kubernetes-based Event Driven Autoscaling component. It provides event driven scale for any container running in Kubernetes
https://keda.sh
Apache License 2.0
8.38k stars 1.06k forks source link

Provide e2e test for all scalers and secret providers (Help Wanted) #2357

Open tomkerkhove opened 2 years ago

tomkerkhove commented 2 years ago

We must provide end-to-end tests for all our scalers and secret providers.

However, the following are missing:

Scalers

Secret providers

RamCohen commented 2 years ago

Hi @tomkerkhove , will be happy to work on this, but for it to be submitted, the issue of having a secret with the credentials for a GCP service account to use should be resolved (https://github.com/kedacore/keda/pull/2648)

v-shenoy commented 2 years ago

@tomkerkhove The Azure Monitor scaler is also missing an e2e test.

tomkerkhove commented 2 years ago

Nice find, opened https://github.com/kedacore/keda/issues/2928 - Thanks!

nilayasiktoprak commented 2 years ago

Would it be good to mention here that now e2e tests are migrating to Go in case someone wants to take any remaining tests above?

v-shenoy commented 2 years ago

@tomkerkhove @JorTurFer @zroubalik We need to create new issues for scalers that have e2e tests in ts in need of migration to go, and add them here.

tomkerkhove commented 2 years ago

Feel free to open them, thanks!

v-shenoy commented 2 years ago

Okay, I created a few. You can edit and add them to the issue. @tomkerkhove

But I also realize that there's too many of them. What's the smoothest way here to transition?

zroubalik commented 2 years ago

I'd create one umbrella issue, with tasks

The task could be transferred into an issue if there's a person who is willing to contribute that.

v-shenoy commented 2 years ago

I'd create one umbrella issue, with tasks

  • [ ] scaler X
  • [ ] scaler Y

The task could be transferred into an issue if there's a person who is willing to contribute that.

Here's a list. Maybe Tom can edit and add them as a section in this issue itself. Or maybe here https://github.com/kedacore/keda/issues/2737

zroubalik commented 2 years ago

Thanks!

zroubalik commented 2 years ago

Updated here: https://github.com/kedacore/keda/issues/2737

tomkerkhove commented 2 years ago

I'd create one umbrella issue, with tasks

  • [ ] scaler X
  • [ ] scaler Y

The task could be transferred into an issue if there's a person who is willing to contribute that.

Here's a list. Maybe Tom can edit and add them as a section in this issue itself. Or maybe here #2737

Why do we have Argo Rollouts in the list? This is not a scaler we have today.

v-shenoy commented 2 years ago

I'd create one umbrella issue, with tasks

  • [ ] scaler X
  • [ ] scaler Y

The task could be transferred into an issue if there's a person who is willing to contribute that.

Here's a list. Maybe Tom can edit and add them as a section in this issue itself. Or maybe here #2737

Why do we have Argo Rollouts in the list? This is not a scaler we have today.

I think it's to test that KEDA is able to scale custom resources besides scaling deployments, and spawning jobs.

JorTurFer commented 2 years ago

yes, we use argo-rollouts to test KEDA with custom resources with /scale but we don't support scaling base on them

tomkerkhove commented 1 year ago

Is the above list still accurate @JorTurFer ?

JorTurFer commented 1 year ago

Is the above list still accurate @JorTurFer ?

I'll check it during the day and I'll update the issue if needed

JorTurFer commented 1 year ago

I have just updated the description with the new issues. All the Azure Workload Identity apply also for AAD-Pod-Idendity, I have to update the issues to include both (as the tests are quite similar, we can do both at once)

tomkerkhove commented 1 year ago

Awesome, thanks! Do you think it makes sense splitting scalers from auth support?

I'm asking because the list gets long, while majority is just for Azure auth e2e.

JorTurFer commented 1 year ago

Yeah, As the authentication code is partially part of the scaler code, I think we should have those test cases covered. I mean, for example, in azure scalers (where we use multiple SDKs) having one covered with Workload Identity doesn't guarantee that the other scalers will work. The only case where it can be duplicated is in scalers like servicebus where we have topics and queues, but the majority of the code is the same. We can remove them if you think isn't worth, but I prefer to cover them, because otherwise we don't have guarantees of them (a typo could be introduced in scaler code for example, removing the pod identity support). From the e2e test duration pov, we could go more aggresive running more test together if we want to reduce the e2e test duration.

I'm asking because the list gets long, while majority is just for Azure auth e2e.

Yes, because I already added them for AWS and GCP scalers xD By AWS and GCP scalers are also covered alone and using pod Identity, the problem here is that azure has 2 supported pod identities and has more scalers than others, which means more effort covering them.

My only doubt is if we should cover aad-pod-identity as has EOL already defined, but... a year means 3-4 releases where we can potentially break it during the support period