support to merge complex commands when to inject commands #185

cwen0 · 2020-02-04T03:52:25Z

What problem does this PR solve?

What is changed and how does it work?

support to merge complex commands
fix update annotations when to inject the sidecar container

eg:

inject: []string{"bash", "-c", "/check.sh"}, origin: []string{"bash", "-c", "set -ex\n/tiflash server --config-file /data/config.toml"}
merged commands: []string{"/bin/sh", "-ec", "/check.sh\nset -ex\n/tiflash server --config-file /data/config.toml\n"}

Check List

Tests

Unit test
Manual test (add detailed scripts or steps below)

log:

kubectl logs -f --namespace app-ns tiflash-0 -c tiflash
fuse server not running, /tmp/fuse/pid not found, retry...
fuse server not running, /tmp/fuse/pid not found, retry...
fuse server not running, /tmp/fuse/pid not found, retry...
fuse server not running, /tmp/fuse/pid not found, retry...
fuse server not running, /tmp/fuse/pid not found, retry...
fuse server is started
++ hostname
+ [[ tiflash-0 =~ -([0-9]+)$ ]]
+ ordinal=0
+ set +ex
Server:         10.96.0.10
Address:        10.96.0.10#53

Name:   tiflash-0.tiflash.app-ns.svc.cluster.local
Address: 10.244.5.18

Logging trace to /logs/server.log
Logging errors to /logs/error.log

Code changes

Has Go code change

Signed-off-by: cwen0 <cwenyin0@gmail.com>

codecov-io · 2020-02-04T03:54:23Z

Codecov Report

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

@@            Coverage Diff            @@
##             master     #185   +/-   ##
=========================================
  Coverage          ?   44.29%           
=========================================
  Files             ?       18           
  Lines             ?      675           
  Branches          ?        0           
=========================================
  Hits              ?      299           
  Misses            ?      343           
  Partials          ?       33

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 f6f0cfb...cf2370d. Read the comment docs.

Signed-off-by: cwen0 <cwenyin0@gmail.com>

… fix_inject_scripts

pkg/utils/command.go

pkg/webhook/inject/inject.go

pkg/utils/command_test.go

zhouqiang-cl

rest LGTM

Signed-off-by: cwen0 <cwenyin0@gmail.com>

… fix_inject_scripts

mahjonp · 2020-02-04T09:23:00Z

pkg/utils/command.go

+
+		scripts += cmd
+		scripts += "\n"
+		continue


emm, this continue is not necessary

zhouqiang-cl

LGTM

Signed-off-by: cwen0 <cwenyin0@gmail.com>

mahjonp · 2020-02-04T09:44:49Z

seems this PR is workable, but I'm not sure. @you06 @mapleFU PTAL

mahjonp · 2020-02-04T09:55:30Z

please comments on documents? list the limitation of current implementation, Rest LGTM

mapleFU · 2020-02-04T09:50:33Z

pkg/utils/command.go

