Skip to content

Conversation

@liangchenye
Copy link
Member

Signed-off-by: Liang Chenye [email protected]

@liangchenye
Copy link
Member Author

  • add 'kill' interface to container.go
    our interface docucment --signal is different with current runc.
    I'm using runc way now.
  • fix bug in test.go
  • add kill.go
    I did not find a good way to test this spec error Attempting to send a signal to a container that is neither created nor running MUST have no effect on the container, so it is missing in this PR.
  • add killsig.go
    now 'TERM' and 'KILL' are MUST supported, 'KILL' is verified in 'kill.go'.
    In killsig.go, we only verify 'TERM' now.
    In the future we might add more signal tests.

Tested in my environment with 'runc' , it works.
PTAL @wking @q384566678

@liangchenye liangchenye mentioned this pull request Feb 12, 2018
44 tasks
util.WaitingForStatus(*r, util.LifecycleStatusCreated|util.LifecycleStatusStopped, time.Second*10, time.Second*1)
// KILL MUST be supported and KILL cannot be trapped
err := r.Kill("KILL")
// wait before the container been deleted
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think the next line needs a comment; util.WaitingForStatus(*r, util.LifecycleStatusStopped, ...) is pretty clear on it's own. And if you do want a comment, you probably want "been deleted" ->"has stopped".

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 remove this line, updated.

@liangchenye
Copy link
Member Author

liangchenye commented Feb 13, 2018

Another issue I found, 'util.Fatal' exits immediately, the 'defer' functions will not be called.
So the bunde dir might not be removed and the container might not be deleted.
It needs to be fixed.

Add an issue to keep track of this. #582

@liangchenye liangchenye mentioned this pull request Feb 13, 2018
t := tap.New()
t.Header(0)
bundleDir, err := util.PrepareBundle()
fmt.Println(bundleDir)

Choose a reason for hiding this comment

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

Should delete it?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: Liang Chenye <[email protected]>
@zhouhao3
Copy link

zhouhao3 commented Feb 26, 2018

LGTM

Approved with PullApprove

@zhouhao3 zhouhao3 merged commit 35924b6 into opencontainers:master Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants