Repository: firecracker-microvm/firecracker-containerd
Pull request #434 (2020-06-08 17:33:26)

Fix DebugHelper
*Issue #, if available:*

NA

*Description of changes:*

There are 2 commits:

* Initialize DebugHelper correctly. It was not reading LogLevels correctly.
* Change Makefile to test config/ directory/package correctly. "go test" is smart enough to find all tests. It would be better to let it find tests, rather than explicitly having Makefile.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/Makefile b/Makefile
index fd315a4..80f467c 100644
--- a/Makefile
+++ b/Makefile
@@ -11,7 +11,7 @@
 # express or implied. See the License for the specific language governing
 # permissions and limitations under the License.
 
-SUBDIRS:=agent runtime internal examples firecracker-control/cmd/containerd eventbridge
+SUBDIRS:=agent runtime examples firecracker-control/cmd/containerd
 TEST_SUBDIRS:=$(addprefix test-,$(SUBDIRS))
 INTEG_TEST_SUBDIRS:=$(addprefix integ-test-,$(SUBDIRS))
 
@@ -121,6 +121,7 @@ image: $(RUNC_BIN) agent-in-docker
 	$(MAKE) -C tools/image-builder all-in-docker
 
 test: $(TEST_SUBDIRS)
+	go test ./... $(EXTRAGOARGS)
 
 # test-in-docker runs all unit tests inside a docker container. Use "integ-test" to
 # run the integ-tests inside containers.
diff --git a/config/config.go b/config/config.go
index 49ef485..ef45032 100644
--- a/config/config.go
+++ b/config/config.go
@@ -94,14 +94,14 @@ func LoadConfig(path string) (*Config, error) {
 		},
 	}
 
+	if err := json.Unmarshal(data, cfg); err != nil {
+		return nil, errors.Wrapf(err, "failed to unmarshal config from %q", path)
+	}
+
 	cfg.DebugHelper, err = debug.New(cfg.LogLevels...)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := json.Unmarshal(data, cfg); err != nil {
-		return nil, errors.Wrapf(err, "failed to unmarshal config from %q", path)
-	}
-
 	return cfg, nil
 }
diff --git a/eventbridge/Makefile b/eventbridge/Makefile
deleted file mode 100644
index 35baaf2..0000000
--- a/eventbridge/Makefile
+++ /dev/null
@@ -1,35 +0,0 @@
-# Copyright 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.
-
-EXTRAGOARGS?=
-
-GOMOD := $(shell go env GOMOD)
-GOSUM := $(GOMOD:.mod=.sum)
-SOURCES:=$(shell find . -name '*.go')
-DOCKER_IMAGE_TAG?=latest
-
-all:
-
-test:
-	go test ./... $(EXTRAGOARGS)
-
-# integ tests involving the eventbridge are currently only in runtime package
-integ-test:
-
-clean:
-
-distclean: clean
-
-install:
-
-.PHONY: clean distclean all install test integ-test
diff --git a/internal/Makefile b/internal/Makefile
deleted file mode 100644
index 519dfba..0000000
--- a/internal/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-# Copyright 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.
-
-EXTRAGOARGS?=
-
-all:
-
-test:
-	go test ./... $(EXTRAGOARGS)
-
-# integ tests involving the all internal packages are currently only in runtime package
-integ-test:
-
-clean:
-
-distclean: clean
-
-install:
-
-.PHONY: clean distclean all install test integ-test


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index d936c90..be3634b 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -47,7 +47,8 @@ func TestLoadConfigOverrides(t *testing.T) {
 			"kernel_image_path":"%s",
 			"root_drive":"%s",
 			"cpu_count": %d,
-            "cpu_template": "%s"
+            "cpu_template": "%s",
+			"log_levels": ["debug"]
 		}`, overrideKernelArgs, overrideKernelPath, overrideRootfsPath, overrideCPUCount, overrideCPUTemplate)
 	configFile, cleanup := createTempConfig(t, configContent)
 	defer cleanup()
@@ -58,6 +59,8 @@ func TestLoadConfigOverrides(t *testing.T) {
 	assert.Equal(t, overrideKernelPath, cfg.KernelImagePath, "expected overridden kernel path")
 	assert.Equal(t, overrideRootfsPath, cfg.RootDrive, "expected overridden rootfs path")
 	assert.Equal(t, overrideCPUTemplate, cfg.CPUTemplate, "expected overridden CPU template")
+
+	assert.True(t, cfg.DebugHelper.LogFirecrackerOutput())
 }
 
 func createTempConfig(t *testing.T, contents string) (string, func()) {