Skip to content

Commit 3642087

Browse files
marceljkmarceljk
committed
implement review findings
1 parent 806c48d commit 3642087

File tree

10 files changed

+194
-149
lines changed

10 files changed

+194
-149
lines changed

‎services/logme/wait/wait.go‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,21 @@ import (
1515
const (
1616
InstanceStatusActive = "active"
1717
InstanceStatusFailed = "failed"
18+
InstanceStatusStopped = "stopped"
19+
InstanceStatusCreating = "creating"
1820
InstanceStatusDeleting = "deleting"
19-
InstanceStateSuccess = "succeeded"
20-
InstanceTypeCreate = "create"
21-
InstanceTypeUpdate = "update"
22-
InstanceTypeDelete = "delete"
21+
InstanceStatusUpdating = "updating"
22+
23+
// Deprecated: InstanceStateSuccess is deprecated and will be removed after 2nd October 2025.
24+
InstanceStateSuccess = "succeeded"
25+
// Deprecated: InstanceStateFailed is deprecated and will be removed after 2nd October 2025.
26+
InstanceStateFailed = "failed"
27+
// Deprecated: InstanceTypeCreate is deprecated and will be removed after 2nd October 2025.
28+
InstanceTypeCreate = "create"
29+
// Deprecated: InstanceTypeUpdate is deprecated and will be removed after 2nd October 2025.
30+
InstanceTypeUpdate = "update"
31+
// Deprecated: InstanceTypeDelete is deprecated and will be removed after 2nd October 2025.
32+
InstanceTypeDelete = "delete"
2333
)
2434

2535
// Interface needed for tests
@@ -39,13 +49,13 @@ func CreateInstanceWaitHandler(ctx context.Context, a APIClientInstanceInterface
3949
if err != nil {
4050
return false, nil, err
4151
}
42-
if s.InstanceId == nil || s.Status == nil {
43-
return false, nil, fmt.Errorf("create failed for instance with id %s. The response is not valid: the instance id or the status are missing", instanceId)
52+
if s.Status == nil {
53+
return false, nil, fmt.Errorf("create failed for instance with id %s. The response is not valid: the status is missing", instanceId)
4454
}
45-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusActive {
55+
switch *s.Status {
56+
case InstanceStatusActive:
4657
return true, s, nil
47-
}
48-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusFailed {
58+
case InstanceStatusFailed:
4959
var failedDescription string
5060
if s.LastOperation != nil && s.LastOperation.Description != nil {
5161
failedDescription = *s.LastOperation.Description
@@ -65,13 +75,13 @@ func PartialUpdateInstanceWaitHandler(ctx context.Context, a APIClientInstanceIn
6575
if err != nil {
6676
return false, nil, err
6777
}
68-
if s.InstanceId == nil || s.Status == nil {
78+
if s.Status == nil {
6979
return false, nil, fmt.Errorf("update failed for instance with id %s. The response is not valid: the instance id or the status are missing", instanceId)
7080
}
71-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusActive {
81+
switch *s.Status {
82+
case InstanceStatusActive:
7283
return true, s, nil
73-
}
74-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusFailed {
84+
case InstanceStatusFailed:
7585
var failedDescription string
7686
if s.LastOperation != nil && s.LastOperation.Description != nil {
7787
failedDescription = *s.LastOperation.Description
@@ -89,13 +99,13 @@ func DeleteInstanceWaitHandler(ctx context.Context, a APIClientInstanceInterface
8999
handler := wait.New(func() (waitFinished bool, response *struct{}, err error) {
90100
s, err := a.GetInstanceExecute(ctx, projectId, instanceId)
91101
if err == nil {
92-
if s.LastOperation == nil || s.Status == nil || s.LastOperation.State == nil || s.LastOperation.Description == nil {
93-
return false, nil, fmt.Errorf("delete failed for instance with id %s. The response is not valid: The status last operation type, description or the state are missing", instanceId)
102+
if s.LastOperation == nil || s.LastOperation.Description == nil || s.Status == nil {
103+
return false, nil, fmt.Errorf("delete failed for instance with id %s. The response is not valid: The status or last operation description are missing", instanceId)
94104
}
95105
if *s.Status != InstanceStatusDeleting {
96106
return false, nil, nil
97107
}
98-
if *s.LastOperation.State == InstanceStateSuccess {
108+
if *s.Status == InstanceStatusActive {
99109
if strings.Contains(*s.LastOperation.Description, "DeleteFailed") || strings.Contains(*s.LastOperation.Description, "failed") {
100110
return true, nil, fmt.Errorf("instance was deleted successfully but has errors: %s", *s.LastOperation.Description)
101111
}

‎services/logme/wait/wait_test.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,21 @@ type apiClientInstanceMocked struct {
2121
resourceDescription string
2222
}
2323

24+
const deleteOperation = "delete"
25+
2426
func (a *apiClientInstanceMocked) GetInstanceExecute(_ context.Context, _, _ string) (*logme.Instance, error) {
2527
if a.getFails {
2628
return nil, &oapierror.GenericOpenAPIError{
2729
StatusCode: 500,
2830
}
2931
}
30-
if a.resourceOperation != nil && *a.resourceOperation == InstanceTypeDelete && a.resourceState == InstanceStateSuccess {
32+
if a.resourceOperation != nil && *a.resourceOperation == deleteOperation && a.resourceState == InstanceStatusActive {
3133
if a.deletionSucceedsWithErrors {
3234
return &logme.Instance{
3335
InstanceId: &a.resourceId,
3436
Status: &a.resourceState,
3537
LastOperation: &logme.InstanceLastOperation{
3638
Description: &a.resourceDescription,
37-
Type: a.resourceOperation,
38-
State: &a.resourceState,
3939
},
4040
}, nil
4141
}
@@ -117,10 +117,9 @@ func TestCreateInstanceWaitHandler(t *testing.T) {
117117
instanceId := "foo-bar"
118118

119119
apiClient := &apiClientInstanceMocked{
120-
getFails: tt.getFails,
121-
resourceId: instanceId,
122-
resourceOperation: utils.Ptr(InstanceTypeCreate),
123-
resourceState: tt.resourceState,
120+
getFails: tt.getFails,
121+
resourceId: instanceId,
122+
resourceState: tt.resourceState,
124123
}
125124

126125
var wantRes *logme.Instance
@@ -187,10 +186,9 @@ func TestUpdateInstanceWaitHandler(t *testing.T) {
187186
instanceId := "foo-bar"
188187

189188
apiClient := &apiClientInstanceMocked{
190-
getFails: tt.getFails,
191-
resourceId: instanceId,
192-
resourceOperation: utils.Ptr(InstanceTypeUpdate),
193-
resourceState: tt.resourceState,
189+
getFails: tt.getFails,
190+
resourceId: instanceId,
191+
resourceState: tt.resourceState,
194192
}
195193

196194
var wantRes *logme.Instance
@@ -229,7 +227,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
229227
desc: "delete_succeeded",
230228
getFails: false,
231229
deleteSucceeedsWithErrors: false,
232-
resourceState: InstanceStateSuccess,
230+
resourceState: InstanceStatusActive,
233231
wantErr: false,
234232
wantResp: true,
235233
},
@@ -244,7 +242,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
244242
{
245243
desc: "delete_succeeds_with_errors",
246244
getFails: false,
247-
resourceState: InstanceStateSuccess,
245+
resourceState: InstanceStatusActive,
248246
deleteSucceeedsWithErrors: true,
249247
resourceDescription: "Deleting resource: cf failed with error: DeleteFailed",
250248
wantErr: true,
@@ -266,7 +264,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
266264
getFails: tt.getFails,
267265
deletionSucceedsWithErrors: tt.deleteSucceeedsWithErrors,
268266
resourceId: instanceId,
269-
resourceOperation: utils.Ptr(InstanceTypeDelete),
267+
resourceOperation: utils.Ptr(deleteOperation),
270268
resourceDescription: tt.resourceDescription,
271269
resourceState: tt.resourceState,
272270
}

‎services/mariadb/wait/wait.go‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,21 @@ import (
1515
const (
1616
InstanceStatusActive = "active"
1717
InstanceStatusFailed = "failed"
18+
InstanceStatusStopped = "stopped"
19+
InstanceStatusCreating = "creating"
1820
InstanceStatusDeleting = "deleting"
19-
InstanceStateSuccess = "succeeded"
20-
InstanceTypeCreate = "create"
21-
InstanceTypeUpdate = "update"
22-
InstanceTypeDelete = "delete"
21+
InstanceStatusUpdating = "updating"
22+
23+
// Deprecated: InstanceStateSuccess is deprecated and will be removed after 2nd October 2025.
24+
InstanceStateSuccess = "succeeded"
25+
// Deprecated: InstanceStateFailed is deprecated and will be removed after 2nd October 2025.
26+
InstanceStateFailed = "failed"
27+
// Deprecated: InstanceTypeCreate is deprecated and will be removed after 2nd October 2025.
28+
InstanceTypeCreate = "create"
29+
// Deprecated: InstanceTypeUpdate is deprecated and will be removed after 2nd October 2025.
30+
InstanceTypeUpdate = "update"
31+
// Deprecated: InstanceTypeDelete is deprecated and will be removed after 2nd October 2025.
32+
InstanceTypeDelete = "delete"
2333
)
2434

2535
// Interface needed for tests
@@ -39,13 +49,13 @@ func CreateInstanceWaitHandler(ctx context.Context, a APIClientInstanceInterface
3949
if err != nil {
4050
return false, nil, err
4151
}
42-
if s.InstanceId == nil || s.Status == nil {
43-
return false, nil, fmt.Errorf("create failed for instance with id %s. The response is not valid: the instance id or the status are missing", instanceId)
52+
if s.Status == nil {
53+
return false, nil, fmt.Errorf("create failed for instance with id %s. The response is not valid: the status is missing", instanceId)
4454
}
45-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusActive {
55+
switch *s.Status {
56+
case InstanceStatusActive:
4657
return true, s, nil
47-
}
48-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusFailed {
58+
case InstanceStatusFailed:
4959
var failedDescription string
5060
if s.LastOperation != nil && s.LastOperation.Description != nil {
5161
failedDescription = *s.LastOperation.Description
@@ -65,13 +75,13 @@ func PartialUpdateInstanceWaitHandler(ctx context.Context, a APIClientInstanceIn
6575
if err != nil {
6676
return false, nil, err
6777
}
68-
if s.InstanceId == nil || s.Status == nil {
78+
if s.Status == nil {
6979
return false, nil, fmt.Errorf("update failed for instance with id %s. The response is not valid: the instance id or the status are missing", instanceId)
7080
}
71-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusActive {
81+
switch *s.Status {
82+
case InstanceStatusActive:
7283
return true, s, nil
73-
}
74-
if *s.InstanceId == instanceId && *s.Status == InstanceStatusFailed {
84+
case InstanceStatusFailed:
7585
var failedDescription string
7686
if s.LastOperation != nil && s.LastOperation.Description != nil {
7787
failedDescription = *s.LastOperation.Description
@@ -89,13 +99,13 @@ func DeleteInstanceWaitHandler(ctx context.Context, a APIClientInstanceInterface
8999
handler := wait.New(func() (waitFinished bool, response *struct{}, err error) {
90100
s, err := a.GetInstanceExecute(ctx, projectId, instanceId)
91101
if err == nil {
92-
if s.LastOperation == nil || s.Status == nil || s.LastOperation.State == nil || s.LastOperation.Description == nil {
93-
return false, nil, fmt.Errorf("delete failed for instance with id %s. The response is not valid: The status, last operation type, description or the state are missing", instanceId)
102+
if s.LastOperation == nil || s.LastOperation.Description == nil || s.Status == nil {
103+
return false, nil, fmt.Errorf("delete failed for instance with id %s. The response is not valid: The status or last operation description are missing", instanceId)
94104
}
95105
if *s.Status != InstanceStatusDeleting {
96106
return false, nil, nil
97107
}
98-
if *s.LastOperation.State == InstanceStateSuccess {
108+
if *s.Status == InstanceStatusActive {
99109
if strings.Contains(*s.LastOperation.Description, "DeleteFailed") || strings.Contains(*s.LastOperation.Description, "failed") {
100110
return true, nil, fmt.Errorf("instance was deleted successfully but has errors: %s", *s.LastOperation.Description)
101111
}

‎services/mariadb/wait/wait_test.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,21 @@ type apiClientInstanceMocked struct {
2121
resourceDescription string
2222
}
2323

24+
const deleteOperation = "delete"
25+
2426
func (a *apiClientInstanceMocked) GetInstanceExecute(_ context.Context, _, _ string) (*mariadb.Instance, error) {
2527
if a.getFails {
2628
return nil, &oapierror.GenericOpenAPIError{
2729
StatusCode: 500,
2830
}
2931
}
30-
if a.resourceOperation != nil && *a.resourceOperation == InstanceTypeDelete && a.resourceState == InstanceStateSuccess {
32+
if a.resourceOperation != nil && *a.resourceOperation == deleteOperation && a.resourceState == InstanceStatusActive {
3133
if a.deletionSucceedsWithErrors {
3234
return &mariadb.Instance{
3335
InstanceId: &a.resourceId,
3436
Status: &a.resourceState,
3537
LastOperation: &mariadb.InstanceLastOperation{
3638
Description: &a.resourceDescription,
37-
Type: a.resourceOperation,
38-
State: &a.resourceState,
3939
},
4040
}, nil
4141
}
@@ -117,10 +117,9 @@ func TestCreateInstanceWaitHandler(t *testing.T) {
117117
instanceId := "foo-bar"
118118

119119
apiClient := &apiClientInstanceMocked{
120-
getFails: tt.getFails,
121-
resourceId: instanceId,
122-
resourceOperation: utils.Ptr(InstanceTypeCreate),
123-
resourceState: tt.resourceState,
120+
getFails: tt.getFails,
121+
resourceId: instanceId,
122+
resourceState: tt.resourceState,
124123
}
125124

126125
var wantRes *mariadb.Instance
@@ -187,10 +186,9 @@ func TestUpdateInstanceWaitHandler(t *testing.T) {
187186
instanceId := "foo-bar"
188187

189188
apiClient := &apiClientInstanceMocked{
190-
getFails: tt.getFails,
191-
resourceId: instanceId,
192-
resourceOperation: utils.Ptr(InstanceTypeUpdate),
193-
resourceState: tt.resourceState,
189+
getFails: tt.getFails,
190+
resourceId: instanceId,
191+
resourceState: tt.resourceState,
194192
}
195193

196194
var wantRes *mariadb.Instance
@@ -228,7 +226,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
228226
desc: "delete_succeeded",
229227
getFails: false,
230228
deleteSucceeedsWithErrors: false,
231-
resourceState: InstanceStateSuccess,
229+
resourceState: InstanceStatusActive,
232230
wantErr: false,
233231
},
234232
{
@@ -241,7 +239,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
241239
{
242240
desc: "delete_succeeds_with_errors",
243241
getFails: false,
244-
resourceState: InstanceStateSuccess,
242+
resourceState: InstanceStatusActive,
245243
deleteSucceeedsWithErrors: true,
246244
resourceDescription: "Deleting resource: cf failed with error: DeleteFailed",
247245
wantErr: true,
@@ -261,7 +259,7 @@ func TestDeleteInstanceWaitHandler(t *testing.T) {
261259
getFails: tt.getFails,
262260
deletionSucceedsWithErrors: tt.deleteSucceeedsWithErrors,
263261
resourceId: instanceId,
264-
resourceOperation: utils.Ptr(InstanceTypeDelete),
262+
resourceOperation: utils.Ptr(deleteOperation),
265263
resourceDescription: tt.resourceDescription,
266264
resourceState: tt.resourceState,
267265
}

0 commit comments

Comments
 (0)