Merge pull request #31055 from Microsoft/jjh/fix30278

Windows: Retry workaround for RS1/RS2 compute system enumeration

daemon/graphdriver/windows/windows.go

@@ -31,6 +31,7 @@ import (
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/longpath"
 	"github.com/docker/docker/pkg/reexec"
+	"github.com/docker/docker/pkg/system"
 	units "github.com/docker/go-units"
 	"golang.org/x/sys/windows"
 )
@@ -265,19 +266,35 @@ func (d *Driver) Remove(id string) error {
 	// it is a transient error. Retry until it succeeds.
 	var computeSystems []hcsshim.ContainerProperties
 	retryCount := 0
+	osv := system.GetOSVersion()
 	for {
-		// Get and terminate any template VMs that are currently using the layer
+		// Get and terminate any template VMs that are currently using the layer.
+		// Note: It is unfortunate that we end up in the graphdrivers Remove() call
+		// for both containers and images, but the logic for template VMs is only
+		// needed for images - specifically we are looking to see if a base layer
+		// is in use by a template VM as a result of having started a Hyper-V
+		// container at some point.
+		//
+		// We have a retry loop for ErrVmcomputeOperationInvalidState and
+		// ErrVmcomputeOperationAccessIsDenied as there is a race condition
+		// in RS1 and RS2 building during enumeration when a silo is going away
+		// for example under it, in HCS. AccessIsDenied added to fix 30278.
+		//
+		// TODO @jhowardmsft - For RS3, we can remove the retries. Also consider
+		// using platform APIs (if available) to get this more succinctly. Also
+		// consider enlighting the Remove() interface to have context of why
+		// the remove is being called - that could improve efficiency by not
+		// enumerating compute systems during a remove of a container as it's
+		// not required.
 		computeSystems, err = hcsshim.GetContainers(hcsshim.ComputeSystemQuery{})
 		if err != nil {
-			if err == hcsshim.ErrVmcomputeOperationInvalidState {
-				if retryCount >= 5 {
-					// If we are unable to get the list of containers
-					// go ahead and attempt to delete the layer anyway
-					// as it will most likely work.
+			if (osv.Build < 15139) &&
+				((err == hcsshim.ErrVmcomputeOperationInvalidState) || (err == hcsshim.ErrVmcomputeOperationAccessIsDenied)) {
+				if retryCount >= 500 {
 					break
 				}
 				retryCount++
-				time.Sleep(2 * time.Second)
+				time.Sleep(10 * time.Millisecond)
 				continue
 			}
 			return err

vendor.conf

@@ -1,6 +1,6 @@
 # the following lines are in sorted order, FYI
 github.com/Azure/go-ansiterm 388960b655244e76e24c75f48631564eaefade62
-github.com/Microsoft/hcsshim v0.5.10
+github.com/Microsoft/hcsshim v0.5.12
 github.com/Microsoft/go-winio v0.3.8
 github.com/Sirupsen/logrus v0.11.0
 github.com/davecgh/go-spew 6d212800a42e8ab5c146b8ace3490ee17e5225f9

vendor/github.com/Microsoft/hcsshim/errors.go

@@ -50,6 +50,10 @@ var (
 
 	// ErrProcNotFound is an error encountered when the the process cannot be found
 	ErrProcNotFound = syscall.Errno(0x7f)
+
+	// ErrVmcomputeOperationAccessIsDenied is an error which can be encountered when enumerating compute systems in RS1/RS2
+	// builds when the underlying silo might be in the process of terminating. HCS was fixed in RS3.
+	ErrVmcomputeOperationAccessIsDenied = syscall.Errno(0x5)
 )
 
 // ProcessError is an error encountered in HCS during an operation on a Process object

vendor/github.com/Microsoft/hcsshim/interface.go

@@ -62,6 +62,7 @@ type ContainerConfig struct {
 	SandboxPath                string      `json:",omitempty"` // Location of unmounted sandbox. Used by Hyper-V containers only. Format %root%\windowsfilter
 	HvPartition                bool        // True if it a Hyper-V Container
 	EndpointList               []string    // List of networking endpoints to be attached to container
+	NetworkSharedContainerName string      `json:",omitempty"` // Name (ID) of the container that we will share the network stack with.
 	HvRuntime                  *HvRuntime  `json:",omitempty"` // Hyper-V container settings. Used by Hyper-V containers only. Format ImagePath=%root%\BaseLayerID\UtilityVM
 	Servicing                  bool        // True if this container is for servicing
 	AllowUnqualifiedDNSQuery   bool        // True to allow unqualified DNS name resolution