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

add web-show example #304

Merged
merged 6 commits into from
Mar 3, 2020
Merged

add web-show example #304

merged 6 commits into from
Mar 3, 2020

Conversation

cwen0
Copy link
Member

@cwen0 cwen0 commented Mar 2, 2020

Signed-off-by: cwen0 [email protected]

What problem does this PR solve?

add web-show examples for demo

What is changed and how does it work?

use Deployment to deploy web-show

Check List

  • Manual test (add detailed scripts or steps below)

@zhouqiang-cl
Copy link
Contributor

/test

zhouqiang-cl
zhouqiang-cl previously approved these changes Mar 2, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

yeya24
yeya24 reviewed Mar 2, 2020
View reviewed changes
examples/web-show/deploy.sh Outdated

sed "s/TARGETIP/$TARGET_IP/g" deployment.yaml > deployment-target.yaml

docker pull
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor

Choose a reason for hiding this comment

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

docker pull what?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have fixed it, PTAL again! thanks!

examples/web-show/network-delay.yaml Outdated
jitter: "0ms"
duration: "30s"
scheduler:
cron: "@every 60s"
Copy link
Contributor

Choose a reason for hiding this comment

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

new line

examples/web-show/service.yaml Outdated
ports:
- protocol: TCP
port: 8081
targetPort: 8081
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@cwen0
add docker mirror
b25e4f5 
Signed-off-by: cwen0 <[email protected]>
@cwen0 cwen0 removed the status/WIP label Mar 2, 2020
@zhouqiang-cl
Copy link
Contributor

/test

@codecov-io
Copy link

codecov-io commented Mar 2, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@65de8c3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #304   +/-   ##
=========================================
  Coverage          ?   60.62%           
=========================================
  Files             ?       40           
  Lines             ?     2494           
  Branches          ?        0           
=========================================
  Hits              ?     1512           
  Misses            ?      882           
  Partials          ?      100

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65de8c3...576690a. Read the comment docs.

yeya24
yeya24 reviewed Mar 2, 2020
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A small nit otherwise looks good! But can I ask why add an empty deployment-target.yaml?

examples/web-show/deploy.sh

kill $(lsof -t -i:8081) 2>&1 >/dev/null | True

nohup kubectl port-forward svc/web-show 8081:8081 >/dev/null 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

Umm, actually I think just kubectl port-forward svc/web-show 8081:8081 is fine. Is it necessary to make it run at background?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think if this script is blocked, it is easy for users to misunderstand that it was not successfully installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@cwen0 cwen0 added the documentation Improvements or additions to documentation label Mar 3, 2020
@cwen0
Copy link
Member Author

cwen0 commented Mar 3, 2020

/test

zhouqiang-cl
zhouqiang-cl previously approved these changes Mar 3, 2020
View reviewed changes
examples/web-show/deploy.sh

kill $(lsof -t -i:8081) 2>&1 >/dev/null | True

nohup kubectl port-forward svc/web-show 8081:8081 >/dev/null 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@cwen0
Copy link
Member Author

cwen0 commented Mar 3, 2020

A small nit otherwise looks good! But can I ask why add an empty deployment-target.yaml?

In deployment-target.yaml, the script will use a target IP instead of TARGET and I just want to keep the deployment.yaml without change

yeya24
yeya24 previously approved these changes Mar 3, 2020
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM

@cwen0
fix a little bug
3e205da 
Signed-off-by: cwen0 <[email protected]>
zhouqiang-cl
zhouqiang-cl approved these changes Mar 3, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl
Copy link
Contributor

/merge

@cwen0
Copy link
Member Author

cwen0 commented Mar 3, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 3, 2020

/run-all-tests

@sre-bot sre-bot merged commit 866651d into chaos-mesh:master Mar 3, 2020
@cwen0 cwen0 deleted the add_web_show_example branch July 14, 2020 02:04
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
@cwen0
add web-show example (chaos-mesh#304)
bb1026d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gallardot Gallardot Gallardot left review comments

@zhouqiang-cl zhouqiang-cl zhouqiang-cl approved these changes

@yeya24 yeya24 yeya24 approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation status/can-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cwen0 @zhouqiang-cl @codecov-io @sre-bot @Gallardot @yeya24