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

Add eagerScalingStrategy for ScaledJob #5872

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

junekhan
Copy link
Contributor

@junekhan junekhan commented Jun 8, 2024

Added eagerScalingStrategy for ScaledJob

Checklist

Fixes #5114

Relates to #

Signed-off-by: June Han <sorrowitsch@gmail.com>
Signed-off-by: June Han <sorrowitsch@gmail.com>
@junekhan junekhan requested a review from a team as a code owner June 8, 2024 08:05
Signed-off-by: June Han <sorrowitsch@gmail.com>
Signed-off-by: June Han <sorrowitsch@gmail.com>
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.

@junekhan thanks for the contribution, could you please also cover this scenario in e2e tests?

@junekhan junekhan force-pushed the feat/eager-mode branch 4 times, most recently from dc4f7ba to 29006d3 Compare June 13, 2024 08:34
Signed-off-by: June Han <sorrowitsch@gmail.com>
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.

Could you please open pr in https://github.com/kedacore/keda-docs/ to cover this new feature?

@junekhan
Copy link
Contributor Author

Could you please open pr in https://github.com/kedacore/keda-docs/ to cover this new feature?

Sure, thanks! Will do that.

@zroubalik
Copy link
Member

zroubalik commented Jun 24, 2024

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

@junekhan
Copy link
Contributor Author

@zroubalik Hi there, I'm sorry that the e2e test didn't go well. I'm glad to analyze the logs, although it seems that I don't have access to it.

@zroubalik
Copy link
Member

@junekhan you should be able to access the logs by following these steps (let me know if you are not):
Screenshot 2024-06-25 at 22 18 29
Screenshot 2024-06-25 at 22 17 34
Screenshot 2024-06-25 at 22 18 01

@junekhan
Copy link
Contributor Author

@zroubalik Thanks for your input. I have no problem accessing the test suite log you mentioned, but I was looking for the log from inside the cluster, which can contain information about how the scaling decision was made. The test suite log only shows that the scaling didn't happen a single time, otherwise, I didn't find any clues.

@zroubalik
Copy link
Member

@zroubalik Thanks for your input. I have no problem accessing the test suite log you mentioned, but I was looking for the log from inside the cluster, which can contain information about how the scaling decision was made. The test suite log only shows that the scaling didn't happen a single time, otherwise, I didn't find any clues.

At the bottom of the output, there's log from KEDA operator as well. That's the best we can do, you can always run the failed test locally: https://github.com/kedacore/keda/tree/main/tests#specific-test

@junekhan
Copy link
Contributor Author

@zroubalik Thanks for your input. I have no problem accessing the test suite log you mentioned, but I was looking for the log from inside the cluster, which can contain information about how the scaling decision was made. The test suite log only shows that the scaling didn't happen a single time, otherwise, I didn't find any clues.

At the bottom of the output, there's log from KEDA operator as well. That's the best we can do, you can always run the failed test locally: https://github.com/kedacore/keda/tree/main/tests#specific-test

@zroubalik Thanks for your hint! That looks daunting 😨 but I will try.

…uration

Signed-off-by: June Han <sorrowitsch@gmail.com>
password = fmt.Sprintf("%s-password", testName)
vhost = "/"
connectionString = fmt.Sprintf("amqp://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace)
httpConnectionString = fmt.Sprintf("http://%s:%s@rabbitmq.%s.svc.cluster.local/", user, password, rmqNamespace)
Copy link

Choose a reason for hiding this comment

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

Semgrep found a possible database connection string built with string concatenation. Check for proper encoding/escaping of components to prevent parse errors and injection vulnerabilities.

Ignore this finding from db-connection-string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks okay.

Signed-off-by: June Han <sorrowitsch@gmail.com>
@junekhan
Copy link
Contributor Author

Could you please rerun the checks? The result doesn't make sense.

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.

Add Eager mode for ScaledJobs
2 participants