From e0a0b7169f5c74b066a521fcd1a36455c3d75f3e Mon Sep 17 00:00:00 2001 From: Emma Date: Fri, 3 Mar 2017 12:47:37 -0600 Subject: [PATCH] fix abnormal behavior caused by migration error status --- lib/apiservers/engine/backends/container.go | 10 +++++ .../engine/backends/container_proxy.go | 37 ++++++++++++++----- .../restapi/handlers/containers_handlers.go | 6 +-- lib/migration/migrator.go | 4 +- 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/lib/apiservers/engine/backends/container.go b/lib/apiservers/engine/backends/container.go index 279989a21c..7b21926db6 100644 --- a/lib/apiservers/engine/backends/container.go +++ b/lib/apiservers/engine/backends/container.go @@ -481,8 +481,18 @@ func (c *Container) ContainerRm(name string, config *types.ContainerRmConfig) er // Use the force and stop the container first secs := 0 + if config.ForceRemove { c.containerProxy.Stop(vc, name, &secs, true) + } else { + broken, err := c.containerProxy.IsBroken(vc) + if err != nil { + return err + } + // force stop if container is broken to make sure container is deletable later + if broken { + c.containerProxy.Stop(vc, name, &secs, true) + } } //call the remove directly on the name. No need for using a handle. diff --git a/lib/apiservers/engine/backends/container_proxy.go b/lib/apiservers/engine/backends/container_proxy.go index d0fd0ee543..3a0ca03300 100644 --- a/lib/apiservers/engine/backends/container_proxy.go +++ b/lib/apiservers/engine/backends/container_proxy.go @@ -88,6 +88,7 @@ type VicContainerProxy interface { Stop(vc *viccontainer.VicContainer, name string, seconds *int, unbound bool) error IsRunning(vc *viccontainer.VicContainer) (bool, error) + IsBroken(vc *viccontainer.VicContainer) (bool, error) Wait(vc *viccontainer.VicContainer, timeout time.Duration) (exitCode int32, processStatus string, containerState string, reterr error) Signal(vc *viccontainer.VicContainer, sig uint64) error Resize(vc *viccontainer.VicContainer, height, width int32) error @@ -477,12 +478,13 @@ func (c *ContainerProxy) Stop(vc *viccontainer.VicContainer, name string, second // we have a container on the PL side lets check the state before proceeding // ignore the error since others will be checking below..this is an attempt to short circuit the op // TODO: can be replaced with simple cache check once power events are propagated to persona - running, err := c.IsRunning(vc) + inspectJSON, err := c.dockerInfo(vc) if err != nil && IsNotFoundError(err) { cache.ContainerCache().DeleteContainer(vc.ContainerID) return err } - if !running { + // attempt to stop container if status is running or broken + if !inspectJSON.State.Running && inspectJSON.State.Status != "broken" { return nil } @@ -541,29 +543,44 @@ func (c *ContainerProxy) Stop(vc *viccontainer.VicContainer, name string, second func (c *ContainerProxy) IsRunning(vc *viccontainer.VicContainer) (bool, error) { defer trace.End(trace.Begin("")) + inspectJSON, err := c.dockerInfo(vc) + if err != nil { + return false, err + } + return inspectJSON.State.Running, nil +} + +func (c *ContainerProxy) dockerInfo(vc *viccontainer.VicContainer) (*types.ContainerJSON, error) { + defer trace.End(trace.Begin("")) + if c.client == nil { - return false, InternalServerError("ContainerProxy.IsRunning failed to get a portlayer client") + return nil, InternalServerError("ContainerProxy.dockerInfo failed to get a portlayer client") } results, err := c.client.Containers.GetContainerInfo(containers.NewGetContainerInfoParamsWithContext(ctx).WithID(vc.ContainerID)) if err != nil { switch err := err.(type) { case *containers.GetContainerInfoNotFound: - return false, NotFoundError(fmt.Sprintf("No such container: %s", vc.ContainerID)) + return nil, NotFoundError(fmt.Sprintf("No such container: %s", vc.ContainerID)) case *containers.GetContainerInfoInternalServerError: - return false, InternalServerError(err.Payload.Message) + return nil, InternalServerError(err.Payload.Message) default: - return false, InternalServerError(fmt.Sprintf("Unknown error from the interaction port layer: %s", err)) + return nil, InternalServerError(fmt.Sprintf("Unknown error from the interaction port layer: %s", err)) } } - inspectJSON, err := ContainerInfoToDockerContainerInspect(vc, results.Payload, c.portlayerName) + return ContainerInfoToDockerContainerInspect(vc, results.Payload, c.portlayerName) +} + +// IsBroken returns true if the given container is broken, e.g. migration failed +func (c *ContainerProxy) IsBroken(vc *viccontainer.VicContainer) (bool, error) { + defer trace.End(trace.Begin("")) + + inspectJSON, err := c.dockerInfo(vc) if err != nil { - log.Errorf("containerInfoToDockerContainerInspect failed with %s", err) return false, err } - - return inspectJSON.State.Running, nil + return inspectJSON.State.Status == "broken", nil } func (c *ContainerProxy) Wait(vc *viccontainer.VicContainer, timeout time.Duration) ( diff --git a/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go b/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go index cbc254feba..60120154a8 100644 --- a/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go +++ b/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go @@ -424,9 +424,9 @@ func convertContainerToContainerInfo(container *exec.ContainerInfo) *models.Cont var state string if container.MigrationError != nil { - state = "Migration failed" - info.ProcessConfig.ErrorMsg = container.MigrationError.Error() - info.ProcessConfi.Status = state + state = "broken" + info.ProcessConfig.ErrorMsg = fmt.Sprintf("Migration failed: %s", container.MigrationError.Error()) + info.ProcessConfig.Status = state } else { state = container.State().String() } diff --git a/lib/migration/migrator.go b/lib/migration/migrator.go index 60f653cb1d..792ef6f908 100644 --- a/lib/migration/migrator.go +++ b/lib/migration/migrator.go @@ -33,7 +33,7 @@ import ( // // Note: Input map conf is VCH appliance guestinfo map, and returned map is the new guestinfo. // Returns false without error means no need to migrate, and returned map is the copy of input map -// If there is error returned, return true and half-migrated value +// If there is error returned, returns true and half-migrated value func MigrateApplianceConfig(ctx context.Context, s *session.Session, conf map[string]string) (map[string]string, bool, error) { return migrateConfig(ctx, s, conf, manager.ApplianceConfigure, manager.ApplianceVersionKey) } @@ -42,7 +42,7 @@ func MigrateApplianceConfig(ctx context.Context, s *session.Session, conf map[st // // Note: Migrated data will be returned in map, and input object is not changed. // Returns false without error means no need to migrate, and returned map is the copy of input map -// If there is error returned, return true and half-migrated value +// If there is error returned, returns true and half-migrated value func MigrateContainerConfig(conf map[string]string) (map[string]string, bool, error) { return migrateConfig(nil, nil, conf, manager.ContainerConfigure, manager.ContainerVersionKey) }