Skip to content

test: add e2e test for openstack metrics scaler#7638

Open
Abhicodeitout wants to merge 9 commits intokedacore:mainfrom
Abhicodeitout:metrics-scaler
Open

test: add e2e test for openstack metrics scaler#7638
Abhicodeitout wants to merge 9 commits intokedacore:mainfrom
Abhicodeitout:metrics-scaler

Conversation

@Abhicodeitout
Copy link
Copy Markdown

@Abhicodeitout Abhicodeitout commented Apr 10, 2026

Provide a description of what has been changed

Checklist

  • When introducing a new scaler, I agree with the scaling governance policy
  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added (if applicable)
  • Ensure make generate-scalers-schema has been run to update any outdated generated files
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4155

@Abhicodeitout Abhicodeitout requested a review from a team as a code owner April 10, 2026 14:15
@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Apr 10, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@rickbrouwer
Copy link
Copy Markdown
Member

Hi! I see in the description that you want to provide an e2e test, for which thanks.
However, I also see a large number of adjustments to other files, which seems unnecessary to me.

@Abhicodeitout
Copy link
Copy Markdown
Author

Abhicodeitout commented Apr 10, 2026

@rickbrouwer So should I revert those and fix only tests one

@rickbrouwer
Copy link
Copy Markdown
Member

Yes please, that keeps the PR scoped

@Abhicodeitout Abhicodeitout force-pushed the metrics-scaler branch 2 times, most recently from a942e02 to 3021816 Compare April 12, 2026 06:36
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer is there some issue in the Github actions

@rickbrouwer
Copy link
Copy Markdown
Member

@rickbrouwer is there some issue in the Github actions

You need to run ‘go fmt .’ in the directory where you added the file

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer is there some issue in the Github actions

You need to run ‘go fmt .’ in the directory where you added the file

I have ran that but then also

@Abhicodeitout
Copy link
Copy Markdown
Author

@dangerousplay is there any plublic group on discord slack or telegram where we discuss to the issues or workspace

@rickbrouwer
Copy link
Copy Markdown
Member

I have ran that but then also

Perhaps you ran it from the wrong place. Otherwise, go to the directory and run it there.

cd tests/scalers/openstack_metrics
go fmt .

Also, please sign your commits (DCO).

@rickbrouwer
Copy link
Copy Markdown
Member

@dangerousplay is there any plublic group on discord slack or telegram where we discuss to the issues or workspace

I think you're posting this in the wrong place?

@Abhicodeitout Abhicodeitout force-pushed the metrics-scaler branch 2 times, most recently from b1ff136 to 85a87c4 Compare April 13, 2026 12:32
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can you review ?

Abhicodeitout added a commit to Abhicodeitout/keda that referenced this pull request Apr 14, 2026
Revert unrelated changes from this branch so PR kedacore#7638 only contains the intended OpenStack metrics e2e test work.

This commit removes accidental modifications to workflow files, changelog, and metrics API scaler implementation/tests that belong to a separate effort.

Also aligns with reviewer request to keep the PR narrowly scoped.

Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can you review ?

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented Apr 14, 2026

Can you undo the GitHub workflows and changelog changes?

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer i have reverted changes can you review ?

@rickbrouwer
Copy link
Copy Markdown
Member

There are still adjustments out of scope.

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can u look now hope this time it goes well

@rickbrouwer
Copy link
Copy Markdown
Member

Looks good overall, but a few things caught my eye.

PostMeasure constructs the URL as metricsURL + "/v1/metric/" + metricID + "/measures", while the scaler does path.Join(openstackMetricsURL.Path, metricID+"/measures"). Are those two hitting the same endpoint for the same OPENSTACK_METRICS_URL value?

Further, with granularity=300, the scaler computes (300/60) - 1 = 4 and then adds that to time.Now(), setting start roughly 4 minutes into the future. Wouldn't it return an empty result for that window, meaning the scaler always returns 0, which would make testScaleOut fail regardless of the value posted?

@Abhicodeitout
Copy link
Copy Markdown
Author

Abhicodeitout commented Apr 16, 2026

@rickbrouwer

