Skip to content

Commit c42bf4c

Browse files
authored
Merge pull request #2201 from hajiler/automated-cherry-pick-of-#2178-upstream-release-1.21
Automated cherry pick of #2178: Add NodeStageVolume disk size validation before mounting
2 parents 25095b3 + c2fe2e7 commit c42bf4c

File tree

6 files changed

+80
-4
lines changed

6 files changed

+80
-4
lines changed

cmd/gce-pd-csi-driver/main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ var (
100100

101101
diskCacheSyncPeriod = flag.Duration("disk-cache-sync-period", 10*time.Minute, "Period for the disk cache to check the /dev/disk/by-id/ directory and evaluate the symlinks")
102102

103+
enableDiskSizeValidation = flag.Bool("enable-disk-size-validation", false, "If set to true, the driver will validate that the requested disk size is matches the physical disk size. This flag is disabled by default.")
104+
103105
version string
104106
)
105107

@@ -251,7 +253,8 @@ func handle() {
251253
maxBackoffDuration := time.Duration(*errorBackoffMaxDurationMs) * time.Millisecond
252254
// TODO(2042): Move more of the constructor args into this struct
253255
args := &driver.GCEControllerServerArgs{
254-
EnableDiskTopology: *diskTopology,
256+
EnableDiskTopology: *diskTopology,
257+
EnableDiskSizeValidation: *enableDiskSizeValidation,
255258
}
256259

257260
controllerServer = driver.NewControllerServer(gceDriver, cloudProvider, initialBackoffDuration, maxBackoffDuration, fallbackRequisiteZones, *enableStoragePoolsFlag, *enableDataCacheFlag, multiZoneVolumeHandleConfig, listVolumesConfig, provisionableDisksConfig, *enableHdHAFlag, args)

pkg/common/constants.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const (
4848

4949
ContextDataCacheSize = "data-cache-size"
5050
ContextDataCacheMode = "data-cache-mode"
51+
ContextDiskSizeGB = "disk-size"
5152

5253
// Keys in the publish context
5354
ContexLocalSsdCacheSize = "local-ssd-cache-size"

pkg/gce-pd-csi-driver/controller.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"math/rand"
2222
neturl "net/url"
2323
"sort"
24+
"strconv"
2425
"strings"
2526
"time"
2627

@@ -121,11 +122,13 @@ type GCEControllerServer struct {
121122
// new RPC methods that might be introduced in future versions of the spec.
122123
csi.UnimplementedControllerServer
123124

124-
EnableDiskTopology bool
125+
EnableDiskTopology bool
126+
EnableDiskSizeValidation bool
125127
}
126128

127129
type GCEControllerServerArgs struct {
128-
EnableDiskTopology bool
130+
EnableDiskTopology bool
131+
EnableDiskSizeValidation bool
129132
}
130133

131134
type MultiZoneVolumeHandleConfig struct {
@@ -1113,7 +1116,7 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
11131116
volumeCapability := req.GetVolumeCapability()
11141117

11151118
pubVolResp := &csi.ControllerPublishVolumeResponse{
1116-
PublishContext: nil,
1119+
PublishContext: map[string]string{},
11171120
}
11181121

11191122
// Set data cache publish context
@@ -1162,6 +1165,9 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
11621165
}
11631166
return nil, common.LoggedError("Failed to getDisk: ", err), disk
11641167
}
1168+
if gceCS.EnableDiskSizeValidation && pubVolResp.GetPublishContext() != nil {
1169+
pubVolResp.PublishContext[common.ContextDiskSizeGB] = strconv.FormatInt(disk.GetSizeGb(), 10)
1170+
}
11651171
instance, err := gceCS.CloudProvider.GetInstanceOrError(ctx, project, instanceZone, instanceName)
11661172
if err != nil {
11671173
if gce.IsGCENotFoundError(err) {

pkg/gce-pd-csi-driver/gce-pd-driver.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ func NewControllerServer(gceDriver *GCEDriver, cloudProvider gce.GCECompute, err
178178
provisionableDisksConfig: provisionableDisksConfig,
179179
enableHdHA: enableHdHA,
180180
EnableDiskTopology: args.EnableDiskTopology,
181+
EnableDiskSizeValidation: args.EnableDiskSizeValidation,
181182
}
182183
}
183184

pkg/gce-pd-csi-driver/node.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,17 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
435435
klog.V(4).Infof("CSI volume is read-only, mounting with extra option ro")
436436
}
437437

438+
// If a disk size is provided in the publish context, ensure it matches the actual device size.
439+
if expectedSize := req.GetPublishContext()[common.ContextDiskSizeGB]; expectedSize != "" {
440+
actualSize, err := getBlockSizeBytes(devicePath, ns.Mounter)
441+
if err != nil {
442+
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to get block size for '%s': %v", devicePath, err.Error()))
443+
}
444+
if expectedSize != strconv.FormatInt(actualSize, 10) {
445+
return nil, status.Error(codes.Internal, fmt.Sprintf("expected block size %q, got %q", expectedSize, strconv.FormatInt(actualSize, 10)))
446+
}
447+
}
448+
438449
err = ns.formatAndMount(devicePath, stagingTargetPath, fstype, options, ns.Mounter)
439450
if err != nil {
440451
// If a volume is created from a content source like snapshot or cloning, the filesystem might get marked

pkg/gce-pd-csi-driver/node_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"google.golang.org/grpc/codes"
3333
"google.golang.org/grpc/status"
3434
"k8s.io/mount-utils"
35+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
3536
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/deviceutils"
3637
metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata"
3738
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/linkcache"
@@ -1104,6 +1105,41 @@ func TestNodeStageVolume(t *testing.T) {
11041105
},
11051106
},
11061107
},
1108+
{
1109+
name: "Valid request with disk size check",
1110+
req: &csi.NodeStageVolumeRequest{
1111+
VolumeId: volumeID,
1112+
StagingTargetPath: stagingPath,
1113+
VolumeCapability: stdVolCap,
1114+
PublishContext: map[string]string{common.ContextDiskSizeGB: "1"},
1115+
},
1116+
deviceSize: 1,
1117+
blockExtSize: 1,
1118+
readonlyBit: "1",
1119+
expResize: false,
1120+
expCommandList: []fakeCmd{
1121+
{
1122+
cmd: "blockdev",
1123+
args: "--getsize64 /dev/disk/fake-path",
1124+
stdout: "%v",
1125+
},
1126+
{
1127+
cmd: "blkid",
1128+
args: "-p -s TYPE -s PTTYPE -o export /dev/disk/fake-path",
1129+
stdout: "DEVNAME=/dev/sdb\nTYPE=%v",
1130+
},
1131+
{
1132+
cmd: "fsck",
1133+
args: "-a /dev/disk/fake-path",
1134+
stdout: "",
1135+
},
1136+
{
1137+
cmd: "blockdev",
1138+
args: "--getro /dev/disk/fake-path",
1139+
stdout: "%v",
1140+
},
1141+
},
1142+
},
11071143
{
11081144
name: "Invalid request (Bad Access Mode)",
11091145
req: &csi.NodeStageVolumeRequest{
@@ -1183,6 +1219,24 @@ func TestNodeStageVolume(t *testing.T) {
11831219
},
11841220
expErrCode: codes.InvalidArgument,
11851221
},
1222+
{
1223+
name: "Invalid request, block size mismatch",
1224+
req: &csi.NodeStageVolumeRequest{
1225+
VolumeId: volumeID,
1226+
StagingTargetPath: stagingPath,
1227+
VolumeCapability: stdVolCap,
1228+
PublishContext: map[string]string{common.ContextDiskSizeGB: "10"},
1229+
},
1230+
deviceSize: 5,
1231+
expErrCode: codes.Internal,
1232+
expCommandList: []fakeCmd{
1233+
{
1234+
cmd: "blockdev",
1235+
args: "--getsize64 /dev/disk/fake-path",
1236+
stdout: "%v",
1237+
},
1238+
},
1239+
},
11861240
}
11871241
for _, tc := range testCases {
11881242
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)