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.

Sign up for GitHub

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

Jump to bottom

feat: Add scaler for Google Cloud Tasks #4834

Merged
merged 24 commits into from
Sep 11, 2023
Merged

feat: Add scaler for Google Cloud Tasks #4834

merged 24 commits into from
Sep 11, 2023

Conversation

jmalvarezf-lmes
Copy link
Contributor

@jmalvarezf-lmes jmalvarezf-lmes commented Jul 31, 2023 •
edited by zroubalik
Loading

Hi,

This is a PR to add scaler based on google cloud tasks depth (#3613). I've added basic tests, but before adding e2e tests, I wanted to know if this continues to be interesting.
If so, I will add this, and the docs part.
It is a copy of the structure of pubsub scaler, but much easier as there is only one metric that makes sense when talking about scaling up/down.

Hope it makes sense,

Regards,

Jose Maria.

Fixes: #3613

@jmalvarezf-lmes jmalvarezf-lmes requested a review from a team as a code owner July 31, 2023 10:11
@github-actions
Copy link

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

🏖️ Over the summer, the response time will be longer than usual due to maintainers taking time off so please bear with us.

While you are waiting, make sure to:

Learn more about:

@jmalvarezf-lmes jmalvarezf-lmes force-pushed the feature/google_cloud_tasks_scaler branch from 06da142 to 4f737e4 Compare July 31, 2023 10:14
@JorTurFer
Copy link
Member

Hi @jmalvarezf-lmes
I think that improving the catalog offer is always nice. I'm not an expert on GCP to know if this service has demand or not (I don't know either if it can run on Kubernetes) but I having it is a good addition 😄

Based on how GCP works, majority of the scalers are just copies from others because almost all are based on StackDriver to get information about them, so I don't think that it's a problem.

Let us know if you need help with docs or e2e test stuff 😄

@jmalvarezf-lmes
Copy link
Contributor Author

@JorTurFer Ok will do. I'll start with the e2e test. Do I have to wait till the night to see the result execution?

Thank you,

feature: add cloud tasks scaler
8de014a 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@jmalvarezf-lmes jmalvarezf-lmes force-pushed the feature/google_cloud_tasks_scaler branch from 330e21e to 97b7d58 Compare August 1, 2023 12:02
Jose Maria Alvarez added 2 commits August 1, 2023 14:55
Add cloud tasks e2e test
0b7cda8 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: Return files to the original state
9312364 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@jmalvarezf-lmes jmalvarezf-lmes force-pushed the feature/google_cloud_tasks_scaler branch from 8771c93 to 9312364 Compare August 1, 2023 12:55
Jose Maria Alvarez added 2 commits August 1, 2023 16:05
fix: static checks
5dfb46b 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: more static checks fixed
a6f5044 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@JorTurFer
Copy link
Member

@JorTurFer Ok will do. I'll start with the e2e test. Do I have to wait till the night to see the result execution?

Thank you,

No no, nightly execution is from main, from PRs we can trigger them manually. Do you need to deploy some extra resources in GCP? If yes, you need to spin up them with a PR to this repo: https://github.com/kedacore/testing-infrastructure

@JorTurFer
Copy link
Member

JorTurFer commented Aug 1, 2023 •
edited
Loading

It'd be interesting if we add support (and coverage with e2e tests) for pod identity. Could you take a look to that part too?

@JorTurFer
Copy link
Member

JorTurFer commented Aug 1, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@jmalvarezf-lmes
Copy link
Contributor Author

@JorTurFer I've seen that the cloud tasks api is not enabled in the project. Does it make sense to add the api enablement here?

You haven't had to enable an api before in gcp.

Thanks,

@JorTurFer
Copy link
Member

What api do you need? I can enable it later on

@jmalvarezf-lmes
Copy link
Contributor Author

cloudtasks.googleapis.com

@JorTurFer
Copy link
Member

I have just enabled the api: kedacore/testing-infrastructure@20cc3a9
I have also restarted e2e tests, let's check how they go now 😄

@JorTurFer
Copy link
Member

JorTurFer commented Aug 1, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@JorTurFer
Copy link
Member

APIs are enabled now, but there is still an error:
image

@jmalvarezf-lmes
Copy link
Contributor Author

jmalvarezf-lmes commented Aug 1, 2023 via email

@JorTurFer
Copy link
Member

These are the apis enabled:
https://github.com/kedacore/testing-infrastructure/blob/2ca08b562a1187ab25dbbf43dc567cc4129c2cb9/terraform/main.tf#L13-L26

