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

fix scale with windows nodes in cluster issue 4908 #5917

Closed
wants to merge 39 commits into from
Closed

fix scale with windows nodes in cluster issue 4908 #5917

wants to merge 39 commits into from

Conversation

Doofus100500
Copy link

@Doofus100500 Doofus100500 commented Jun 27, 2024

Add

Checklist

Fixes #
#4908

@Wolfe1
Copy link
Contributor

Wolfe1 commented Jun 27, 2024

In terms of simply ignoring any windows nodes I think this would do the trick. a few things that I think you would need before this got merged:

  • Add to the unit tests for scenarios with windows nodes present
  • Add End 2 end tests with windows nodes present
  • Update documentation to show that windows nodes are currently ignored

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.

What would happen if user would like to scale windows nodes?

@Wolfe1
Copy link
Contributor

Wolfe1 commented Jun 27, 2024

What would happen if user would like to scale windows nodes?

With this implementation, it would not work.

Looking at the code again, we already say what platform we wish to scale from in the Keda settings. Now that we pull the platformName from the grid sessions I would think we just use that to make sure it matches the expected platformName in our Keda settings rather than just completely excluding windows.

@Doofus100500
Copy link
Author

In my case, it’s enough to simply exclude Windows. This is because Windows nodes live in a VM. However, even before my PR, the user wouldn’t have been able to scale Windows nodes. I don’t quite understand which settings you are referring to, as I am not that deeply involved in your project.

@Doofus100500
Copy link
Author

Now that we pull the platformName from the grid sessions I would think we just use that to make sure it matches the expected platformName in our Keda settings rather than just completely excluding windows.

@Wolfe1 Like that?

@Doofus100500 Doofus100500 requested a review from zroubalik July 8, 2024 06:41
@Doofus100500
Copy link
Author

@zroubalik @Wolfe1 Hi, is there a chance this PR will be accepted?

@Doofus100500
Copy link
Author

Doofus100500 commented Aug 1, 2024

@zroubalik @Wolfe1 Hi, could you take another look? I added unit tests. Are you sure we need to add changes to e2e tests? There is nothing about the platform in the existing ones.

novoselov and others added 19 commits August 1, 2024 16:50
Signed-off-by: novoselov <[email protected]>
Signed-off-by: circa10a <[email protected]>
Signed-off-by: Caleb Lemoine <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
Signed-off-by: novoselov <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Tom Kerkhove <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: novoselov <[email protected]>
* Add support to fetch username from env for MYSQL Scaler

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

* Add changelog

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

* Change if-else block to switch block

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

* Refactor scaler switch block

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

* Refactor method

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

* Refactor Mysql Scaler

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

* Address failure in static checks

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

* Address Review Comments

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

---------

