Repository: appc/spec (go)

Issue:
ace: add sysctl isolator
Applications running inside pods may need to tweak several namespaced sysctl parameters. There is a need for executor to support this (https://github.com/coreos/rkt/issues/2694), and OCI Linux runtime manifest already features a sysctl stanza (https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#sysctl).

In short, ACE should provide an `os/linux/sysctl` isolator.

Pull request:
spec: add os/unix/sysctl isolator
This PR introduces an "os/unix/sysctl" isolator, which applies at pod-level.

Closes #645

---

Arbitrary/controversial choices made here:
 * isolator type, as it applies inside pod-context (like other isolators) instead of the host (like top-level stanzas: volumes, ports)
 * ~~new "environment" isolator category, as it can also accommodate "ulimit" ones~~

Patch:
diff --git a/ace/validator.go b/ace/validator.go
index f92652c..fdd27ac 100644
--- a/ace/validator.go
+++ b/ace/validator.go
@@ -328,10 +328,14 @@ func validatePodMetadata(metadataURL string, pm *schema.PodManifest) results {
 func validateAppAnnotations(metadataURL string, pm *schema.PodManifest, app *schema.RuntimeApp, img *schema.ImageManifest) results {
 	r := results{}
 
-	// build a map of expected annotations by merging img.Annotations
+	// build a map of expected annotations by merging app.Annotations
 	// with PodManifest overrides
-	expectedAnnots := img.Annotations
-	for _, annot := range app.Annotations {
+	expectedAnnots := app.Annotations
+	a := pm.Apps.Get(app.Name)
+	if a == nil {
+		panic("could not find app in manifest!")
+	}
+	for _, annot := range a.Annotations {
 		expectedAnnots.Set(annot.Name, annot.Value)
 	}
 	if len(expectedAnnots) == 0 {
diff --git a/examples/pod_runtime.json b/examples/pod_runtime.json
index 1407610..658dba1 100644
--- a/examples/pod_runtime.json
+++ b/examples/pod_runtime.json
@@ -98,6 +98,12 @@
         {
             "name": "resource/memory",
             "value": {"limit": "4G"}
+        },
+        {
+            "name": "os/unix/sysctl",
+            "value": {
+                "net.ipv4.tcp_fin_timeout": "6"
+            }
         }
     ],
     "annotations": [
diff --git a/schema/types/isolator_unix.go b/schema/types/isolator_unix.go
new file mode 100644
index 0000000..ac0111a
--- /dev/null
+++ b/schema/types/isolator_unix.go
@@ -0,0 +1,83 @@
+// Copyright 2016 The appc Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License 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 types
+
+import (
+	"encoding/json"
+)
+
+var (
+	UnixIsolatorNames = make(map[ACIdentifier]struct{})
+)
+
+const (
+	//TODO(lucab): add "ulimit" isolators
+	UnixSysctlName = "os/unix/sysctl"
+)
+
+func init() {
+	for name, con := range map[ACIdentifier]IsolatorValueConstructor{
+		UnixSysctlName: func() IsolatorValue { return &UnixSysctl{} },
+	} {
+		AddIsolatorName(name, UnixIsolatorNames)
+		AddIsolatorValueConstructor(name, con)
+	}
+}
+
+type UnixSysctl map[string]string
+
+func (s *UnixSysctl) UnmarshalJSON(b []byte) error {
+	var v map[string]string
+	err := json.Unmarshal(b, &v)
+	if err != nil {
+		return err
+	}
+	*s = UnixSysctl(v)
+	return err
+}
+
+func (s UnixSysctl) AssertValid() error {
+	return nil
+}
+
+func (s UnixSysctl) multipleAllowed() bool {
+	return false
+}
+func (s UnixSysctl) Conflicts() []ACIdentifier {
+	return nil
+}
+
+func (s UnixSysctl) AsIsolator() Isolator {
+	isol := isolatorMap[UnixSysctlName]()
+
+	b, err := json.Marshal(s)
+	if err != nil {
+		panic(err)
+	}
+	valRaw := json.RawMessage(b)
+	return Isolator{
+		Name:     UnixSysctlName,
+		ValueRaw: &valRaw,
+		value:    isol,
+	}
+}
+
+func NewUnixSysctlIsolator(cfg map[string]string) (*UnixSysctl, error) {
+	s := UnixSysctl(cfg)
+	if err := s.AssertValid(); err != nil {
+		return nil, err
+	}
+	return &s, nil
+}
diff --git a/spec/ace.md b/spec/ace.md
index 87712a6..e05dab6 100644
--- a/spec/ace.md
+++ b/spec/ace.md
@@ -383,6 +383,36 @@ Small quantities can be represented directly as decimals (e.g., 0.3), or using m
 
 **NOTE**: Network limits MUST NOT apply to localhost communication between apps in a pod.
 
+### Unix Isolators
+
+These isolators take care of configuring several settings that are typically available in UNIX-like environments.
+This set includes isolators to tweak some standard POSIX features as well as other technologies that have been ported across multiple kernel families.
+
+#### os/unix/sysctl
+
+Sysctl parameters allow an application to configure kernel level options for its environment.
+A number of these values are namespace-aware as well, which makes them a natural fit for for an ACE to configure.
+
+* Scope: pod
+
+**Parameters:**
+
+* dictionary of `sysctl(8)` key-value entries to set kernel runtime parameters.
+
+```json
+"name": "os/unix/sysctl",
+"value": {
+    "net.ipv4.tcp_mtu_probing": "1",
+    "net.ipv4.tcp_keepalive_time": "300",
+    "net.ipv4.tcp_keepalive_probes": "5",
+    "net.ipv4.tcp_keepalive_intvl": "15"
+}
+```
+**Notes**:
+ 1. Only a single `os/unix/sysctl` isolator can be specified per-pod.
+ 2. Implementations SHOULD validate keys before applying sysctl parameters.
+    For example, a Linux implementation may want to only allow entries related to a specific namespaced subtree like `net.*`.
+    In such case, implementations MUST either ignore forbidden parameters or refuse to run the pod alltogether.
 
 ## App Container Metadata Service
 


Test patch:
diff --git a/schema/types/isolator_test.go b/schema/types/isolator_test.go
index e7be9e7..3a317e9 100644
--- a/schema/types/isolator_test.go
+++ b/schema/types/isolator_test.go
@@ -172,6 +172,27 @@ func TestIsolatorUnmarshal(t *testing.T) {
 			}`,
 			true,
 		},
+		{
+			`{
+				"name": "os/unix/sysctl",
+				"value": {"net.ipv4.tcp_rfc1337": "1"}
+			}`,
+			false,
+		},
+		{
+			`{
+				"name": "os/unix/sysctl",
+				"value": {"net.ipv4.tcp_rfc1337": 1}
+			}`,
+			true,
+		},
+		{
+			`{
+				"name": "os/unix/sysctl",
+				"value": {["net.ipv4.tcp_rfc1337"]}
+			}`,
+			true,
+		},
 	}
 
 	for i, tt := range tests {
@@ -205,6 +226,10 @@ func TestIsolatorsGetByName(t *testing.T) {
 			{
 				"name": "os/linux/no-new-privileges",
 				"value": true
+			},
+			{
+				"name": "os/unix/sysctl",
+				"value": {"net.ipv4.tcp_rfc1337": "1"}
 			}
 		]
 	`
@@ -215,12 +240,14 @@ func TestIsolatorsGetByName(t *testing.T) {
 		wrequest int64
 		wset     []LinuxCapability
 		wprivs   LinuxNoNewPrivileges
+		wsysctl  UnixSysctl
 	}{
-		{"resource/cpu", 1, 30, nil, false},
-		{"resource/memory", 2147483648, 1000000000, nil, false},
-		{"os/linux/capabilities-retain-set", 0, 0, []LinuxCapability{"CAP_KILL"}, false},
-		{"os/linux/capabilities-remove-set", 0, 0, []LinuxCapability{"CAP_KILL"}, false},
-		{"os/linux/no-new-privileges", 0, 0, nil, LinuxNoNewPrivileges(true)},
+		{"resource/cpu", 1, 30, nil, false, nil},
+		{"resource/memory", 2147483648, 1000000000, nil, false, nil},
+		{"os/linux/capabilities-retain-set", 0, 0, []LinuxCapability{"CAP_KILL"}, false, nil},
+		{"os/linux/capabilities-remove-set", 0, 0, []LinuxCapability{"CAP_KILL"}, false, nil},
+		{"os/linux/no-new-privileges", 0, 0, nil, LinuxNoNewPrivileges(true), nil},
+		{"os/unix/sysctl", 0, 0, nil, false, UnixSysctl{"net.ipv4.tcp_rfc1337": "1"}},
 	}
 
 	var is Isolators
@@ -269,8 +296,13 @@ func TestIsolatorsGetByName(t *testing.T) {
 				t.Errorf("#%d: got %v, want %v", i, v, tt.wprivs)
 			}
 
+		case *UnixSysctl:
+			if !reflect.DeepEqual(*v, tt.wsysctl) {
+				t.Errorf("#%d: got %v, want %v", i, *v, tt.wsysctl)
+			}
+
 		default:
-			panic("unexecpected type")
+			panic("unexpected type")
 		}
 	}
 }
diff --git a/schema/types/isolator_unix_test.go b/schema/types/isolator_unix_test.go
new file mode 100644
index 0000000..55e7e1e
--- /dev/null
+++ b/schema/types/isolator_unix_test.go
@@ -0,0 +1,57 @@
+// Copyright 2016 The appc Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License 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 types
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestUnixSysctlIsolator(t *testing.T) {
+	tests := []struct {
+		inCfg map[string]string
+
+		expectedErr bool
+		expectedRes UnixSysctl
+	}{
+		// empty isolator - valid
+		{
+			make(map[string]string),
+
+			false,
+			UnixSysctl{},
+		},
+		// simple isolator - valid
+		{
+			map[string]string{
+				"foo": "bar",
+			},
+
+			false,
+			UnixSysctl{
+				"foo": "bar",
+			},
+		},
+	}
+	for i, tt := range tests {
+		gotRes, err := NewUnixSysctlIsolator(tt.inCfg)
+		if gotErr := err != nil; gotErr != tt.expectedErr {
+			t.Errorf("#%d: want err=%t, got %t (err=%v)", i, tt.expectedErr, gotErr, err)
+		}
+		if !reflect.DeepEqual(tt.expectedRes, *gotRes) {
+			t.Errorf("#%d: want %s, got %s", i, tt.expectedRes, *gotRes)
+		}
+	}
+}