Remove SYS_RESOURCE capability from launcher pod

cmd/chroot/main.go

@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
-	"unsafe"
 
 	"github.com/spf13/cobra"
 	"golang.org/x/sys/unix"
@@ -61,10 +60,9 @@ func main() {
 					Cur: uint64(cpuTime),
 					Max: uint64(cpuTime),
 				}
-				// PID 0 means that we assign the limit to us, all commands will inherit it
-				_, _, e1 := syscall.RawSyscall6(syscall.SYS_PRLIMIT64, uintptr(0), uintptr(unix.RLIMIT_CPU), uintptr(unsafe.Pointer(value)), 0, 0, 0)
-				if e1 != 0 {
-					return fmt.Errorf("error setting prlimit on cpu time with value %d: %v", value, e1)
+				err := syscall.Setrlimit(unix.RLIMIT_CPU, value)
+				if err != nil {
+					return fmt.Errorf("error setting prlimit on cpu time with value %d: %v", value, err)
 				}
 			}
 
@@ -73,10 +71,9 @@ func main() {
 					Cur: uint64(megabyte) * 1000000,
 					Max: uint64(megabyte) * 1000000,
 				}
-				// PID 0 means that we assign the limit to us, all commands will inherit it
-				_, _, e1 := syscall.RawSyscall6(syscall.SYS_PRLIMIT64, uintptr(0), uintptr(unix.RLIMIT_AS), uintptr(unsafe.Pointer(value)), 0, 0, 0)
-				if e1 != 0 {
-					return fmt.Errorf("error setting prlimit on virtual memory with value %d: %v", value, e1)
+				err := syscall.Setrlimit(unix.RLIMIT_AS, value)
+				if err != nil {
+					return fmt.Errorf("error setting prlimit on virtual memory with value %d: %v", value, err)
 				}
 			}
 

go.mod

@@ -39,6 +39,7 @@ require (
 	github.com/libvirt/libvirt-go v5.0.0+incompatible
 	github.com/mattn/go-colorable v0.1.1 // indirect
 	github.com/mattn/go-runewidth v0.0.0-20181218000649-703b5e6b11ae // indirect
+	github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b
 	github.com/onsi/ginkgo v1.8.0
 	github.com/onsi/gomega v1.5.1-0.20190515112211-6a48b4839f85
 	github.com/openshift/api v3.9.1-0.20190401220125-3a6077f1f910+incompatible

go.sum

@@ -194,6 +194,8 @@ github.com/mattn/go-sqlite3 v1.10.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsO
 github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/maxbrunsfeld/counterfeiter v0.0.0-20181017030959-1aadac120687/go.mod h1:aoVsckWnsNzazwF2kmD+bzgdr4GBlbK91zsdivQJ2eU=
+github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b h1:9+ke9YJ9KGWw5ANXK6ozjoK47uI3uNbXv4YVINBnGm8=
+github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b/go.mod h1:r1VsdOzOPt1ZSrGZWFoNhsAedKnEd6r9Np1+5blZCWk=
 github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=

pkg/util/BUILD.bazel

@@ -8,4 +8,5 @@ go_library(
     ],
     importpath = "kubevirt.io/kubevirt/pkg/util",
     visibility = ["//visibility:public"],
+    deps = ["//staging/src/kubevirt.io/client-go/api/v1:go_default_library"],
 )

pkg/util/util.go

@@ -1,6 +1,19 @@
 package util
 
+import (
+	v1 "kubevirt.io/client-go/api/v1"
+)
+
 const ExtensionAPIServerAuthenticationConfigMap = "extension-apiserver-authentication"
 const RequestHeaderClientCAFileKey = "requestheader-client-ca-file"
 const VirtShareDir = "/var/run/kubevirt"
 const VirtLibDir = "/var/lib/kubevirt"
+
+func IsSRIOVVmi(vmi *v1.VirtualMachineInstance) bool {
+	for _, iface := range vmi.Spec.Domain.Devices.Interfaces {
+		if iface.SRIOV != nil {
+			return true
+		}
+	}
+	return false
+}

pkg/virt-controller/services/BUILD.bazel

