diff --git a/pkg/volume/csi/csi_client.go b/pkg/volume/csi/csi_client.go index 108bfb93bbb..79731928033 100644 --- a/pkg/volume/csi/csi_client.go +++ b/pkg/volume/csi/csi_client.go @@ -612,10 +612,7 @@ func (c *csiDriverClient) NodeGetVolumeStats(ctx context.Context, volID string, if err != nil { return nil, err } - usages := resp.GetUsage() - if usages == nil { - return nil, fmt.Errorf("failed to get usage from response. usage is nil") - } + metrics := &volume.Metrics{ Used: resource.NewQuantity(int64(0), resource.BinarySI), Capacity: resource.NewQuantity(int64(0), resource.BinarySI), @@ -625,8 +622,9 @@ func (c *csiDriverClient) NodeGetVolumeStats(ctx context.Context, volID string, InodesFree: resource.NewQuantity(int64(0), resource.BinarySI), } + var isSupportNodeVolumeCondition bool if utilfeature.DefaultFeatureGate.Enabled(features.CSIVolumeHealth) { - isSupportNodeVolumeCondition, err := c.nodeSupportsVolumeCondition(ctx) + isSupportNodeVolumeCondition, err = c.nodeSupportsVolumeCondition(ctx) if err != nil { return nil, err } @@ -637,6 +635,12 @@ func (c *csiDriverClient) NodeGetVolumeStats(ctx context.Context, volID string, } } + usages := resp.GetUsage() + // If the driver does not support volume condition and usages is nil, return an error + if !isSupportNodeVolumeCondition && usages == nil { + return nil, fmt.Errorf("failed to get usage from response. usage is nil") + } + for _, usage := range usages { if usage == nil { continue diff --git a/pkg/volume/csi/csi_client_test.go b/pkg/volume/csi/csi_client_test.go index 7b45ca7c198..239925ed1e7 100644 --- a/pkg/volume/csi/csi_client_test.go +++ b/pkg/volume/csi/csi_client_test.go @@ -66,10 +66,10 @@ func newFakeCsiDriverClientWithVolumeStats(t *testing.T, volumeStatsSet bool) *f } } -func newFakeCsiDriverClientWithVolumeStatsAndCondition(t *testing.T, volumeStatsSet, volumeConditionSet bool) *fakeCsiDriverClient { +func newFakeCsiDriverClientWithVolumeStatsAndCondition(t *testing.T, volumeStatsSet, volumeConditionSet, setVolumeStat, setVolumeCondition bool) *fakeCsiDriverClient { return &fakeCsiDriverClient{ t: t, - nodeClient: fake.NewNodeClientWithVolumeStatsAndCondition(volumeStatsSet, volumeConditionSet), + nodeClient: fake.NewNodeClientWithVolumeStatsAndCondition(volumeStatsSet, volumeConditionSet, setVolumeStat, setVolumeCondition), } } @@ -100,18 +100,23 @@ func (c *fakeCsiDriverClient) NodeGetVolumeStats(ctx context.Context, volID stri VolumeId: volID, VolumePath: targetPath, } + fakeResp := &csipbv1.NodeGetVolumeStatsResponse{} + if c.nodeClient.SetVolumeStats { + fakeResp = getRawVolumeInfo() + } + if c.nodeClient.SetVolumecondition { + fakeResp.VolumeCondition = &csipbv1.VolumeCondition{ + Abnormal: true, + Message: "Volume is abnormal", + } + } + c.nodeClient.SetNodeVolumeStatsResp(fakeResp) - c.nodeClient.SetNodeVolumeStatsResp(getRawVolumeInfo()) resp, err := c.nodeClient.NodeGetVolumeStats(ctx, req) if err != nil { return nil, err } - usages := resp.GetUsage() - if usages == nil { - return nil, nil - } - metrics := &volume.Metrics{} isSupportNodeVolumeCondition, err := c.nodeSupportsVolumeCondition(ctx) @@ -124,6 +129,10 @@ func (c *fakeCsiDriverClient) NodeGetVolumeStats(ctx context.Context, volID stri metrics.Abnormal, metrics.Message = &abnormal, &message } + usages := resp.GetUsage() + if !isSupportNodeVolumeCondition && usages == nil { + return nil, errors.New("volume usage is nil") + } for _, usage := range usages { if usage == nil { continue @@ -377,8 +386,8 @@ func setupClientWithExpansion(t *testing.T, stageUnstageSet bool, expansionSet b return newFakeCsiDriverClientWithExpansion(t, stageUnstageSet, expansionSet) } -func setupClientWithVolumeStatsAndCondition(t *testing.T, volumeStatsSet, volumeConditionSet bool) csiClient { - return newFakeCsiDriverClientWithVolumeStatsAndCondition(t, volumeStatsSet, volumeConditionSet) +func setupClientWithVolumeStatsAndCondition(t *testing.T, volumeStatsSet, volumeConditionSet, setVolumeStat, setVolumecondition bool) csiClient { + return newFakeCsiDriverClientWithVolumeStatsAndCondition(t, volumeStatsSet, volumeConditionSet, setVolumeStat, setVolumecondition) } func setupClientWithVolumeStats(t *testing.T, volumeStatsSet bool) csiClient { @@ -839,13 +848,17 @@ func TestVolumeHealthEnable(t *testing.T) { tests := []struct { name string volumeStatsSet bool + setVolumeStat bool + setVolumecondition bool volumeConditionSet bool volumeData VolumeStatsOptions success bool }{ { - name: "when nodeVolumeStats=on, VolumeID=on, DeviceMountPath=on, VolumeCondition=on", + name: "when nodeVolumeStats=on, volumeStatsSet=on, setVolumeStat=on, volumeCondition=on, setVolumecondition=on", volumeStatsSet: true, + setVolumeStat: true, + setVolumecondition: true, volumeConditionSet: true, volumeData: VolumeStatsOptions{ VolumeSpec: spec, @@ -855,8 +868,10 @@ func TestVolumeHealthEnable(t *testing.T) { success: true, }, { - name: "when nodeVolumeStats=on, VolumeID=on, DeviceMountPath=on, VolumeCondition=off", + name: "when nodeVolumeStats=on, volumeStatsSet=on, setVolumeStat=on, volumeCondition=off, setVolumecondition=off", volumeStatsSet: true, + setVolumeStat: true, + setVolumecondition: false, volumeConditionSet: false, volumeData: VolumeStatsOptions{ VolumeSpec: spec, @@ -865,6 +880,30 @@ func TestVolumeHealthEnable(t *testing.T) { }, success: true, }, + { + name: "when nodeVolumeStats=on, volumeStatsSet=off, setVolumeStat=off, volumeCondition=on, setVolumecondition=on", + volumeStatsSet: false, + setVolumeStat: false, + setVolumecondition: true, + volumeConditionSet: true, + volumeData: VolumeStatsOptions{ + VolumeSpec: spec, + VolumeID: "volume1", + DeviceMountPath: "/foo/bar", + }, + success: true, + }, + { + name: "when nodeVolumeStats=on, volumeStatsSet=off, setVolumeStat=off, volumeCondition=off, setVolumecondition=off", + setVolumeStat: false, + volumeConditionSet: false, + volumeData: VolumeStatsOptions{ + VolumeSpec: spec, + VolumeID: "volume1", + DeviceMountPath: "/foo/bar", + }, + success: false, + }, } for _, tc := range tests { @@ -872,24 +911,24 @@ func TestVolumeHealthEnable(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) defer cancel() csiSource, _ := getCSISourceFromSpec(tc.volumeData.VolumeSpec) - csClient := setupClientWithVolumeStatsAndCondition(t, tc.volumeStatsSet, tc.volumeConditionSet) + csClient := setupClientWithVolumeStatsAndCondition(t, tc.volumeStatsSet, tc.volumeConditionSet, tc.setVolumeStat, tc.setVolumecondition) metrics, err := csClient.NodeGetVolumeStats(ctx, csiSource.VolumeHandle, tc.volumeData.DeviceMountPath) - if tc.success { - assert.Nil(t, err) + if err != nil && tc.success { + t.Errorf("For %s : expected %v got %v", tc.name, tc.success, err) } - - if metrics == nil { - t.Errorf("csi.NodeGetVolumeStats returned nil metrics for volume %s", tc.volumeData.VolumeID) - } else { - if tc.volumeConditionSet { - assert.NotNil(t, metrics.Abnormal) - assert.NotNil(t, metrics.Message) + if tc.success { + if metrics == nil { + t.Errorf("csi.NodeGetVolumeStats returned nil metrics for volume %s", tc.volumeData.VolumeID) } else { - assert.Nil(t, metrics.Abnormal) - assert.Nil(t, metrics.Message) + if tc.volumeConditionSet { + assert.NotNil(t, metrics.Abnormal) + assert.NotNil(t, metrics.Message) + } else { + assert.Nil(t, metrics.Abnormal) + assert.Nil(t, metrics.Message) + } } } - }) } } @@ -919,7 +958,7 @@ func TestVolumeHealthDisable(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) defer cancel() csiSource, _ := getCSISourceFromSpec(tc.volumeData.VolumeSpec) - csClient := setupClientWithVolumeStatsAndCondition(t, tc.volumeStatsSet, false) + csClient := setupClientWithVolumeStatsAndCondition(t, tc.volumeStatsSet, false, true, false) metrics, err := csClient.NodeGetVolumeStats(ctx, csiSource.VolumeHandle, tc.volumeData.DeviceMountPath) if tc.success { assert.Nil(t, err) diff --git a/pkg/volume/csi/fake/fake_client.go b/pkg/volume/csi/fake/fake_client.go index 037beaa4909..865ad6e7b5b 100644 --- a/pkg/volume/csi/fake/fake_client.go +++ b/pkg/volume/csi/fake/fake_client.go @@ -80,6 +80,8 @@ type NodeClient struct { expansionSet bool volumeStatsSet bool volumeConditionSet bool + SetVolumeStats bool + SetVolumecondition bool singleNodeMultiWriterSet bool volumeMountGroupSet bool nodeGetInfoResp *csipb.NodeGetInfoResponse @@ -110,13 +112,16 @@ func NewNodeClientWithExpansion(stageUnstageSet bool, expansionSet bool) *NodeCl func NewNodeClientWithVolumeStats(volumeStatsSet bool) *NodeClient { return &NodeClient{ volumeStatsSet: volumeStatsSet, + SetVolumeStats: true, } } -func NewNodeClientWithVolumeStatsAndCondition(volumeStatsSet, volumeConditionSet bool) *NodeClient { +func NewNodeClientWithVolumeStatsAndCondition(volumeStatsSet, volumeConditionSet, setVolumeStats, setVolumeCondition bool) *NodeClient { return &NodeClient{ volumeStatsSet: volumeStatsSet, volumeConditionSet: volumeConditionSet, + SetVolumeStats: setVolumeStats, + SetVolumecondition: setVolumeCondition, } }