Refactor volume.Builder.IsReadOnly() to volume.Builder.GetAttributes()

pkg/kubelet/kubelet.go

@@ -1028,10 +1028,10 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap)
 	rootDirSELinuxOptions.Level = pod.Spec.SecurityContext.SELinuxOptions.Level
 	volumeContext := fmt.Sprintf("%s:%s:%s:%s", rootDirSELinuxOptions.User, rootDirSELinuxOptions.Role, rootDirSELinuxOptions.Type, rootDirSELinuxOptions.Level)
 
-	for _, volume := range volumes {
-		if volume.Builder.SupportsSELinux() && !volume.Builder.IsReadOnly() {
+	for _, vol := range volumes {
+		if vol.Builder.GetAttributes().Managed && vol.Builder.GetAttributes().SupportsSELinux {
 			// Relabel the volume and its content to match the 'Level' of the pod
-			err := filepath.Walk(volume.Builder.GetPath(), func(path string, info os.FileInfo, err error) error {
+			err := filepath.Walk(vol.Builder.GetPath(), func(path string, info os.FileInfo, err error) error {
 				if err != nil {
 					return err
 				}
@@ -1040,7 +1040,7 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap)
 			if err != nil {
 				return err
 			}
-			volume.SELinuxLabeled = true
+			vol.SELinuxLabeled = true
 		}
 	}
 	return nil
@@ -1067,7 +1067,7 @@ func makeMounts(pod *api.Pod, podDir string, container *api.Container, podVolume
 		// If the volume supports SELinux and it has not been
 		// relabeled already and it is not a read-only volume,
 		// relabel it and mark it as labeled
-		if vol.Builder.SupportsSELinux() && !vol.SELinuxLabeled && !vol.Builder.IsReadOnly() {
+		if vol.Builder.GetAttributes().SupportsSELinux && !vol.SELinuxLabeled && !vol.Builder.GetAttributes().Managed {
 			vol.SELinuxLabeled = true
 			relabelVolume = true
 		}

pkg/kubelet/kubelet_test.go

@@ -515,8 +515,8 @@ func (f *stubVolume) GetPath() string {
 	return f.path
 }
 
-func (f *stubVolume) IsReadOnly() bool {
-	return false
+func (f *stubVolume) GetAttributes() volume.Attributes {
+	return volume.Attributes{}
 }
 
 func (f *stubVolume) SetUp() error {
@@ -527,14 +527,6 @@ func (f *stubVolume) SetUpAt(dir string) error {
 	return nil
 }
 
-func (f *stubVolume) SupportsSELinux() bool {
-	return false
-}
-
-func (f *stubVolume) SupportsOwnershipManagement() bool {
-	return false
-}
-
 func TestMakeVolumeMounts(t *testing.T) {
 	container := api.Container{
 		VolumeMounts: []api.VolumeMount{

pkg/kubelet/volumes.go

@@ -140,10 +140,15 @@ func (kl *Kubelet) mountExternalVolumes(pod *api.Pod) (kubecontainer.VolumeMap,
 		if err != nil {
 			return nil, err
 		}
-		if hasFSGroup && builder.SupportsOwnershipManagement() && !builder.IsReadOnly() {
+		if hasFSGroup &&
+			builder.GetAttributes().Managed &&
+			builder.GetAttributes().SupportsOwnershipManagement {
 			err := kl.manageVolumeOwnership(pod, internal, builder, fsGroup)
 			if err != nil {
+				glog.Errorf("Error managing ownership of volume %v for pod %v/%v: %v", internal.Name(), pod.Namespace, pod.Name, err)
 				return nil, err
+			} else {
+				glog.V(3).Infof("Managed ownership of volume %v for pod %v/%v", internal.Name(), pod.Namespace, pod.Name)
 			}
 		}
 		podVolumes[volSpec.Name] = kubecontainer.VolumeInfo{Builder: builder}

pkg/kubelet/volumes_linux.go

@@ -28,8 +28,11 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 )
 
-// Bitmask to OR with current ownership of volumes that allow ownership management by the Kubelet
-const managedOwnershipBitmask = os.FileMode(0660)
+// Bitmasks to OR with current ownership of volumes that allow ownership management by the Kubelet
+const (
+	rwMask = os.FileMode(0660)
+	roMask = os.FileMode(0440)
+)
 
 // manageVolumeOwnership modifies the given volume to be owned by fsGroup.
 func (kl *Kubelet) manageVolumeOwnership(pod *api.Pod, volSpec *volume.Spec, builder volume.Builder, fsGroup int64) error {
@@ -53,7 +56,12 @@ func (kl *Kubelet) manageVolumeOwnership(pod *api.Pod, volSpec *volume.Spec, bui
 			glog.Errorf("Chown failed on %v: %v", path, err)
 		}
 
-		err = kl.chmodRunner.Chmod(path, info.Mode()|managedOwnershipBitmask|os.ModeSetgid)
+		mask := rwMask
+		if builder.GetAttributes().ReadOnly {
+			mask = roMask
+		}
+
+		err = kl.chmodRunner.Chmod(path, info.Mode()|mask|os.ModeSetgid)
 		if err != nil {
 			glog.Errorf("Chmod failed on %v: %v", path, err)
 		}

pkg/volume/aws_ebs/aws_ebs.go

@@ -177,8 +177,13 @@ type awsElasticBlockStoreBuilder struct {
 
 var _ volume.Builder = &awsElasticBlockStoreBuilder{}
 
-func (_ *awsElasticBlockStoreBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *awsElasticBlockStoreBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     !b.readOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 // SetUp attaches the disk and bind mounts to the volume path.
@@ -246,14 +251,6 @@ func (b *awsElasticBlockStoreBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (b *awsElasticBlockStoreBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *awsElasticBlockStoreBuilder) SupportsSELinux() bool {
-	return true
-}
-
 func makeGlobalPDPath(host volume.VolumeHost, volumeID string) string {
 	// Clean up the URI to be more fs-friendly
 	name := volumeID

pkg/volume/aws_ebs/aws_ebs_test.go

@@ -225,7 +225,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/cephfs/cephfs.go

@@ -151,8 +151,13 @@ type cephfsBuilder struct {
 
 var _ volume.Builder = &cephfsBuilder{}
 
-func (_ *cephfsBuilder) SupportsOwnershipManagement() bool {
-	return false
+func (cephfsVolume *cephfsBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    cephfsVolume.readonly,
+		Managed:                     false,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             false,
+	}
 }
 
 // SetUp attaches the disk and bind mounts to the volume path.
@@ -183,14 +188,6 @@ func (cephfsVolume *cephfsBuilder) SetUpAt(dir string) error {
 	return err
 }
 
-func (cephfsVolume *cephfsBuilder) IsReadOnly() bool {
-	return cephfsVolume.readonly
-}
-
-func (cephfsVolume *cephfsBuilder) SupportsSELinux() bool {
-	return false
-}
-
 type cephfsCleaner struct {
 	*cephfs
 }

pkg/volume/cinder/cinder.go

@@ -153,8 +153,13 @@ func detachDiskLogError(cd *cinderVolume) {
 	}
 }
 
-func (_ *cinderVolumeBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *cinderVolumeBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     !b.readOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 func (b *cinderVolumeBuilder) SetUp() error {
@@ -221,14 +226,6 @@ func (b *cinderVolumeBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (b *cinderVolumeBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *cinderVolumeBuilder) SupportsSELinux() bool {
-	return true
-}
-
 func makeGlobalPDName(host volume.VolumeHost, devName string) string {
 	return path.Join(host.GetPluginDir(cinderVolumePluginName), "mounts", devName)
 }

pkg/volume/downwardapi/downwardapi.go

@@ -107,8 +107,14 @@ type downwardAPIVolumeBuilder struct {
 // downwardAPIVolumeBuilder implements volume.Builder interface
 var _ volume.Builder = &downwardAPIVolumeBuilder{}
 
-func (_ *downwardAPIVolumeBuilder) SupportsOwnershipManagement() bool {
-	return false
+// downward API volumes are always ReadOnlyManaged
+func (d *downwardAPIVolume) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    true,
+		Managed:                     true,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 // SetUp puts in place the volume plugin.
@@ -152,15 +158,6 @@ func (b *downwardAPIVolumeBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-// IsReadOnly func to fulfill volume.Builder interface
-func (d *downwardAPIVolume) IsReadOnly() bool {
-	return true
-}
-
-func (d *downwardAPIVolume) SupportsSELinux() bool {
-	return true
-}
-
 // collectData collects requested downwardAPI in data map.
 // Map's key is the requested name of file to dump
 // Map's value is the (sorted) content of the field to be dumped in the file.

pkg/volume/empty_dir/empty_dir.go

@@ -135,8 +135,13 @@ type emptyDir struct {
 	rootContext   string
 }
 
-func (_ *emptyDir) SupportsOwnershipManagement() bool {
-	return true
+func (ed *emptyDir) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    false,
+		Managed:                     true,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 // SetUp creates new directory.
@@ -187,14 +192,6 @@ func (ed *emptyDir) SetUpAt(dir string) error {
 	return err
 }
 
-func (ed *emptyDir) IsReadOnly() bool {
-	return false
-}
-
-func (ed *emptyDir) SupportsSELinux() bool {
-	return true
-}
-
 // setupTmpfs creates a tmpfs mount at the specified directory with the
 // specified SELinux context.
 func (ed *emptyDir) setupTmpfs(dir string, selinuxContext string) error {

pkg/volume/fc/fc.go

@@ -164,10 +164,14 @@ type fcDiskBuilder struct {
 
 var _ volume.Builder = &fcDiskBuilder{}
 
-func (_ *fcDiskBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *fcDiskBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     !b.readOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
-
 func (b *fcDiskBuilder) SetUp() error {
 	return b.SetUpAt(b.GetPath())
 }
@@ -187,14 +191,6 @@ type fcDiskCleaner struct {
 
 var _ volume.Cleaner = &fcDiskCleaner{}
 
-func (b *fcDiskBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *fcDiskBuilder) SupportsSELinux() bool {
-	return true
-}
-
 // Unmounts the bind mount, and detaches the disk only if the disk
 // resource was the last reference to that disk on the kubelet.
 func (c *fcDiskCleaner) TearDown() error {

pkg/volume/fc/fc_test.go

@@ -246,7 +246,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/flocker/plugin.go

@@ -113,10 +113,14 @@ type flockerBuilder struct {
 	readOnly bool
 }
 
-func (_ *flockerBuilder) SupportsOwnershipManagement() bool {
-	return false
+func (b flockerBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     false,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             false,
+	}
 }
-
 func (b flockerBuilder) GetPath() string {
 	return b.flocker.path
 }
@@ -201,14 +205,6 @@ func (b flockerBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (b flockerBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b flockerBuilder) SupportsSELinux() bool {
-	return false
-}
-
 // updateDatasetPrimary will update the primary in Flocker and wait for it to
 // be ready. If it never gets to ready state it will timeout and error.
 func (b flockerBuilder) updateDatasetPrimary(datasetID, primaryUUID string) error {

pkg/volume/flocker/plugin_test.go

@@ -146,8 +146,8 @@ func TestNewCleaner(t *testing.T) {
 }
 
 func TestIsReadOnly(t *testing.T) {
-	b := flockerBuilder{readOnly: true}
-	assert.True(t, b.IsReadOnly())
+	b := &flockerBuilder{readOnly: true}
+	assert.True(t, b.GetAttributes().ReadOnly)
 }
 
 func TestGetPath(t *testing.T) {

pkg/volume/gce_pd/gce_pd.go

@@ -165,8 +165,13 @@ type gcePersistentDiskBuilder struct {
 
 var _ volume.Builder = &gcePersistentDiskBuilder{}
 
-func (_ *gcePersistentDiskBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *gcePersistentDiskBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     !b.readOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 // SetUp attaches the disk and bind mounts to the volume path.
@@ -234,14 +239,6 @@ func (b *gcePersistentDiskBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (b *gcePersistentDiskBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *gcePersistentDiskBuilder) SupportsSELinux() bool {
-	return true
-}
-
 func makeGlobalPDName(host volume.VolumeHost, devName string) string {
 	return path.Join(host.GetPluginDir(gcePersistentDiskPluginName), "mounts", devName)
 }

pkg/volume/gce_pd/gce_pd_test.go

@@ -240,7 +240,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/git_repo/git_repo.go

@@ -109,8 +109,13 @@ type gitRepoVolumeBuilder struct {
 
 var _ volume.Builder = &gitRepoVolumeBuilder{}
 
-func (_ *gitRepoVolumeBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *gitRepoVolumeBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    false,
+		Managed:                     true,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             true, // xattr change should be okay, TODO: double check
+	}
 }
 
 // SetUp creates new directory and clones a git repo.
@@ -118,14 +123,6 @@ func (b *gitRepoVolumeBuilder) SetUp() error {
 	return b.SetUpAt(b.GetPath())
 }
 
-func (b *gitRepoVolumeBuilder) IsReadOnly() bool {
-	return false
-}
-
-func (b *gitRepoVolumeBuilder) SupportsSELinux() bool {
-	return true
-}
-
 // This is the spec for the volume that this plugin wraps.
 var wrappedVolumeSpec = &volume.Spec{
 	Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},

pkg/volume/glusterfs/glusterfs.go

@@ -154,8 +154,13 @@ type glusterfsBuilder struct {
 
 var _ volume.Builder = &glusterfsBuilder{}
 
-func (_ *glusterfsBuilder) SupportsOwnershipManagement() bool {
-	return false
+func (b *glusterfsBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     false,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             false,
+	}
 }
 
 // SetUp attaches the disk and bind mounts to the volume path.
@@ -185,14 +190,6 @@ func (b *glusterfsBuilder) SetUpAt(dir string) error {
 	return err
 }
 
-func (b *glusterfsBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *glusterfsBuilder) SupportsSELinux() bool {
-	return false
-}
-
 func (glusterfsVolume *glusterfs) GetPath() string {
 	name := glusterfsPluginName
 	return glusterfsVolume.plugin.host.GetPodVolumeDir(glusterfsVolume.pod.UID, util.EscapeQualifiedNameForDisk(name), glusterfsVolume.volName)

pkg/volume/glusterfs/glusterfs_test.go

@@ -209,7 +209,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/host_path/host_path.go

@@ -167,8 +167,13 @@ type hostPathBuilder struct {
 
 var _ volume.Builder = &hostPathBuilder{}
 
-func (_ *hostPathBuilder) SupportsOwnershipManagement() bool {
-	return false
+func (b *hostPathBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     false,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             false,
+	}
 }
 
 // SetUp does nothing.
@@ -181,14 +186,6 @@ func (b *hostPathBuilder) SetUpAt(dir string) error {
 	return fmt.Errorf("SetUpAt() does not make sense for host paths")
 }
 
-func (b *hostPathBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *hostPathBuilder) SupportsSELinux() bool {
-	return false
-}
-
 func (b *hostPathBuilder) GetPath() string {
 	return b.path
 }

pkg/volume/host_path/host_path_test.go

@@ -268,7 +268,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/iscsi/iscsi.go

@@ -158,8 +158,13 @@ type iscsiDiskBuilder struct {
 
 var _ volume.Builder = &iscsiDiskBuilder{}
 
-func (_ *iscsiDiskBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *iscsiDiskBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     !b.readOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 func (b *iscsiDiskBuilder) SetUp() error {
@@ -181,14 +186,6 @@ type iscsiDiskCleaner struct {
 
 var _ volume.Cleaner = &iscsiDiskCleaner{}
 
-func (b *iscsiDiskBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *iscsiDiskBuilder) SupportsSELinux() bool {
-	return true
-}
-
 // Unmounts the bind mount, and detaches the disk only if the disk
 // resource was the last reference to that disk on the kubelet.
 func (c *iscsiDiskCleaner) TearDown() error {

pkg/volume/iscsi/iscsi_test.go

@@ -246,7 +246,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/nfs/nfs.go

@@ -147,8 +147,13 @@ type nfsBuilder struct {
 
 var _ volume.Builder = &nfsBuilder{}
 
-func (_ *nfsBuilder) SupportsOwnershipManagement() bool {
-	return false
+func (b *nfsBuilder) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.readOnly,
+		Managed:                     false,
+		SupportsOwnershipManagement: false,
+		SupportsSELinux:             false,
+	}
 }
 
 // SetUp attaches the disk and bind mounts to the volume path.
@@ -200,14 +205,6 @@ func (b *nfsBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (b *nfsBuilder) IsReadOnly() bool {
-	return b.readOnly
-}
-
-func (b *nfsBuilder) SupportsSELinux() bool {
-	return false
-}
-
 //
 //func (c *nfsCleaner) GetPath() string {
 //	name := nfsPluginName

pkg/volume/nfs/nfs_test.go

@@ -247,7 +247,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/persistent_claim/persistent_claim.go

@@ -88,10 +88,6 @@ func (plugin *persistentClaimPlugin) NewBuilder(spec *volume.Spec, pod *api.Pod,
 	return builder, nil
 }
 
-func (plugin *persistentClaimPlugin) IsReadOnly() bool {
-	return plugin.readOnly
-}
-
 func (plugin *persistentClaimPlugin) NewCleaner(_ string, _ types.UID) (volume.Cleaner, error) {
 	return nil, fmt.Errorf("This will never be called directly. The PV backing this claim has a cleaner.  Kubelet uses that cleaner, not this one, when removing orphaned volumes.")
 }

pkg/volume/rbd/disk_manager.go

@@ -62,7 +62,7 @@ func diskSetUp(manager diskManager, b rbdBuilder, volPath string, mounter mount.
 	}
 	// Perform a bind mount to the full path to allow duplicate mounts of the same disk.
 	options := []string{"bind"}
-	if b.IsReadOnly() {
+	if (&b).GetAttributes().ReadOnly {
 		options = append(options, "ro")
 	}
 	err = mounter.Mount(globalPDPath, volPath, "", options)

pkg/volume/rbd/rbd.go

@@ -192,8 +192,13 @@ type rbdBuilder struct {
 
 var _ volume.Builder = &rbdBuilder{}
 
-func (_ *rbdBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (b *rbd) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    b.ReadOnly,
+		Managed:                     !b.ReadOnly,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 func (b *rbdBuilder) SetUp() error {
@@ -215,14 +220,6 @@ type rbdCleaner struct {
 
 var _ volume.Cleaner = &rbdCleaner{}
 
-func (b *rbd) IsReadOnly() bool {
-	return b.ReadOnly
-}
-
-func (b *rbd) SupportsSELinux() bool {
-	return true
-}
-
 // Unmounts the bind mount, and detaches the disk only if the disk
 // resource was the last reference to that disk on the kubelet.
 func (c *rbdCleaner) TearDown() error {

pkg/volume/rbd/rbd_test.go

@@ -203,7 +203,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
 	pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}}
 	builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{})
 
-	if !builder.IsReadOnly() {
+	if !builder.GetAttributes().ReadOnly {
 		t.Errorf("Expected true for builder.IsReadOnly")
 	}
 }

pkg/volume/rbd/rbd_util.go

@@ -193,7 +193,7 @@ func (util *RBDUtil) loadRBD(builder *rbdBuilder, mnt string) error {
 
 func (util *RBDUtil) fencing(b rbdBuilder) error {
 	// no need to fence readOnly
-	if b.IsReadOnly() {
+	if (&b).GetAttributes().ReadOnly {
 		return nil
 	}
 	return util.rbdLock(b, true)

pkg/volume/secret/secret.go

@@ -97,10 +97,14 @@ type secretVolumeBuilder struct {
 
 var _ volume.Builder = &secretVolumeBuilder{}
 
-func (_ *secretVolumeBuilder) SupportsOwnershipManagement() bool {
-	return true
+func (sv *secretVolume) GetAttributes() volume.Attributes {
+	return volume.Attributes{
+		ReadOnly:                    true,
+		Managed:                     true,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
-
 func (b *secretVolumeBuilder) SetUp() error {
 	return b.SetUpAt(b.GetPath())
 }
@@ -172,14 +176,6 @@ func (b *secretVolumeBuilder) SetUpAt(dir string) error {
 	return nil
 }
 
-func (sv *secretVolume) IsReadOnly() bool {
-	return false
-}
-
-func (sv *secretVolume) SupportsSELinux() bool {
-	return true
-}
-
 func totalSecretBytes(secret *api.Secret) int {
 	totalSize := 0
 	for _, bytes := range secret.Data {

pkg/volume/testing.go

@@ -156,8 +156,13 @@ type FakeVolume struct {
 	Plugin  *FakeVolumePlugin
 }
 
-func (_ *FakeVolume) SupportsOwnershipManagement() bool {
-	return false
+func (_ *FakeVolume) GetAttributes() Attributes {
+	return Attributes{
+		ReadOnly:                    false,
+		Managed:                     true,
+		SupportsOwnershipManagement: true,
+		SupportsSELinux:             true,
+	}
 }
 
 func (fv *FakeVolume) SetUp() error {
@@ -168,14 +173,6 @@ func (fv *FakeVolume) SetUpAt(dir string) error {
 	return os.MkdirAll(dir, 0750)
 }
 
-func (fv *FakeVolume) IsReadOnly() bool {
-	return false
-}
-
-func (fv *FakeVolume) SupportsSELinux() bool {
-	return false
-}
-
 func (fv *FakeVolume) GetPath() string {
 	return path.Join(fv.Plugin.Host.GetPodVolumeDir(fv.PodUID, util.EscapeQualifiedNameForDisk(fv.Plugin.PluginName), fv.VolName))
 }

pkg/volume/volume.go

@@ -30,6 +30,14 @@ type Volume interface {
 	GetPath() string
 }
 
+// Attributes represents the attributes of this builder.
+type Attributes struct {
+	ReadOnly                    bool
+	Managed                     bool
+	SupportsOwnershipManagement bool
+	SupportsSELinux             bool
+}
+
 // Builder interface provides methods to set up/mount the volume.
 type Builder interface {
 	// Uses Interface to provide the path for Docker binds.
@@ -42,21 +50,8 @@ type Builder interface {
 	// directory path, which may or may not exist yet.  This may be called
 	// more than once, so implementations must be idempotent.
 	SetUpAt(dir string) error
-	// IsReadOnly is a flag that gives the builder's ReadOnly attribute.
-	// All persistent volumes have a private readOnly flag in their builders.
-	IsReadOnly() bool
-	// SupportsOwnershipManagement returns whether this builder wants
-	// ownership management for the volume.  If this method returns true,
-	// the Kubelet will:
-	//
-	// 1. Make the volume owned by group FSGroup
-	// 2. Set the setgid bit is set (new files created in the volume will be owned by FSGroup)
-	// 3. Logical OR the permission bits with rw-rw----
-	SupportsOwnershipManagement() bool
-	// SupportsSELinux reports whether the given builder supports
-	// SELinux and would like the kubelet to relabel the volume to
-	// match the pod to which it will be attached.
-	SupportsSELinux() bool
+	// GetAttributes returns the attributes of the builder.
+	GetAttributes() Attributes
 }
 
 // Cleaner interface provides methods to cleanup/unmount the volumes.

test/e2e/downwardapi_volume.go

@@ -29,69 +29,90 @@ var _ = Describe("Downward API volume", func() {
 	f := NewFramework("downward-api")
 
 	It("should provide labels and annotations files [Conformance]", func() {
+		podName := "downwardapi-volume-" + string(util.NewUUID())
+		pod := downwardAPIVolumePod(podName)
+
+		testContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+			fmt.Sprintf("cluster=\"rack10\"\n"),
+			fmt.Sprintf("builder=\"john-doe\"\n"),
+			fmt.Sprintf("%s\n", podName),
+		}, f.Namespace.Name)
+	})
+
+	It("should provide labels and annotations files as non-root with fsgroup [Conformance] [Skipped]", func() {
 		podName := "metadata-volume-" + string(util.NewUUID())
-		pod := &api.Pod{
-			ObjectMeta: api.ObjectMeta{
-				Name:        podName,
-				Labels:      map[string]string{"cluster": "rack10"},
-				Annotations: map[string]string{"builder": "john-doe"},
-			},
-			Spec: api.PodSpec{
-				Containers: []api.Container{
-					{
-						Name:    "client-container",
-						Image:   "gcr.io/google_containers/busybox",
-						Command: []string{"sh", "-c", "cat /etc/labels /etc/annotations /etc/podname"},
-						VolumeMounts: []api.VolumeMount{
-							{
-								Name:      "podinfo",
-								MountPath: "/etc",
-								ReadOnly:  false,
-							},
+		uid := int64(1001)
+		gid := int64(1234)
+		pod := downwardAPIVolumePod(podName)
+		pod.Spec.SecurityContext = &api.PodSecurityContext{
+			RunAsUser: &uid,
+			FSGroup:   &gid,
+		}
+		testContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+			fmt.Sprintf("cluster=\"rack10\"\n"),
+			fmt.Sprintf("builder=\"john-doe\"\n"),
+			fmt.Sprintf("%s\n", podName),
+		}, f.Namespace.Name)
+	})
+})
+
+func downwardAPIVolumePod(name string) *api.Pod {
+	return &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name:        name,
+			Labels:      map[string]string{"cluster": "rack10"},
+			Annotations: map[string]string{"builder": "john-doe"},
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Name:    "client-container",
+					Image:   "gcr.io/google_containers/busybox",
+					Command: []string{"sh", "-c", "cat /etc/labels /etc/annotations /etc/podname"},
+					VolumeMounts: []api.VolumeMount{
+						{
+							Name:      "podinfo",
+							MountPath: "/etc",
+							ReadOnly:  false,
 						},
 					},
 				},
-				Volumes: []api.Volume{
-					{
-						Name: "podinfo",
-						VolumeSource: api.VolumeSource{
-							DownwardAPI: &api.DownwardAPIVolumeSource{
-								Items: []api.DownwardAPIVolumeFile{
-									{
-										Path: "labels",
-										FieldRef: api.ObjectFieldSelector{
-											APIVersion: "v1",
-											FieldPath:  "metadata.labels",
-										},
+			},
+			Volumes: []api.Volume{
+				{
+					Name: "podinfo",
+					VolumeSource: api.VolumeSource{
+						DownwardAPI: &api.DownwardAPIVolumeSource{
+							Items: []api.DownwardAPIVolumeFile{
+								{
+									Path: "labels",
+									FieldRef: api.ObjectFieldSelector{
+										APIVersion: "v1",
+										FieldPath:  "metadata.labels",
 									},
-									{
-										Path: "annotations",
-										FieldRef: api.ObjectFieldSelector{
-											APIVersion: "v1",
-											FieldPath:  "metadata.annotations",
-										},
+								},
+								{
+									Path: "annotations",
+									FieldRef: api.ObjectFieldSelector{
+										APIVersion: "v1",
+										FieldPath:  "metadata.annotations",
 									},
-									{
-										Path: "podname",
-										FieldRef: api.ObjectFieldSelector{
-											APIVersion: "v1",
-											FieldPath:  "metadata.name",
-										},
+								},
+								{
+									Path: "podname",
+									FieldRef: api.ObjectFieldSelector{
+										APIVersion: "v1",
+										FieldPath:  "metadata.name",
 									},
 								},
 							},
 						},
 					},
 				},
-				RestartPolicy: api.RestartPolicyNever,
 			},
-		}
-		testContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
-			fmt.Sprintf("cluster=\"rack10\"\n"),
-			fmt.Sprintf("builder=\"john-doe\"\n"),
-			fmt.Sprintf("%s\n", podName),
-		}, f.Namespace.Name)
-	})
-})
+			RestartPolicy: api.RestartPolicyNever,
+		},
+	}
+}
 
 // TODO: add test-webserver example as pointed out in https://github.com/kubernetes/kubernetes/pull/5093#discussion-diff-37606771