Merge pull request #138 from mauriciopoppe/volume-api-changes

Update Volume API

Author: k8s-ci-robot

client/api/volume/v1beta3/api.pb.go

@@ -5,128 +5,130 @@ package v1beta3;
 option go_package = "github.com/kubernetes-csi/csi-proxy/client/api/volume/v1beta3";
 
 service Volume {
-    // ListVolumesOnDisk returns the volume IDs (in \\.\Volume{GUID} format) for
-    // all volumes on a Disk device
+    // ListVolumesOnDisk returns the volume IDs (in \\.\Volume{GUID} format) for all volumes from a
+    // given disk number and partition number (optional)
     rpc ListVolumesOnDisk(ListVolumesOnDiskRequest) returns (ListVolumesOnDiskResponse) {}
-    // MountVolume mounts the volume at the requested global staging path
+    // MountVolume mounts the volume at the requested global staging path.
     rpc MountVolume(MountVolumeRequest) returns (MountVolumeResponse) {}
-    // DismountVolume gracefully dismounts a volume
-    rpc DismountVolume(DismountVolumeRequest) returns (DismountVolumeResponse) {}
-    // IsVolumeFormatted checks if a volume is formatted with NTFS
+    // UnmountVolume flushes data cache to disk and removes the global staging path.
+    rpc UnmountVolume(UnmountVolumeRequest) returns (UnmountVolumeResponse) {}
+    // IsVolumeFormatted checks if a volume is formatted.
     rpc IsVolumeFormatted(IsVolumeFormattedRequest) returns (IsVolumeFormattedResponse) {}
-    // FormatVolume formats a volume with the provided file system
+    // FormatVolume formats a volume with NTFS.
     rpc FormatVolume(FormatVolumeRequest) returns (FormatVolumeResponse) {}
-    // ResizeVolume performs resizing of the partition and file system for a block based volume
+    // ResizeVolume performs resizing of the partition and file system for a block based volume.
     rpc ResizeVolume(ResizeVolumeRequest) returns (ResizeVolumeResponse) {}
-    // VolumeStats gathers DiskSize, VolumeSize and VolumeUsedSize for a volume
-    rpc VolumeStats(VolumeStatsRequest) returns (VolumeStatsResponse) {}
-    // GetVolumeDiskNumber gets the disk number of the disk where the volume is located
-    rpc GetVolumeDiskNumber(VolumeDiskNumberRequest) returns (VolumeDiskNumberResponse) {}
-    // GetVolumeIDFromMount gets the volume id for a given mount
-    rpc GetVolumeIDFromMount(VolumeIDFromMountRequest) returns (VolumeIDFromMountResponse) {}
-    // WriteVolumeCache write volume cache to disk
+    // GetVolumeStats gathers total bytes and used bytes for a volume.
+    rpc GetVolumeStats(GetVolumeStatsRequest) returns (GetVolumeStatsResponse) {}
+    // GetDiskNumberFromVolumeID gets the disk number of the disk where the volume is located.
+    rpc GetDiskNumberFromVolumeID(GetDiskNumberFromVolumeIDRequest) returns (GetDiskNumberFromVolumeIDResponse ) {}
+    // GetVolumeIDFromTargetPath gets the volume id for a given target path.
+    rpc GetVolumeIDFromTargetPath(GetVolumeIDFromTargetPathRequest) returns (GetVolumeIDFromTargetPathResponse) {}
+    // WriteVolumeCache write volume cache to disk.
     rpc WriteVolumeCache(WriteVolumeCacheRequest) returns (WriteVolumeCacheResponse) {}
 }
 
 message ListVolumesOnDiskRequest {
-    // Disk device ID of the disk to query for volumes
-    string disk_id = 1;
+    // Disk device number of the disk to query for volumes.
+    uint32 disk_number = 1;
+    // The partition number (optional), by default it uses the first partition of the disk.
+    uint32 partition_number = 2;
 }
 
 message ListVolumesOnDiskResponse {
-    // Volume device IDs of volumes on the specified disk
+    // Volume device IDs of volumes on the specified disk.
     repeated string volume_ids = 1;
 }
 
 message MountVolumeRequest {
-    // Volume device ID of the volume to mount
+    // Volume device ID of the volume to mount.
     string volume_id = 1;
-    // Path in the host's file system where the volume needs to be mounted
-    string path = 2;
+    // Path in the host's file system where the volume needs to be mounted.
+    string target_path = 2;
 }
 
 message MountVolumeResponse {
-    // Intentionally empty
+    // Intentionally empty.
 }
 