+
+func isShellScripts(cmd string) bool {
+	if cmd == "bash" || cmd == "sh" ||
+		strings.HasPrefix(cmd, "bash ") || strings.HasPrefix(cmd, "sh ") ||


Why not just strings.HasPrefix(cmd, "bash ") || strings.HasPrefix(cmd, "sh ")

avoid some commands with prefix bash or sh, eg: shell2 xxx

mapleFU · 2020-02-04T09:54:17Z

pkg/utils/command.go

+}
+
+func mergeOriginCommandsAndArgs(origin []string, args []string) string {
+	commands := []string{}


commands := append(origin, args...)

Signed-off-by: cwen0 <cwenyin0@gmail.com>

… fix_inject_scripts

cwen0 · 2020-02-04T12:57:36Z

@mahjonp @zhouqiang-cl @YangKeao @mapleFU PTAL again, thanks!

zhouqiang-cl

LGTM

Signed-off-by: cwen0 <cwenyin0@gmail.com>

zhouqiang-cl

LGTM

Signed-off-by: cwen0 <cwenyin0@gmail.com>

doc/io_chaos.md

zhouqiang-cl

LGTM

Co-Authored-By: mahjonp <junpeng.man@gmail.com>

zhouqiang-cl

LGTM

…#185) * support to merge complex commands when to inject commands Signed-off-by: cwen0 <cwenyin0@gmail.com>

support to merge complex commands when to inject commands

739d1f4

Signed-off-by: cwen0 <cwenyin0@gmail.com>

cwen0 added status/WIP component/operator labels Feb 4, 2020

cwen0 and others added 4 commits February 4, 2020 11:54

fix comments

943f0fb

Signed-off-by: cwen0 <cwenyin0@gmail.com>

Merge branch 'master' into fix_inject_scripts

9bf981f

fix update annotations

662dc73

Signed-off-by: cwen0 <cwenyin0@gmail.com>

Merge branch 'fix_inject_scripts' of github.com:cwen0/chaos-mesh into…

b0de5bd

… fix_inject_scripts

cwen0 changed the title ~~[WIP] support to merge complex commands when to inject commands~~ support to merge complex commands when to inject commands Feb 4, 2020

cwen0 added status/PTAL and removed status/WIP labels Feb 4, 2020

cwen0 requested review from zhouqiang-cl, YangKeao and mahjonp February 4, 2020 06:59

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

pkg/utils/command.go Show resolved Hide resolved

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

pkg/utils/command.go Outdated Show resolved Hide resolved

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

pkg/utils/command.go Show resolved Hide resolved

zhouqiang-cl added this to In progress in Weekly Plan 2020-02-02 ~ 2020-02-07 Feb 4, 2020

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

pkg/webhook/inject/inject.go Show resolved Hide resolved

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

pkg/utils/command_test.go Show resolved Hide resolved

Merge branch 'master' into fix_inject_scripts

7ecc6e5

zhouqiang-cl reviewed Feb 4, 2020

View reviewed changes

cwen0 added 2 commits February 4, 2020 17:03

address comments

4885b91

Signed-off-by: cwen0 <cwenyin0@gmail.com>

Merge branch 'fix_inject_scripts' of github.com:cwen0/chaos-mesh into…

0c7a643

… fix_inject_scripts

mahjonp reviewed Feb 4, 2020

View reviewed changes

pkg/utils/command.go Outdated

scripts += cmd

scripts += "\n"

continue

Copy link

Contributor

mahjonp Feb 4, 2020

Choose a reason for hiding this comment

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

emm, this continue is not necessary

zhouqiang-cl previously approved these changes Feb 4, 2020

View reviewed changes

address comments

6024b87

Signed-off-by: cwen0 <cwenyin0@gmail.com>

cwen0 dismissed zhouqiang-cl’s stale review via 6024b87 February 4, 2020 09:34

Weekly Plan 2020-02-02 ~ 2020-02-07 automation moved this from In progress to Review in progress Feb 4, 2020

address comments

0f415b2

Signed-off-by: cwen0 <cwenyin0@gmail.com>

mapleFU reviewed Feb 4, 2020

View reviewed changes

zhouqiang-cl and others added 4 commits February 4, 2020 19:09

Merge branch 'master' into fix_inject_scripts

88607ac

address comments

33ddd9c

Signed-off-by: cwen0 <cwenyin0@gmail.com>

Merge branch 'fix_inject_scripts' of github.com:cwen0/chaos-mesh into…

f6f0cfb

… fix_inject_scripts

Merge remote-tracking branch 'upstream/master' into fix_inject_scripts

2fc2f8c

zhouqiang-cl previously approved these changes Feb 4, 2020

View reviewed changes

mahjonp previously approved these changes Feb 4, 2020

View reviewed changes

Weekly Plan 2020-02-02 ~ 2020-02-07 automation moved this from Review in progress to Reviewer approved Feb 4, 2020

fix comments

d29241e

Signed-off-by: cwen0 <cwenyin0@gmail.com>

cwen0 dismissed stale reviews from mahjonp and zhouqiang-cl via d29241e February 4, 2020 13:06

Weekly Plan 2020-02-02 ~ 2020-02-07 automation moved this from Reviewer approved to Review in progress Feb 4, 2020

zhouqiang-cl previously approved these changes Feb 4, 2020

View reviewed changes

fix typo

9a4cca7

Signed-off-by: cwen0 <cwenyin0@gmail.com>

cwen0 dismissed zhouqiang-cl’s stale review via 9a4cca7 February 4, 2020 13:09

mahjonp reviewed Feb 4, 2020

View reviewed changes

doc/io_chaos.md Outdated Show resolved Hide resolved

zhouqiang-cl previously approved these changes Feb 4, 2020

View reviewed changes

Update doc/io_chaos.md

cf2370d

Co-Authored-By: mahjonp <junpeng.man@gmail.com>

cwen0 dismissed zhouqiang-cl’s stale review via cf2370d February 4, 2020 13:15

zhouqiang-cl approved these changes Feb 4, 2020

View reviewed changes

mahjonp approved these changes Feb 4, 2020

View reviewed changes

Weekly Plan 2020-02-02 ~ 2020-02-07 automation moved this from Review in progress to Reviewer approved Feb 4, 2020

zhouqiang-cl merged commit fe0b928 into chaos-mesh:master Feb 4, 2020

Weekly Plan 2020-02-02 ~ 2020-02-07 automation moved this from Reviewer approved to Done Feb 4, 2020

cwen0 deleted the fix_inject_scripts branch July 14, 2020 02:01

sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021

support to merge complex commands when to inject commands (chaos-mesh…

176254d

…#185) * support to merge complex commands when to inject commands Signed-off-by: cwen0 <cwenyin0@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support to merge complex commands when to inject commands #185

support to merge complex commands when to inject commands #185

cwen0 commented Feb 4, 2020 •

edited

codecov-io commented Feb 4, 2020 •

edited

zhouqiang-cl left a comment

mahjonp Feb 4, 2020

zhouqiang-cl left a comment

mahjonp commented Feb 4, 2020

mahjonp commented Feb 4, 2020

mapleFU Feb 4, 2020

cwen0 Feb 4, 2020

mapleFU Feb 4, 2020

cwen0 commented Feb 4, 2020

zhouqiang-cl left a comment

zhouqiang-cl left a comment

zhouqiang-cl left a comment

zhouqiang-cl left a comment

support to merge complex commands when to inject commands #185

support to merge complex commands when to inject commands #185

Conversation

cwen0 commented Feb 4, 2020 • edited

What problem does this PR solve?

What is changed and how does it work?

Check List

codecov-io commented Feb 4, 2020 • edited

Codecov Report

zhouqiang-cl left a comment

Choose a reason for hiding this comment

mahjonp Feb 4, 2020

Choose a reason for hiding this comment

zhouqiang-cl left a comment

Choose a reason for hiding this comment

mahjonp commented Feb 4, 2020

mahjonp commented Feb 4, 2020

mapleFU Feb 4, 2020

Choose a reason for hiding this comment

cwen0 Feb 4, 2020

Choose a reason for hiding this comment

mapleFU Feb 4, 2020

Choose a reason for hiding this comment

cwen0 commented Feb 4, 2020

zhouqiang-cl left a comment

Choose a reason for hiding this comment

zhouqiang-cl left a comment

Choose a reason for hiding this comment

zhouqiang-cl left a comment

Choose a reason for hiding this comment

zhouqiang-cl left a comment

Choose a reason for hiding this comment

cwen0 commented Feb 4, 2020 •

edited

codecov-io commented Feb 4, 2020 •

edited