both fixed.

  1. URL mismatchPostMeasure was appending /v1/metric/<id>/measures to the raw base URL, while the scaler appends <id>/measures directly to metricsURL. Aligned PostMeasure to use metricsURL + "/" + metricID + "/measures" so both paths hit the same endpoint, assuming OPENSTACK_METRICS_URL already includes /v1/metric.

  2. Time windowgranularity=300 was computing (300/60)-1 = 4 and adding that to time.Now(), setting start ~4 minutes in the future and guaranteeing an empty response.

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented Apr 18, 2026

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

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer @JorTurFer CAN YOU TRIGGER TEST RUN ?

Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer @JorTurFer CAN YOU RUN THE TESTS ?

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer what happened are there any mis changes ?

@rickbrouwer
Copy link
Copy Markdown
Member

Please avoid all caps, it comes across as impolite.

@rickbrouwer
Copy link
Copy Markdown
Member

I see that the fix you made earlier is suddenly gone. Is it no longer needed?

@Abhicodeitout
Copy link
Copy Markdown
Author

Abhicodeitout commented May 1, 2026

@rickbrouwer actually earlier rebase went wrong now i have corrected it can you trigger it now ? i tried to mock as suggested by @JorTurFer for gnoochi and tested it went well

@rickbrouwer
Copy link
Copy Markdown
Member

I mean, you initially made a change in the OpenStack scaler itself due to a bug, right? But I don't see that in this pr anymore.

Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer i have updated the code and fixed the bug and tested by mocking. Can you trigger the test ?

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented May 2, 2026

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

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented May 5, 2026

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

@Abhicodeitout
Copy link
Copy Markdown
Author

Abhicodeitout commented May 5, 2026

If its good can i get a merge @rickbrouwer @JorTurFer

@rickbrouwer
Copy link
Copy Markdown
Member

The test was skipped due to a missing variable OPENSTACK_METRICS_URL:

2026-05-05T06:29:22.8359169Z     openstack_metrics_test.go:***5***: skipping OpenStack metrics test: unable to discover metric service from catalog and OPENSTACK_METRICS_URL is not set: scaler could not find the service URL dynamically. Either provide it in the scaler parameters or check your OpenStack configuration: service 'metric' not found in catalog. Please, provide different credentials or reach to your OpenStack cluster manager
2026-05-05T06:29:22.8362021Z --- SKIP: TestScaler (***.66s)

@Abhicodeitout
Copy link
Copy Markdown
Author

The test was skipped due to a missing variable OPENSTACK_METRICS_URL:

2026-05-05T06:29:22.8359169Z     openstack_metrics_test.go:***5***: skipping OpenStack metrics test: unable to discover metric service from catalog and OPENSTACK_METRICS_URL is not set: scaler could not find the service URL dynamically. Either provide it in the scaler parameters or check your OpenStack configuration: service 'metric' not found in catalog. Please, provide different credentials or reach to your OpenStack cluster manager
2026-05-05T06:29:22.8362021Z --- SKIP: TestScaler (***.66s)

So what should be that url ? @rickbrouwer

Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can you start test

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can you trigger test?

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented May 5, 2026

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

@rickbrouwer
Copy link
Copy Markdown
Member

=== RUN TestScaler
openstack_metrics_test.go:5: skipping OpenStack metrics test: unable to discover metric service from catalog and OPENSTACK_METRICS_URL is not set: metric lookup failed: scaler could not find the service URL dynamically. Either provide it in the scaler parameters or check your OpenStack configuration: service 'metric' not found in catalog. Please, provide different credentials or reach to your OpenStack cluster manager; gnocchi lookup failed: scaler could not find the service URL dynamically. Either provide it in the scaler parameters or check your OpenStack configuration: service 'gnocchi' not found in catalog. Please, provide different credentials or reach to your OpenStack cluster manager
--- SKIP: TestScaler (***.***5s)

@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer
If your cloud truly has no metrics endpoint in its Keystone catalog, the e2e test will still skip unless OPENSTACK_METRICS_URL is set. The change only fixes the false-negative case where the endpoint exists under service type metric but was not being resolved

Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
Signed-off-by: Abhishek Kumar Kushwaha <abhithegabbar@gmail.com>
@Abhicodeitout
Copy link
Copy Markdown
Author

@rickbrouwer can you trigger test run

@rickbrouwer
Copy link
Copy Markdown
Member

rickbrouwer commented May 8, 2026

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

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 e2e test for Openstack Metrics Scaler

4 participants