Skip to content

Commit 5c1e6e8

Browse files
Added filtering for incorrect slo boosting logs
1 parent 28f5cc8 commit 5c1e6e8

File tree

3 files changed

+145
-0
lines changed

3 files changed

+145
-0
lines changed

pkg/gce-cloud-provider/compute/gce.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"net/http"
2222
"net/url"
2323
"os"
24+
"regexp"
2425
"runtime"
2526
"sync"
2627
"time"
@@ -441,3 +442,27 @@ func IsGCENotFoundError(err error) bool {
441442
func IsGCEInvalidError(err error) bool {
442443
return IsGCEError(err, "invalid")
443444
}
445+
446+
// ErrorContainsRegex checks if the error message contains a specific regex pattern.
447+
func ErrorContainsRegex(err error, regexPattern string) bool {
448+
if err == nil {
449+
return false
450+
}
451+
re, compileErr := regexp.Compile(regexPattern)
452+
if compileErr != nil {
453+
fmt.Printf("Error compiling regex '%s': %v\n", regexPattern, compileErr)
454+
return false
455+
}
456+
errorMessage := err.Error()
457+
return re.MatchString(errorMessage)
458+
}
459+
460+
// IsSnapshotAlreadyExistsError checks if the error is a snapshot already exists error.
461+
func IsSnapshotAlreadyExistsError(err error) bool {
462+
return ErrorContainsRegex(err, "The resource '[^']+' already exists")
463+
}
464+
465+
// IsGCPOrgViolationError checks if the error is a GCP organization policy violation error.
466+
func IsGCPOrgViolationError(err error) bool {
467+
return ErrorContainsRegex(err, "violates constraint constraints/gcp.")
468+
}

pkg/gce-cloud-provider/compute/gce_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import (
3030

3131
"google.golang.org/api/compute/v1"
3232
"google.golang.org/api/googleapi"
33+
"google.golang.org/grpc/codes"
34+
"google.golang.org/grpc/status"
3335
)
3436

3537
type mockTokenSource struct{}
@@ -101,6 +103,114 @@ func TestIsGCEError(t *testing.T) {
101103
}
102104
}
103105

106+
func TestErrorContainsRegex(t *testing.T) {
107+
testCases := []struct {
108+
name string
109+
inputErr error
110+
regex string
111+
expectedResult bool
112+
}{
113+
{
114+
name: "regex found in error message",
115+
inputErr: errors.New("The resource 'my-resource' already exists"),
116+
regex: "The resource '[^']+' already exists",
117+
expectedResult: true,
118+
},
119+
{
120+
name: "wrapped valid error message",
121+
inputErr: status.Errorf(codes.AlreadyExists, "Snapshot already exists: %v", errors.New("The resource 'my-resource' already exists")),
122+
regex: "The resource '[^']+' already exists",
123+
expectedResult: true,
124+
},
125+
{
126+
name: "regex not found in error message",
127+
inputErr: errors.New("The resource 'my-resource' does not exist"),
128+
regex: "The resource '[^']+' already exists",
129+
expectedResult: false,
130+
},
131+
{
132+
name: "empty",
133+
inputErr: errors.New(" "),
134+
regex: "The resource '[^']+' already exists",
135+
expectedResult: false,
136+
},
137+
{
138+
name: "empty 2",
139+
inputErr: errors.New(""),
140+
regex: "The resource '[^']+' already exists",
141+
expectedResult: false,
142+
},
143+
{
144+
name: "nil",
145+
inputErr: nil,
146+
regex: "The resource '[^']+' already exists",
147+
expectedResult: false,
148+
},
149+
}
150+
151+
for _, tc := range testCases {
152+
t.Logf("Running test: %v", tc.name)
153+
result := ErrorContainsRegex(tc.inputErr, tc.regex)
154+
if tc.expectedResult != result {
155+
t.Fatalf("Got '%t', expected '%t'", result, tc.expectedResult)
156+
}
157+
}
158+
}
159+
160+
func TestErrorIsGCPViolationRegex(t *testing.T) {
161+
testCases := []struct {
162+
name string
163+
inputErr error
164+
expectedResult bool
165+
}{
166+
{
167+
name: "is gcp org violation error",
168+
inputErr: errors.New("Your api request violates constraint constraints/gcp.resourceLocations"),
169+
expectedResult: true,
170+
},
171+
{
172+
name: "is not gcp org violation error",
173+
inputErr: errors.New("Some incorrect error message"),
174+
expectedResult: false,
175+
},
176+
}
177+
178+
for _, tc := range testCases {
179+
t.Logf("Running test: %v", tc.name)
180+
result := IsGCPOrgViolationError(tc.inputErr)
181+
if tc.expectedResult != result {
182+
t.Fatalf("Got '%t', expected '%t'", result, tc.expectedResult)
183+
}
184+
}
185+
}
186+
187+
func TestErrorIsSnapshotExistsError(t *testing.T) {
188+
testCases := []struct {
189+
name string
190+
inputErr error
191+
expectedResult bool
192+
}{
193+
{
194+
name: "is gcp org violation error",
195+
inputErr: errors.New("Your api request violates constraint constraints/gcp.resourceLocations"),
196+
expectedResult: true,
197+
},
198+
{
199+
name: "is not gcp org violation error",
200+
inputErr: errors.New("Some incorrect error message"),
201+
expectedResult: false,
202+
},
203+
}
204+
205+
for _, tc := range testCases {
206+
t.Logf("Running test: %v", tc.name)
207+
result := IsGCPOrgViolationError(tc.inputErr)
208+
if tc.expectedResult != result {
209+
t.Fatalf("Got '%t', expected '%t'", result, tc.expectedResult)
210+
}
211+
}
212+
}
213+
104214
func TestGetComputeVersion(t *testing.T) {
105215
testCases := []struct {
106216
name string

pkg/gce-pd-csi-driver/controller.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,6 +1668,16 @@ func (gceCS *GCEControllerServer) createPDSnapshot(ctx context.Context, project
16681668
if gce.IsGCEError(err, "notFound") {
16691669
return nil, status.Errorf(codes.NotFound, "Could not find volume with ID %v: %v", volKey.String(), err.Error())
16701670
}
1671+
1672+
// Identified as incorrect error handling in https://b.corp.google.com/issues/398008774
1673+
if gce.IsSnapshotAlreadyExistsError(err) {
1674+
return nil, status.Errorf(codes.AlreadyExists, "Snapshot already exists: %v", err.Error())
1675+
}
1676+
1677+
// Identified as incorrect error handling in https://b.corp.google.com/issues/398008774
1678+
if gce.IsGCPOrgViolationError(err) {
1679+
return nil, status.Errorf(codes.FailedPrecondition, "Violates GCP org policy: %v", err.Error())
1680+
}
16711681
return nil, common.LoggedError("Failed to create snapshot: ", err)
16721682
}
16731683
}

0 commit comments

Comments
 (0)