Repository: firecracker-microvm/firecracker-containerd
Pull request #140 (2019-03-20 01:24:54)

runtime: specify default kernel arguments
*Issue #, if available:*
https://github.com/firecracker-microvm/firecracker-containerd/issues/59

*Description of changes:*
Sets default kernel arguments so that it is not required to specify.

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/docs/getting-started.md b/docs/getting-started.md
index c27d3fb..1dc8185 100644
--- a/docs/getting-started.md
+++ b/docs/getting-started.md
@@ -214,7 +214,9 @@ configuration file has the following fields:
   changes every execution when run by containerd.
 * `kernel_image_path` (required) - A path where the kernel image file is
   located.  A fully-qualified path is recommended.
-* `kernel_args` (required) - Arguments for the kernel command line.
+* `kernel_args` (optional) - Arguments for the kernel command line.  If left
+  undefined, the runtime specifies "console=ttyS0 noapic reboot=k panic=1
+  pci=off nomodules rw".
 * `root_drive` (required) - A path where the root drive image file is located. A
   fully-qualified path is recommended.
 * `cpu_count` (required) - The number of vCPUs to make available to a microVM.
diff --git a/docs/quickstart.md b/docs/quickstart.md
index 6c08f55..ce5a404 100644
--- a/docs/quickstart.md
+++ b/docs/quickstart.md
@@ -139,7 +139,6 @@ sudo tee -a /etc/containerd/firecracker-runtime.json <<EOF
 {
   "firecracker_binary_path": "/usr/local/bin/firecracker",
   "kernel_image_path": "/var/lib/firecracker-containerd/runtime/hello-vmlinux.bin",
-  "kernel_args": "console=ttyS0 noapic reboot=k panic=1 pci=off nomodules rw",
   "root_drive": "/var/lib/firecracker-containerd/runtime/hello-rootfs.ext4",
   "cpu_count": 1,
   "cpu_template": "T2",
diff --git a/runtime/README.md b/runtime/README.md
index 4b32b5c..e3490e9 100644
--- a/runtime/README.md
+++ b/runtime/README.md
@@ -40,7 +40,9 @@ configuration file has the following fields:
   changes every execution when run by containerd.
 * `kernel_image_path` (required) - A path where the kernel image file is
   located.  A fully-qualified path is recommended.
-* `kernel_args` (required) - Arguments for the kernel command line.
+* `kernel_args` (optional) - Arguments for the kernel command line.  If left
+  undefined, the runtime specifies "console=ttyS0 noapic reboot=k panic=1
+  pci=off nomodules rw".
 * `root_drive` (required) - A path where the root drive image file is located. A
   fully-qualified path is recommended.
 * `cpu_count` (required) - The number of vCPUs to make available to a microVM.
diff --git a/runtime/config.go b/runtime/config.go
index 51d1840..6a9a999 100644
--- a/runtime/config.go
+++ b/runtime/config.go
@@ -25,6 +25,7 @@ const (
 	configPathEnvName = "FIRECRACKER_CONTAINERD_RUNTIME_CONFIG_PATH"
 	defaultConfigPath = "/etc/containerd/firecracker-runtime.json"
 	defaultSocketPath = "./firecracker.sock"
+	defaultKernelArgs = "console=ttyS0 noapic reboot=k panic=1 pci=off nomodules rw"
 )
 
 // Config represents runtime configuration parameters
@@ -58,10 +59,12 @@ func LoadConfig(path string) (*Config, error) {
 		return nil, errors.Wrapf(err, "failed to read config from %q", path)
 	}
 
-	var cfg Config
-	if err := json.Unmarshal(data, &cfg); err != nil {
+	cfg := &Config{
+		KernelArgs: defaultKernelArgs,
+	}
+	if err := json.Unmarshal(data, cfg); err != nil {
 		return nil, errors.Wrapf(err, "failed to unmarshal config from %q", path)
 	}
 
-	return &cfg, nil
+	return cfg, nil
 }


Test patch:
diff --git a/runtime/config_test.go b/runtime/config_test.go
new file mode 100644
index 0000000..35c4738
--- /dev/null
+++ b/runtime/config_test.go
@@ -0,0 +1,62 @@
+// 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 (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLoadConfigDefaults(t *testing.T) {
+	configContent := `{}`
+	configFile, cleanup := createTempConfig(t, configContent)
+	defer cleanup()
+	cfg, err := LoadConfig(configFile)
+	if err != nil {
+		t.Error(err, "failed to load config")
+	}
+
+	assert.Equal(t, cfg.KernelArgs, defaultKernelArgs, "expected default kernel args")
+}
+
+func TestLoadConfigOverrides(t *testing.T) {
+	overrideKernelArgs := "OVERRIDE KERNEL ARGS"
+	configContent := fmt.Sprintf(`{"kernel_args":"%s"}`, overrideKernelArgs)
+	configFile, cleanup := createTempConfig(t, configContent)
+	defer cleanup()
+	cfg, err := LoadConfig(configFile)
+	if err != nil {
+		t.Error(err, "failed to load config")
+	}
+
+	assert.Equal(t, cfg.KernelArgs, overrideKernelArgs, "expected overridden kernel args")
+}
+
+func createTempConfig(t *testing.T, contents string) (string, func()) {
+	t.Helper()
+	configFile, err := ioutil.TempFile("", "config")
+	if err != nil {
+		t.Fatal(err, "failed to create temp config file")
+	}
+	err = ioutil.WriteFile(configFile.Name(), []byte(contents), 0644)
+	if err != nil {
+		os.Remove(configFile.Name())
+		t.Fatal(err, "failed to write contents to temp config file")
+	}
+	return configFile.Name(), func() { os.Remove(configFile.Name()) }
+}