fix the race in case sandbox start failed during pod creating

daemon/pod/container.go

@@ -20,6 +20,7 @@ import (
 	"github.com/docker/engine-api/types/strslice"
 
 	"github.com/hyperhq/hypercontainer-utils/hlog"
+	"github.com/hyperhq/hyperd/errors"
 	apitypes "github.com/hyperhq/hyperd/types"
 	"github.com/hyperhq/hyperd/utils"
 	runv "github.com/hyperhq/runv/api"
@@ -233,6 +234,11 @@ func (c *Container) Add() error {
 
 func (c *Container) start() error {
 	if err := c.status.Start(); err != nil {
+		if err == errors.ErrContainerAlreadyRunning {
+			err = nil
+			c.Log(INFO, "container in running state, do not need start")
+			return nil
+		}
 		c.Log(ERROR, err)
 		return err
 	}
@@ -1196,8 +1202,10 @@ func (cs *ContainerStatus) Start() error {
 	cs.Lock()
 	defer cs.Unlock()
 
-	if cs.State != S_CONTAINER_CREATED {
-		return fmt.Errorf("only CREATING container could be set to creatd, current: %d", cs.State)
+	if cs.State == S_CONTAINER_RUNNING {
+		return errors.ErrContainerAlreadyRunning
+	} else if cs.State != S_CONTAINER_CREATED {
+		return fmt.Errorf("only CREATED container could be set to running, current: %d", cs.State)
 	}
 
 	cs.Killed = false

daemon/pod/decommission.go

@@ -536,14 +536,14 @@ func (p *XPod) cleanup() {
 	p.resourceLock.Lock()
 	defer p.resourceLock.Unlock()
 
-	p.statusLock.RLock()
+	p.statusLock.Lock()
 	if p.status == S_POD_STOPPED || p.status == S_POD_NONE {
-		p.statusLock.RUnlock()
+		p.statusLock.Unlock()
 		return
 	} else {
 		p.status = S_POD_STOPPING
 	}
-	p.statusLock.RUnlock()
+	p.statusLock.Unlock()
 
 	err := p.decommissionResources()
 	if err != nil {

daemon/pod/migration.go

@@ -198,9 +198,6 @@ func persistLagecyPod(factory *PodFactory, spec *apitypes.UserPod) error {
 	if err = p.initResources(spec, false); err != nil {
 		return err
 	}
-	if err = p.prepareResources(); err != nil {
-		return err
-	}
 
 	if err = p.savePod(); err != nil {
 		return err

daemon/pod/pod.go

@@ -74,6 +74,7 @@ type XPod struct {
 	// got a value from this chan, it should put an element to it again, in case other procedure may wait
 	// on it too.
 	stoppedChan chan bool
+	initCond    *sync.Cond
 }
 
 // The Log infrastructure, to add pod name as prefix of the log message.
@@ -110,29 +111,37 @@ func (p *XPod) SandboxName() string {
 }
 
 func (p *XPod) IsNone() bool {
-	p.statusLock.Lock()
+	p.statusLock.RLock()
 	isNone := p.status == S_POD_NONE
-	p.statusLock.Unlock()
+	p.statusLock.RUnlock()
 
 	return isNone
 }
 
 func (p *XPod) IsRunning() bool {
-	p.statusLock.Lock()
+	p.statusLock.RLock()
 	running := p.status == S_POD_RUNNING
-	p.statusLock.Unlock()
+	p.statusLock.RUnlock()
 
 	return running
 }
 
 func (p *XPod) IsAlive() bool {
-	p.statusLock.Lock()
+	p.statusLock.RLock()
 	alive := (p.status == S_POD_RUNNING) || (p.status == S_POD_STARTING)
-	p.statusLock.Unlock()
+	p.statusLock.RUnlock()
 
 	return alive
 }
 
+func (p *XPod) IsStopped() bool {
+	p.statusLock.RLock()
+	stopped := p.status == S_POD_STOPPED
+	p.statusLock.RUnlock()
+
+	return stopped
+}
+
 func (p *XPod) IsContainerRunning(cid string) bool {
 	if !p.IsRunning() {
 		return false

daemon/pod/provision.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/hyperhq/hypercontainer-utils/hlog"
+	"github.com/hyperhq/hyperd/errors"
 	apitypes "github.com/hyperhq/hyperd/types"
 	"github.com/hyperhq/hyperd/utils"
 	runv "github.com/hyperhq/runv/api"
@@ -84,7 +85,7 @@ func newXPod(factory *PodFactory, spec *apitypes.UserPod) (*XPod, error) {
 	}
 	factory.hosts = HostsCreator(spec.Id)
 	factory.logCreator = initLogCreator(factory, spec)
-	return &XPod{
+	p := &XPod{
 		name:         spec.Id,
 		logPrefix:    fmt.Sprintf("Pod[%s] ", spec.Id),
 		globalSpec:   spec.CloneGlobalPart(),
@@ -98,7 +99,9 @@ func newXPod(factory *PodFactory, spec *apitypes.UserPod) (*XPod, error) {
 		statusLock:   &sync.RWMutex{},
 		stoppedChan:  make(chan bool, 1),
 		factory:      factory,
-	}, nil
+	}
+	p.initCond = sync.NewCond(p.statusLock.RLocker())
+	return p, nil
 }
 
 func (p *XPod) ContainerCreate(c *apitypes.UserContainer) (string, error) {
@@ -211,7 +214,7 @@ func (p *XPod) ContainerStart(cid string) error {
 // Start() means start a STOPPED pod.
 func (p *XPod) Start() error {
 
-	if p.status == S_POD_STOPPED {
+	if p.IsStopped() {
 		if err := p.createSandbox(p.globalSpec); err != nil {
 			p.Log(ERROR, "failed to create sandbox for the stopped pod: %v", err)
 			return err
@@ -226,13 +229,12 @@ func (p *XPod) Start() error {
 		}
 	}
 
-	if p.status == S_POD_RUNNING {
-		if err := p.startAll(); err != nil {
-			return err
-		}
-	} else {
-		err := fmt.Errorf("not in proper status and could not be started: %v", p.status)
-		p.Log(ERROR, err)
+	err := p.waitPodRun("start pod")
+	if err != nil {
+		p.Log(ERROR, "wait running failed, cannot start pod")
+		return err
+	}
+	if err := p.startAll(); err != nil {
 		return err
 	}
 
@@ -299,20 +301,20 @@ func (p *XPod) waitVMInit() {
 	}
 	r := p.sandbox.WaitInit()
 	p.Log(INFO, "sandbox init result: %#v", r)
+	p.statusLock.Lock()
 	if r.IsSuccess() {
-		p.statusLock.Lock()
 		if p.status == S_POD_STARTING {
 			p.status = S_POD_RUNNING
 		}
-		p.statusLock.Unlock()
 	} else {
 		p.statusLock.Lock()
 		if p.sandbox != nil {
 			go p.sandbox.Shutdown()
 		}
 		p.status = S_POD_STOPPING
-		p.statusLock.Unlock()
 	}
+	p.initCond.Broadcast()
+	p.statusLock.Unlock()
 }
 
 func (p *XPod) reserveNames(containers []*apitypes.UserContainer) error {
@@ -391,6 +393,15 @@ func (p *XPod) prepareResources() error {
 	var (
 		err error
 	)
+
+	p.resourceLock.Lock()
+	defer p.resourceLock.Unlock()
+
+	if !p.IsAlive() {
+		p.Log(ERROR, "pod is not alive, can not prepare resources")
+		return errors.ErrPodNotAlive.WithArgs(p.Id())
+	}
+
 	//generate /etc/hosts
 	p.factory.hosts.Do()
 
@@ -414,6 +425,14 @@ func (p *XPod) prepareResources() error {
 // addResourcesToSandbox() add resources to sandbox parallelly, it issues
 // runV API parallelly to send the NIC, Vols, and Containers to sandbox
 func (p *XPod) addResourcesToSandbox() error {
+	p.resourceLock.Lock()
+	defer p.resourceLock.Unlock()
+
+	if !p.IsAlive() {
+		p.Log(ERROR, "pod is not alive, can not add resources to sandbox")
+		return errors.ErrPodNotAlive.WithArgs(p.Id())
+	}
+
 	p.Log(INFO, "adding resource to sandbox")
 	future := utils.NewFutureSet()
 
@@ -471,3 +490,25 @@ func (p *XPod) sandboxShareDir() string {
 	}
 	return filepath.Join(hypervisor.BaseDir, p.sandbox.Id, hypervisor.ShareDirTag)
 }
+
+func (p *XPod) waitPodRun(activity string) error {
+	p.statusLock.RLock()
+	for {
+		if p.status == S_POD_RUNNING || p.status == S_POD_PAUSED {
+			p.statusLock.RUnlock()
+			p.Log(DEBUG, "pod is running, proceed %s", activity)
+			return nil
+		}
+		if p.status != S_POD_STARTING {
+			p.statusLock.RUnlock()
+			// only starting could transit to running, if not starting, that's mean failed
+			p.Log(ERROR, "pod is not running, cannot %s", activity)
+			return errors.ErrPodNotRunning.WithArgs(p.Id())
+		}
+		p.Log(TRACE, "wait for pod running")
+		p.initCond.Wait()
+	}
+	// should never reach here
+	p.statusLock.RUnlock()
+	return errors.ErrorCodeCommon.WithArgs("reach unreachable code...")
+}

errors/errors.go

@@ -0,0 +1,35 @@
+package errors
+
+import (
+	"net/http"
+
+	"github.com/docker/distribution/registry/api/errcode"
+)
+
+const errGroup = "hyperd"
+
+var (
+	ErrorCodeCommon = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "COMMONERROR",
+		Message:        "%v",
+		HTTPStatusCode: http.StatusInternalServerError,
+	})
+
+	ErrPodNotAlive = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "HYPER_POD_NOT_ALIVE",
+		Message:        "cannot complete the operation, because the pod %s is not alive",
+		HTTPStatusCode: http.StatusPreconditionFailed,
+	})
+
+	ErrPodNotRunning = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "HYPER_POD_NOT_RUNNING",
+		Message:        "cannot complete the operation, because the pod %s is not running",
+		HTTPStatusCode: http.StatusPreconditionFailed,
+	})
+
+	ErrContainerAlreadyRunning = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "HYPER_CONTAINER_RUNNING",
+		Message:        "container %s is in running state",
+		HTTPStatusCode: http.StatusPreconditionFailed,
+	})
+)