From 69614f5091a12ef30c681f50351fca496cdd3c49 Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Mon, 24 May 2021 16:27:46 +0800 Subject: [PATCH] Add error return for handle When vcenter is unaccessible, we cannot always return nil which indicates the container VM does not exist anymore so get removed from cache. Manual tests for container vm start/stop/exec/delete passed after power off vcenter and enable vcenter again. --- .../portlayer/restapi/handlers/containers_handlers.go | 5 ++++- lib/portlayer/exec/container.go | 10 +++++----- lib/portlayer/logging/logging.go | 6 +++--- lib/portlayer/network/container.go | 5 ++++- lib/portlayer/network/network.go | 11 +++++++++-- lib/portlayer/portlayer.go | 6 +++--- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go b/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go index 344dff53d82..2227670f1f7 100644 --- a/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go +++ b/lib/apiservers/portlayer/restapi/handlers/containers_handlers.go @@ -198,7 +198,10 @@ func (handler *ContainersHandlersImpl) GetHandler(params containers.GetParams) m op := trace.NewOperationFromID(context.Background(), params.OpID, "containers.GetHandler(%s)", params.ID) defer trace.End(trace.Begin("GetHandler", op)) - h := exec.GetContainer(context.Background(), uid.Parse(params.ID)) + h, err := exec.GetContainer(context.Background(), uid.Parse(params.ID)) + if err != nil { + return containers.NewGetDefault(503).WithPayload(&models.Error{Message: err.Error()}) + } if h == nil { return containers.NewGetNotFound().WithPayload(&models.Error{Message: fmt.Sprintf("container %s not found", params.ID)}) } diff --git a/lib/portlayer/exec/container.go b/lib/portlayer/exec/container.go index 2b562edbcc8..764b9a24f23 100644 --- a/lib/portlayer/exec/container.go +++ b/lib/portlayer/exec/container.go @@ -217,14 +217,14 @@ func newContainer(base *containerBase) *Container { return c } -func GetContainer(ctx context.Context, id uid.UID) *Handle { +func GetContainer(ctx context.Context, id uid.UID) (*Handle, error) { // get from the cache container := Containers.Container(id.String()) if container != nil { return container.NewHandle(ctx) } - return nil + return nil, nil } func (c *ContainerInfo) String() string { @@ -318,20 +318,20 @@ func (c *Container) WaitForState(s State) <-chan struct{} { return eventChan } -func (c *Container) NewHandle(ctx context.Context) *Handle { +func (c *Container) NewHandle(ctx context.Context) (*Handle, error) { // Call property collector to fill the data if c.vm != nil { op := trace.FromContext(ctx, "") // FIXME: this should be calling the cache to decide if a refresh is needed if err := c.Refresh(op); err != nil { op.Errorf("refreshing container %s failed: %s", c, err) - return nil // nil indicates error + return nil, err } } // return a handle that represents zero changes over the current configuration // for this container - return newHandle(c) + return newHandle(c), nil } // Refresh updates config and runtime info, holding a lock only while swapping diff --git a/lib/portlayer/logging/logging.go b/lib/portlayer/logging/logging.go index 94acb47d405..f9bc8880bde 100644 --- a/lib/portlayer/logging/logging.go +++ b/lib/portlayer/logging/logging.go @@ -73,9 +73,9 @@ func eventCallback(ie events.Event) { operation := func() error { var err error - handle := container.NewHandle(op) - if handle == nil { - err = fmt.Errorf("Handle for %s cannot be created", ie.Reference()) + handle, err := container.NewHandle(op) + if err != nil { + err = fmt.Errorf("Handle for %s cannot be created: %s", ie.Reference(), err) log.Error(err) return err } diff --git a/lib/portlayer/network/container.go b/lib/portlayer/network/container.go index d3fced23073..a072f28bff4 100644 --- a/lib/portlayer/network/container.go +++ b/lib/portlayer/network/container.go @@ -101,7 +101,10 @@ func (c *Container) Refresh(ctx context.Context) error { // this will "refresh" the container executor config that contains // the current ip addresses - h := exec.GetContainer(ctx, c.ID()) + h, err := exec.GetContainer(ctx, c.ID()) + if err != nil { + return fmt.Errorf("could not find container %s: %s", c.ID(), err) + } if h == nil { return fmt.Errorf("could not find container %s", c.ID()) } diff --git a/lib/portlayer/network/network.go b/lib/portlayer/network/network.go index bf0c7a2a290..d27bf48add8 100644 --- a/lib/portlayer/network/network.go +++ b/lib/portlayer/network/network.go @@ -129,7 +129,11 @@ func handleEvent(netctx *Context, ie events.Event) { op := trace.NewOperation(context.Background(), fmt.Sprintf("handleEvent(%s)", ie.EventID())) op.Infof("Handling Event: %s", ie.EventID()) // grab the operation from the event - handle := exec.GetContainer(op, uid.Parse(ie.Reference())) + handle, err := exec.GetContainer(op, uid.Parse(ie.Reference())) + if err != nil { + op.Errorf("Could not find container: %s", err) + return + } if handle == nil { _, err := netctx.RemoveIDFromScopes(op, ie.Reference()) if err != nil { @@ -196,7 +200,10 @@ func engageContext(op trace.Operation, netctx *Context, em event.EventManager) e defer s.Resume() for _, c := range exec.Containers.Containers(nil) { log.Debugf("adding container %s", c) - h := c.NewHandle(op) + h, err := c.NewHandle(op) + if err != nil { + return err + } defer h.Close() // add any user created networks that show up in container's config diff --git a/lib/portlayer/portlayer.go b/lib/portlayer/portlayer.go index 0ed6301d1b5..2a75c424dd5 100644 --- a/lib/portlayer/portlayer.go +++ b/lib/portlayer/portlayer.go @@ -148,9 +148,9 @@ func TakeCareOfSerialPorts(op trace.Operation, sess *session.Session) { operation := func() error { // Obtain a container handle - handle := containers[i].NewHandle(op) - if handle == nil { - err := fmt.Errorf("unable to obtain a handle for container %s", containerID) + handle, err := containers[i].NewHandle(op) + if err != nil { + err := fmt.Errorf("unable to obtain a handle for container %s: %s", containerID, err) op.Errorf("%s", err) return err