Skip to content

Commit

Permalink
runtime: fsGroup support for direct-assigned volume
Browse files Browse the repository at this point in the history
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 <yibzhuang@gmail.com>
(cherry picked from commit 532d539)
Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
  • Loading branch information
yibozhuang authored and fidencio committed Apr 27, 2022
1 parent b62cced commit 97ad1d5
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 1 deletion.
19 changes: 19 additions & 0 deletions src/runtime/pkg/direct-volume/utils.go
Expand Up @@ -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"
Expand Down
7 changes: 6 additions & 1 deletion src/runtime/pkg/direct-volume/utils_test.go
Expand Up @@ -27,7 +27,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)
Expand All @@ -41,6 +45,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)

_, err = os.Stat(filepath.Join(kataDirectVolumeRootPath, b64.URLEncoding.EncodeToString([]byte(volumePath))))
assert.Nil(t, err)
Expand Down
20 changes: 20 additions & 0 deletions src/runtime/virtcontainers/container.go
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions src/runtime/virtcontainers/kata_agent.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions src/runtime/virtcontainers/kata_agent_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -234,6 +235,7 @@ func TestHandleLocalStorage(t *testing.T) {
}

func TestHandleDeviceBlockVolume(t *testing.T) {
var gid = 2000
k := kataAgent{}

// nolint: govet
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions src/runtime/virtcontainers/mount.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 97ad1d5

Please sign in to comment.