Merged PR 4188258: update hcsshim ado to 43a75bb

Related work items: microsoft#173, microsoft#839, microsoft#856, microsoft#877, microsoft#881, microsoft#886, microsoft#887, microsoft#888, microsoft#889, microsoft#890, microsoft#893, microsoft#894, microsoft#896, microsoft#899, microsoft#900, microsoft#902, microsoft#904, microsoft#905, microsoft#906, microsoft#907, microsoft#908, microsoft#910, microsoft#912, microsoft#913, microsoft#914, microsoft#916, microsoft#918, microsoft#923, microsoft#925, microsoft#926, microsoft#928, microsoft#929, microsoft#932, microsoft#933, microsoft#934, microsoft#938, microsoft#939, microsoft#942, microsoft#943, microsoft#945, microsoft#946, microsoft#947, microsoft#949, microsoft#951, microsoft#952, microsoft#954

.gitattributes

@@ -0,0 +1 @@
+* text=auto eol=lf

.gitignore

@@ -1 +1,3 @@
 *.exe
+.idea
+.vscode

CODEOWNERS

@@ -1,3 +1,3 @@
-* @microsoft/containerplat
-
+* @microsoft/containerplat
+
 /hcn/* @nagiesek

appveyor.yml

@@ -1,14 +1,14 @@
 version: 0.1.{build}
 
-image: Visual Studio 2017
+image: Visual Studio 2019
 
 clone_folder: c:\gopath\src\github.com\Microsoft\hcsshim
 
 environment:
   GOPATH: c:\gopath
   PATH: "%GOPATH%\\bin;C:\\gometalinter-2.0.12-windows-amd64;%PATH%"
 
-stack: go 1.13.4
+stack: go 1.15
 
 build_script:
   - appveyor DownloadFile https://github.com/alecthomas/gometalinter/releases/download/v2.0.12/gometalinter-2.0.12-windows-amd64.zip
@@ -19,16 +19,17 @@ build_script:
   - go build ./cmd/tar2ext4
   - go build ./cmd/wclayer
   - go build ./cmd/device-util
-  - go build ./internal/tools/grantvmgroupaccess 
+  - go build ./internal/tools/grantvmgroupaccess
   - go build ./internal/tools/uvmboot
   - go build ./internal/tools/zapdir
-  - go test -v ./... -tags admin
+  - go test -gcflags=all=-d=checkptr -v ./... -tags admin
   - cd test
-  - go test -v ./internal -tags admin
-  - go test -c ./containerd-shim-runhcs-v1/ -tags functional
-  - go test -c ./cri-containerd/ -tags functional
-  - go test -c ./functional/ -tags functional
-  - go test -c ./runhcs/ -tags functional
+  - go test -gcflags=all=-d=checkptr -v ./internal -tags admin
+  - go test -gcflags=all=-d=checkptr -c ./containerd-shim-runhcs-v1/ -tags functional
+  - go test -gcflags=all=-d=checkptr -c ./cri-containerd/ -tags functional
+  - go test -gcflags=all=-d=checkptr -c ./functional/ -tags functional
+  - go test -gcflags=all=-d=checkptr -c ./runhcs/ -tags functional
+  - go build -o sample-logging-driver.exe ./cri-containerd/helpers/log.go
 
 artifacts:
   - path: 'containerd-shim-runhcs-v1.exe'
@@ -43,3 +44,4 @@ artifacts:
   - path: './test/cri-containerd.test.exe'
   - path: './test/functional.test.exe'
   - path: './test/runhcs.test.exe'
+  - path: './test/sample-logging-driver.exe'

cmd/containerd-shim-runhcs-v1/clone.go

@@ -0,0 +1,50 @@
+package main
+
+import (
+	"context"
+
+	"github.com/Microsoft/hcsshim/internal/clone"
+	"github.com/Microsoft/hcsshim/internal/uvm"
+)
+
+// saveAsTemplate saves the UVM and container inside it as a template and also stores the
+// relevant information in the registry so that clones can be created from this template.
+// Every cloned uvm gets its own NIC and we do not want to create clones of a template
+// which still has a NIC attached to it. So remove the NICs attached to the template uvm
+// before saving it.
+// Similar to the NIC scenario we do not want to create clones from a template with an
+// active GCS connection so close the GCS connection too.
+func saveAsTemplate(ctx context.Context, templateTask *hcsTask) (err error) {
+	var utc *uvm.UVMTemplateConfig
+	var templateConfig *clone.TemplateConfig
+
+	if err = templateTask.host.RemoveAllNICs(ctx); err != nil {
+		return err
+	}
+
+	if err = templateTask.host.CloseGCSConnection(); err != nil {
+		return err
+	}
+
+	utc, err = templateTask.host.GenerateTemplateConfig()
+	if err != nil {
+		return err
+	}
+
+	templateConfig = &clone.TemplateConfig{
+		TemplateUVMID:         utc.UVMID,
+		TemplateUVMResources:  utc.Resources,
+		TemplateUVMCreateOpts: utc.CreateOpts,
+		TemplateContainerID:   templateTask.id,
+		TemplateContainerSpec: *templateTask.taskSpec,
+	}
+
+	if err = clone.SaveTemplateConfig(ctx, templateConfig); err != nil {
+		return err
+	}
+
+	if err = templateTask.host.SaveAsTemplate(ctx); err != nil {
+		return err
+	}
+	return nil
+}

cmd/containerd-shim-runhcs-v1/delete.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"time"
 
 	"github.com/Microsoft/hcsshim/internal/hcs"
@@ -29,7 +30,6 @@ The delete command will be executed in the container's bundle as its cwd.
 		// We cant write anything to stdout for this cmd other than the
 		// task.DeleteResponse by protcol. We can write to stderr which will be
 		// warning logged in containerd.
-		logrus.SetOutput(ioutil.Discard)
 
 		ctx, span := trace.StartSpan(gcontext.Background(), "delete")
 		defer span.End()
@@ -82,6 +82,13 @@ The delete command will be executed in the container's bundle as its cwd.
 			}
 		}
 
+		// hcsshim shim writes panic logs in the bundle directory in a file named "panic.log"
+		// log those messages (if any) on stderr so that it shows up in containerd's log.
+		logs, err := ioutil.ReadFile(filepath.Join(bundleFlag, "panic.log"))
+		if err == nil && len(logs) > 0 {
+			logrus.WithField("log", string(logs)).Error("found shim panic logs during delete")
+		}
+
 		// Remove the bundle on disk
 		if err := os.RemoveAll(bundleFlag); err != nil && !os.IsNotExist(err) {
 			return err

cmd/containerd-shim-runhcs-v1/exec_clone.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"context"
+
+	"github.com/Microsoft/hcsshim/internal/cmd"
+	"github.com/Microsoft/hcsshim/internal/cow"
+	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/uvm"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/sirupsen/logrus"
+)
+
+func newClonedExec(
+	ctx context.Context,
+	events publisher,
+	tid string,
+	host *uvm.UtilityVM,
+	c cow.Container,
+	id, bundle string,
+	isWCOW bool,
+	spec *specs.Process,
+	io cmd.UpstreamIO) *clonedExec {
+	log.G(ctx).WithFields(logrus.Fields{
+		"tid":    tid,
+		"eid":    id, // Init exec ID is always same as Task ID
+		"bundle": bundle,
+	}).Debug("newClonedExec")
+
+	he := &hcsExec{
+		events:      events,
+		tid:         tid,
+		host:        host,
+		c:           c,
+		id:          id,
+		bundle:      bundle,
+		isWCOW:      isWCOW,
+		spec:        spec,
+		io:          io,
+		processDone: make(chan struct{}),
+		state:       shimExecStateCreated,
+		exitStatus:  255, // By design for non-exited process status.
+		exited:      make(chan struct{}),
+	}
+
+	ce := &clonedExec{
+		he,
+	}
+	go he.waitForContainerExit()
+	return ce
+}
+
+var _ = (shimExec)(&clonedExec{})
+
+// clonedExec inherits from hcsExec. The only difference between these two is that
+// on starting a clonedExec it doesn't attempt to start the container even if the
+// exec is the init process. This is because in case of clonedExec the container is
+// already running inside the pod.
+type clonedExec struct {
+	*hcsExec
+}
+
+func (ce *clonedExec) Start(ctx context.Context) (err error) {
+	// A cloned exec should never initialize the container as it should
+	// already be running.
+	return ce.startInternal(ctx, false)
+}

cmd/containerd-shim-runhcs-v1/exec_hcs.go

@@ -181,7 +181,7 @@ func (he *hcsExec) Status() *task.StateResponse {
 	}
 }
 
-func (he *hcsExec) Start(ctx context.Context) (err error) {
+func (he *hcsExec) startInternal(ctx context.Context, initializeContainer bool) (err error) {
 	he.sl.Lock()
 	defer he.sl.Unlock()
 	if he.state != shimExecStateCreated {
@@ -192,8 +192,7 @@ func (he *hcsExec) Start(ctx context.Context) (err error) {
 			he.exitFromCreatedL(ctx, 1)
 		}
 	}()
-	if he.id == he.tid {
-		// This is the init exec. We need to start the container itself
+	if initializeContainer {
 		err = he.c.Start(ctx)
 		if err != nil {
 			return err
@@ -257,6 +256,12 @@ func (he *hcsExec) Start(ctx context.Context) (err error) {
 	return nil
 }
 
+func (he *hcsExec) Start(ctx context.Context) (err error) {
+	// If he.id == he.tid then this is the init exec.
+	// We need to initialize the container itself before starting this exec.
+	return he.startInternal(ctx, he.id == he.tid)
+}
+
 func (he *hcsExec) Kill(ctx context.Context, signal uint32) error {
 	he.sl.Lock()
 	defer he.sl.Unlock()
@@ -414,6 +419,8 @@ func (he *hcsExec) exitFromCreatedL(ctx context.Context, status int) {
 //
 // 6. Close `he.exited` channel to unblock any waiters who might have called
 // `Create`/`Wait`/`Start` which is a valid pattern.
+//
+// 7. Finally, save the UVM and this container as a template if specified.
 func (he *hcsExec) waitForExit() {
 	ctx, span := trace.StartSpan(context.Background(), "hcsExec::waitForExit")
 	defer span.End()

cmd/containerd-shim-runhcs-v1/main.go

@@ -55,15 +55,6 @@ func stack() []byte {
 	}
 }
 
-func panicRecover() {
-	if r := recover(); r != nil {
-		logrus.WithFields(logrus.Fields{
-			"panic": r,
-			"stack": string(stack()),
-		}).Error("containerd-shim-runhcs-v1: panic")
-	}
-}
-
 func etwCallback(sourceID guid.GUID, state etw.ProviderState, level etw.Level, matchAnyKeyword uint64, matchAllKeyword uint64, filterData uintptr) {
 	if state == etw.ProviderStateCaptureState {
 		resp, err := svc.DiagStacks(context.Background(), &shimdiag.StacksRequest{})
@@ -92,8 +83,6 @@ func main() {
 		}
 	}
 
-	defer panicRecover()
-
 	provider.WriteEvent(
 		"ShimLaunched",
 		nil,