Signed-off-by: Indresh2410 <[email protected]>
Signed-off-by: novoselov <[email protected]>
| datasource  | package              | from   | to     |
| ----------- | -------------------- | ------ | ------ |
| github-tags | actions/setup-python | v5.1.0 | v5.1.1 |

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: novoselov <[email protected]>
…/sdk/azidentity` (#5471)

* Migrate EventHub SDK

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

* remove old sdk

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

* sort

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

* migrate storage SDK

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

* update changelog and style

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

* migrate key vault

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

* fix typos

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

* Update azure monitor

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

* fix typo

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

* fix e2e tests

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

* fix error

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

* use AZQUERY FOR MONITOR

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

* fix style

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

* use client optiosn

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

* use sdk parsing function

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

* migrate log analytics

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

* update tokens

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

* style

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

* fix typo

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

* Add missing changes

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

* bump azure deps

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

* fix packages

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

* fix code

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

* fix test

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

* fix styles

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

* remove todos

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

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
* feat: add TLS auth support for IBM MQ scaler

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

* fix: fix ibmmq scaler test

Signed-off-by: Rick Brouwer <[email protected]>

* fix: correct httpClient

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

---------

Signed-off-by: rickbrouwer <[email protected]>
Signed-off-by: Rick Brouwer <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: novoselov <[email protected]>
…ic value found (#5897)

Signed-off-by: Benjamin Woodley <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
* add core logic to support access token in postgres scaler

Signed-off-by: Ferdinand de Baecque <[email protected]>

* minor fix

Signed-off-by: Ferdinand de Baecque <[email protected]>

* run make build to fmt code

Signed-off-by: Ferdinand de Baecque <[email protected]>

* make regexp password pattern global

Signed-off-by: Ferdinand de Baecque <[email protected]>

* adapt to use placeholder for regexp

Signed-off-by: Ferdinand de Baecque <[email protected]>

* add missing authPodIdentity variable

Signed-off-by: Ferdinand de Baecque <[email protected]>

* lint code using gci write... command

Signed-off-by: Ferdinand de Baecque <[email protected]>

* lint import + add 2 unite tests

Signed-off-by: Ferdinand de Baecque <[email protected]>

* lint with make fmt

Signed-off-by: Ferdinand de Baecque <[email protected]>

* remove podIdentityAzure references (but keep AzureWorkload ones)

Signed-off-by: Ferdinand de Baecque <[email protected]>

* replace switch by if statements + fix error when comparing + close connection before recreating it

Signed-off-by: Ferdinand de Baecque <[email protected]>

* generate a new token if the current one has expired + add log info statement

Signed-off-by: Ferdinand de Baecque <[email protected]>

* minor change + add entry in CHANGELOG.md

Signed-off-by: Ferdinand de Baecque <[email protected]>

* Add first draft of an e2e test

Signed-off-by: Ferdinand de Baecque <[email protected]>

* Add comment and change package name

Signed-off-by: Ferdinand de Baecque <[email protected]>

* fix golanci lint

Signed-off-by: Ferdinand de Baecque <[email protected]>

* use identity 1 in e2e tests

Signed-off-by: Ferdinand de Baecque <[email protected]>

* fix e2e tests after testing it + change .env file

Signed-off-by: Ferdinand de Baecque <[email protected]>

* go fmt

Signed-off-by: Ferdinand de Baecque <[email protected]>

* remove entries in .env file

Signed-off-by: Ferdinand de Baecque <[email protected]>

* Add Postgres env variables

Signed-off-by: Ferdinand de Baecque <[email protected]>

* remove useless variables

Signed-off-by: Ferdinand de Baecque <[email protected]>

* Update e2e test to reset all the task using a query

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

* missing changes after rebase

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

* fix typo in the query

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

* remove the load

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

* fix style

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

---------

Signed-off-by: Ferdinand de Baecque <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Co-authored-by: Jorge Turrado <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: novoselov <[email protected]>
This scaledjob test template was missing its namespace, which doesn't
generally matter for the test -- the test is just checking if the
webhook works, it doesn't care where the scaledjob ends up.

Where it does matter, is if you happen to run this test suite in a more
restrictive environment where you can't write to the default namespace,
because you fail with a namespace-related creation error instead of the
expected "no triggers defined in the ScaledObject/ScaledJob" error.

This just adds the namespace to the template so it's just like all the
other ones in the test suite.

Signed-off-by: John Kyros <[email protected]>
Signed-off-by: novoselov <[email protected]>
JorTurFer and others added 18 commits August 1, 2024 16:51
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
…5496)

* Add new Datadog External scaler to talk to the DCA

Signed-off-by: Ara Pulido <[email protected]>

* Add ability to retrieve a metric value from the DCA

Signed-off-by: Ara Pulido <[email protected]>

* Use datadogmetric naming convention

Signed-off-by: Ara Pulido <[email protected]>

* Merge both Datadog scalers into one

Signed-off-by: Ara Pulido <[email protected]>

* Add authMode to the TriggerAuthentication

Signed-off-by: Ara Pulido <[email protected]>

* Add unit tests for Datadog scaler with Cluster Agent proxy

Signed-off-by: Ara Pulido <[email protected]>

* Fix activation

Signed-off-by: Ara Pulido <[email protected]>

* Add E2E tests for the Datadog scaler using the Cluster Agent proxy

Signed-off-by: Ara Pulido <[email protected]>

* Fixes after rebase

Signed-off-by: Ara Pulido <[email protected]>

* Rearrange Datadog tests

Signed-off-by: Ara Pulido <[email protected]>

* Fix linting errors

Signed-off-by: Ara Pulido <[email protected]>

* Keep token only auth

Signed-off-by: Ara Pulido <[email protected]>

* Remove references to ca metadata

Signed-off-by: Ara Pulido <[email protected]>

* remove trailing space

Signed-off-by: Ara Pulido <[email protected]>

* Add changelog entry for cluster agent as proxy feature

Signed-off-by: Ara Pulido <[email protected]>

* Fix activation parameter for Datadog API

Signed-off-by: Ara Pulido <[email protected]>

* Adjust Datadog test values

Signed-off-by: Ara Pulido <[email protected]>

* Fix typo in Datadog test

Signed-off-by: Ara Pulido <[email protected]>

* Make cluster agent metric server parameter mandatory

Signed-off-by: Ara Pulido <[email protected]>

* Use only service name and namespace to resolve the Cluster Agent service IP

Signed-off-by: Ara Pulido <[email protected]>

* Fix linting issues

Signed-off-by: Ara Pulido <[email protected]>

---------

Signed-off-by: Ara Pulido <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
…queueLengthStrategy (#5875)

Signed-off-by: Leonardo D'Ippolito <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: June Han <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: novoselov <[email protected]>
* Annotate Jobs with parent ScaledJob generation

Signed-off-by: Josef Karasek <[email protected]>

* fix tests

Signed-off-by: Josef Karasek <[email protected]>

* fix lint

Signed-off-by: Josef Karasek <[email protected]>

* fix log message

Signed-off-by: Josef Karasek <[email protected]>

* update changelog

Signed-off-by: Josef Karasek <[email protected]>

* update changelog

Signed-off-by: Josef Karasek <[email protected]>

* update changelog

Signed-off-by: Josef Karasek <[email protected]>

---------

Signed-off-by: Josef Karasek <[email protected]>
Signed-off-by: Zbynek Roubalik <[email protected]>
Co-authored-by: Zbynek Roubalik <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
* chore: build with keda-tools:1.22.5
to resolve CVE-2024-24790, CVE-2024-24789, and CVE-2024-24791
bump github.com/Azure/azure-sdk-for-go/sdk/azidentity to resolve CVE-2024-35255

Signed-off-by: Paul Yu <[email protected]>

* chore: use go install instead of go get and replacing deprecated tools

Signed-off-by: Paul Yu <[email protected]>

* chore: vendor dependency cleanup

Signed-off-by: Paul Yu <[email protected]>

* Update missing references to 1.21

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

---------

Signed-off-by: Paul Yu <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: novoselov <[email protected]>
* Add first scaler version

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

* small refactor for response validation

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

* Add 'from' property, rename host/token

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

* Add parsing tests

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

* update changelog

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

* Update CHANGELOG.md

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

* Update values type to float64

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

* Remove unnecessary conversion

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

* e2e tests

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

* Apply suggestions from code review

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

* Update dynatrace_test.go

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

* Fix bad templating for e2e tests

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

* Revert unnecessary (?) template variable change

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

* Apply suggestions from code review

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

* Update tests/scalers/dynatrace/dynatrace_test.go

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

* Do not allow token to be passed in scaledobject trigger

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

* Remove bad secret, tweak dynakube test config

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

* Rename property in response parsing

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

* Update tests/scalers/dynatrace/dynatrace_test.go

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

* use new operator secret, update template variable naming

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

* forgotten correct variable definition

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

* try default value in query for e2e tests

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

* fix missing closing parenthesis, bad indenting

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

* Update e2e test to use custom metrics

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

* Close the body to fix static checks

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

* use declarative scaler config

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

---------

Signed-off-by: cyrilico <[email protected]>
Signed-off-by: damas <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: novoselov <[email protected]>
* Remove deprecated cortexOrgId in prometheus scaler

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

* Move to breaking changes

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

---------

Signed-off-by: dttung2905 <[email protected]>
Signed-off-by: novoselov <[email protected]>
fix
Signed-off-by: novoselov <[email protected]>
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
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.

@Doofus100500 could you please keep here only relevant commits?

@Doofus100500 Doofus100500 closed this by deleting the head repository Aug 7, 2024
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.