Merge pull request #25902 from euank/changeVolumeMounts

Automatic merge from submit-queue

rkt: Use volumes from RunContainerOptions

This replaces the previous creation of mounts from the `volumeGetter`
with mounts provided via RunContainerOptions.

This is motivated by the fact that the latter has a more complete set of
mounts (e.g. the `/etc/hosts` one created in kubelet.go in the case an IP is available).

This does not induce further e2e failures as far as I can tell.

cc @yifan-gu

pkg/kubelet/kubelet.go

@@ -445,7 +445,6 @@ func NewMainKubelet(
 			containerRefManager,
 			klet.podManager,
 			klet.livenessManager,
-			klet.volumeManager,
 			klet.httpClient,
 			klet.networkPlugin,
 			klet.hairpinMode == componentconfig.HairpinVeth,

pkg/kubelet/rkt/mock_rkt/mock_volume_getter.go

@@ -1,59 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors 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.
-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.
-*/
-
-// Generated via `mockgen k8s.io/kubernetes/pkg/kubelet/rkt VolumeGetter > mock_rkt/mock_volume_getter.go`
-// Edited to include required boilerplate
-// Source: k8s.io/kubernetes/pkg/kubelet/rkt (interfaces: VolumeGetter)
-
-package mock_rkt
-
-import (
-	gomock "github.com/golang/mock/gomock"
-	container "k8s.io/kubernetes/pkg/kubelet/container"
-	types "k8s.io/kubernetes/pkg/types"
-)
-
-// Mock of VolumeGetter interface
-type MockVolumeGetter struct {
-	ctrl     *gomock.Controller
-	recorder *_MockVolumeGetterRecorder
-}
-
-// Recorder for MockVolumeGetter (not exported)
-type _MockVolumeGetterRecorder struct {
-	mock *MockVolumeGetter
-}
-
-func NewMockVolumeGetter(ctrl *gomock.Controller) *MockVolumeGetter {
-	mock := &MockVolumeGetter{ctrl: ctrl}
-	mock.recorder = &_MockVolumeGetterRecorder{mock}
-	return mock
-}
-
-func (_m *MockVolumeGetter) EXPECT() *_MockVolumeGetterRecorder {
-	return _m.recorder
-}
-
-func (_m *MockVolumeGetter) GetVolumes(_param0 types.UID) (container.VolumeMap, bool) {
-	ret := _m.ctrl.Call(_m, "GetVolumes", _param0)
-	ret0, _ := ret[0].(container.VolumeMap)
-	ret1, _ := ret[1].(bool)
-	return ret0, ret1
-}
-
-func (_mr *_MockVolumeGetterRecorder) GetVolumes(arg0 interface{}) *gomock.Call {
-	return _mr.mock.ctrl.RecordCall(_mr.mock, "GetVolumes", arg0)
-}

pkg/kubelet/rkt/rkt.go

@@ -139,7 +139,6 @@ type Runtime struct {
 	runtimeHelper       kubecontainer.RuntimeHelper
 	recorder            record.EventRecorder
 	livenessManager     proberesults.Manager
-	volumeGetter        VolumeGetter
 	imagePuller         kubecontainer.ImagePuller
 	runner              kubecontainer.HandlerRunner
 	execer              utilexec.Interface
@@ -162,11 +161,6 @@ type Runtime struct {
 
 var _ kubecontainer.Runtime = &Runtime{}
 
-// TODO(yifan): Remove this when volumeManager is moved to separate package.
-type VolumeGetter interface {
-	GetVolumes(podUID kubetypes.UID) (kubecontainer.VolumeMap, bool)
-}
-
 // TODO(yifan): This duplicates the podGetter in dockertools.
 type podGetter interface {
 	GetPodByUID(kubetypes.UID) (*api.Pod, bool)
@@ -190,7 +184,6 @@ func New(
 	containerRefManager *kubecontainer.RefManager,
 	podGetter podGetter,
 	livenessManager proberesults.Manager,
-	volumeGetter VolumeGetter,
 	httpClient types.HttpGetter,
 	networkPlugin network.NetworkPlugin,
 	hairpinMode bool,
@@ -243,7 +236,6 @@ func New(
 		runtimeHelper:       runtimeHelper,
 		recorder:            recorder,
 		livenessManager:     livenessManager,
-		volumeGetter:        volumeGetter,
 		networkPlugin:       networkPlugin,
 		execer:              execer,
 		touchPath:           touchPath,
@@ -621,20 +613,6 @@ func (r *Runtime) makePodManifest(pod *api.Pod, pullSecrets []api.Secret) (*appc
 		}
 	}
 
-	volumeMap, ok := r.volumeGetter.GetVolumes(pod.UID)
-	if !ok {
-		return nil, fmt.Errorf("cannot get the volumes for pod %q", format.Pod(pod))
-	}
-
-	// Set global volumes.
-	for vname, volume := range volumeMap {
-		manifest.Volumes = append(manifest.Volumes, appctypes.Volume{
-			Name:   convertToACName(vname),
-			Kind:   "host",
-			Source: volume.Mounter.GetPath(),
-		})
-	}
-
 	// TODO(yifan): Set pod-level isolators once it's supported in kubernetes.
 	return manifest, nil
 }
@@ -780,6 +758,16 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, c api.Container, requiresPrivi
 		return err
 	}
 
+	for _, mnt := range opts.Mounts {
+		readOnly := mnt.ReadOnly
+		manifest.Volumes = append(manifest.Volumes, appctypes.Volume{
+			Name:     convertToACName(mnt.Name),
+			Source:   mnt.HostPath,
+			Kind:     "host",
+			ReadOnly: &readOnly,
+		})
+	}
+
 	ra := appcschema.RuntimeApp{
 		Name:           convertToACName(c.Name),
 		Image:          appcschema.RuntimeImage{ID: *hash},

pkg/kubelet/rkt/rkt_test.go

@@ -36,7 +36,6 @@ import (
 	kubetesting "k8s.io/kubernetes/pkg/kubelet/container/testing"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 	"k8s.io/kubernetes/pkg/kubelet/rkt/mock_os"
-	"k8s.io/kubernetes/pkg/kubelet/rkt/mock_rkt"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 	kubetypes "k8s.io/kubernetes/pkg/types"
 	"k8s.io/kubernetes/pkg/util/errors"
@@ -1636,11 +1635,9 @@ func TestMakePodManifestAnnotations(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	mockVolumeGetter := mock_rkt.NewMockVolumeGetter(ctrl)
-
 	fr := newFakeRktInterface()
 	fs := newFakeSystemd()
-	r := &Runtime{apisvc: fr, systemd: fs, volumeGetter: mockVolumeGetter}
+	r := &Runtime{apisvc: fr, systemd: fs}
 
 	testCases := []struct {
 		in     *api.Pod
@@ -1691,7 +1688,6 @@ func TestMakePodManifestAnnotations(t *testing.T) {
 
 	for i, testCase := range testCases {
 		hint := fmt.Sprintf("case #%d", i)
-		mockVolumeGetter.EXPECT().GetVolumes(gomock.Any()).Return(kubecontainer.VolumeMap{}, true)
 
 		result, err := r.makePodManifest(testCase.in, []api.Secret{})
 		assert.Equal(t, err, testCase.outerr, hint)