@@ -10,6 +10,7 @@ go_library(
         "//pkg/container-disk:go_default_library",
         "//pkg/hooks:go_default_library",
         "//pkg/host-disk:go_default_library",
+        "//pkg/util:go_default_library",
         "//pkg/util/hardware:go_default_library",
         "//pkg/util/net/dns:go_default_library",
         "//pkg/util/types:go_default_library",

pkg/virt-controller/services/template.go

@@ -42,6 +42,7 @@ import (
 	"kubevirt.io/kubevirt/pkg/config"
 	containerdisk "kubevirt.io/kubevirt/pkg/container-disk"
 	"kubevirt.io/kubevirt/pkg/hooks"
+	"kubevirt.io/kubevirt/pkg/util"
 	"kubevirt.io/kubevirt/pkg/util/hardware"
 	"kubevirt.io/kubevirt/pkg/util/net/dns"
 	"kubevirt.io/kubevirt/pkg/util/types"
@@ -60,7 +61,6 @@ const GenieNetworksAnnotation = "cni"
 
 const CAP_NET_ADMIN = "NET_ADMIN"
 const CAP_SYS_NICE = "SYS_NICE"
-const CAP_SYS_RESOURCE = "SYS_RESOURCE"
 
 // LibvirtStartupDelay is added to custom liveness and readiness probes initial delay value.
 // Libvirt needs roughly 10 seconds to start.
@@ -98,15 +98,6 @@ type templateService struct {
 
 type PvcNotFoundError error
 
-func isSRIOVVmi(vmi *v1.VirtualMachineInstance) bool {
-	for _, iface := range vmi.Spec.Domain.Devices.Interfaces {
-		if iface.SRIOV != nil {
-			return true
-		}
-	}
-	return false
-}
-
 func isFeatureStateEnabled(fs *v1.FeatureState) bool {
 	return fs != nil && fs.Enabled != nil && *fs.Enabled
 }
@@ -355,7 +346,7 @@ func (t *templateService) RenderLaunchManifest(vmi *v1.VirtualMachineInstance) (
 		MountPath: "/var/run/libvirt",
 	})
 
-	if isSRIOVVmi(vmi) {
+	if util.IsSRIOVVmi(vmi) {
 		// libvirt needs this volume to access PCI device config;
 		// note that the volume should not be read-only because libvirt
 		// opens the config for writing
@@ -945,12 +936,6 @@ func getRequiredCapabilities(vmi *v1.VirtualMachineInstance) []k8sv1.Capability
 	}
 	// add a CAP_SYS_NICE capability to allow setting cpu affinity
 	res = append(res, CAP_SYS_NICE)
-
-	if isSRIOVVmi(vmi) {
-		// this capability is needed for libvirt to be able to change ulimits for device passthrough:
-		// "error : cannot limit locked memory to 2098200576: Operation not permitted"
-		res = append(res, CAP_SYS_RESOURCE)
-	}
 	return res
 }
 

pkg/virt-handler/isolation/BUILD.bazel

@@ -11,10 +11,14 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/container-disk:go_default_library",
+        "//pkg/util:go_default_library",
         "//pkg/virt-handler/cmd-client:go_default_library",
         "//staging/src/kubevirt.io/client-go/api/v1:go_default_library",
         "//staging/src/kubevirt.io/client-go/log:go_default_library",
         "//vendor/github.com/golang/mock/gomock:go_default_library",
+        "//vendor/github.com/mitchellh/go-ps:go_default_library",
+        "//vendor/golang.org/x/sys/unix:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
     ],
 )
 

pkg/virt-handler/isolation/generated_mock_isolation.go

@@ -61,3 +61,13 @@ func (_m *MockPodIsolationDetector) Whitelist(controller []string) PodIsolationD
 func (_mr *_MockPodIsolationDetectorRecorder) Whitelist(arg0 interface{}) *gomock.Call {
 	return _mr.mock.ctrl.RecordCall(_mr.mock, "Whitelist", arg0)
 }
