-
Notifications
You must be signed in to change notification settings - Fork 159
Fix #556 #557
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
Fix #556 #557
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "errors" | ||
| "io/ioutil" | ||
| "os" | ||
| "path/filepath" | ||
| "strconv" | ||
| "time" | ||
|
|
||
| tap "github.com/mndrix/tap-go" | ||
| "github.com/opencontainers/runtime-tools/validation/util" | ||
| ) | ||
|
|
||
| func main() { | ||
| t := tap.New() | ||
| t.Header(0) | ||
|
|
||
| tempDir, err := ioutil.TempDir("", "oci-pid") | ||
| if err != nil { | ||
| util.Fatal(err) | ||
| } | ||
| defer os.RemoveAll(tempDir) | ||
| tempPidFile := filepath.Join(tempDir, "pidfile") | ||
|
|
||
| config := util.LifecycleConfig{ | ||
| Actions: util.LifecycleActionCreate | util.LifecycleActionDelete, | ||
| PreCreate: func(r *util.Runtime) error { | ||
| r.PidFile = tempPidFile | ||
| return nil | ||
| }, | ||
| PostCreate: func(r *util.Runtime) error { | ||
| pidData, err := ioutil.ReadFile(tempPidFile) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| pid, err := strconv.Atoi(string(pidData)) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| state, err := r.State() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if state.Pid != pid { | ||
| return errors.New("Wrong pid in the pidfile") | ||
| } | ||
| return nil | ||
| }, | ||
| PreDelete: func(r *util.Runtime) error { | ||
| return util.WaitingForStatus(*r, util.LifecycleStatusCreated, time.Second*10, time.Second*1) | ||
| }, | ||
| } | ||
|
|
||
| g := util.GetDefaultGenerator() | ||
| g.SetProcessArgs([]string{"true"}) | ||
| err = util.RuntimeLifecycleValidate(g, config) | ||
| if err != nil { | ||
| util.Fatal(err) | ||
| } | ||
|
|
||
| t.AutoPlan() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import ( | |
| type Runtime struct { | ||
| RuntimeCommand string | ||
| BundleDir string | ||
| PidFile string | ||
| ID string | ||
| stdout *os.File | ||
| stderr *os.File | ||
|
|
@@ -63,7 +64,9 @@ func (r *Runtime) Create() (stderr []byte, err error) { | |
| if r.ID != "" { | ||
| args = append(args, r.ID) | ||
| } | ||
|
|
||
| if r.PidFile != "" { | ||
| args = append(args, "--pid-file", r.PidFile) | ||
| } | ||
| if r.BundleDir != "" { | ||
| args = append(args, "--bundle", r.BundleDir) | ||
| } | ||
|
|
@@ -146,23 +149,32 @@ func (r *Runtime) State() (rspecs.State, error) { | |
| } | ||
|
|
||
| // Delete a container | ||
| func (r *Runtime) Delete() error { | ||
| func (r *Runtime) Delete() ([]byte, error) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we returning
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use this to debug why 'Delete' fails. |
||
| var args []string | ||
| var stderr []byte | ||
| args = append(args, "delete") | ||
| if r.ID != "" { | ||
| args = append(args, r.ID) | ||
| } | ||
|
|
||
| cmd := exec.Command(r.RuntimeCommand, args...) | ||
| return cmd.Run() | ||
| stdout, err := cmd.Output() | ||
| if e, ok := err.(*exec.ExitError); ok { | ||
| stderr = e.Stderr | ||
| } | ||
| if err != nil && len(stderr) == 0 { | ||
| stderr = stdout | ||
| } | ||
|
|
||
| return stderr, err | ||
| } | ||
|
|
||
| // Clean deletes the container. If removeBundle is set, the bundle | ||
| // directory is removed after the container is deleted succesfully or, if | ||
| // forceRemoveBundle is true, after the deletion attempt regardless of | ||
| // whether it was successful or not. | ||
| func (r *Runtime) Clean(removeBundle bool, forceRemoveBundle bool) error { | ||
| err := r.Delete() | ||
| _, err := r.Delete() | ||
|
|
||
| if removeBundle && (err == nil || forceRemoveBundle) { | ||
| err2 := os.RemoveAll(r.bundleDir()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package util | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "os" | ||
|
|
@@ -21,6 +22,50 @@ var ( | |
| RuntimeCommand = "runc" | ||
| ) | ||
|
|
||
| // LifecycleAction defines the phases will be called. | ||
| type LifecycleAction int | ||
|
|
||
| const ( | ||
| // LifecycleActionCreate creates a container | ||
| LifecycleActionCreate = 1 << iota | ||
| // LifecycleActionStart starts a container | ||
| LifecycleActionStart | ||
| // LifecycleActionDelete deletes a container | ||
| LifecycleActionDelete | ||
| ) | ||
|
|
||
| // LifecycleStatus follows https://github.com/opencontainers/runtime-spec/blob/master/runtime.md#state | ||
| type LifecycleStatus int | ||
|
|
||
| const ( | ||
| // LifecycleStatusCreating "creating" | ||
| LifecycleStatusCreating = 1 << iota | ||
| // LifecycleStatusCreated "created" | ||
| LifecycleStatusCreated | ||
| // LifecycleStatusRunning "running" | ||
| LifecycleStatusRunning | ||
| // LifecycleStatusStopped "stopped" | ||
| LifecycleStatusStopped | ||
| ) | ||
|
|
||
| var lifecycleStatusMap = map[string]LifecycleStatus{ | ||
| "creating": LifecycleStatusCreating, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or these could just be strings: type LivecycleStatus string
const (
LifecycleStatusCreating = "creating"
…
)and then you cast with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An 'integer' is useful when we expect to have multiple statuses. For example, if we want to delete a container, we expected to have 'created' or 'stopped' status. So we can 'WaitingForStatus(Created|Stopped)'. |
||
| "created": LifecycleStatusCreated, | ||
| "running": LifecycleStatusRunning, | ||
| "stopped": LifecycleStatusStopped, | ||
| } | ||
|
|
||
| // LifecycleConfig includes | ||
| // 1. Actions to define the default running lifecycles. | ||
| // 2. Four phases for user to add his/her own operations. | ||
| type LifecycleConfig struct { | ||
| Actions LifecycleAction | ||
| PreCreate func(runtime *Runtime) error | ||
| PostCreate func(runtime *Runtime) error | ||
| PreDelete func(runtime *Runtime) error | ||
| PostDelete func(runtime *Runtime) error | ||
| } | ||
|
|
||
| // PreFunc initializes the test environment after preparing the bundle | ||
| // but before creating the container. | ||
| type PreFunc func(string) error | ||
|
|
@@ -78,6 +123,28 @@ func GetDefaultGenerator() *generate.Generator { | |
| return &g | ||
| } | ||
|
|
||
| // WaitingForStatus waits an expected runtime status, return error if | ||
| // 1. fail to query the status | ||
| // 2. timeout | ||
| func WaitingForStatus(r Runtime, status LifecycleStatus, retryTimeout time.Duration, pollInterval time.Duration) error { | ||
| for start := time.Now(); time.Since(start) < retryTimeout; time.Sleep(pollInterval) { | ||
| state, err := r.State() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if v, ok := lifecycleStatusMap[state.Status]; ok { | ||
| if status&v != 0 { | ||
| return nil | ||
| } | ||
| } else { | ||
| // In spec, it says 'Additional values MAY be defined by the runtime'. | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| return errors.New("timeout in waiting for the container status") | ||
| } | ||
|
|
||
| // RuntimeInsideValidate runs runtimetest inside a container. | ||
| func RuntimeInsideValidate(g *generate.Generator, f PreFunc) (err error) { | ||
| bundleDir, err := PrepareBundle() | ||
|
|
@@ -184,3 +251,74 @@ func RuntimeOutsideValidate(g *generate.Generator, f AfterFunc) error { | |
| } | ||
| return nil | ||
| } | ||
|
|
||
| // RuntimeLifecycleValidate validates runtime lifecycle. | ||
| func RuntimeLifecycleValidate(g *generate.Generator, config LifecycleConfig) error { | ||
| bundleDir, err := PrepareBundle() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r, err := NewRuntime(RuntimeCommand, bundleDir) | ||
| if err != nil { | ||
| os.RemoveAll(bundleDir) | ||
| return err | ||
| } | ||
| defer r.Clean(true, true) | ||
| err = r.SetConfig(g) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r.SetID(uuid.NewV4().String()) | ||
|
|
||
| if config.PreCreate != nil { | ||
| if err := config.PreCreate(&r); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.Actions&LifecycleActionCreate != 0 { | ||
| stderr, err := r.Create() | ||
| if err != nil { | ||
| os.Stderr.WriteString("failed to create the container\n") | ||
| os.Stderr.Write(stderr) | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.PostCreate != nil { | ||
| if err := config.PostCreate(&r); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.Actions&LifecycleActionStart != 0 { | ||
| stderr, err := r.Start() | ||
| if err != nil { | ||
| os.Stderr.WriteString("failed to start the container\n") | ||
| os.Stderr.Write(stderr) | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.PreDelete != nil { | ||
| if err := config.PreDelete(&r); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.Actions&LifecycleActionDelete != 0 { | ||
| stderr, err := r.Delete() | ||
| if err != nil { | ||
| os.Stderr.WriteString("failed to delete the container\n") | ||
| os.Stderr.Write(stderr) | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if config.PostDelete != nil { | ||
| if err := config.PostDelete(&r); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need this, because a container must be created before
createreturns. Maybe a runc bug?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runcdoes not implement 'creating' lifecycle ,it is always 'created' or 'fails'.
From the spec, it should be like this:
runX create xrunX status xit should be 'creating' (assuming the creating takes a long time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's definitely a bug. And there's also some CLI spec wording about
create's exit timing. I think we can drop this, and I'll file a runc patch.