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

Refactor the Load Generator and Support SteadyState Types #7265

Merged
merged 10 commits into from Apr 22, 2022

Conversation

rthallisey
Copy link
Contributor

@rthallisey rthallisey commented Feb 22, 2022

Refactor the load generator code.

  • Define workload types as an interface so it can be extended to accommodate each job type
  • Add support for steady-state job
  • Simplify the config (rely on outside scripts to handle ns creation, iterations, ect...)

Test below shows a steady-state workload with churn: 2 count: 5. This will create 5 VMIs and delete 2 VMIs frequently over a 5m period.

name: kubevirt-steady-state-test
timeout: 5m
count: 5
churn: 2
minChurnSleep: 20s
type: "steady-state"
object:
  templateFile: vmi-ephemeral.yaml
  inputVars:
    containerPrefix: quay.io/kubevirt
    containerImg: cirros-container-disk-demo
    containerTag: ""
    namespace: default

Screen Shot 2022-02-22 at 9 31 36 AM

Release note:

Support steady-state job types in the load-generator tool

Future work:

  • The SteadyState CreateWorkload is really the same as Burst CreateWorkload. Call Burst create from SteadyState.
  • Refacore the Watcher package

@rthallisey
Copy link
Contributor Author

/cc @marceloamaral @davidvossel

Copy link
Contributor

@marceloamaral marceloamaral left a comment

Choose a reason for hiding this comment

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

This PR will affect two prow jobs. So before merging this, you also need another PR to fix those jobs.

periodic-kubevirt-performance-cluster-100-density-test

periodic-kubevirt-performance-cluster-scale-density-test

Both jobs execute the automation/perfscale-test.sh, which run the perfscale-load-generator tool. You also need to update this script accordingly to the new logic.

/hold

config.QPS = float32(t.Global.QPS)
config.Burst = t.Global.Burst
config.QPS = QPS
config.Burst = Burst
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the client QPS and Burst are not configurable anymore?
In fact, by default I was disabling rate limiting for the client library (i.e., 0 values)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to reduce config options. I default to 20 which should be plenty for the time being.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we should hard-code this.... Actually, we should avoid hard-coded parameters....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed both to unlimited. We can come back and change this as the tool matures.

tools/perfscale-load-generator/object/unstructured.go Outdated Show resolved Hide resolved
@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2022
@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. size/XXL labels Mar 5, 2022
@rthallisey rthallisey force-pushed the refactor-lg branch 2 times, most recently from db0867a to 6c50d82 Compare March 7, 2022 18:46
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/network and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Mar 7, 2022
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2022
@rthallisey
Copy link
Contributor Author

/retest

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Mar 7, 2022
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Mar 7, 2022
@rthallisey
Copy link
Contributor Author

created kubevirt/project-infra#1946. I think this change can go in first and 1946 can quickly follow.

@rthallisey
Copy link
Contributor Author

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 7, 2022
@rthallisey
Copy link
Contributor Author

/retest

@kubevirt-bot kubevirt-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 10, 2022
@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Mar 10, 2022
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Mar 10, 2022
@marceloamaral
Copy link
Contributor

/retest
/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 7, 2022
@rthallisey
Copy link
Contributor Author

/retest

Copy link
Member

@davidvossel davidvossel left a comment

Choose a reason for hiding this comment

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

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2022
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
Signed-off-by: Ryan Hallisey <rhallisey@nvidia.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 20, 2022
@rthallisey
Copy link
Contributor Author

rebased. @marceloamaral need another lgtm from you

@rthallisey
Copy link
Contributor Author

/retest

@marceloamaral
Copy link
Contributor

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2022
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/network size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants