Merge pull request #3572 from grahamwhaley/20170130_kvm_min_ram

stage1/kvm: Change RAM calculation, and increase minimum

stage1/init/kvm/resources.go

@@ -21,14 +21,25 @@ import (
 	"github.com/appc/spec/schema/types"
 )
 
+// The algorithm/reasoning:
+// When running the VM takes a small amount of memory out of the container
+//  (systemMemOverhead below)
+// The VM also has a transient startup need - some extra 'space' really only
+//  needed to 'get going' before we get to the workload.
+//
+// So, we have two rules:
+// - we always add on the system overhead
+// - we bump up to the minimum required for boot if we need to
+//
+// From those, and the app memory asks, calculate how much memory to hand to
+//  the VM.
 const (
-	defaultMem        = 128 // MB
-	systemMemOverhead = 128 // MB
+	minMem            = 512 // MB - minimum we need in VM
+	systemMemOverhead = 128 // MB - overhead we need for VM
 )
 
 // findResources finds value of last isolator for particular type.
 func findResources(isolators types.Isolators) (mem, cpus int64) {
-	mem = defaultMem
 	for _, i := range isolators {
 		switch v := i.Value().(type) {
 		case *types.ResourceMemory:
@@ -66,5 +77,10 @@ func GetAppsResources(apps schema.AppList) (totalCpus, totalMem int64) {
 	// Add an overhead for the VM system
 	totalMem += systemMemOverhead
 
+	// Always ensure we have at least the minimum RAM needed
+	if totalMem < minMem {
+		totalMem = minMem
+	}
+
 	return totalCpus, totalMem
 }

stage1/init/kvm/resources_test.go

@@ -20,33 +20,85 @@ import (
 	"github.com/appc/spec/schema/types"
 )
 
+// Check that findResources is returning the answers we expect.
+// In the current implementation, that is exactly what we pass in to it for
+// the cpu value, and the memory value is divided by (1024 * 1024) - MB
 func TestFindResources(t *testing.T) {
 	tests := []struct {
 		in types.Isolators
 
 		wmem int64
 		wcpu int64
 	}{
+		//empty values should return us empty results
 		{
 			types.Isolators{},
 
-			defaultMem,
+			0,
 			0,
 		},
+		//check values in isolators actually come back
 		{
 			types.Isolators([]types.Isolator{
 				newIsolator(`
 				{
 					"name":     "resource/cpu",
 					"value": {
-						"limit": 100,
-						"request": 100
+						"limit": 0,
+						"request": 0
 						}
 				}`),
 			}),
 
-			defaultMem,
-			100,
+			0,
+			0,
+		},
+		{
+			types.Isolators([]types.Isolator{
+				newIsolator(`
+				{
+					"name":     "resource/cpu",
+					"value": {
+						"limit": 1,
+						"request": 1
+						}
+				}`),
+			}),
+
+			0,
+			1,
+		},
+		{
+			// 1M test
+			types.Isolators([]types.Isolator{
+				newIsolator(`
+				{
+					"name":     "resource/memory",
+					"value": {
+						"limit": 1048576,
+						"request": 1048576
+						}
+				}`),
+			}),
+
+			1,
+			0,
+		},
+		{
+			// 128M test
+			types.Isolators([]types.Isolator{
+				newIsolator(`
+				{
+					"name":     "resource/memory",
+					"value": {
+						"limit": 134217728,
+						"request": 134217728
+						}
+				}`),
+			}),
+
+			128,
+			0,
 		},
 	}