diff --git a/lib/apiservers/engine/backends/container.go b/lib/apiservers/engine/backends/container.go index ef3884a2df..c9c39fa456 100644 --- a/lib/apiservers/engine/backends/container.go +++ b/lib/apiservers/engine/backends/container.go @@ -58,7 +58,6 @@ import ( "github.com/vmware/vic/lib/apiservers/engine/backends/portmap" "github.com/vmware/vic/lib/apiservers/portlayer/client/containers" "github.com/vmware/vic/lib/apiservers/portlayer/client/scopes" - "github.com/vmware/vic/lib/apiservers/portlayer/client/tasks" "github.com/vmware/vic/lib/apiservers/portlayer/models" "github.com/vmware/vic/lib/archive" "github.com/vmware/vic/lib/config/executor" @@ -189,52 +188,18 @@ const ( defaultEnvPath = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ) +// TODO: we should probably just make this purely a proxy thing. This is now just a passthrough func (c *Container) Handle(id, name string) (string, error) { - resp, err := c.containerProxy.Client().Containers.Get(containers.NewGetParamsWithContext(ctx).WithID(id)) - if err != nil { - switch err := err.(type) { - case *containers.GetNotFound: - cache.ContainerCache().DeleteContainer(id) - return "", NotFoundError(name) - case *containers.GetDefault: - return "", InternalServerError(err.Payload.Message) - default: - return "", InternalServerError(err.Error()) - } - } - return resp.Payload, nil + return c.containerProxy.Handle(id, name) } // docker's container.execBackend func (c *Container) TaskWaitToStart(cid, cname, eid string) error { - // obtain a portlayer client - client := c.containerProxy.Client() - - handle, err := c.Handle(cid, cname) - if err != nil { - return err - } - - // wait the Task to start - config := &models.TaskWaitConfig{ - Handle: handle, - ID: eid, - } - - params := tasks.NewWaitParamsWithContext(ctx).WithConfig(config) - // FIXME: NEEDS CONTAINER PROXY - _, err = client.Tasks.Wait(params) - if err != nil { - switch err := err.(type) { - case *tasks.WaitInternalServerError: - return InternalServerError(err.Payload.Message) - default: - return InternalServerError(err.Error()) - } - } + op := trace.NewOperation(context.TODO(), "") + defer trace.End(trace.Begin(fmt.Sprintf("%s: cname=(%s), cid=(%s), eid=(%s)", op, cname, cid, eid))) - return nil + return c.containerProxy.WaitTask(op, cid, cname, eid) } // ContainerExecCreate sets up an exec in a running container. @@ -270,7 +235,7 @@ func (c *Container) ContainerExecCreate(name string, config *types.ExecConfig) ( // NOTE: we should investigate what we can add or manipulate in the handle in the portlayer to make this check even more granular. Maybe Add the actually State into the handle config or part of the container info could be "snapshotted" // This check is now done off of the handle. if state != "RUNNING" { - return ConflictError(fmt.Sprintf("Container %s is restarting, wait until the container is running", id)) + return ConflictError(fmt.Sprintf("Container (%s) is not powered on, please start the container before attempting to an exec an operation", name)) } op.Debugf("State checks succeeded for exec operation on container(%s)", id) diff --git a/lib/apiservers/engine/backends/container_proxy.go b/lib/apiservers/engine/backends/container_proxy.go index b39e2484f2..a3f58058f2 100644 --- a/lib/apiservers/engine/backends/container_proxy.go +++ b/lib/apiservers/engine/backends/container_proxy.go @@ -93,6 +93,7 @@ type VicContainerProxy interface { // TODO: we should not be returning a swagger model here, however we do not have a solid architected return for this yet. InspectTask(op trace.Operation, handle string, eid string, cid string) (*models.TaskInspectResponse, error) BindTask(op trace.Operation, handle string, eid string) (string, error) + WaitTask(op trace.Operation, cid string, cname string, eid string) error BindInteraction(handle string, name string, id string) (string, error) UnbindInteraction(handle string, name string, id string) (string, error) @@ -390,6 +391,37 @@ func (c *ContainerProxy) BindTask(op trace.Operation, handle string, eid string) return respHandle, nil } +func (c *ContainerProxy) WaitTask(op trace.Operation, cid string, cname string, eid string) error { + handle, err := c.Handle(cid, cname) + if err != nil { + return err + } + + // wait the Task to start + config := &models.TaskWaitConfig{ + Handle: handle, + ID: eid, + } + + params := tasks.NewWaitParamsWithContext(ctx).WithConfig(config) + _, err = c.client.Tasks.Wait(params) + if err != nil { + switch err := err.(type) { + case *tasks.WaitNotFound: + return InternalServerError("the Container(%s) has been shutdown during execution ofthe exec operation") + case *tasks.WaitPreconditionRequired: + return InternalServerError("container(%s) must be powered on in order perform the desired exec operation") + case *tasks.WaitInternalServerError: + return InternalServerError(err.Payload.Message) + default: + return InternalServerError(err.Error()) + } + } + + return nil + +} + // AddContainerToScope adds a container, referenced by handle, to a scope. // If an error is return, the returned handle should not be used. // diff --git a/lib/apiservers/portlayer/restapi/handlers/task_handlers.go b/lib/apiservers/portlayer/restapi/handlers/task_handlers.go index 51cff44c90..f7b9d9efec 100644 --- a/lib/apiservers/portlayer/restapi/handlers/task_handlers.go +++ b/lib/apiservers/portlayer/restapi/handlers/task_handlers.go @@ -277,11 +277,20 @@ func (handler *TaskHandlersImpl) WaitHandler(params tasks.WaitParams) middleware // wait task to set started field to something err := task.Wait(&op, handle, params.Config.ID) if err != nil { - op.Errorf("%s", err.Error()) - - return tasks.NewWaitInternalServerError().WithPayload( - &models.Error{Message: err.Error()}, - ) + switch err := err.(type) { + case *task.TaskPowerStateError: + op.Debugf("PowerStateError occured for task (%s) on handle (%s) while attempting to wait for the task to complete", params.Config.ID, handle) + return tasks.NewWaitPreconditionRequired().WithPayload( + &models.Error{Message: err.Error()}) + case *task.TaskNotFoundError: + op.Debugf("TaskNotFoundError occured for task (%s) on handle (%s) while attempting to wait for the task to complete", params.Config.ID, handle) + return tasks.NewWaitNotFound().WithPayload( + &models.Error{Message: err.Error()}) + default: + op.Errorf("%s", err.Error()) + return tasks.NewWaitInternalServerError().WithPayload( + &models.Error{Message: err.Error()}) + } } return tasks.NewWaitOK() diff --git a/lib/apiservers/portlayer/swagger.json b/lib/apiservers/portlayer/swagger.json index 6878308332..c5ed6c6bf7 100644 --- a/lib/apiservers/portlayer/swagger.json +++ b/lib/apiservers/portlayer/swagger.json @@ -1772,6 +1772,12 @@ "$ref": "#/definitions/Error" } }, + "428": { + "description": "target resource is not powered on", + "schema": { + "$ref": "#/definitions/Error" + } + }, "500": { "description": "Wait of task failed", "schema": { diff --git a/lib/portlayer/task/wait.go b/lib/portlayer/task/wait.go index 5308cd1a67..8ff80e1e8c 100644 --- a/lib/portlayer/task/wait.go +++ b/lib/portlayer/task/wait.go @@ -35,7 +35,7 @@ func Wait(op *trace.Operation, h interface{}, id string) error { if handle.Runtime != nil && handle.Runtime.PowerState != types.VirtualMachinePowerStatePoweredOn { err := fmt.Errorf("Unable to wait for task when container %s is not running", id) op.Errorf("%s", err) - return err + return TaskPowerStateError{Err: err} } _, okS := handle.ExecConfig.Sessions[id] @@ -59,3 +59,11 @@ func Wait(op *trace.Operation, h interface{}, id string) error { } return c.WaitForExec(timeout, id) } + +type TaskPowerStateError struct { + Err error +} + +func (t TaskPowerStateError) Error() string { + return t.Err.Error() +} diff --git a/tests/test-cases/Group1-Docker-Commands/1-38-Docker-Exec.robot b/tests/test-cases/Group1-Docker-Commands/1-38-Docker-Exec.robot index e4d56af359..f665a53fcf 100644 --- a/tests/test-cases/Group1-Docker-Commands/1-38-Docker-Exec.robot +++ b/tests/test-cases/Group1-Docker-Commands/1-38-Docker-Exec.robot @@ -128,12 +128,16 @@ Exec During PowerOff ${rc} ${output}= Run And Return Rc And Output docker %{VCH-PARAMS} pull ${busybox} Should Be Equal As Integers ${rc} 0 Should Not Contain ${output} Error - ${rc} ${id}= Run And Return Rc And Output docker %{VCH-PARAMS} run -d ${busybox} /bin/top + ${suffix}= Evaluate '%{DRONE_BUILD_NUMBER}-' + str(random.randint(1000,9999)) modules=random + Set Test Variable ${ExecPowerOffContainer} I-Have-Two-Anonymous-Volumes-${suffix} + ${rc} ${id}= Run And Return Rc And Output docker %{VCH-PARAMS} run -itd --name ${ExecPoweroffContainer} ${busybox} /bin/top Should Be Equal As Integers ${rc} 0 + :FOR ${idx} IN RANGE 1 20 \ Start Process docker %{VCH-PARAMS} exec ${id} /bin/top alias=exec-%{VCH-NAME}-${idx} shell=true - - Start Process docker %{VCH-PARAMS} stop ${id} alias=stop-%{VCH-NAME}-${id} shell=true + + ${rc} ${id}= Run And Return Rc And Output docker %{VCH-PARAMS} stop ${id} + Should Be Equal As Integers ${rc} 0 ${combinedoutput}= Set Variable @@ -141,7 +145,4 @@ Exec During PowerOff \ ${result}= Wait For Process exec-%{VCH-NAME}-${idx} timeout=2 mins \ ${combinedOutput}= Catenate ${combinedOutput} ${result.stderr}${\n} - ${stopResult}= Wait For Process stop-%{VCH-NAME}-${id} - Should Be Equal As Integers ${stopResult.rc} 0 - - Should Contain ${combinedOutput} container (${id}) has been powered off + Should Contain ${combinedOutput} Conflict error from portlayer: Container (${ExecPoweroffContainer}) is not powered on, please start the container before attempting to an exec an operation