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

Update container/pod benchmarking procedures. #894

Merged
merged 9 commits into from May 23, 2022

Conversation

aznashwan
Copy link
Contributor

@aznashwan aznashwan commented Feb 15, 2022

What type of PR is this?

/kind documentation

What this PR does / why we need it:

This PR updates the benchmarking procedures for containers and pods.
The number of container/pod benchmarks as well as how many can be run in parallel are now configurable via an external YAML file, and the results of the benchmarks are output as JSON files within a provided directory.

The main purpose behind parametrizing the benchmarks is to allow for analysis of larger sample sizes on the same host to spot any performance degradations within the runtime.
These changes are also intended to be integrated within GitHub workflows on the main containerd repo to monitor performance across supported OSes/runtimes for any new RC/full release.

Which issue(s) this PR fixes:

None.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added optional `--benchmarking-params-file` CLI flag for providing the path to a YAML file defining custom parameters which provide control over the total and parallel number of benchmarks performed. Please review the docs for full details.
Added optional `--benchmarking-output-dir` CLI flag for providing the path to a pre-existing directory in which the results of benchmarks will be saved.

@k8s-ci-robot
Copy link
Contributor

@aznashwan: The label(s) kind/benchmarking cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind benchmarking
/kind documentation

What this PR does / why we need it:

This PR updates the benchmarking procedures for containers and pods.
The number of container/pod benchmarks as well as how many can be run in parallel are now configurable via an external YAML file, and the results of the benchmarks are output as JSON files within a provided directory.

The main purpose behind parametrizing the benchmarks is to allow for analysis of larger sample sizes on the same host to spot any performance degradations within the runtime.
These changes are also intended to be integrated within GitHub workflows on the main containerd repo to monitor performance across supported OSes/runtimes for any new RC/full release.

Which issue(s) this PR fixes:

None.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added optional `--benchmarking-params-file` CLI flag for providing the path to a YAML file defining custom parameters which provide control over the total and parallel number of benchmarks performed. Please review the docs for full details.
Added optional `--benchmarking-output-dir` CLI flag for providing the path to a pre-existing directory in which the results of benchmarks will be saved.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 15, 2022
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 15, 2022
@aznashwan aznashwan force-pushed the benchmarking branch 2 times, most recently from 59e8753 to 0153cf3 Compare February 15, 2022 15:56
aznashwan added a commit to aznashwan/containerd that referenced this pull request Feb 21, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Feb 21, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Mar 16, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
@dcantah
Copy link
Member

dcantah commented Mar 17, 2022

@feiskyer are you able to take a look here?

Copy link
Member

@feiskyer feiskyer left a comment

Choose a reason for hiding this comment

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

Thanks for adding those customizations for benchmarking.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 21, 2022
@feiskyer
Copy link
Member

@aznashwan could you fix the build failures reported on https://github.com/kubernetes-sigs/cri-tools/runs/5203016894?check_suite_focus=true?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 21, 2022
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 21, 2022
@aznashwan
Copy link
Contributor Author

@feiskyer thanks a lot for taking a look!
I've just rebased the PR and am awaiting all CI checks.

could you fix the build failures reported on https://github.com/kubernetes-sigs/cri-tools/runs/5203016894?check_suite_focus=true

I had noticed that issue but was unable to replicate it outside of GitHub runners (I have run a make release on x86_64-based Linux, Windows, and macOS just to be sure) so I suspect it's something related to either the image/host the GitHub runners are using, or something related to how the actions/setup-go@v2 action library sets up Golang within the runner.

Either way, I have created this gomega PR which should address the root cause of the issue, and will re-vendor it if/when it gets fixed.

aznashwan added a commit to aznashwan/containerd that referenced this pull request Mar 21, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Mar 21, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
@aznashwan
Copy link
Contributor Author

Minor update: added a function to defer the check on the pod container timeout here

@aznashwan
Copy link
Contributor Author

@feiskyer I'm glad to say my gmeasure patch was recently merged so this PR now only depends on the module getting bumped.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 2, 2022
@aznashwan aznashwan force-pushed the benchmarking branch 2 times, most recently from e0b9a56 to d3df361 Compare May 5, 2022 12:26
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2022
aznashwan and others added 6 commits May 9, 2022 14:30
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
This patch defines new types and mechanisms for managing benchmark
results using a channel-based appriach, as the previous
gmeasure.Stopwatch-based approach did not provide a mechanism
for associating operations which are part of a larger lifecycle
being benchmarked. (e.g. container CRUD operations)

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2022
aznashwan added a commit to aznashwan/containerd that referenced this pull request May 10, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Bring the image-related benchmarks in line with the container and pod
benchmaks by parametrizing the benchmark settings and switching to
`gmeasure.experiment` for running the benchmarks.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request May 11, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aznashwan, feiskyer, saschagrunert

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:
  • OWNERS [feiskyer,saschagrunert]

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

@k8s-ci-robot k8s-ci-robot merged commit 4b7a280 into kubernetes-sigs:master May 23, 2022
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jun 7, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jun 14, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jun 14, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jun 15, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jun 28, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Jul 19, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
aznashwan added a commit to aznashwan/containerd that referenced this pull request Oct 26, 2022
This patch leverages the new benchmarking features added in cri-tools
in [this PR](kubernetes-sigs/cri-tools#894) to
add GitHub workflows for automatically running the benchmarks on
Azure-based VMs for both Linux and Windows, as well as adding a Python
script which generates plot graphs for the results.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants