Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
31 of 35 tasks
tomkerkhove opened this issue Nov 29, 2021 · 19 comments
Open
31 of 35 tasks
Labels
Epic help wanted Looking for support from community stale-bot-ignore All issues that should not be automatically closed by our stale bot testing

Comments

@tomkerkhove
Copy link
Member

tomkerkhove commented Nov 29, 2021

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

However, the following are missing:

Scalers

Secret providers

@tomkerkhove tomkerkhove added help wanted Looking for support from community Epic testing stale-bot-ignore All issues that should not be automatically closed by our stale bot labels Nov 29, 2021
@tomkerkhove tomkerkhove pinned this issue Nov 29, 2021
@RamCohen
Copy link
Contributor

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 (#2648)

@JorTurFer JorTurFer changed the title Provide e2e test for all scalers (Help Wanted) Provide e2e test for all scalers and secret providers (Help Wanted) Mar 29, 2022
@v-shenoy
Copy link
Contributor

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

@tomkerkhove
Copy link
Member Author

Nice find, opened #2928 - Thanks!

@nilayasiktoprak
Copy link
Contributor

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
Copy link
Contributor

v-shenoy commented Jun 13, 2022

@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
Copy link
Member Author

Feel free to open them, thanks!

@v-shenoy
Copy link
Contributor

v-shenoy commented Jun 13, 2022

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
Copy link
Member

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.

@v-shenoy
Copy link
Contributor

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

@zroubalik
Copy link
Member

Thanks!

@zroubalik
Copy link
Member

Updated here: #2737

@tomkerkhove
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member

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

@tomkerkhove
Copy link
Member Author

Is the above list still accurate @JorTurFer ?

@JorTurFer
Copy link
Member

Is the above list still accurate @JorTurFer ?

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

@JorTurFer
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

JorTurFer commented Jan 25, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic help wanted Looking for support from community stale-bot-ignore All issues that should not be automatically closed by our stale bot testing
Projects
Status: To Do
Development

No branches or pull requests

6 participants