Skip to content

Commit d468f4e

Browse files
ashimagarg27ashimagarg27
authored
Fix Error Code In Pod Events (#244)
* return response if non empty Signed-off-by: Ashima-Ashima1 <[email protected]> * publish tag Signed-off-by: Ashima-Ashima1 <[email protected]> * return response if non empty Signed-off-by: Ashima-Ashima1 <[email protected]> * return response if non empty Signed-off-by: Ashima-Ashima1 <[email protected]> * empty-commit Signed-off-by: Ashima-Ashima1 <[email protected]> * publish v0.5.8 Signed-off-by: Ashima-Ashima1 <[email protected]> * check errors Signed-off-by: Ashima-Ashima1 <[email protected]> * publish v0.5.9 Signed-off-by: Ashima-Ashima1 <[email protected]> * fix error returns Signed-off-by: Ashima-Ashima1 <[email protected]> * publish v0.5.10 Signed-off-by: Ashima-Ashima1 <[email protected]> * fix error returns Signed-off-by: Ashima-Ashima1 <[email protected]> * fix error returns Signed-off-by: Ashima-Ashima1 <[email protected]> * address review comments Signed-off-by: Ashima-Ashima1 <[email protected]> * address review comments Signed-off-by: Ashima-Ashima1 <[email protected]> --------- Signed-off-by: Ashima-Ashima1 <[email protected]>
1 parent 6026fcb commit d468f4e

File tree

6 files changed

+54
-133
lines changed

6 files changed

+54
-133
lines changed

‎.secrets.baseline‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "go.sum|^.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2025-07-28T06:01:06Z",
6+
"generated_at": "2025-07-29T06:33:49Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"
@@ -190,7 +190,7 @@
190190
"hashed_secret": "2ace62c1befa19e3ea37dd52be9f6d508c5163e6",
191191
"is_secret": false,
192192
"is_verified": false,
193-
"line_number": 270,
193+
"line_number": 264,
194194
"type": "Secret Keyword",
195195
"verified_result": null
196196
}

