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

Automate scale test #1804

Closed
wants to merge 1 commit into from

Conversation

ciarams87
Copy link
Member

Proposed changes

Problem: Give a brief overview of the problem or feature being addressed.

Solution: Explain the approach you took to implement the solution, highlighting any significant design decisions or
considerations.

Testing: Describe any testing that you did.

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #ISSUE

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.


@github-actions github-actions bot added dependencies Pull requests that update a dependency file tests Pull requests that update tests labels Apr 4, 2024
Comment on lines +12 to +13
monitoring "cloud.google.com/go/monitoring/apiv3/v2"
"cloud.google.com/go/monitoring/apiv3/v2/monitoringpb"
Copy link
Contributor

Choose a reason for hiding this comment

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

Per a conversation offline, I bet it would be easier to get metrics if we just deploy our own Prometheus, and not use the GCP monitoring. Then we also don't need this extra dependency.

// NewCSVEncoder returns a vegeta CSV encoder.
func NewCSVEncoder(w io.Writer) vegeta.Encoder {
// NewVegetaCSVEncoder returns a vegeta CSV encoder.
func NewVegetaCSVEncoder(w io.Writer) vegeta.Encoder {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the reason we kept this agnostic is if we ever change the test framework, we don't have to update the function calls.

resultsDir, err = framework.CreateResultsDir("scale", version)
Expect(err).ToNot(HaveOccurred())

filename := filepath.Join(resultsDir, fmt.Sprintf("%s.md", version))
Copy link
Contributor

Choose a reason for hiding this comment

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

This filename formatting is slightly changed now in other tests to account for N+


BeforeAll(func() {
// Need longer create timeout to allow for all the apps
resourceManager.TimeoutConfig.CreateTimeout = 180 * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

This will change it for all tests since this is a global object

Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale Pull requests/issues with no activity label Apr 19, 2024
@pleshakov pleshakov removed the stale Pull requests/issues with no activity label Apr 19, 2024
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this pull request May 2, 2024
Problem:
Non-functional scale test needs to be run manually.

Solution:
- Automate scale test.
- Use in-cluster Prometheus to collect CPU, memory and NGF metrics.
- Use Kubernetes API server to get NGF logs.

For development and troubleshooting, it is possible to run scale test
locally in Kind cluster. However, it is necessary to bring down
the number of HTTPRoutes to 50 or less (roughly).

Testing:
- Ran this test locally with 64 listeners, 50 routes and 50 upstreams.
- Ran this test on GKE with the default configuration.

Out of scope: ensuring this test runs successfully via GitHub pipeline.

Closes nginxinc#1368

Largely based on work by Ciara in
nginxinc#1804

Co-authored-by: Ciara Stacke <c.stacke@f5.com>
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this pull request May 2, 2024
Problem:
Non-functional scale test needs to be run manually.

Solution:
- Automate scale test.
- Use in-cluster Prometheus to collect CPU, memory and NGF metrics.
- Use Kubernetes API server to get NGF logs.

For development and troubleshooting, it is possible to run scale test
locally in Kind cluster. However, it is necessary to bring down
the number of HTTPRoutes to 50 or less (roughly).

Testing:
- Ran this test locally with 64 listeners, 50 routes and 50 upstreams.
- Ran this test on GKE with the default configuration.

Out of scope: ensuring this test runs successfully via GitHub pipeline.

Closes nginxinc#1368

Largely based on work by Ciara in
nginxinc#1804

Co-authored-by: Ciara Stacke <c.stacke@f5.com>
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this pull request May 2, 2024
Problem:
Non-functional scale test needs to be run manually.

Solution:
- Automate scale test.
- Use in-cluster Prometheus to collect CPU, memory and NGF metrics.
- Use Kubernetes API server to get NGF logs.

For development and troubleshooting, it is possible to run scale test
locally in Kind cluster. However, it is necessary to bring down
the number of HTTPRoutes to 50 or less (roughly).

Testing:
- Ran this test locally with 64 listeners, 50 routes and 50 upstreams
  with NGINX OSS.
- Ran this test on GKE with the default configuration with NGINX OSS.

Out of scope: ensuring this test runs successfully via GitHub pipeline.

Closes nginxinc#1368

Largely based on work by Ciara in
nginxinc#1804

Co-authored-by: Ciara Stacke <c.stacke@f5.com>
@pleshakov pleshakov mentioned this pull request May 2, 2024
6 tasks
@sjberman
Copy link
Contributor

sjberman commented May 2, 2024

Shall we close in favor of #1926?

@ciarams87 ciarams87 closed this May 3, 2024
@ciarams87 ciarams87 deleted the tests/automate-scale-test branch May 3, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file tests Pull requests that update tests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants