Merge pull request #47700 from JulienBalestra/rkt-systemd-unit-limitnofile

Automatic merge from submit-queue (batch tested with PRs 47700, 48464, 48502)

Provide a way to setup the limit NO files for rkt Pods

**What this PR does / why we need it**:

This PR allows to customize the Systemd unit files for rkt pods.
We start with the `systemd-unit-option.rkt.kubernetes.io/LimitNOFILE` to allows to run workloads like etcd, ES in kubernetes with rkt.

**Special notes for your reviewer**:

Once again, I followed @yifan-gu guidelines.
I made a basic check over the values given inside the `systemd-unit-option.rkt.kubernetes.io/LimitNOFILE` (integer and > 0).
If this check fails: I simply ignore the field.
The other implementation would be to fail the whole SetUpPod.

We discussed using a key like `rkt.kubernetes.io/systemd-unit-option/LimitNOFILE` but the validation only allows a single `/` in this field:
```The Deployment "tiller" is invalid: spec.template.annotations: Invalid value: "rkt.kubernetes.io/systemd-unit-option/LimitNOFILE": a qualified name must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]') with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName')```

**Release note**:

```release-note 
NONE
```

pkg/kubelet/rkt/BUILD

@@ -82,6 +82,7 @@ go_test(
         "//vendor/github.com/appc/spec/schema:go_default_library",
         "//vendor/github.com/appc/spec/schema/types:go_default_library",
         "//vendor/github.com/coreos/go-systemd/dbus:go_default_library",
+        "//vendor/github.com/coreos/go-systemd/unit:go_default_library",
         "//vendor/github.com/coreos/rkt/api/v1alpha:go_default_library",
         "//vendor/github.com/golang/mock/gomock:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",

pkg/kubelet/rkt/rkt.go

@@ -95,6 +95,8 @@ const (
 	k8sRktRestartCountAnno           = "rkt.kubernetes.io/restart-count"
 	k8sRktTerminationMessagePathAnno = "rkt.kubernetes.io/termination-message-path"
 
+	k8sRktLimitNoFileAnno = "systemd-unit-option.rkt.kubernetes.io/LimitNOFILE"
+
 	// TODO(euank): This has significant security concerns as a stage1 image is
 	// effectively root.
 	// Furthermore, this (using an annotation) is a hack to pass an extra
@@ -1148,6 +1150,23 @@ func constructSyslogIdentifier(generateName string, podName string) string {
 	return podName
 }
 
+// Setup additional systemd field specified in the Pod Annotation
+func setupSystemdCustomFields(annotations map[string]string, unitOptionArray []*unit.UnitOption) ([]*unit.UnitOption, error) {
+	// LimitNOFILE
+	if strSize := annotations[k8sRktLimitNoFileAnno]; strSize != "" {
+		size, err := strconv.Atoi(strSize)
+		if err != nil {
+			return unitOptionArray, err
+		}
+		if size < 1 {
+			return unitOptionArray, fmt.Errorf("invalid value for %s: %s", k8sRktLimitNoFileAnno, strSize)
+		}
+		unitOptionArray = append(unitOptionArray, newUnitOption("Service", "LimitNOFILE", strSize))
+	}
+
+	return unitOptionArray, nil
+}
+
 // preparePod will:
 //
 // 1. Invoke 'rkt prepare' to prepare the pod, and get the rkt pod uuid.
@@ -1235,6 +1254,11 @@ func (r *Runtime) preparePod(pod *v1.Pod, podIP string, pullSecrets []v1.Secret,
 		units = append(units, newUnitOption("Service", "SELinuxContext", selinuxContext))
 	}
 
+	units, err = setupSystemdCustomFields(pod.Annotations, units)
+	if err != nil {
+		glog.Warningf("fail to add custom systemd fields provided by pod Annotations: %q", err)
+	}
+
 	serviceName := makePodServiceFileName(uuid)
 	glog.V(4).Infof("rkt: Creating service file %q for pod %q", serviceName, format.Pod(pod))
 	serviceFile, err := r.os.Create(serviceFilePath(serviceName))

pkg/kubelet/rkt/rkt_test.go

@@ -28,6 +28,7 @@ import (
 
 	appcschema "github.com/appc/spec/schema"
 	appctypes "github.com/appc/spec/schema/types"
+	"github.com/coreos/go-systemd/unit"
 	rktapi "github.com/coreos/rkt/api/v1alpha"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
@@ -2075,3 +2076,53 @@ func TestGetPodSystemdServiceFiles(t *testing.T) {
 		}
 	}
 }
+
+func TestSetupSystemdCustomFields(t *testing.T) {
+	testCases := []struct {
+		unitOpts       []*unit.UnitOption
+		podAnnotations map[string]string
+		expectedValues []string
+		raiseErr       bool
+	}{
+		// without annotation
+		{
+			[]*unit.UnitOption{
+				{Section: "Service", Name: "ExecStart", Value: "/bin/true"},
+			},
+			map[string]string{},
+			[]string{"/bin/true"},
+			false,
+		},
+		// with valid annotation for LimitNOFile
+		{
+			[]*unit.UnitOption{
+				{Section: "Service", Name: "ExecStart", Value: "/bin/true"},
+			},
+			map[string]string{k8sRktLimitNoFileAnno: "1024"},
+			[]string{"/bin/true", "1024"},
+			false,
+		},
+		// with invalid annotation for LimitNOFile
+		{
+			[]*unit.UnitOption{
+				{Section: "Service", Name: "ExecStart", Value: "/bin/true"},
+			},
+			map[string]string{k8sRktLimitNoFileAnno: "-1"},
+			[]string{"/bin/true"},
+			true,
+		},
+	}
+
+	for i, tt := range testCases {
+		raiseErr := false
+		newUnitsOpts, err := setupSystemdCustomFields(tt.podAnnotations, tt.unitOpts)
+		if err != nil {
+			raiseErr = true
+		}
+		assert.Equal(t, tt.raiseErr, raiseErr, fmt.Sprintf("Test case #%d", i))
+		for _, opt := range newUnitsOpts {
+			assert.Equal(t, "Service", opt.Section, fmt.Sprintf("Test case #%d", i))
+			assert.Contains(t, tt.expectedValues, opt.Value, fmt.Sprintf("Test case #%d", i))
+		}
+	}
+}