Skip to content

Set MaxVolumesPerNode in NodeGetInfo #240

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

Merged
merged 1 commit into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/gce-cloud-provider/metadata/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
FakeProject = "test-project"
)

var FakeMachineType = "n1-standard-1"

func NewFakeService() MetadataService {
return &fakeServiceManager{}
}
Expand All @@ -41,3 +43,11 @@ func (manager *fakeServiceManager) GetProject() string {
func (manager *fakeServiceManager) GetName() string {
return "test-name"
}

func (manager *fakeServiceManager) GetMachineType() string {
return FakeMachineType
}

func SetMachineType(s string) {
FakeMachineType = s
}
27 changes: 21 additions & 6 deletions pkg/gce-cloud-provider/metadata/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package metadata

import (
"fmt"
"strings"

"cloud.google.com/go/compute/metadata"
)
Expand All @@ -28,13 +29,15 @@ type MetadataService interface {
GetZone() string
GetProject() string
GetName() string
GetMachineType() string
}

type metadataServiceManager struct {
// Current zone the driver is running in
zone string
project string
name string
zone string
project string
name string
machineType string
}

var _ MetadataService = &metadataServiceManager{}
Expand All @@ -52,11 +55,19 @@ func NewMetadataService() (MetadataService, error) {
if err != nil {
return nil, fmt.Errorf("failed to get instance name: %v", err)
}
fullMachineType, err := metadata.Get("instance/machine-type")
if err != nil {
return nil, fmt.Errorf("failed to get machine-type: %v", err)
}
// Response format: "projects/[NUMERIC_PROJECT_ID]/machineTypes/[MACHINE_TYPE]"
splits := strings.Split(fullMachineType, "/")
machineType := splits[len(splits)-1]

return &metadataServiceManager{
project: projectID,
zone: zone,
name: name,
project: projectID,
zone: zone,
name: name,
machineType: machineType,
}, nil
}

Expand All @@ -71,3 +82,7 @@ func (manager *metadataServiceManager) GetProject() string {
func (manager *metadataServiceManager) GetName() string {
return manager.name
}

func (manager *metadataServiceManager) GetMachineType() string {
return manager.machineType
}
28 changes: 26 additions & 2 deletions pkg/gce-pd-csi-driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package gceGCEDriver
import (
"fmt"
"os"
"strings"
"sync"

csi "github.com/container-storage-interface/spec/lib/go/csi"
Expand All @@ -42,6 +43,14 @@ type GCENodeServer struct {

var _ csi.NodeServer = &GCENodeServer{}

// The constants are used to map from the machine type to the limit of
// persistent disks that can be attached to an instance. Please refer to gcloud doc
// https://cloud.google.com/compute/docs/disks/#pdnumberlimits
const (
volumeLimit16 int64 = 16
volumeLimit128 int64 = 128
)

func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
ns.mux.Lock()
defer ns.mux.Unlock()
Expand Down Expand Up @@ -284,15 +293,17 @@ func (ns *GCENodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRe

nodeID := common.CreateNodeID(ns.MetadataService.GetProject(), ns.MetadataService.GetZone(), ns.MetadataService.GetName())

volumeLimits, err := ns.GetVolumeLimits()

resp := &csi.NodeGetInfoResponse{
NodeId: nodeID,
// TODO(#19): Set MaxVolumesPerNode based on Node Type
// Default of 0 means that CO Decides how many nodes can be published
// Can get from metadata server "machine-type"
MaxVolumesPerNode: 0,
MaxVolumesPerNode: volumeLimits,
AccessibleTopology: top,
}
return resp, nil
return resp, err
}

func (ns *GCENodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) {
Expand All @@ -302,3 +313,16 @@ func (ns *GCENodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGe
func (ns *GCENodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) {
return nil, status.Error(codes.Unimplemented, fmt.Sprintf("NodeExpandVolume is not yet implemented"))
}

func (ns *GCENodeServer) GetVolumeLimits() (int64, error) {
var volumeLimits int64

// Machine-type format: n1-type-CPUS or custom-CPUS-RAM or f1/g1-type
machineType := ns.MetadataService.GetMachineType()
if strings.HasPrefix(machineType, "n1-") || strings.HasPrefix(machineType, "custom-") {
volumeLimits = volumeLimit128
} else {
volumeLimits = volumeLimit16
}
return volumeLimits, nil
}
73 changes: 73 additions & 0 deletions pkg/gce-pd-csi-driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,76 @@ limitations under the License.
*/

package gceGCEDriver

import (
"testing"

csi "github.com/container-storage-interface/spec/lib/go/csi"
"golang.org/x/net/context"
metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata"
mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
)

func getTestGCEDriver(t *testing.T) *GCEDriver {
gceDriver := GetGCEDriver()
err := gceDriver.SetupGCEDriver(nil, mountmanager.NewFakeSafeMounter(), mountmanager.NewFakeDeviceUtils(), metadataservice.NewFakeService(), driver, "test-vendor")
if err != nil {
t.Fatalf("Failed to setup GCE Driver: %v", err)
}
return gceDriver
}

func TestNodeGetVolumeLimits(t *testing.T) {

gceDriver := getTestGCEDriver(t)
ns := gceDriver.ns
req := &csi.NodeGetInfoRequest{}

testCases := []struct {
name string
machineType string
expVolumeLimit int64
}{
{
name: "Predifined standard machine",
machineType: "n1-standard-1",
expVolumeLimit: volumeLimit128,
},
{
name: "Predifined micro machine",
machineType: "f1-micro",
expVolumeLimit: volumeLimit16,
},
{
name: "Predifined small machine",
machineType: "g1-small",
expVolumeLimit: volumeLimit16,
},
{
name: "Custom machine with 1GiB Mem",
machineType: "custom-1-1024",
expVolumeLimit: volumeLimit128,
},
{
name: "Custom machine with 4GiB Mem",
machineType: "custom-2-4096",
expVolumeLimit: volumeLimit128,
},
}

for _, tc := range testCases {
t.Logf("Test case: %s", tc.name)
metadataservice.SetMachineType(tc.machineType)
res, err := ns.NodeGetInfo(context.Background(), req)
if err != nil {
t.Fatalf("Failed to get node info: %v", err)
} else {
volumeLimit := res.GetMaxVolumesPerNode()
if volumeLimit != tc.expVolumeLimit {
t.Fatalf("Expected volume limit: %v, got %v, for machine-type: %v",
tc.expVolumeLimit, volumeLimit, tc.machineType)
}
t.Logf("Get node info: %v", res)
}
}
}
19 changes: 14 additions & 5 deletions test/e2e/tests/single_zone_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,24 @@ import (
const (
testNamePrefix = "gcepd-csi-e2e-"

defaultSizeGb int64 = 5
defaultRepdSizeGb int64 = 200
readyState = "READY"
standardDiskType = "pd-standard"
ssdDiskType = "pd-ssd"
defaultSizeGb int64 = 5
defaultRepdSizeGb int64 = 200
readyState = "READY"
standardDiskType = "pd-standard"
ssdDiskType = "pd-ssd"
defaultVolumeLimit int64 = 128
)

var _ = Describe("GCE PD CSI Driver", func() {

It("Should get reasonable volume limits from nodes with NodeGetInfo", func() {
testContext := getRandomTestContext()
resp, err := testContext.Client.NodeGetInfo()
Expect(err).To(BeNil())
volumeLimit := resp.GetMaxVolumesPerNode()
Expect(volumeLimit).To(Equal(defaultVolumeLimit))
})

It("Should create->attach->stage->mount volume and check if it is writable, then unmount->unstage->detach->delete and check disk is deleted", func() {
testContext := getRandomTestContext()

Expand Down