-message DismountVolumeRequest {
-    // Volume device ID of the volume to dismount
+message UnmountVolumeRequest {
+    // Volume device ID of the volume to dismount.
     string volume_id = 1;
     // Path where the volume has been mounted.
-    string path = 2;
+    string target_path = 2;
 }
 
-message DismountVolumeResponse {
-    // Intentionally empty
+message UnmountVolumeResponse {
+    // Intentionally empty.
 }
 
 message IsVolumeFormattedRequest {
-    // Volume device ID of the volume to check
+    // Volume device ID of the volume to check.
     string volume_id = 1;
 }
 
 message IsVolumeFormattedResponse {
-    // Is the volume formatted with NTFS
+    // Is the volume formatted with NTFS.
     bool formatted = 1;
 }
 
 message FormatVolumeRequest {
-    // Volume device ID of the volume to format
+    // Volume device ID of the volume to format.
     string volume_id = 1;
 }
 
 message FormatVolumeResponse {
-    // Intentionally empty
+    // Intentionally empty.
 }
 
 message ResizeVolumeRequest {
-    // Volume device ID of the volume to dismount
+    // Volume device ID of the volume to resize.
     string volume_id = 1;
-    // New size of the volume
-    int64 size = 2;
+    // New size in bytes of the volume.
+    int64 size_bytes = 2;
 }
 
 message ResizeVolumeResponse {
-    // Intentionally empty
+    // Intentionally empty.
 }
 
-message VolumeStatsRequest{
-    // Volume device Id of the volume to get the stats for
+message GetVolumeStatsRequest{
+    // Volume device Id of the volume to get the stats for.
     string volume_id = 1;
 }
 
-message VolumeStatsResponse{
-    // Capacity of the volume
-    int64 volumeSize = 1;
+message GetVolumeStatsResponse{
+    // Total bytes
+    int64 total_bytes = 1;
     // Used bytes
-    int64 volumeUsedSize = 2;
+    int64 used_bytes = 2;
 }
 
-message VolumeDiskNumberRequest{
-    // Volume device Id of the volume to get the disk number for
+message GetDiskNumberFromVolumeIDRequest {
+    // Volume device ID of the volume to get the disk number for.
     string volume_id = 1;
 }
 
-message VolumeDiskNumberResponse{
-    // Corresponding disk number
-    int64 diskNumber = 1;
+message GetDiskNumberFromVolumeIDResponse {
+    // Corresponding disk number.
+    uint32 disk_number = 1;
 }
 
-message VolumeIDFromMountRequest {
-    // Mount
-    string mount = 1;
+message GetVolumeIDFromTargetPathRequest {
+    // The target path.
+    string target_path = 1;
 }
 
-message VolumeIDFromMountResponse {
-    // Mount
+message GetVolumeIDFromTargetPathResponse {
+    // The volume device ID.
     string volume_id = 1;
 }
 
 message WriteVolumeCacheRequest {
-    // Volume device ID of the volume to flush the cache
+    // Volume device ID of the volume to flush the cache.
     string volume_id = 1;
 }
 
 message WriteVolumeCacheResponse {
-    // Intentionally empty
+    // Intentionally empty.
 }

client/api/volume/v1beta3/api.proto

@@ -1,12 +1,12 @@
 package integrationtests
 
 import (
-	"fmt"
-	"io/ioutil"
 	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
 	"testing"
 
-	"github.com/kubernetes-csi/csi-proxy/cmd/csi-proxy-api-gen/generators"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -18,16 +18,28 @@ import (
 
 // TestNewAPIGroup tests that bootstraping a new group works as intended.
 func TestNewAPIGroup(t *testing.T) {
-	skipTestOnCondition(t, isRunningOnGhActions())
+	// TODO(mauriciopoppe): this test about the generator isn't working at all
+	// the generator looks like it's working but the steps to make the diffs between the actual and desired
+	// aren't correct, on top of that the generator desired files are out of date
+	t.Skip("Skipping csi-api-generator test (ref 139#)")
+
 	// clean slate
 	require.Nil(t, os.RemoveAll("csiapigen/new_group/actual_output"))
-	logLevel := "3"
-	stdout, _ := runGenerator(t, "TestNewAPIGroup",
+
+	// check that the csi-proxy-api-gen binary exists
+	_, b, _, _ := runtime.Caller(0)
+	csiAPIGenPath := filepath.Join(filepath.Dir(b), "../build/csi-proxy-api-gen")
+	_, err := os.Lstat(csiAPIGenPath)
+	require.Truef(t, err == nil, "expected err=nil, instead got err=%+v", err)
+
+	// run the generator
+	stdout := runGenerator(t, csiAPIGenPath, "TestNewAPIGroup",
 		"--input-dirs", "github.com/kubernetes-csi/csi-proxy/integrationtests/csiapigen/new_group/api",
 		// might as well check that logging CLI args work as expected
-		"-v", logLevel)
+		"-v=3")
 
-	assert.Contains(t, stdout, "Verbosity level set to "+logLevel)
+	assert.Contains(t, stdout, "Verbosity level set to 3")
+	assert.Contains(t, stdout, "Generation successful!")
 
 	// now check the generated files are exactly what we expect
 	// the files in expected_output have had their `.go` extension changed to `go_code` so that one
@@ -46,64 +58,15 @@ func TestNewAPIGroup(t *testing.T) {
 // runGenerator runs csi-proxy-api-gen with the given CLI args, and returns stdout and stderr.
 // It will also fail the test immediately if there any panics during the generation (but
 // will handle those graciously).
-func runGenerator(t *testing.T, testName string, cliArgs ...string) (string, string) {
-	stdoutFile, err := ioutil.TempFile("", "test-csi-proxy-api-gen-stdout-"+testName)
-	require.Nil(t, err)
-	stderrFile, err := ioutil.TempFile("", "test-csi-proxy-api-gen-stderr-"+testName)
-	require.Nil(t, err)
-
-	oldStdout := os.Stdout
-	oldStderr := os.Stderr
-	os.Stdout = stdoutFile
-	os.Stderr = stderrFile
-
-	restored := false
-
-	restoreStdOutAndErr := func() {
-		if restored {
-			return
-		}
-		restored = true
-
-		os.Stdout = oldStdout
-		os.Stderr = oldStderr
-
-		assert.Nil(t, stdoutFile.Close())
-		assert.Nil(t, stderrFile.Close())
+func runGenerator(t *testing.T, csiAPIGenPath string, testName string, cliArgs ...string) string {
+	// run generator through powershell
+	cmd := exec.Command(csiAPIGenPath, cliArgs...)
+	t.Logf("executing command %q", cmd.String())
+	out, err := cmd.CombinedOutput()
+	t.Logf("%s", out)
+	if err != nil {
+		t.Fatalf("command %q failed with err=%+v", cmd.String(), err)
 	}
 
-	defer func() {
-		restoreStdOutAndErr()
-
-		panicErr := recover()
-		failedErrorMsg := ""
-		if panicErr != nil {
-			failedErrorMsg = fmt.Sprintf("panic when generating code: %v\n", panicErr)
-
-			readLogFile := func(logFile *os.File) string {
-				contents, err := ioutil.ReadFile(logFile.Name())
-				if err != nil {
-					return fmt.Sprintf("<unable to read: %v>", err)
-				}
-				return string(contents)
-			}
-
-			failedErrorMsg += fmt.Sprintf("stdout:\n%s\n", readLogFile(stdoutFile))
-			failedErrorMsg += fmt.Sprintf("stderr:\n%s\n", readLogFile(stderrFile))
-		}
-
-		assert.Nil(t, os.Remove(stdoutFile.Name()))
-		assert.Nil(t, os.Remove(stderrFile.Name()))
-
-		require.Fail(t, failedErrorMsg)
-	}()
-
-	// show time
-	generators.Execute(testName, cliArgs...)
-
-	// to flush & close the log files
-	restoreStdOutAndErr()
-
-	return readFile(t, readFile(t, stdoutFile.Name())),
-		readFile(t, readFile(t, stderrFile.Name()))
+	return string(out)
 }