Skip to content

Commit 70bb095

Browse files
nicolai86nicolai86
authored
Merge pull request scaleway#74 from terraform-providers/bug/va-race
fix race-condition between server & volume_attachment
2 parents 2f8ddba + 91dd117 commit 70bb095

8 files changed

+98
-30
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
## 1.6.0 (Unreleased)
1+
## 1.5.1 (Unreleased)
22

33
IMPROVEMENTS:
44

55
* provider: update documentation ([#75](https://github.com/terraform-providers/terraform-provider-scaleway/pull/75))
66

7+
BUG FIXES:
8+
9+
* resource/scaleway_server & resource/scaleway_volume_attachment: race condition between startup & shutdown of servers ([#74](https://github.com/terraform-providers/terraform-provider-scaleway/pull/74))
10+
711
## 1.5.0 (June 29, 2018)
812

913
IMPROVEMENTS:

‎scaleway/helpers.go

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package scaleway
33
import (
44
"fmt"
55
"strings"
6+
"sync"
67
"time"
78

89
"github.com/hashicorp/terraform/helper/resource"
@@ -45,9 +46,56 @@ func validateVolumeType(v interface{}, k string) (ws []string, errors []error) {
4546
return
4647
}
4748

49+
var waitForServer = map[string]*sync.WaitGroup{}
50+
51+
func getWaitForServerLock(serverID string) *sync.WaitGroup {
52+
mu.Lock()
53+
defer mu.Unlock()
54+
wg, ok := waitForServer[serverID]
55+
if !ok {
56+
wg = &sync.WaitGroup{}
57+
waitForServer[serverID] = wg
58+
}
59+
return wg
60+
}
61+
62+
func startServer(scaleway *api.API, server *api.Server) error {
63+
wg := getWaitForServerLock(server.Identifier)
64+
wg.Wait()
65+
66+
mu.Lock()
67+
task, err := scaleway.PostServerAction(server.Identifier, "poweron")
68+
mu.Unlock()
69+
70+
if err != nil {
71+
return err
72+
}
73+
74+
return waitForTaskCompletion(scaleway, task.Identifier, server.Identifier)
75+
}
76+
77+
func stopServer(scaleway *api.API, server *api.Server) error {
78+
wg := getWaitForServerLock(server.Identifier)
79+
wg.Wait()
80+
81+
mu.Lock()
82+
task, err := scaleway.PostServerAction(server.Identifier, "poweroff")
83+
mu.Unlock()
84+
85+
if err != nil {
86+
return err
87+
}
88+
return waitForTaskCompletion(scaleway, task.Identifier, server.Identifier)
89+
}
90+
4891
// deleteRunningServer terminates the server and waits until it is removed.
4992
func deleteRunningServer(scaleway *api.API, server *api.Server) error {
93+
wg := getWaitForServerLock(server.Identifier)
94+
wg.Wait()
95+
96+
mu.Lock()
5097
task, err := scaleway.PostServerAction(server.Identifier, "terminate")
98+
mu.Unlock()
5199

52100
if err != nil {
53101
if serr, ok := err.(api.APIError); ok {
@@ -59,12 +107,14 @@ func deleteRunningServer(scaleway *api.API, server *api.Server) error {
59107
return err
60108
}
61109

62-
return waitForTaskCompletion(scaleway, task.Identifier)
110+
return waitForTaskCompletion(scaleway, task.Identifier, server.Identifier)
63111
}
64112

65113
// deleteStoppedServer needs to cleanup attached root volumes. this is not done
66114
// automatically by Scaleway
67115
func deleteStoppedServer(scaleway *api.API, server *api.Server) error {
116+
mu.Lock()
117+
defer mu.Unlock()
68118
if err := scaleway.DeleteServer(server.Identifier); err != nil {
69119
return err
70120
}
@@ -77,11 +127,27 @@ func deleteStoppedServer(scaleway *api.API, server *api.Server) error {
77127
return nil
78128
}
79129

80-
func waitForTaskCompletion(scaleway *api.API, taskID string) error {
130+
func waitForTaskCompletion(scaleway *api.API, taskID, serverID string) error {
131+
wg := getWaitForServerLock(serverID)
132+
wg.Wait()
133+
134+
mu.Lock()
135+
wg.Add(1)
136+
mu.Unlock()
137+
138+
defer func() {
139+
mu.Lock()
140+
wg.Done()
141+
mu.Unlock()
142+
}()
143+
81144
stateConf := &resource.StateChangeConf{
82145
Pending: []string{"pending", "started"},
83146
Target: []string{"success"},
84147
Refresh: func() (interface{}, string, error) {
148+
mu.Lock()
149+
defer mu.Unlock()
150+
85151
task, err := scaleway.GetTask(taskID)
86152
if err != nil {
87153
return 42, "error", err
@@ -92,12 +158,17 @@ func waitForTaskCompletion(scaleway *api.API, taskID string) error {
92158
MinTimeout: 10 * time.Second,
93159
Delay: 15 * time.Second,
94160
}
161+
95162
_, err := stateConf.WaitForState()
163+
96164
return err
97165
}
98166

99167
func withStoppedServer(scaleway *api.API, serverID string, run func(*api.Server) error) error {
168+
mu.Lock()
100169
server, err := scaleway.GetServer(serverID)
170+
mu.Unlock()
171+
101172
if err != nil {
102173
return err
103174
}
@@ -107,27 +178,21 @@ func withStoppedServer(scaleway *api.API, serverID string, run func(*api.Server)
107178
if server.State != "stopped" {
108179
startServerAgain = true
109180

110-
task, err := scaleway.PostServerAction(server.Identifier, "poweroff")
181+
err := stopServer(scaleway, server)
111182
if err != nil {
112183
return err
113184
}
114-
if err := waitForTaskCompletion(scaleway, task.Identifier); err != nil {
115-
return err
116-
}
117185
}
118186

119187
if err := run(server); err != nil {
120188
return err
121189
}
122190

123191
if startServerAgain {
124-
task, err := scaleway.PostServerAction(serverID, "poweron")
192+
err := startServer(scaleway, server)
125193
if err != nil {
126194
return err
127195
}
128-
if err := waitForTaskCompletion(scaleway, task.Identifier); err != nil {
129-
return err
130-
}
131196
}
132197
return nil
133198
}

‎scaleway/resource_ip_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func TestAccScalewayIP_Basic(t *testing.T) {
7979
resource.TestStep{
8080
Config: testAccCheckScalewayIPAttachConfig,
8181
Check: resource.ComposeTestCheckFunc(
82+
testAccCheckScalewayServerExists("scaleway_server.base"),
8283
testAccCheckScalewayIPExists("scaleway_ip.base"),
8384
testAccCheckScalewayIPAttachment("scaleway_ip.base", func(serverID string) bool {
8485
return serverID != ""
@@ -204,7 +205,6 @@ resource "scaleway_server" "base" {
204205
# ubuntu 14.04
205206
image = "%s"
206207
type = "C1"
207-
state = "stopped"
208208
}
209209
210210
resource "scaleway_ip" "base" {

‎scaleway/resource_server.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,13 @@ func resourceScalewayServerCreate(d *schema.ResourceData, m interface{}) error {
203203
sizeInGB := uint64(volume["size_in_gb"].(int))
204204

205205
if sizeInGB > 0 {
206+
mu.Lock()
206207
v, err := scaleway.CreateVolume(api.VolumeDefinition{
207208
Size: sizeInGB * gb,
208209
Type: volume["type"].(string),
209210
Name: fmt.Sprintf("%s-%d", req.Name, sizeInGB),
210211
})
212+
mu.Unlock()
211213
if err != nil {
212214
return err
213215
}
@@ -234,15 +236,11 @@ func resourceScalewayServerCreate(d *schema.ResourceData, m interface{}) error {
234236

235237
d.SetId(server.Identifier)
236238
if d.Get("state").(string) != "stopped" {
237-
mu.Lock()
238-
task, err := scaleway.PostServerAction(server.Identifier, "poweron")
239-
mu.Unlock()
239+
err := startServer(scaleway, server)
240240
if err != nil {
241241
return err
242242
}
243243

244-
err = waitForTaskCompletion(scaleway, task.Identifier)
245-
246244
if v, ok := d.GetOk("public_ip"); ok {
247245
if err := attachIP(scaleway, d.Id(), v.(string)); err != nil {
248246
return err
@@ -259,8 +257,8 @@ func resourceScalewayServerCreate(d *schema.ResourceData, m interface{}) error {
259257

260258
func resourceScalewayServerRead(d *schema.ResourceData, m interface{}) error {
261259
scaleway := m.(*Client).scaleway
262-
server, err := scaleway.GetServer(d.Id())
263260

261+
server, err := scaleway.GetServer(d.Id())
264262
if err != nil {
265263
if serr, ok := err.(api.APIError); ok {
266264
log.Printf("[DEBUG] Error reading server: %q\n", serr.APIMessage)
@@ -373,9 +371,6 @@ func resourceScalewayServerUpdate(d *schema.ResourceData, m interface{}) error {
373371
func resourceScalewayServerDelete(d *schema.ResourceData, m interface{}) error {
374372
scaleway := m.(*Client).scaleway
375373

376-
mu.Lock()
377-
defer mu.Unlock()
378-
379374
s, err := scaleway.GetServer(d.Id())
380375
if err != nil {
381376
return err
@@ -386,7 +381,6 @@ func resourceScalewayServerDelete(d *schema.ResourceData, m interface{}) error {
386381
}
387382

388383
err = deleteRunningServer(scaleway, s)
389-
390384
if err == nil {
391385
d.SetId("")
392386
}

‎scaleway/resource_server_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,10 @@ func testAccCheckScalewayServerExists(n string) resource.TestCheckFunc {
315315
return fmt.Errorf("Record not found")
316316
}
317317

318+
if server.State != "running" {
319+
return fmt.Errorf("expected server to be running, but was %q", server.State)
320+
}
321+
318322
return nil
319323
}
320324
}

‎scaleway/resource_user_data_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func TestAccScalewayUserData_Basic(t *testing.T) {
1717
resource.TestStep{
1818
Config: testAccCheckScalewayUserDataConfig,
1919
Check: resource.ComposeTestCheckFunc(
20+
testAccCheckScalewayServerExists("scaleway_server.base"),
2021
testAccCheckScalewayUserDataExists("scaleway_user_data.base"),
2122
resource.TestCheckResourceAttr("scaleway_user_data.base", "value", "supersecret"),
2223
resource.TestCheckResourceAttr("scaleway_user_data.base", "key", "gcp_username"),
@@ -69,7 +70,6 @@ resource "scaleway_server" "base" {
6970
# ubuntu 14.04
7071
image = "%s"
7172
type = "C1"
72-
state = "stopped"
7373
}
7474
7575
resource "scaleway_user_data" "base" {

‎scaleway/resource_volume_attachment.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ func resourceScalewayVolumeAttachment() *schema.Resource {
3535
var errVolumeAlreadyAttached = fmt.Errorf("Scaleway volume already attached")
3636

3737
func resourceScalewayVolumeAttachmentCreate(d *schema.ResourceData, m interface{}) error {
38-
mu.Lock()
39-
defer mu.Unlock()
40-
4138
scaleway := m.(*Client).scaleway
4239

40+
mu.Lock()
4341
vol, err := scaleway.GetVolume(d.Get("volume").(string))
42+
mu.Unlock()
43+
4444
if err != nil {
4545
return err
4646
}
@@ -74,7 +74,9 @@ func resourceScalewayVolumeAttachmentCreate(d *schema.ResourceData, m interface{
7474
var req = api.ServerPatchDefinition{
7575
Volumes: &volumes,
7676
}
77+
mu.Lock()
7778
err := scaleway.PatchServer(server.Identifier, req)
79+
mu.Unlock()
7880

7981
if err == nil {
8082
return nil
@@ -141,9 +143,6 @@ func resourceScalewayVolumeAttachmentRead(d *schema.ResourceData, m interface{})
141143
const serverWaitTimeout = 5 * time.Minute
142144

143145
func resourceScalewayVolumeAttachmentDelete(d *schema.ResourceData, m interface{}) error {
144-
mu.Lock()
145-
defer mu.Unlock()
146-
147146
scaleway := m.(*Client).scaleway
148147

149148
if err := withStoppedServer(scaleway, d.Get("server").(string), func(server *api.Server) error {
@@ -171,7 +170,9 @@ func resourceScalewayVolumeAttachmentDelete(d *schema.ResourceData, m interface{
171170
var req = api.ServerPatchDefinition{
172171
Volumes: &volumes,
173172
}
173+
mu.Lock()
174174
err := scaleway.PatchServer(server.Identifier, req)
175+
mu.Unlock()
175176

176177
if err == nil {
177178
return nil

‎scaleway/resource_volume_attachment_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func TestAccScalewayVolumeAttachment_Basic(t *testing.T) {
1717
resource.TestStep{
1818
Config: testAccCheckScalewayVolumeAttachmentConfig,
1919
Check: resource.ComposeTestCheckFunc(
20+
testAccCheckScalewayServerExists("scaleway_server.base"),
2021
testAccCheckScalewayVolumeAttachmentExists("scaleway_volume_attachment.test"),
2122
),
2223
},
@@ -76,7 +77,6 @@ resource "scaleway_server" "base" {
7677
# ubuntu 14.04
7778
image = "%s"
7879
type = "C1"
79-
# state = "stopped"
8080
}
8181
8282
resource "scaleway_volume" "test" {

0 commit comments

Comments
 (0)