From 532d53977ec276ecbb298a9755439bd3d95dbebf Mon Sep 17 00:00:00 2001 From: Yibo Zhuang Date: Wed, 6 Apr 2022 19:31:49 -0700 Subject: [PATCH] runtime: fsGroup support for direct-assigned volume The fsGroup will be specified by the fsGroup key in the direct-assign mountinfo metadate field. This will be set when invoking the kata-runtime binary and providing the key, value pair in the metadata field. Similarly, the fsGroupChangePolicy will also be provided in the mountinfo metadate field. Adding an extra fields FsGroup and FSGroupChangePolicy in the Mount construct for container mount which will be populated when creating block devices by parsing out the mountInfo.json. And in handleDeviceBlockVolume of the kata-agent client, it checks if the mount FSGroup is not nil, which indicates that fsGroup change is required in the guest, and will provide the FSGroup field in the protobuf to pass the value to the agent. Fixes #4018 Signed-off-by: Yibo Zhuang --- src/runtime/pkg/direct-volume/utils.go | 19 +++++++++++++++ src/runtime/pkg/direct-volume/utils_test.go | 7 +++++- src/runtime/virtcontainers/container.go | 20 ++++++++++++++++ src/runtime/virtcontainers/kata_agent.go | 16 +++++++++++++ src/runtime/virtcontainers/kata_agent_test.go | 23 +++++++++++++++++++ src/runtime/virtcontainers/mount.go | 10 ++++++++ 6 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/runtime/pkg/direct-volume/utils.go b/src/runtime/pkg/direct-volume/utils.go index 275b0508f8..847b704bd8 100644 --- a/src/runtime/pkg/direct-volume/utils.go +++ b/src/runtime/pkg/direct-volume/utils.go @@ -17,6 +17,25 @@ import ( const ( mountInfoFileName = "mountInfo.json" + + FSGroupMetadataKey = "fsGroup" + FSGroupChangePolicyMetadataKey = "fsGroupChangePolicy" +) + +// FSGroupChangePolicy holds policies that will be used for applying fsGroup to a volume. +// This type and the allowed values are tracking the PodFSGroupChangePolicy defined in +// https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/core/v1/types.go +// It is up to the client using the direct-assigned volume feature (e.g. CSI drivers) to determine +// the optimal setting for this change policy (i.e. from Pod spec or assuming volume ownership +// based on the storage offering). +type FSGroupChangePolicy string + +const ( + // FSGroupChangeAlways indicates that volume's ownership should always be changed. + FSGroupChangeAlways FSGroupChangePolicy = "Always" + // FSGroupChangeOnRootMismatch indicates that volume's ownership will be changed + // only when ownership of root directory does not match with the desired group id. + FSGroupChangeOnRootMismatch FSGroupChangePolicy = "OnRootMismatch" ) var kataDirectVolumeRootPath = "/run/kata-containers/shared/direct-volumes" diff --git a/src/runtime/pkg/direct-volume/utils_test.go b/src/runtime/pkg/direct-volume/utils_test.go index 485f8f9ce8..54f2ad388e 100644 --- a/src/runtime/pkg/direct-volume/utils_test.go +++ b/src/runtime/pkg/direct-volume/utils_test.go @@ -25,7 +25,11 @@ func TestAdd(t *testing.T) { VolumeType: "block", Device: "/dev/sda", FsType: "ext4", - Options: []string{"journal_dev", "noload"}, + Metadata: map[string]string{ + FSGroupMetadataKey: "3000", + FSGroupChangePolicyMetadataKey: string(FSGroupChangeOnRootMismatch), + }, + Options: []string{"journal_dev", "noload"}, } buf, err := json.Marshal(actual) assert.Nil(t, err) @@ -39,6 +43,7 @@ func TestAdd(t *testing.T) { assert.Equal(t, expected.Device, actual.Device) assert.Equal(t, expected.FsType, actual.FsType) assert.Equal(t, expected.Options, actual.Options) + assert.Equal(t, expected.Metadata, actual.Metadata) // Remove the file err = Remove(volumePath) diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index d82124c1d6..4db6403ffa 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -639,6 +639,26 @@ func (c *Container) createBlockDevices(ctx context.Context) error { c.mounts[i].Type = mntInfo.FsType c.mounts[i].Options = mntInfo.Options c.mounts[i].ReadOnly = readonly + + for key, value := range mntInfo.Metadata { + switch key { + case volume.FSGroupMetadataKey: + gid, err := strconv.Atoi(value) + if err != nil { + c.Logger().WithError(err).Errorf("invalid group id value %s provided for key %s", value, volume.FSGroupMetadataKey) + continue + } + c.mounts[i].FSGroup = &gid + case volume.FSGroupChangePolicyMetadataKey: + if _, exists := mntInfo.Metadata[volume.FSGroupMetadataKey]; !exists { + c.Logger().Errorf("%s specified without provding the group id with key %s", volume.FSGroupChangePolicyMetadataKey, volume.FSGroupMetadataKey) + continue + } + c.mounts[i].FSGroupChangePolicy = volume.FSGroupChangePolicy(value) + default: + c.Logger().Warnf("Ignoring unsupported direct-assignd volume metadata key: %s, value: %s", key, value) + } + } } var stat unix.Stat_t diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index aad6269d75..97a09d091c 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -19,6 +19,7 @@ import ( "time" "github.com/docker/go-units" + volume "github.com/kata-containers/kata-containers/src/runtime/pkg/direct-volume" "github.com/kata-containers/kata-containers/src/runtime/pkg/katautils/katatrace" resCtrl "github.com/kata-containers/kata-containers/src/runtime/pkg/resourcecontrol" "github.com/kata-containers/kata-containers/src/runtime/pkg/uuid" @@ -167,6 +168,15 @@ func getPagesizeFromOpt(fsOpts []string) string { return "" } +func getFSGroupChangePolicy(policy volume.FSGroupChangePolicy) pbTypes.FSGroupChangePolicy { + switch policy { + case volume.FSGroupChangeOnRootMismatch: + return pbTypes.FSGroupChangePolicy_OnRootMismatch + default: + return pbTypes.FSGroupChangePolicy_Always + } +} + // Shared path handling: // 1. create three directories for each sandbox: // -. /run/kata-containers/shared/sandboxes/$sbx_id/mounts/, a directory to hold all host/guest shared mounts @@ -1468,6 +1478,12 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, m Mount, device api.De if len(vol.Options) == 0 { vol.Options = m.Options } + if m.FSGroup != nil { + vol.FsGroup = &grpc.FSGroup{ + GroupId: uint32(*m.FSGroup), + GroupChangePolicy: getFSGroupChangePolicy(m.FSGroupChangePolicy), + } + } return vol, nil } diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index 029bc2a8d0..af26edc0c6 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" "code.cloudfoundry.org/bytefmt" + volume "github.com/kata-containers/kata-containers/src/runtime/pkg/direct-volume" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/drivers" @@ -234,6 +235,7 @@ func TestHandleLocalStorage(t *testing.T) { } func TestHandleDeviceBlockVolume(t *testing.T) { + var gid = 2000 k := kataAgent{} // nolint: govet @@ -315,6 +317,27 @@ func TestHandleDeviceBlockVolume(t *testing.T) { Source: testSCSIAddr, }, }, + { + BlockDeviceDriver: config.VirtioBlock, + inputMount: Mount{ + FSGroup: &gid, + FSGroupChangePolicy: volume.FSGroupChangeOnRootMismatch, + }, + inputDev: &drivers.BlockDevice{ + BlockDrive: &config.BlockDrive{ + PCIPath: testPCIPath, + VirtPath: testVirtPath, + }, + }, + resultVol: &pb.Storage{ + Driver: kataBlkDevType, + Source: testPCIPath.String(), + FsGroup: &pb.FSGroup{ + GroupId: uint32(gid), + GroupChangePolicy: pbTypes.FSGroupChangePolicy_OnRootMismatch, + }, + }, + }, } for _, test := range tests { diff --git a/src/runtime/virtcontainers/mount.go b/src/runtime/virtcontainers/mount.go index 0e83bc6899..5e75826199 100644 --- a/src/runtime/virtcontainers/mount.go +++ b/src/runtime/virtcontainers/mount.go @@ -14,6 +14,7 @@ import ( "syscall" merr "github.com/hashicorp/go-multierror" + volume "github.com/kata-containers/kata-containers/src/runtime/pkg/direct-volume" "github.com/kata-containers/kata-containers/src/runtime/pkg/katautils/katatrace" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" "github.com/pkg/errors" @@ -325,6 +326,7 @@ func bindMountContainerRootfs(ctx context.Context, shareDir, cid, cRootFs string } // Mount describes a container mount. +// nolint: govet type Mount struct { // Source is the source of the mount. Source string @@ -352,6 +354,14 @@ type Mount struct { // ReadOnly specifies if the mount should be read only or not ReadOnly bool + + // FSGroup a group ID that the group ownership of the files for the mounted volume + // will need to be changed when set. + FSGroup *int + + // FSGroupChangePolicy specifies the policy that will be used when applying + // group id ownership change for a volume. + FSGroupChangePolicy volume.FSGroupChangePolicy } func isSymlink(path string) bool {