Skip to content

Commit 90710b3

Browse files
Fix flaky service account tests (#1304)
Sometimes, the SA tests fail with the "service account already exists" error The issue is due to the fact that Grafana sometimes 500s on SA POSTs, but the SA is actually created, so on the retry, there's a conflict This PR changes the retry process so that if we 500, we try to find the SA and "import" it before retrying
1 parent eba5b73 commit 90710b3

File tree

4 files changed

+43
-10
lines changed

4 files changed

+43
-10
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/go-openapi/strfmt v0.22.0
1111
github.com/grafana/amixr-api-go-client v0.0.11
1212
github.com/grafana/grafana-api-golang-client v0.27.0
13-
github.com/grafana/grafana-openapi-client-go v0.0.0-20240118162741-b884e1a072bf
13+
github.com/grafana/grafana-openapi-client-go v0.0.0-20240126032018-bd23c00af697
1414
github.com/grafana/machine-learning-go-client v0.5.0
1515
github.com/grafana/slo-openapi-client/go v0.0.0-20240112175006-de02e75b9d73
1616
github.com/grafana/synthetic-monitoring-agent v0.19.3

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ github.com/grafana/amixr-api-go-client v0.0.11 h1:jlE+5t0tRuCtjbpM81j70Dr2J4eCyS
102102
github.com/grafana/amixr-api-go-client v0.0.11/go.mod h1:N6x26XUrM5zGtK5zL5vNJnAn2JFMxLFPPLTw/6pDkFE=
103103
github.com/grafana/grafana-api-golang-client v0.27.0 h1:zIwMXcbCB4n588i3O2N6HfNcQogCNTd/vPkEXTr7zX8=
104104
github.com/grafana/grafana-api-golang-client v0.27.0/go.mod h1:uNLZEmgKtTjHBtCQMwNn3qsx2mpMb8zU+7T4Xv3NR9Y=
105-
github.com/grafana/grafana-openapi-client-go v0.0.0-20240118162741-b884e1a072bf h1:bi08tzZ4QEEe4KBDXfJlcO9QwUdvM/ndzrmV3mozzkU=
106-
github.com/grafana/grafana-openapi-client-go v0.0.0-20240118162741-b884e1a072bf/go.mod h1:af7rlJw/VtbvAfI5VWzYO4p/pT58FXrN6XqZBnkwBxo=
105+
github.com/grafana/grafana-openapi-client-go v0.0.0-20240126032018-bd23c00af697 h1:8io6OuKJrhH8SEfwXV1ZpAwgaGhJ25/pjaA3vYmMxxE=
106+
github.com/grafana/grafana-openapi-client-go v0.0.0-20240126032018-bd23c00af697/go.mod h1:af7rlJw/VtbvAfI5VWzYO4p/pT58FXrN6XqZBnkwBxo=
107107
github.com/grafana/machine-learning-go-client v0.5.0 h1:Q1K+MPSy8vfMm2jsk3WQ7O77cGr2fM5hxwtPSoPc5NU=
108108
github.com/grafana/machine-learning-go-client v0.5.0/go.mod h1:QFfZz8NkqVF8++skjkKQXJEZfpCYd8S0yTWJUpsLLTA=
109109
github.com/grafana/slo-openapi-client/go v0.0.0-20240112175006-de02e75b9d73 h1:E5vAeB5q1H3BVeNhtd1dI8RubucJdPwpx/ElNtKD3ls=

internal/resources/grafana/data_source_service_account.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package grafana
22

33
import (
44
"context"
5+
"fmt"
56

7+
"github.com/grafana/grafana-openapi-client-go/client"
68
"github.com/grafana/grafana-openapi-client-go/client/service_accounts"
9+
"github.com/grafana/grafana-openapi-client-go/models"
710
"github.com/grafana/terraform-provider-grafana/internal/common"
811
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
912
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -30,23 +33,31 @@ func DatasourceServiceAccount() *schema.Resource {
3033
func DatasourceServiceAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
3134
client, orgID := OAPIClientFromNewOrgResource(meta, d)
3235
name := d.Get("name").(string)
36+
sa, err := findServiceAccountByName(client, name)
37+
if err != nil {
38+
return diag.FromErr(err)
39+
}
40+
d.SetId(MakeOrgResourceID(orgID, sa.ID))
41+
return ReadServiceAccount(ctx, d, meta)
42+
}
43+
44+
func findServiceAccountByName(client *client.GrafanaHTTPAPI, name string) (*models.ServiceAccountDTO, error) {
3345
var page int64 = 0
3446
for {
3547
params := service_accounts.NewSearchOrgServiceAccountsWithPagingParams().WithPage(&page)
3648
resp, err := client.ServiceAccounts.SearchOrgServiceAccountsWithPaging(params)
3749
if err != nil {
38-
return diag.FromErr(err)
50+
return nil, err
3951
}
4052
serviceAccounts := resp.Payload.ServiceAccounts
4153
if len(serviceAccounts) == 0 {
4254
break
4355
}
4456
for _, sa := range serviceAccounts {
4557
if sa.Name == name {
46-
d.SetId(MakeOrgResourceID(orgID, sa.ID))
47-
return ReadServiceAccount(ctx, d, meta)
58+
return sa, nil
4859
}
4960
}
5061
}
51-
return diag.Errorf("service account %q not found", name)
62+
return nil, fmt.Errorf("service account %q not found", name)
5263
}

internal/resources/grafana/resource_service_account.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import (
44
"context"
55
"strconv"
66
"sync"
7+
"time"
78

89
"github.com/grafana/grafana-openapi-client-go/client/service_accounts"
910
"github.com/grafana/grafana-openapi-client-go/models"
1011
"github.com/grafana/terraform-provider-grafana/internal/common"
1112
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
13+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1214
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1315
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1416
)
@@ -60,18 +62,38 @@ func CreateServiceAccount(ctx context.Context, d *schema.ResourceData, meta inte
6062
defer serviceAccountCreateMutex.Unlock()
6163

6264
client, orgID := OAPIClientFromNewOrgResource(meta, d)
65+
client = client.WithRetries(0, 0) // Disable retries to have our own retry logic
6366
req := models.CreateServiceAccountForm{
6467
Name: d.Get("name").(string),
6568
Role: d.Get("role").(string),
6669
IsDisabled: d.Get("is_disabled").(bool),
6770
}
6871

69-
params := service_accounts.NewCreateServiceAccountParams().WithBody(&req)
70-
resp, err := client.ServiceAccounts.CreateServiceAccount(params)
72+
var sa *models.ServiceAccountDTO
73+
err := retry.RetryContext(ctx, 10*time.Second, func() *retry.RetryError {
74+
params := service_accounts.NewCreateServiceAccountParams().WithBody(&req)
75+
resp, err := client.ServiceAccounts.CreateServiceAccount(params)
76+
if err == nil {
77+
sa = resp.Payload
78+
return nil
79+
}
80+
81+
if err, ok := err.(*service_accounts.CreateServiceAccountInternalServerError); ok {
82+
// Sometimes on 500s, the service account is created but the response is not returned.
83+
// If we just retry, it will conflict because the SA was actually created.
84+
foundSa, readErr := findServiceAccountByName(client, req.Name)
85+
if readErr != nil {
86+
return retry.RetryableError(err)
87+
}
88+
sa = foundSa
89+
return nil
90+
}
91+
return retry.NonRetryableError(err)
92+
})
7193
if err != nil {
7294
return diag.FromErr(err)
7395
}
74-
d.SetId(MakeOrgResourceID(orgID, resp.Payload.ID))
96+
d.SetId(MakeOrgResourceID(orgID, sa.ID))
7597

7698
return ReadServiceAccount(ctx, d, meta)
7799
}

0 commit comments

Comments
 (0)