‎pkg/mounter/mounter-rclone.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,9 @@ func (rclone *RcloneMounter) Mount(source string, target string) error {
210210

211211
payload := fmt.Sprintf(`{"path":"%s","bucket":"%s","mounter":"%s","args":%s}`, target, bucketName, constants.RClone, jsonData)
212212

213-
response, err := mounterRequest(payload, "http://unix/api/cos/mount")
214-
klog.Info("Worker Mounting...", response)
213+
err = mounterRequest(payload, "http://unix/api/cos/mount")
215214
if err != nil {
216-
if strings.TrimSpace(response) != "" {
217-
return parseErrFromResponse(response)
218-
}
215+
klog.Error("failed to mount on worker...", err)
219216
return err
220217
}
221218
return nil
@@ -232,12 +229,9 @@ func (rclone *RcloneMounter) Unmount(target string) error {
232229

233230
payload := fmt.Sprintf(`{"path":"%s"}`, target)
234231

235-
response, err := mounterRequest(payload, "http://unix/api/cos/unmount")
236-
klog.Info("Worker Unmounting...", response)
232+
err := mounterRequest(payload, "http://unix/api/cos/unmount")
237233
if err != nil {
238-
if strings.TrimSpace(response) != "" {
239-
return parseErrFromResponse(response)
240-
}
234+
klog.Error("failed to unmount on worker...", err)
241235
return err
242236
}
243237

‎pkg/mounter/mounter-rclone_test.go‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ func TestRcloneMount_WorkerNode_Positive(t *testing.T) {
149149
createConfigWrap = func(_ string, _ *RcloneMounter) error {
150150
return nil
151151
}
152-
mounterRequest = func(_, _ string) (string, error) {
153-
return "", nil
152+
mounterRequest = func(_, _ string) error {
153+
return nil
154154
}
155155

156156
err := rclone.Mount(source, target)
@@ -177,37 +177,15 @@ func TestRcloneMount_WorkerNode_Negative(t *testing.T) {
177177
createConfigWrap = func(_ string, _ *RcloneMounter) error {
178178
return nil
179179
}
180-
mounterRequest = func(_, _ string) (string, error) {
181-
return "", errors.New("failed to create http request")
180+
mounterRequest = func(_, _ string) error {
181+
return errors.New("failed to create http request")
182182
}
183183

184184
err := rclone.Mount(source, target)
185185
assert.Error(t, err)
186186
assert.Contains(t, err.Error(), "failed to create http request")
187187
}
188188

189-
func TestRcloneMount_WorkerNode_FailedToParseError(t *testing.T) {
190-
mountWorker = true
191-
192-
MakeDir = func(path string, perm os.FileMode) error {
193-
return nil
194-
}
195-
writePassWrap = func(_, _ string) error {
196-
return nil
197-
}
198-
mounterRequest = func(_, _ string) (string, error) {
199-
return "{\"error\": \"failed to perform http request\"}", errors.New("error")
200-
}
201-
202-
defer func() { MakeDir = os.MkdirAll }()
203-
204-
rclone := &RcloneMounter{}
205-
206-
err := rclone.Mount(source, target)
207-
assert.Error(t, err)
208-
assert.Contains(t, err.Error(), "failed to perform http request")
209-
}
210-
211189
func TestRcloneUnmount_NodeServer(t *testing.T) {
212190
mountWorker = false
213191

@@ -234,8 +212,8 @@ func TestRcloneUnmount_WorkerNode(t *testing.T) {
234212
},
235213
})}
236214

237-
mounterRequest = func(_, _ string) (string, error) {
238-
return "", nil
215+
mounterRequest = func(_, _ string) error {
216+
return nil
239217
}
240218

241219
err := rclone.Unmount(target)
@@ -251,29 +229,15 @@ func TestRcloneUnmount_WorkerNode_Negative(t *testing.T) {
251229
},
252230
})}
253231

254-
mounterRequest = func(_, _ string) (string, error) {
255-
return "", errors.New("failed to create http request")
232+
mounterRequest = func(_, _ string) error {
233+
return errors.New("failed to create http request")
256234
}
257235

258236
err := rclone.Unmount(target)
259237
assert.Error(t, err)
260238
assert.Contains(t, err.Error(), "failed to create http request")
261239
}
262240

263-
func TestRcloneUnmount_WorkerNode_FailedToParseError(t *testing.T) {
264-
mountWorker = true
265-
266-
mounterRequest = func(_, _ string) (string, error) {
267-
return "{\"error\": \"failed to perform http request\"}", errors.New("error")
268-
}
269-
270-
rclone := &RcloneMounter{}
271-
272-
err := rclone.Unmount(target)
273-
assert.Error(t, err)
274-
assert.Contains(t, err.Error(), "failed to perform http request")
275-
}
276-
277241
func TestRcloneUnmount_NodeServer_Negative(t *testing.T) {
278242
mountWorker = false
279243

‎pkg/mounter/mounter-s3fs.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,9 @@ func (s3fs *S3fsMounter) Mount(source string, target string) error {
164164

165165
klog.Info("Worker Mounting Payload...", payload)
166166

167-
response, err := mounterRequest(payload, "http://unix/api/cos/mount")
168-
klog.Info("Worker Mounting...", response)
167+
err = mounterRequest(payload, "http://unix/api/cos/mount")
169168
if err != nil {
170-
if strings.TrimSpace(response) != "" {
171-
return parseErrFromResponse(response)
172-
}
169+
klog.Error("failed to mount on worker...", err)
173170
return err
174171
}
175172
return nil
@@ -187,12 +184,9 @@ func (s3fs *S3fsMounter) Unmount(target string) error {
187184

188185
payload := fmt.Sprintf(`{"path":"%s"}`, target)
189186

190-
response, err := mounterRequest(payload, "http://unix/api/cos/unmount")
191-
klog.Info("Worker Unmounting...", response)
187+
err := mounterRequest(payload, "http://unix/api/cos/unmount")
192188
if err != nil {
193-
if strings.TrimSpace(response) != "" {
194-
return parseErrFromResponse(response)
195-
}
189+
klog.Error("failed to unmount on worker...", err)
196190
return err
197191
}
198192

‎pkg/mounter/mounter-s3fs_test.go‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ func TestS3FSMount_WorkerNode_Positive(t *testing.T) {
9999
writePassWrap = func(_, _ string) error {
100100
return nil
101101
}
102-
mounterRequest = func(_, _ string) (string, error) {
103-
return "", nil
102+
mounterRequest = func(_, _ string) error {
103+
return nil
104104
}
105105

106106
s3fs := &S3fsMounter{
@@ -152,8 +152,8 @@ func TestS3FSMount_WorkerNode_Negative(t *testing.T) {
152152
writePassWrap = func(_, _ string) error {
153153
return nil
154154
}
155-
mounterRequest = func(_, _ string) (string, error) {
156-
return "", errors.New("failed to perform http request")
155+
mounterRequest = func(_, _ string) error {
156+
return errors.New("failed to perform http request")
157157
}
158158

159159
s3fs := &S3fsMounter{}
@@ -163,28 +163,6 @@ func TestS3FSMount_WorkerNode_Negative(t *testing.T) {
163163
assert.Contains(t, err.Error(), "failed to perform http request")
164164
}
165165

166-
func TestS3FSMount_WorkerNode_FailedToParseError(t *testing.T) {
167-
mountWorker = true
168-
169-
MakeDir = func(path string, perm os.FileMode) error {
170-
return nil
171-
}
172-
writePassWrap = func(_, _ string) error {
173-
return nil
174-
}
175-
mounterRequest = func(_, _ string) (string, error) {
176-
return "{\"error\": \"failed to perform http request\"}", errors.New("error")
177-
}
178-
179-
defer func() { MakeDir = os.MkdirAll }()
180-
181-
s3fs := &S3fsMounter{}
182-
183-
err := s3fs.Mount(source, target)
184-
assert.Error(t, err)
185-
assert.Contains(t, err.Error(), "failed to perform http request")
186-
}
187-
188166
func TestUnmount_NodeServer(t *testing.T) {
189167
mountWorker = false
190168

@@ -211,8 +189,8 @@ func TestUnmount_WorkerNode(t *testing.T) {
211189
},
212190
})}
213191

214-
mounterRequest = func(_, _ string) (string, error) {
215-
return "", nil
192+
mounterRequest = func(_, _ string) error {
193+
return nil
216194
}
217195

218196
err := s3fs.Unmount(target)
@@ -228,29 +206,15 @@ func TestUnmount_WorkerNode_Negative(t *testing.T) {
228206
},
229207
})}
230208

231-
mounterRequest = func(_, _ string) (string, error) {
232-
return "", errors.New("failed to create http request")
209+
mounterRequest = func(_, _ string) error {
210+
return errors.New("failed to create http request")
233211
}
234212

235213
err := s3fs.Unmount(target)
236214
assert.Error(t, err)
237215
assert.Contains(t, err.Error(), "failed to create http request")
238216
}
239217

240-
func TestUnmount_WorkerNode_FailedToParseError(t *testing.T) {
241-
mountWorker = true
242-
243-
mounterRequest = func(_, _ string) (string, error) {
244-
return "{\"error\": \"failed to perform http request\"}", errors.New("error")
245-
}
246-
247-
s3fs := &S3fsMounter{}
248-
249-
err := s3fs.Unmount(target)
250-
assert.Error(t, err)
251-
assert.Contains(t, err.Error(), "failed to perform http request")
252-
}
253-
254218
func TestUnmount_NodeServer_Negative(t *testing.T) {
255219
mountWorker = false
256220

‎pkg/mounter/mounter.go‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func writePass(pwFileName string, pwFileContent string) error {
127127
return nil
128128
}
129129

130-
func createCOSCSIMounterRequest(payload string, url string) (string, error) {
130+
func createCOSCSIMounterRequest(payload string, url string) error {
131131
// Get socket path
132132
socketPath := os.Getenv(constants.COSCSIMounterSocketPathEnv)
133133
if socketPath == "" {
@@ -137,7 +137,7 @@ func createCOSCSIMounterRequest(payload string, url string) (string, error) {
137137

138138
err := isGRPCServerAvailable(socketPath)
139139
if err != nil {
140-
return "", err
140+
return err
141141
}
142142

143143
// Create a custom dialer function for Unix socket connection
@@ -156,12 +156,12 @@ func createCOSCSIMounterRequest(payload string, url string) (string, error) {
156156
// Create POST request
157157
req, err := http.NewRequest("POST", url, strings.NewReader(payload))
158158
if err != nil {
159-
return "", err
159+
return err
160160
}
161161
req.Header.Set("Content-Type", "application/json")
162162
response, err := client.Do(req)
163163
if err != nil {
164-
return "", err
164+
return err
165165
}
166166

167167
defer func() {
@@ -172,40 +172,42 @@ func createCOSCSIMounterRequest(payload string, url string) (string, error) {
172172

173173
body, err := io.ReadAll(response.Body)
174174
if err != nil {
175-
return "", err
175+
return err
176176
}
177177

178178
responseBody := string(body)
179179
klog.Infof("response from cos-csi-mounter -> Response body: %s, Response code: %v", responseBody, response.StatusCode)
180180

181181
if response.StatusCode != http.StatusOK {
182-
return responseBody, fetchGRPCReturnCode(response.StatusCode)
182+
return parseGRPCResponse(response.StatusCode, responseBody)
183183
}
184-
return "", nil
184+
return nil
185185
}
186186

187-
func fetchGRPCReturnCode(code int) error {
187+
// parseGRPCResponse takes both response body and error code and frames error message
188+
func parseGRPCResponse(code int, response string) error {
189+
errMsg := parseErrFromResponse(response)
188190
switch code {
189191
case http.StatusBadRequest:
190-
return status.Error(codes.InvalidArgument, "Invalid Argument")
192+
return status.Error(codes.InvalidArgument, errMsg)
191193
case http.StatusNotFound:
192-
return status.Error(codes.NotFound, "Not Found")
194+
return status.Error(codes.NotFound, errMsg)
193195
case http.StatusConflict:
194-
return status.Error(codes.AlreadyExists, "Already Exists")
196+
return status.Error(codes.AlreadyExists, errMsg)
195197
case http.StatusForbidden:
196-
return status.Error(codes.PermissionDenied, "Permission Denied")
198+
return status.Error(codes.PermissionDenied, errMsg)
197199
case http.StatusTooManyRequests:
198-
return status.Error(codes.ResourceExhausted, "Resource Exhausted")
200+
return status.Error(codes.ResourceExhausted, errMsg)
199201
case http.StatusNotImplemented:
200-
return status.Error(codes.Unimplemented, "Unimplemented")
202+
return status.Error(codes.Unimplemented, errMsg)
201203
case http.StatusInternalServerError:
202-
return status.Error(codes.Internal, "Internal")
204+
return status.Error(codes.Internal, errMsg)
203205
case http.StatusServiceUnavailable:
204-
return status.Error(codes.Unavailable, "Unavailable")
206+
return status.Error(codes.Unavailable, errMsg)
205207
case http.StatusUnauthorized:
206-
return status.Error(codes.Unauthenticated, "Unauthenticated")
208+
return status.Error(codes.Unauthenticated, errMsg)
207209
default:
208-
return status.Error(codes.Unknown, "Unknown")
210+
return status.Error(codes.Unknown, errMsg)
209211
}
210212
}
211213

@@ -222,16 +224,19 @@ func isGRPCServerAvailable(socketPath string) error {
222224
return nil
223225
}
224226

225-
func parseErrFromResponse(response string) error {
227+
// parseErrFromResponse fetches error from responseBody
228+
// e.g. ResponseBody: {"error":"invalid args for mounter: invalid s3fs args decode error: json: unknown field \"unknownkey\""}
229+
// parseErrFromResponse returns "invalid args for mounter: invalid s3fs args decode error: json: unknown field \"unknownkey\"
230+
func parseErrFromResponse(response string) string {
226231
var errFromResp map[string]string
227232
err := json.Unmarshal([]byte(response), &errFromResp)
228233
if err != nil {
229-
klog.Warning("failed to unmarshal response from server")
230-
return errors.New(response)
234+
klog.Warning("failed to unmarshal response from server", err)
235+
return response
231236
}
232237
val, exists := errFromResp["error"]
233238
if !exists {
234-
return errors.New(response)
239+
return response
235240
}
236-
return errors.New(val)
241+
return val
237242
}

0 commit comments

Comments
 (0)