If you need any other API enabled, just draft a PR adding it to the list and tomorrow I'll merge it 😄

fix: add location for queue
e7d8ecc 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@JorTurFer
Copy link
Member

JorTurFer commented Aug 2, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

Jose Maria Alvarez added 2 commits August 2, 2023 11:26
@JorTurFer
Copy link
Member

JorTurFer commented Aug 2, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

fix: specify location also in tasks
bb69243 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@JorTurFer
Copy link
Member

JorTurFer commented Aug 2, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@JorTurFer
Copy link
Member

JorTurFer commented Aug 15, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@zroubalik zroubalik changed the title chore: Add scaler for cloud tasks feat: Add scaler for cloud tasks Aug 16, 2023
@zroubalik zroubalik changed the title feat: Add scaler for cloud tasks feat: Add scaler for Google Cloud Tasks Aug 16, 2023
zroubalik
zroubalik reviewed Aug 16, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/scalers/gcp_cloud_tasks_scaler.go Outdated Show resolved Hide resolved
zroubalik
zroubalik reviewed Sep 5, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmalvarezf-lmes any update on this please?

Jose Maria Alvarez added 6 commits September 5, 2023 17:45
fix: add gcp as prefix for naming for clarity
4b24b3a 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: merge main and correct changelog conflicts
7a87d21 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: correct problem in test when changing name in struct
d0936e1 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: order in changelog
850a1f8 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: another order change in changelog
29fa168 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: more renaming
1fc17fe 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@jmalvarezf-lmes
Copy link
Contributor Author

now renaming should be done, you can check if it's ok, and then launch e2e tests if so.

@JorTurFer
Copy link
Member

JorTurFer commented Sep 5, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp_cloud_tasks
Update: You can check the progress here

zroubalik
zroubalik approved these changes Sep 6, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Just one nit in changelog

pkg/scalers/gcp_cloud_tasks_scaler.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Jose Maria Alvarez added 3 commits September 8, 2023 17:27
fix: put it in a new section
eed2952 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: try a new order for the changelog
f03e8b7 
Signed-off-by: Jose Maria Alvarez <[email protected]>
fix: delete unneeded line
ab06637 
Signed-off-by: Jose Maria Alvarez <[email protected]>
zroubalik
zroubalik reviewed Sep 11, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
fix: into the new Changelog section
c50aea8 
Signed-off-by: Jose Maria Alvarez <[email protected]>
zroubalik
zroubalik reviewed Sep 11, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmalvarezf-lmes there are some conflicts blocking us from merging this PR, could you please take a look? Thanks!

@jmalvarezf-lmes
Copy link
Contributor Author

Should be now addressed.
Thanks,

fix: another order fix
346aba9 
Signed-off-by: Jose Maria Alvarez <[email protected]>
@JorTurFer
Copy link
Member

JorTurFer commented Sep 11, 2023 •
edited
Loading

thanks a lot! ❤️

@JorTurFer
Copy link
Member

JorTurFer commented Sep 11, 2023 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@JorTurFer JorTurFer enabled auto-merge (squash) September 11, 2023 21:10
@JorTurFer JorTurFer merged commit 7daa09a into kedacore:main Sep 11, 2023
18 checks passed
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
@jmalvarezf-lmes @toniiiik
feat: Add scaler for Google Cloud Tasks (kedacore#4834)
a262450 
* feature: add cloud tasks scaler

Signed-off-by: Jose Maria Alvarez <[email protected]>

* Add cloud tasks e2e test

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: Return files to the original state

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: static checks

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: more static checks fixed

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: add location for queue

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: add correct command to create test tasks in queue

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: add fixes to test and add pod identity test

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: specify location also in tasks

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: correct indentation and location for purge

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: correct naming, add package correctly to identity tests

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: change test name

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: add gcp as prefix for naming for clarity

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: correct problem in test when changing name in struct

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: order in changelog

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: another order change in changelog

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: more renaming

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: put it in a new section

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: try a new order for the changelog

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: delete unneeded line

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: into the new Changelog section

Signed-off-by: Jose Maria Alvarez <[email protected]>

* fix: another order fix

Signed-off-by: Jose Maria Alvarez <[email protected]>

---------

Signed-off-by: Jose Maria Alvarez <[email protected]>
Signed-off-by: Jose Maria Alvarez Fernandez <[email protected]>
Signed-off-by: anton.lysina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik approved these changes

@JorTurFer JorTurFer JorTurFer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New scaler: Google Cloud Tasks
3 participants
@jmalvarezf-lmes @JorTurFer @zroubalik