Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add retries to circleci CRUD resources #32

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add retries to circleci CRUD resources #32

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
936f3f5
Add retries to circleci CRUD resources
ajlake May 5, 2020
8117371
Update resource_circleci_environment_variable.go
ajlake May 6, 2020
08b2e2c
Update resource_circleci_environment_variable.go
ajlake May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 61 additions & 33 deletions circleci/resource_circleci_environment_variable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package circleci
import (
"crypto/sha256"
"encoding/base64"
"errors"
"fmt"
"net/http"
"strings"
"time"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"

circleciapi "github.com/jszwedko/go-circleci"
Expand All @@ -24,6 +27,8 @@ func resourceCircleCIEnvironmentVariable() *schema.Resource {

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(5 * time.Minute),
Read: schema.DefaultTimeout(5 * time.Minute),
Update: schema.DefaultTimeout(5 * time.Minute),
Delete: schema.DefaultTimeout(5 * time.Minute),
},

Expand Down Expand Up @@ -153,6 +158,21 @@ func hashString(str string) string {
return base64.StdEncoding.EncodeToString(hash[:])
}

func wrap(err error) *resource.RetryError {
if err == nil {
return nil
}

var apiErr circleciapi.APIError
if errors.As(err, &apiErr) {
switch apiErr.HTTPStatusCode {
case http.StatusTooManyRequests, http.StatusServiceUnavailable, http.StatusInternalServerError, http.StatusBadGateway:
return resource.RetryableError(err)
}
}
return resource.NonRetryableError(err)
}

func resourceCircleCIEnvironmentVariableCreate(d *schema.ResourceData, m interface{}) error {
providerClient := m.(*ProviderClient)

Expand All @@ -161,21 +181,23 @@ func resourceCircleCIEnvironmentVariableCreate(d *schema.ResourceData, m interfa
envName := d.Get("name").(string)
envValue := d.Get("value").(string)

exists, err := providerClient.EnvVarExists(organization, projectName, envName)
if err != nil {
return err
}
return resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
exists, err := providerClient.EnvVarExists(organization, projectName, envName)
if err != nil {
return wrap(err)
}

if exists {
return fmt.Errorf("environment variable '%s' already exists for project '%s'", envName, projectName)
}
if exists {
return wrap(fmt.Errorf("environment variable '%s' already exists for project '%s'", envName, projectName))
}

if _, err := providerClient.AddEnvVar(organization, projectName, envName, envValue); err != nil {
return err
}
if _, err := providerClient.AddEnvVar(organization, projectName, envName, envValue); err != nil {
return wrap(err)
}
Comment on lines +185 to +196
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we split the retry here and use a different, one for the call to EnvVarExists() and another retry for AddEnvVar() ?

If the first one succeed (and return false) but the second one fail due to network issue (aka a retryable error), should we do the first request again too?
I'm not sure if it's overkill to do 2 different retries and what we gain from doing only one.

What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitting makes sense to me


d.SetId(generateId(organization, projectName, envName))
return resourceCircleCIEnvironmentVariableRead(d, m)
d.SetId(generateId(organization, projectName, envName))
return wrap(resourceCircleCIEnvironmentVariableRead(d, m))
})
}

func resourceCircleCIEnvironmentVariableRead(d *schema.ResourceData, m interface{}) error {
Expand All @@ -192,18 +214,20 @@ func resourceCircleCIEnvironmentVariableRead(d *schema.ResourceData, m interface
projectName := d.Get("project").(string)
envName := d.Get("name").(string)

envVar, err := providerClient.GetEnvVar(organization, projectName, envName)
if err != nil {
return err
}
return resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
envVar, err := providerClient.GetEnvVar(organization, projectName, envName)
if err != nil {
return wrap(err)
}

if err := d.Set("name", envVar.Name); err != nil {
return err
}
if err := d.Set("name", envVar.Name); err != nil {
return wrap(err)
}

// environment variable value can only be set at creation since CircleCI API return hidden values : https://circleci.com/docs/api/#list-environment-variables
// also it is better to avoid storing sensitive value in terraform state if possible.
return nil
// environment variable value can only be set at creation since CircleCI API return hidden values : https://circleci.com/docs/api/#list-environment-variables
// also it is better to avoid storing sensitive value in terraform state if possible.
return nil
})
}

func resourceCircleCIEnvironmentVariableDelete(d *schema.ResourceData, m interface{}) error {
Expand All @@ -213,14 +237,16 @@ func resourceCircleCIEnvironmentVariableDelete(d *schema.ResourceData, m interfa
projectName := d.Get("project").(string)
envName := d.Get("name").(string)

err := providerClient.DeleteEnvVar(organization, projectName, envName)
if err != nil {
return err
}
return resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
err := providerClient.DeleteEnvVar(organization, projectName, envName)
if err != nil {
return wrap(err)
}

d.SetId("")
d.SetId("")

return nil
return nil
})
}

func resourceCircleCIEnvironmentVariableExists(d *schema.ResourceData, m interface{}) (bool, error) {
Expand All @@ -237,12 +263,14 @@ func resourceCircleCIEnvironmentVariableExists(d *schema.ResourceData, m interfa
projectName := d.Get("project").(string)
envName := d.Get("name").(string)

envVar, err := providerClient.GetEnvVar(organization, projectName, envName)
if err != nil {
return false, err
}
var envVar *circleciapi.EnvVar
err := resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
e, err := providerClient.GetEnvVar(organization, projectName, envName)
envVar = e
return wrap(err)
})

return bool(envVar.Value != ""), nil
return bool(envVar.Value != ""), err
}

func getOrganization(d *schema.ResourceData, providerClient *ProviderClient) string {
Expand Down