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

Implement hooks in libcontainer code base #261

Merged
merged 4 commits into from
Sep 11, 2015

Conversation

crosbymichael
Copy link
Member

This is a little different implementation from #160 to have function based hooks for consumers of libcontainer directly but still supports our hooks for the spec based commands.

crosbymichael and others added 3 commits September 10, 2015 17:57
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>

Conflicts:
	libcontainer/integration/exec_test.go
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
@icecrime
Copy link

👍 (cc @mavenugo)


type Hook interface {
// Run executes the hook with the provided state.
Run(*HookState) error
Copy link
Contributor

Choose a reason for hiding this comment

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

HookState very simple "immutable" struct. Can be passed everywhere by value to avoid confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@mrunalp
Copy link
Contributor

mrunalp commented Sep 11, 2015

👍 to the function hooks idea

config.Hooks = &configs.Hooks{
Prestart: []configs.Hook{
configs.NewFunctionHook(func(s *configs.HookState) error {
f, err := os.Create(filepath.Join(rootfs, "test"))
Copy link
Contributor

Choose a reason for hiding this comment

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

dont think we should reach out to rootfs from here. We should pass it as hook state as @mrunalp mentioned before.

@calavera
Copy link
Contributor

@crosbymichael I've tried to address the comments in this PR in calavera@00a65a1 Feel free to cherry pick it. Unfortunately, the poststop hook test is not working, I'm sure I'm missing something.

@crosbymichael
Copy link
Member Author

@calavera fixed

@mrunalp @LK4D4 updated with your comments

Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
@mrunalp
Copy link
Contributor

mrunalp commented Sep 11, 2015

go vet failed.

@crosbymichael
Copy link
Member Author

go vet is a lair!

@mrunalp
Copy link
Contributor

mrunalp commented Sep 11, 2015

Ha ha

@mrunalp
Copy link
Contributor

mrunalp commented Sep 11, 2015

LGTM

@crosbymichael
Copy link
Member Author

@mrunalp we can update this again when we update the spec, i was just doing both independent so we would not be blocked

@LK4D4
Copy link
Contributor

LK4D4 commented Sep 11, 2015

LGTM

LK4D4 added a commit that referenced this pull request Sep 11, 2015
Implement hooks in libcontainer code base
@LK4D4 LK4D4 merged commit 7d122ff into opencontainers:master Sep 11, 2015
@crosbymichael crosbymichael deleted the hooks branch September 11, 2015 18:44
mavenugo added a commit to mavenugo/docker that referenced this pull request Sep 11, 2015
libnetwork waiting on : moby/libnetwork#515
libcontainer waiting on : opencontainers/runc#261

Due to libseccomp challenges, I cherry-picked opencontainers/runc#261
on top of runc v0.0.3 which is seen in my private fork that is
vendored-in here

Signed-off-by: Madhu Venugopal <madhu@docker.com>
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.

7 participants