-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add volume expansion support #87
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -277,10 +277,57 @@ func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetC | |
}, | ||
}, | ||
}, | ||
{ | ||
Type: &csi.NodeServiceCapability_Rpc{ | ||
Rpc: &csi.NodeServiceCapability_RPC{ | ||
Type: csi.NodeServiceCapability_RPC_EXPAND_VOLUME, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, nil | ||
} | ||
|
||
func (ns *nodeServer) NodeGetVolumeStats(ctx context.Context, in *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) { | ||
return nil, status.Error(codes.Unimplemented, "") | ||
} | ||
|
||
// NodeExpandVolume is only implemented so the driver can be used for e2e testing. | ||
func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) { | ||
|
||
volID := req.GetVolumeId() | ||
if len(volID) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Volume ID not provided") | ||
} | ||
|
||
vol, err := getVolumeByID(volID) | ||
if err != nil { | ||
// Assume not found error | ||
return nil, status.Errorf(codes.NotFound, "Could not get volume %s: %v", volID, err) | ||
} | ||
|
||
volPath := req.GetVolumePath() | ||
if len(volPath) == 0 { | ||
return nil, status.Error(codes.InvalidArgument, "Volume path not provided") | ||
} | ||
|
||
info, err := os.Stat(volPath) | ||
if err != nil { | ||
return nil, status.Errorf(codes.InvalidArgument, "Could not get file information from %s: %v", volPath, err) | ||
} | ||
|
||
switch m := info.Mode(); { | ||
case m.IsDir(): | ||
if vol.VolAccessType != mountAccess { | ||
return nil, status.Errorf(codes.InvalidArgument, "Volume %s is not a directory", volID) | ||
} | ||
case m&os.ModeDevice != 0: | ||
if vol.VolAccessType != blockAccess { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The block and character access are two different types right? |
||
return nil, status.Errorf(codes.InvalidArgument, "Volume %s is not a block device", volID) | ||
} | ||
default: | ||
return nil, status.Errorf(codes.InvalidArgument, "Volume %s is invalid", volID) | ||
} | ||
|
||
return &csi.NodeExpandVolumeResponse{}, nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should also verify if provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deployment spec should be updated to add the external-resizer sidecar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to release a new version of the driver before updating the manifest, otherwise we'll break the deployment script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #90 for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, how does this break the deployment script? The pull jobs running in this PR should be testing a custom built image of the hostpath driver that includes your change.
And before we tag a new image, when the CI jobs run, it will run the older version of the driver that doesn't implement expand volume. Does that break the resizer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, the resizer fails to start because the version of driver specified in the manifest doesn't implement expand volume.