+
+func (_m *MockPodIsolationDetector) AdjustResources(vm *v1.VirtualMachineInstance) error {
+	ret := _m.ctrl.Call(_m, "AdjustResources", vm)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+func (_mr *_MockPodIsolationDetectorRecorder) AdjustResources(arg0 interface{}) *gomock.Call {
+	return _mr.mock.ctrl.RecordCall(_mr.mock, "AdjustResources", arg0)
+}

pkg/virt-handler/isolation/isolation.go

@@ -35,9 +35,16 @@ import (
 	"path/filepath"
 	"strings"
 	"syscall"
+	"unsafe"
+
+	"golang.org/x/sys/unix"
+
+	ps "github.com/mitchellh/go-ps"
+	"k8s.io/apimachinery/pkg/api/resource"
 
 	v1 "kubevirt.io/client-go/api/v1"
 	"kubevirt.io/client-go/log"
+	"kubevirt.io/kubevirt/pkg/util"
 	cmdclient "kubevirt.io/kubevirt/pkg/virt-handler/cmd-client"
 )
 
@@ -53,6 +60,9 @@ type PodIsolationDetector interface {
 	// Whitelist allows specifying cgroup controller which should be considered to detect the cgroup slice
 	// It returns a PodIsolationDetector to allow configuring the PodIsolationDetector via the builder pattern.
 	Whitelist(controller []string) PodIsolationDetector
+
+	// Adjust system resources to run the passed VM
+	AdjustResources(vm *v1.VirtualMachineInstance) error
 }
 
 type MountInfo struct {
@@ -120,6 +130,91 @@ func (s *socketBasedIsolationDetector) Detect(vm *v1.VirtualMachineInstance) (*I
 	return NewIsolationResult(pid, slice, controller), nil
 }
 
+// standard golang libraries don't provide API to set runtime limits
+// for other processes, so we have to directly call to kernel
+func prLimit(pid int, limit uintptr, rlimit *unix.Rlimit) error {
+	_, _, errno := unix.RawSyscall6(unix.SYS_PRLIMIT64,
+		uintptr(pid),
+		limit,
+		uintptr(unsafe.Pointer(rlimit)),
+		0, 0, 0)
+	if errno != 0 {
+		return fmt.Errorf("Error setting prlimit: %v", errno)
+	}
+	return nil
+}
+
+func (s *socketBasedIsolationDetector) AdjustResources(vm *v1.VirtualMachineInstance) error {
+	// only VFIO attached domains require MEMLOCK adjustment
+	if !util.IsSRIOVVmi(vm) {
+		return nil
+	}
+
+	// bump memlock ulimit for libvirtd
+	res, err := s.Detect(vm)
+	if err != nil {
+		return err
+	}
+	launcherPid := res.Pid()
+
+	processes, err := ps.Processes()
+	if err != nil {
+		return fmt.Errorf("failed to get all processes: %v", err)
+	}
+
+	for _, process := range processes {
+		// consider all processes that are virt-launcher children
+		if process.PPid() != launcherPid {
+			continue
+		}
+
+		// libvirtd process sets the memory lock limit before fork/exec-ing into qemu
+		if process.Executable() != "libvirtd" {
+			continue
+		}
+
+		// make the best estimate for memory required by libvirt
+		memlockSize, err := getMemlockSize(vm)
+		if err != nil {
+			return err
+		}
+		rLimit := unix.Rlimit{
+			Max: uint64(memlockSize),
+			Cur: uint64(memlockSize),
+		}
+		err = prLimit(process.Pid(), unix.RLIMIT_MEMLOCK, &rLimit)
+		if err != nil {
+			return fmt.Errorf("failed to set rlimit for memory lock: %v", err)
+		}
+		// we assume a single process should match
+		break
+	}
+	return nil
+}
+
+// consider reusing getMemoryOverhead()
+// This is not scientific, but neither what libvirtd does is. See details in:
+// https://www.redhat.com/archives/libvirt-users/2019-August/msg00051.html
+func getMemlockSize(vm *v1.VirtualMachineInstance) (int64, error) {
+	memlockSize := resource.NewQuantity(0, resource.DecimalSI)
+
+	// start with base memory requested for the VM
+	vmiMemoryReq := vm.Spec.Domain.Resources.Requests.Memory()
+	memlockSize.Add(*resource.NewScaledQuantity(vmiMemoryReq.ScaledValue(resource.Kilo), resource.Kilo))
+
+	// allocate 1Gb for VFIO needs
+	memlockSize.Add(resource.MustParse("1G"))
+
+	// add some more memory for NUMA / CPU topology, platform memory alignment and other needs
+	memlockSize.Add(resource.MustParse("256M"))
+
+	bytes_, ok := memlockSize.AsInt64()
+	if !ok {
+		return 0, fmt.Errorf("could not calculate memory lock size")
+	}
+	return bytes_, nil
+}
+
 func NewIsolationResult(pid int, slice string, controller []string) *IsolationResult {
 	return &IsolationResult{pid: pid, slice: slice, controller: controller}
 }

pkg/virt-handler/isolation/isolation_test.go

@@ -73,3 +73,14 @@ var _ = Describe("Isolation", func() {
 		})
 	})
 })
