Skip to content

Commit b1be551

Browse files
authored
Surface NotSupported error message during AttachVolume call (#3595)
Signed-off-by: Deepak Kinni <[email protected]>
1 parent caa29aa commit b1be551

File tree

2 files changed

+150
-1
lines changed

2 files changed

+150
-1
lines changed

pkg/common/cns-lib/volume/manager.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ func (m *defaultManager) AttachVolume(ctx context.Context,
11191119
}
11201120

11211121
volumeOperationRes := taskResult.GetCnsVolumeOperationResult()
1122-
if volumeOperationRes.Fault != nil {
1122+
if volumeOperationRes.Fault != nil && volumeOperationRes.Fault.Fault != nil {
11231123
faultType = ExtractFaultTypeFromVolumeResponseResult(ctx, volumeOperationRes)
11241124
_, isResourceInUseFault := volumeOperationRes.Fault.Fault.(*vim25types.ResourceInUse)
11251125
if isResourceInUseFault {
@@ -1133,6 +1133,37 @@ func (m *defaultManager) AttachVolume(ctx context.Context,
11331133
return diskUUID, "", nil
11341134
}
11351135
}
1136+
1137+
// Check if this is a CnsFault with NotSupported fault cause
1138+
if cnsFault, isCnsFault := volumeOperationRes.Fault.Fault.(*cnstypes.CnsFault); isCnsFault {
1139+
if cnsFault.FaultCause != nil {
1140+
notSupportedFault, isNotSupportedFault := cnsFault.FaultCause.Fault.(*vim25types.NotSupported)
1141+
if isNotSupportedFault {
1142+
log.Infof("observed CnsFault with NotSupported fault cause while attaching volume: %q with vm: %q",
1143+
volumeID, vm.String())
1144+
1145+
// Extract the specific error message from NotSupported fault's FaultMessage array
1146+
var errorMessages []string
1147+
for _, faultMsg := range notSupportedFault.FaultMessage {
1148+
if faultMsg.Message != "" {
1149+
errorMessages = append(errorMessages, faultMsg.Message)
1150+
}
1151+
}
1152+
1153+
if len(errorMessages) > 0 {
1154+
extractedMessage := strings.Join(errorMessages, " - ")
1155+
log.Infof("NotSupported fault extracted message: %s", extractedMessage)
1156+
return "", faultType, logger.LogNewErrorf(log,
1157+
"%q Failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
1158+
extractedMessage, volumeID, vm.String(), spew.Sdump(volumeOperationRes.Fault), taskInfo.ActivationId)
1159+
}
1160+
1161+
// Fallback to detailed dump for debugging
1162+
log.Debugf("NotSupported fault details: %+v", spew.Sdump(cnsFault.FaultCause))
1163+
}
1164+
}
1165+
}
1166+
11361167
return "", faultType, logger.LogNewErrorf(log, "failed to attach cns volume: %q to node vm: %q. fault: %q. opId: %q",
11371168
volumeID, vm.String(), spew.Sdump(volumeOperationRes.Fault), taskInfo.ActivationId)
11381169
}

pkg/common/cns-lib/volume/manager_test.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package volume
22

33
import (
44
"context"
5+
"strings"
56
"testing"
67
"time"
78

89
"github.com/stretchr/testify/assert"
10+
cnstypes "github.com/vmware/govmomi/cns/types"
911
vim25types "github.com/vmware/govmomi/vim25/types"
1012
)
1113

@@ -50,3 +52,119 @@ func performSlowTask(ch chan TaskResult, delay time.Duration) {
5052
Err: nil,
5153
}
5254
}
55+
56+
// Test CnsFault with NotSupported fault cause detection
57+
func TestCnsFaultNotSupportedDetection(t *testing.T) {
58+
tests := []struct {
59+
name string
60+
fault vim25types.BaseMethodFault
61+
expectCnsFault bool
62+
expectNotSupported bool
63+
expectedMessage string
64+
}{
65+
{
66+
name: "CnsFault with NotSupported cause",
67+
fault: &cnstypes.CnsFault{
68+
MethodFault: vim25types.MethodFault{
69+
FaultCause: &vim25types.LocalizedMethodFault{
70+
LocalizedMessage: "The operation is not supported on the object.",
71+
Fault: &vim25types.NotSupported{
72+
RuntimeFault: vim25types.RuntimeFault{
73+
MethodFault: vim25types.MethodFault{
74+
FaultMessage: []vim25types.LocalizableMessage{
75+
{
76+
Message: "Attaching mutually shared fcd disks to same VM is not supported.",
77+
},
78+
},
79+
},
80+
},
81+
},
82+
},
83+
},
84+
Reason: "VSLM task failed",
85+
},
86+
expectCnsFault: true,
87+
expectNotSupported: true,
88+
expectedMessage: "Attaching mutually shared fcd disks to same VM is not supported.",
89+
},
90+
{
91+
name: "CnsFault with NotSupported cause - multiple messages",
92+
fault: &cnstypes.CnsFault{
93+
MethodFault: vim25types.MethodFault{
94+
FaultCause: &vim25types.LocalizedMethodFault{
95+
LocalizedMessage: "The operation is not supported on the object.",
96+
Fault: &vim25types.NotSupported{
97+
RuntimeFault: vim25types.RuntimeFault{
98+
MethodFault: vim25types.MethodFault{
99+
FaultMessage: []vim25types.LocalizableMessage{
100+
{
101+
Message: "First error message.",
102+
},
103+
{
104+
Message: "Second error message.",
105+
},
106+
},
107+
},
108+
},
109+
},
110+
},
111+
},
112+
Reason: "VSLM task failed",
113+
},
114+
expectCnsFault: true,
115+
expectNotSupported: true,
116+
expectedMessage: "First error message. - Second error message.",
117+
},
118+
{
119+
name: "CnsFault without NotSupported cause",
120+
fault: &cnstypes.CnsFault{
121+
MethodFault: vim25types.MethodFault{
122+
FaultMessage: []vim25types.LocalizableMessage{
123+
{Message: "Some other error"},
124+
},
125+
},
126+
Reason: "VSLM task failed",
127+
},
128+
expectCnsFault: true,
129+
expectNotSupported: false,
130+
},
131+
{
132+
name: "Non-CnsFault",
133+
fault: &vim25types.MethodFault{
134+
FaultMessage: []vim25types.LocalizableMessage{
135+
{Message: "Generic fault"},
136+
},
137+
},
138+
expectCnsFault: false,
139+
expectNotSupported: false,
140+
},
141+
}
142+
143+
for _, tt := range tests {
144+
t.Run(tt.name, func(t *testing.T) {
145+
// Test type assertions that would be used in the AttachVolume function
146+
cnsFault, isCnsFault := tt.fault.(*cnstypes.CnsFault)
147+
assert.Equal(t, tt.expectCnsFault, isCnsFault)
148+
149+
if isCnsFault && cnsFault.FaultCause != nil {
150+
notSupportedFault, isNotSupportedFault := cnsFault.FaultCause.Fault.(*vim25types.NotSupported)
151+
assert.Equal(t, tt.expectNotSupported, isNotSupportedFault)
152+
153+
if isNotSupportedFault && tt.expectedMessage != "" {
154+
// Test message extraction logic - only extract from FaultMessage array
155+
var errorMessages []string
156+
for _, faultMsg := range notSupportedFault.FaultMessage {
157+
if faultMsg.Message != "" {
158+
errorMessages = append(errorMessages, faultMsg.Message)
159+
}
160+
}
161+
162+
if len(errorMessages) > 0 {
163+
extractedMessage := strings.Join(errorMessages, " - ")
164+
assert.Equal(t, tt.expectedMessage, extractedMessage)
165+
}
166+
}
167+
}
168+
})
169+
}
170+
}

0 commit comments

Comments
 (0)