Repository: firecracker-microvm/firecracker-containerd
Pull request #148 (2019-03-25 19:40:38)

agent: fixes panic on cancelAll function
`cancelAll` would panic due to using `len(slice)` as an index.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sipsma: Change itself looks good but we should update the makefiles and/or .`buildkite/pipeline.yml` to run the new unit test (it isn't now because this is the first unit test in the agent package).

Patch:
diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml
index d0306ef..fd55e5c 100644
--- a/.buildkite/pipeline.yml
+++ b/.buildkite/pipeline.yml
@@ -87,3 +87,24 @@ steps:
             cd /firecracker-containerd/runtime
             make test EXTRAGOARGS="-v -count=1"
           '
+
+  - label: ":fencer: agent tests"
+    agents:
+      queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
+    artifact_paths:
+      - "logs/*"
+    command:
+      >
+        mkdir $(pwd)/logs
+        && docker run --rm \
+          --privileged \
+          --volume /dev:/dev \
+          --volume /sys:/sys \
+          --volume /run/udev/control:/run/udev/control \
+          --volume $(pwd)/logs:/var/log/firecracker-containerd-test \
+          --ipc=host \
+          localhost/firecracker-containerd-unittest:${BUILDKITE_BUILD_NUMBER} \
+          '
+            cd /firecracker-containerd/agent
+            make test EXTRAGOARGS="-v -count=1"
+          '
diff --git a/agent/Makefile b/agent/Makefile
index e4eceac..b7e4c80 100644
--- a/agent/Makefile
+++ b/agent/Makefile
@@ -10,9 +10,11 @@
 # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
 # express or implied. See the License for the specific language governing
 # permissions and limitations under the License.
+EXTRAGOARGS:=
 
 GOMOD := $(shell go env GOMOD)
 GOSUM := $(GOMOD:.mod=.sum)
+SOURCES:=$(shell find . -name '*.go')
 
 all: agent
 
@@ -23,6 +25,9 @@ else
 	go build -o agent
 endif
 
+test: $(SOURCES) $(GOMOD) $(GOSUM)
+	go test ./... $(EXTRAGOARGS)
+
 clean:
 	- rm -f agent
 
diff --git a/agent/service.go b/agent/service.go
index 3698de2..1426540 100644
--- a/agent/service.go
+++ b/agent/service.go
@@ -427,7 +427,7 @@ func (ts *TaskService) Shutdown(ctx context.Context, req *shimapi.ShutdownReques
 
 func (ts *TaskService) cancelAll() {
 	// cancel LIFO order
-	for i := len(ts.cancels); i >= 0; i-- {
+	for i := len(ts.cancels) - 1; i >= 0; i-- {
 		log.G(context.Background()).Debug("Cancelling ", i)
 		ts.cancels[i]()
 	}


Test patch:
diff --git a/agent/service_test.go b/agent/service_test.go
new file mode 100644
index 0000000..3583798
--- /dev/null
+++ b/agent/service_test.go
@@ -0,0 +1,30 @@
+// Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package main
+
+import (
+	"context"
+	"testing"
+)
+
+func TestCancelAll(t *testing.T) {
+	ts := &TaskService{
+		cancels: []context.CancelFunc{
+			func() {},
+			func() {},
+		},
+	}
+
+	ts.cancelAll()
+}