Skip to content

Conversation

@wangkirin
Copy link

When I try to use prestart hooks , I found a bug about hook args
If I add folllowing contents in runtime.json

"prestart":[ {
                "path": "/bin/bash",
                "args": ["-c","\"ls\""]
            }
            ],

runc terminated and return an error , but if I change the args to

"prestart":[ {
                "path": "/bin/bash",
                "args": ["test","-c","\"ls\""]
            }
            ],

I can get the list of files from standouput, even I change test to any other string such as adfafadsf, it also works, that means the first argument is useless

I think this problem is caused by the Cmd struct of golang , in Godocs, it says [1]
Args holds command line arguments, including the command as Args[0].
so I make a new string slice and insert a "" string in front of current Args before cmd.Runto slove that

[1] https://golang.org/pkg/os/exec/#Cmd
Signed-off-by: Wang Qilin [email protected]

Signed-off-by: Wang Qilin <[email protected]>
@hqhq
Copy link
Contributor

hqhq commented Nov 24, 2015

No it's not a bug.

// Args holds command line arguments, including the command as Args[0].
// If the Args field is empty or nil, Run uses {Path}.
//
// In typical use, both Path and Args are set by calling Command.
Args []string

So the typical way you set prestart should be like:

"prestart":[ {
                "path": "/bin/bash",
                "args": ["/bin/bash", "-c", "\"ls\""]
            }
            ],

Thought it's not the only way that can work.

@wangkirin
Copy link
Author

@hqhq , thank you for your review, maybe we can't really call it a bug and I have already fix it :)
I think in specs , there is no statment that if we set Args ,we should set the first argument the same with path, and the example given by specs is like

 "poststop": [
            {
                "path": "/usr/sbin/cleanup.sh",
                "args": ["-f"]
            }
        ]

so I think we can modify this

@wangkirin wangkirin changed the title fix a Hook Args Bug fix a Hook Args Usage Nov 24, 2015
@hqhq
Copy link
Contributor

hqhq commented Nov 24, 2015

@wangkirin Yeah, I didn't notice the specs examples, by intuition I think it should be the same as golang packages, I opened a PR opencontainers/runtime-spec#255 , let's see how that works.

@mrunalp
Copy link
Contributor

mrunalp commented Nov 24, 2015

This is working as intended as @hqhq pointed out.

@mrunalp mrunalp closed this Nov 24, 2015
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants