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

run with pidfile option #311

Merged
merged 2 commits into from
Aug 18, 2021
Merged

Conversation

developer-guy
Copy link
Contributor

Signed-off-by: Batuhan Apaydın batuhan.apaydin@trendyol.com

I'm not so sure but It might fix the second option of issue #215.

cmd/nerdctl/run.go Outdated Show resolved Hide resolved
cmd/nerdctl/run.go Outdated Show resolved Hide resolved
@developer-guy developer-guy changed the title run with pid-file option run with pidfile option Aug 10, 2021
README.md Outdated Show resolved Hide resolved
@developer-guy
Copy link
Contributor Author

I think this time we are ready for the big moment @AkihiroSuda, this will be my first PR for the nerdctl and I'm so excited about this 🥳😋

@AkihiroSuda AkihiroSuda added this to the v0.12.0 milestone Aug 12, 2021
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Sorry, this has to be implemented in pkg/ocihook to support restarting like podman:

$ podman run -d --name foo --restart=always --pidfile /tmp/foo.pid nginx:alpine
$ cat /tmp/foo.pid 
41021
$ kill -9 41021
$ cat /tmp/foo.pid
41100

@developer-guy
Copy link
Contributor Author

Sorry, this has to be implemented in pkg/ocihook to support restarting like podman:

$ podman run -d --name foo --restart=always --pidfile /tmp/foo.pid nginx:alpine
$ cat /tmp/foo.pid 
41021
$ kill -9 41021
$ cat /tmp/foo.pid
41100

I did something about OCI hook for supporting pidfile option @AkihiroSuda , but I'm not so sure again :(

pkg/ocihook/ocihook.go Outdated Show resolved Hide resolved
cmd/nerdctl/run.go Outdated Show resolved Hide resolved
Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@developer-guy
Copy link
Contributor Author

any updates @AkihiroSuda 🤝

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

LGTM, and pushed a small fix

Sorry for my recent delay in reviewing PRs

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.

2 participants