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

feat: Add annotations from ScaledObject to HPA #2659

Merged
merged 1 commit into from
Feb 26, 2022

Conversation

sergeyshevch
Copy link
Contributor

@sergeyshevch sergeyshevch commented Feb 18, 2022

Provide a description of what has been changed

Now annotations from ScaledObject will be applied to HPA.

Use Case:

We validate HPA for the required set of annotations. And our Scaled object contains such annotations but it doesn't reflect it to HPA

Checklist

Fixes #

@sergeyshevch sergeyshevch requested a review from a team as a code owner February 18, 2022 12:39
@sergeyshevch sergeyshevch force-pushed the feature/hpa-annotations branch from e41db5b to 55039f4 Compare February 18, 2022 12:40
@tomkerkhove
Copy link
Member

Would be nice to see if we can have automated tests for this

@sergeyshevch
Copy link
Contributor Author

@tomkerkhove Sure. I will work on it on a week

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

Hi,
Thanks for this improvement, could you add some test for this feature please?
We have a specific tests file for scaledObjectController. You can copy/paste one of the and edit it just for testing this feature

@JorTurFer
Copy link
Member

Sorry, I have just seen that Tom already requested it

@sergeyshevch
Copy link
Contributor Author

@JorTurFer Thanks for the example! I've not forgotten about this PR. I will try to make this today.

@sergeyshevch
Copy link
Contributor Author

@JorTurFer I'm not a pro in a go test but I've tried to add a test for this function.

Can you look at it, please? Also, I've not found any guide on how to run these tests locally.

Signed-off-by: Sergey Shevchenko <shevchenko@simple.life>
@sergeyshevch sergeyshevch force-pushed the feature/hpa-annotations branch from 751ac8d to fdcf8f8 Compare February 26, 2022 10:49
@sergeyshevch
Copy link
Contributor Author

Ok, looks like it's working. @tomkerkhove @JorTurFer Can you look at this again?

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for this contribution ❤️

@JorTurFer
Copy link
Member

BTW @sergeyshevch, you can run unit test using go command directly, but the easiest way is running make test in the root path. We use make to simplify a lot of process :)

@JorTurFer JorTurFer merged commit c643679 into kedacore:main Feb 26, 2022
@sergeyshevch
Copy link
Contributor Author

@JorTurFer Yeah, I found it. But it requires local installation of k8s)

@JorTurFer
Copy link
Member

For make test? Are you sure?
I'm AFK but when I arrive home I'll check it

@JorTurFer
Copy link
Member

In theory, you don't need local cluster for it.
make test a mock for the control plane

@sergeyshevch
Copy link
Contributor Author

@JorTurFer Sorry for following up in this thread. When the next release is planned? I want to get this patch into production in some time.

@JorTurFer
Copy link
Member

hi @sergeyshevch
Next release will be during the first half of May

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants