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 new gitlab-runner-scaler #6087

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

fira42073
Copy link
Contributor

@fira42073 fira42073 commented Aug 17, 2024

Provide a description of what has been changed

Checklist

Fixes #5616

@fira42073 fira42073 changed the title Add new gitlab-runner-scaler; bootstrap configuration Add new gitlab-runner-scaler Aug 17, 2024
@fira42073
Copy link
Contributor Author

fira42073 and others added 15 commits August 26, 2024 00:44
… "waiting for resource" status

Signed-off-by: Fira Curie <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
* chore: Prepare v2.15.1

Signed-off-by: Jorge Turrado <[email protected]>

* fix changelog

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
* chore: Enable Az Pipeline e2e

Signed-off-by: Jorge Turrado <[email protected]>

* update manifests

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
* chore: Improve azure e2e coverage

Signed-off-by: Jorge Turrado <[email protected]>

* add missing vars

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
…dacore#6072)

* Update

Signed-off-by: SpiritZhou <[email protected]>

* Update ChangeLog

Signed-off-by: SpiritZhou <[email protected]>

* Update

Signed-off-by: SpiritZhou <[email protected]>

---------

Signed-off-by: SpiritZhou <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
* add errorWhenMetricValueEmpty

Signed-off-by: Rob Pickerill <[email protected]>

* fix golangci-lint

Signed-off-by: Rob Pickerill <[email protected]>

* improve error message for empty results

Signed-off-by: Rob Pickerill <[email protected]>

* add error when empty metric values to changelog

Signed-off-by: Rob Pickerill <[email protected]>

* rename errorWhenMetricValuesEmpty -> errorWhenNullValues

Signed-off-by: Rob Pickerill <[email protected]>

* use getParameterFromConfigV2 to read config for errorWhenNullValues

Signed-off-by: Rob Pickerill <[email protected]>

* add e2e for error state for cw, and improve e2e for min values for cw

Signed-off-by: Rob Pickerill <[email protected]>

* remove erroneous print statement

Signed-off-by: Rob Pickerill <[email protected]>

* remove unused vars

Signed-off-by: Rob Pickerill <[email protected]>

* rename errorWhenMetricValuesEmpty -> ignoreNullValues

Signed-off-by: Rob Pickerill <[email protected]>

* move towards shared package for e2e aws

Signed-off-by: Rob Pickerill <[email protected]>

* minMetricValue optionality based on ignoreNullValues

Signed-off-by: Rob Pickerill <[email protected]>

* fail fast

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>

* Update tests/scalers/aws/aws_cloudwatch_ignore_null_values_false/aws_cloudwatch_ignore_null_values_false_test.go

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>

* fail fast

Signed-off-by: Rob Pickerill <[email protected]>

* fix broken new line

Signed-off-by: Rob Pickerill <[email protected]>

* fix broken new lines

Signed-off-by: Rob Pickerill <[email protected]>

* assert no scaling changes in e2e, and set false for required in minMetricValue

Signed-off-by: robpickerill <[email protected]>

* fix ci checks

Signed-off-by: robpickerill <[email protected]>

* Update tests/scalers/aws/aws_cloudwatch_min_metric_value/aws_cloudwatch_min_metric_value_test.go

fix invalid check

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>

* fix merge conflicts

Signed-off-by: robpickerill <[email protected]>

* fix e2e package names

Signed-off-by: robpickerill <[email protected]>

---------

Signed-off-by: Rob Pickerill <[email protected]>
Signed-off-by: robpickerill <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
* add a static connection name

Signed-off-by: robpickerill <[email protected]>

* Update pkg/scalers/rabbitmq_scaler.go

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>

* add improvement to changelog

Signed-off-by: robpickerill <[email protected]>

* add namepace and so name to conn name

Signed-off-by: robpickerill <[email protected]>

* Update comment

Signed-off-by: Jorge Turrado Ferrero <[email protected]>

---------

Signed-off-by: robpickerill <[email protected]>
Signed-off-by: Rob Pickerill <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
Signed-off-by: Fira Curie <[email protected]>
@fira42073
Copy link
Contributor Author

I don't know why, but I'm shown as coauthor of some commits that I haven't been a part of.

Comment on lines 277 to 290
func setConfigValueURL(_ Params, valFromConfig string, field reflect.Value) error {
u, err := url.Parse(valFromConfig)
if err != nil {
return fmt.Errorf("expected url.URL or *url.URL, unable to parse %q: %w", valFromConfig, err)
}

// If the field type is a pointer to url.URL (`*url.URL`), set the value directly
if field.Kind() == reflect.Ptr && field.Type().Elem() == reflect.TypeOf(url.URL{}) {
field.Set(reflect.ValueOf(u))
return nil
}

return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

can you please add a simple unit test for this? you can take an inspiration in

func TestBasicTypedConfig(t *testing.T) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -273,6 +273,22 @@ func setConfigValueURLParams(params Params, valFromConfig string, field reflect.
return nil
}

// setConfigValueURL is a function that sets the value of the url.URL field
func setConfigValueURL(_ Params, valFromConfig string, field reflect.Value) error {
Copy link
Member

Choose a reason for hiding this comment

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

per our discussion in #5616 (comment), you don't have to pass arguments that the function body doesn't need

Suggested change
func setConfigValueURL(_ Params, valFromConfig string, field reflect.Value) error {
func setConfigValueURL(valFromConfig string, field reflect.Value) error {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed that

Copy link

stale bot commented Nov 7, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale All issues that are marked as stale due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitLab Runner Scaler
7 participants