+
+var _ = Describe("getMemlockSize", func() {
+	vm := v1.NewMinimalVMIWithNS("default", "testvm")
+
+	It("Should return correct number of bytes for memlock limit", func() {
+		bytes_, err := getMemlockSize(vm)
+		Expect(err).ToNot(HaveOccurred())
+		// 1Gb (static part for vfio VMs) + 256Mb (estimated overhead) + 8 Mb (VM)
+		Expect(int(bytes_)).To(Equal(1264389000))
+	})
+})

pkg/virt-handler/vm.go

@@ -1414,6 +1414,11 @@ func (d *VirtualMachineController) processVmUpdate(origVMI *v1.VirtualMachineIns
 			}
 		}
 
+		err = d.podIsolationDetector.AdjustResources(vmi)
+		if err != nil {
+			return fmt.Errorf("failed to adjust resources: %v", err)
+		}
+
 		options := &cmdv1.VirtualMachineOptions{
 			VirtualMachineSMBios: &cmdv1.SMBios{
 				Family:       d.clusterConfig.GetSMBIOS().Family,

pkg/virt-handler/vm_test.go

@@ -73,6 +73,7 @@ var _ = Describe("VirtualMachineInstance", func() {
 	var mockQueue *testutils.MockWorkQueue
 	var mockWatchdog *MockWatchdog
 	var mockGracefulShutdown *MockGracefulShutdown
+	var mockIsolationDetector *isolation.MockPodIsolationDetector
 
 	var vmiFeeder *testutils.VirtualMachineFeeder
 	var domainFeeder *testutils.DomainFeeder
@@ -127,6 +128,10 @@ var _ = Describe("VirtualMachineInstance", func() {
 		mockGracefulShutdown = &MockGracefulShutdown{shareDir}
 		config, _, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 
+		mockIsolationDetector = isolation.NewMockPodIsolationDetector(ctrl)
+		mockIsolationDetector.EXPECT().Detect(gomock.Any()).Return(&isolation.IsolationResult{}, nil).AnyTimes()
+		mockIsolationDetector.EXPECT().AdjustResources(gomock.Any()).Return(nil).AnyTimes()
+
 		controller = NewController(recorder,
 			virtClient,
 			host,
@@ -140,7 +145,7 @@ var _ = Describe("VirtualMachineInstance", func() {
 			10,
 			config,
 			tlsConfig,
-			isolation.NewSocketBasedIsolationDetector(shareDir),
+			mockIsolationDetector,
 		)
 
 		testUUID = uuid.NewUUID()

pkg/virt-operator/creation/components/scc.go

@@ -74,7 +74,7 @@ func NewKubeVirtControllerSCC(namespace string) *secv1.SecurityContextConstraint
 	scc.SELinuxContext = secv1.SELinuxContextStrategyOptions{
 		Type: secv1.SELinuxStrategyRunAsAny,
 	}
-	scc.AllowedCapabilities = []corev1.Capability{"NET_ADMIN", "SYS_NICE", "SYS_RESOURCE"}
+	scc.AllowedCapabilities = []corev1.Capability{"NET_ADMIN", "SYS_NICE"}
 	scc.AllowHostDirVolumePlugin = true
 	scc.Users = []string{fmt.Sprintf("system:serviceaccount:%s:kubevirt-controller", namespace)}