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.

Sign up for GitHub

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

Jump to bottom

Use multiple projects in the load test for 1-click-deployment #2648

Merged
merged 17 commits into from Mar 16, 2019
Merged

Use multiple projects in the load test for 1-click-deployment #2648

merged 17 commits into from Mar 16, 2019

Conversation

zhenghuiwang
Copy link
Contributor

@zhenghuiwang zhenghuiwang commented Mar 7, 2019 •
edited

#2580

This PR extends current load test to use multiple projects:
It now sends requests to X projects and Y deployments per project, where X = number_project and Y=number_deployments_per_project from command line flags. In total that is X*Y concurrent requests for deploying Kubeflow.

In addition to that, I removed the projectNumber flag and get the value from the projectID flag. So there won't be mismatch by any chance.

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Mar 8, 2019
@zhenghuiwang zhenghuiwang marked this pull request as ready for review March 8, 2019 03:18
@zhenghuiwang zhenghuiwang changed the title [WIP] load test for 1-click-deployment Use multiple projects in the load test for 1-click-deployment Mar 8, 2019
@zhenghuiwang
Copy link
Contributor Author

/assign @kunmingg
/assign @jlewi

jlewi
jlewi reviewed Mar 8, 2019
View reviewed changes
testing/run_deploy_app.sh Outdated Show resolved Hide resolved
@jlewi
Copy link
Contributor

jlewi commented Mar 8, 2019

Mostly looks good. one question about the script.

kunmingg
kunmingg reviewed Mar 8, 2019
View reviewed changes
testing/run_deploy_app.sh Outdated Show resolved Hide resolved
@jlewi
Copy link
Contributor

jlewi commented Mar 12, 2019

/lgtm
/approve
/hold
@kunmingg Any additional comments?

@zhenghuiwang zhenghuiwang mentioned this pull request Mar 13, 2019
Closed
@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 13, 2019
@zhenghuiwang
Copy link
Contributor Author

/retest

@jlewi
Copy link
Contributor

jlewi commented Mar 14, 2019

Test failure is spark operator; looks like #2693

@kunmingg
Copy link
Contributor

Only 1 comment:

For insert_ssl_cert, current GCS path is build from test_mode + deploy_name:
https://github.com/kubeflow/kubeflow/blob/master/testing/test_deploy_app.py#L239
Since in our load test we reuse deploy_name across projects, they will write to same GCS path and overwrite each other.

Can we add project as part of the GCS path as well? Or just use hostname as GCS folder name?

@zhenghuiwang
Copy link
Contributor Author

Only 1 comment:

For insert_ssl_cert, current GCS path is build from test_mode + deploy_name:
https://github.com/kubeflow/kubeflow/blob/master/testing/test_deploy_app.py#L239
Since in our load test we reuse deploy_name across projects, they will write to same GCS path and overwrite each other.

Can we add project as part of the GCS path as well? Or just use hostname as GCS folder name?

good suggestion! added the project name into the GCS path.

@kunmingg
Copy link
Contributor

/lgtm
/approve
Thanks!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi, kunmingg

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

@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 16, 2019
@kunmingg
Copy link
Contributor

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 781cb59 into kubeflow:master Mar 16, 2019
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
@zhenghuiwang @k8s-ci-robot
Use multiple projects in the load test for 1-click-deployment (kubefl…
3c3aab9 
…ow#2648)

* code complete

* remove project number

* upate shell scripts

* more logging

* add comments

* comments on script

* change default version

* add prototype

* update the logic of waiting

* move prototype config

* run delete resource in parallel

* change ssl upload path

* rename gcs_path

* unify gcs path

* always upload ssl cert for successful deployment
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
@zhenghuiwang @k8s-ci-robot
Use multiple projects in the load test for 1-click-deployment (kubefl…
3d2a517 
…ow#2648)

* code complete

* remove project number

* upate shell scripts

* more logging

* add comments

* comments on script

* change default version

* add prototype

* update the logic of waiting

* move prototype config

* run delete resource in parallel

* change ssl upload path

* rename gcs_path

* unify gcs path

* always upload ssl cert for successful deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlewi jlewi jlewi left review comments

@kunmingg kunmingg kunmingg left review comments

@ellistarn ellistarn Awaiting requested review from ellistarn

@r2d4 r2d4 Awaiting requested review from r2d4

Assignees

@jlewi jlewi

@kunmingg kunmingg

Labels
approved cla: yes lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@zhenghuiwang @jlewi @kunmingg @k8s-ci-robot @googlebot