Browse code

Use State waiting functions

Docker-DCO-1.1-Signed-off-by: Alexandr Morozov <lk4d4math@gmail.com> (github: LK4D4)

Alexandr Morozov authored on 2014/06/06 20:30:04
Showing 9 changed files
... ...
@@ -53,7 +53,7 @@ type Container struct {
53 53
 	Args []string
54 54
 
55 55
 	Config *runconfig.Config
56
-	State  State
56
+	State  *State
57 57
 	Image  string
58 58
 
59 59
 	NetworkSettings *NetworkSettings
... ...
@@ -74,8 +74,7 @@ type Container struct {
74 74
 	daemon                   *Daemon
75 75
 	MountLabel, ProcessLabel string
76 76
 
77
-	waitLock chan struct{}
78
-	Volumes  map[string]string
77
+	Volumes map[string]string
79 78
 	// Store rw/ro in a separate structure to preserve reverse-compatibility on-disk.
80 79
 	// Easier than migrating older container configs :)
81 80
 	VolumesRW  map[string]bool
... ...
@@ -284,7 +283,6 @@ func (container *Container) Start() (err error) {
284 284
 	if err := container.startLoggingToDisk(); err != nil {
285 285
 		return err
286 286
 	}
287
-	container.waitLock = make(chan struct{})
288 287
 
289 288
 	return container.waitForStart()
290 289
 }
... ...
@@ -293,7 +291,7 @@ func (container *Container) Run() error {
293 293
 	if err := container.Start(); err != nil {
294 294
 		return err
295 295
 	}
296
-	container.Wait()
296
+	container.State.WaitStop(-1 * time.Second)
297 297
 	return nil
298 298
 }
299 299
 
... ...
@@ -307,7 +305,7 @@ func (container *Container) Output() (output []byte, err error) {
307 307
 		return nil, err
308 308
 	}
309 309
 	output, err = ioutil.ReadAll(pipe)
310
-	container.Wait()
310
+	container.State.WaitStop(-1 * time.Second)
311 311
 	return output, err
312 312
 }
313 313
 
... ...
@@ -467,6 +465,7 @@ func (container *Container) monitor(callback execdriver.StartCallback) error {
467 467
 	if err != nil {
468 468
 		utils.Errorf("Error running container: %s", err)
469 469
 	}
470
+	container.State.SetStopped(exitCode)
470 471
 
471 472
 	// Cleanup
472 473
 	container.cleanup()
... ...
@@ -475,28 +474,17 @@ func (container *Container) monitor(callback execdriver.StartCallback) error {
475 475
 	if container.Config.OpenStdin {
476 476
 		container.stdin, container.stdinPipe = io.Pipe()
477 477
 	}
478
-
479 478
 	if container.daemon != nil && container.daemon.srv != nil {
480 479
 		container.daemon.srv.LogEvent("die", container.ID, container.daemon.repositories.ImageName(container.Image))
481 480
 	}
482
-
483
-	close(container.waitLock)
484
-
485 481
 	if container.daemon != nil && container.daemon.srv != nil && container.daemon.srv.IsRunning() {
486
-		container.State.SetStopped(exitCode)
487
-
488
-		// FIXME: there is a race condition here which causes this to fail during the unit tests.
489
-		// If another goroutine was waiting for Wait() to return before removing the container's root
490
-		// from the filesystem... At this point it may already have done so.
491
-		// This is because State.setStopped() has already been called, and has caused Wait()
492
-		// to return.
493
-		// FIXME: why are we serializing running state to disk in the first place?
494
-		//log.Printf("%s: Failed to dump configuration to the disk: %s", container.ID, err)
482
+		// FIXME: here is race condition between two RUN instructions in Dockerfile
483
+		// because they share same runconfig and change image. Must be fixed
484
+		// in server/buildfile.go
495 485
 		if err := container.ToDisk(); err != nil {
496
-			utils.Errorf("Error dumping container state to disk: %s\n", err)
486
+			utils.Errorf("Error dumping container %s state to disk: %s\n", container.ID, err)
497 487
 		}
498 488
 	}
499
-
500 489
 	return err
501 490
 }
502 491
 
... ...
@@ -532,6 +520,7 @@ func (container *Container) cleanup() {
532 532
 }
533 533
 
534 534
 func (container *Container) KillSig(sig int) error {
535
+	utils.Debugf("Sending %d to %s", sig, container.ID)
535 536
 	container.Lock()
536 537
 	defer container.Unlock()
537 538
 
... ...
@@ -577,9 +566,9 @@ func (container *Container) Kill() error {
577 577
 	}
578 578
 
579 579
 	// 2. Wait for the process to die, in last resort, try to kill the process directly
580
-	if err := container.WaitTimeout(10 * time.Second); err != nil {
580
+	if _, err := container.State.WaitStop(10 * time.Second); err != nil {
581 581
 		// Ensure that we don't kill ourselves
582
-		if pid := container.State.Pid; pid != 0 {
582
+		if pid := container.State.GetPid(); pid != 0 {
583 583
 			log.Printf("Container %s failed to exit within 10 seconds of kill - trying direct SIGKILL", utils.TruncateID(container.ID))
584 584
 			if err := syscall.Kill(pid, 9); err != nil {
585 585
 				return err
... ...
@@ -587,7 +576,7 @@ func (container *Container) Kill() error {
587 587
 		}
588 588
 	}
589 589
 
590
-	container.Wait()
590
+	container.State.WaitStop(-1 * time.Second)
591 591
 	return nil
592 592
 }
593 593
 
... ...
@@ -605,11 +594,11 @@ func (container *Container) Stop(seconds int) error {
605 605
 	}
606 606
 
607 607
 	// 2. Wait for the process to exit on its own
608
-	if err := container.WaitTimeout(time.Duration(seconds) * time.Second); err != nil {
608
+	if _, err := container.State.WaitStop(time.Duration(seconds) * time.Second); err != nil {
609 609
 		log.Printf("Container %v failed to exit within %d seconds of SIGTERM - using the force", container.ID, seconds)
610 610
 		// 3. If it doesn't, then send SIGKILL
611 611
 		if err := container.Kill(); err != nil {
612
-			container.Wait()
612
+			container.State.WaitStop(-1 * time.Second)
613 613
 			return err
614 614
 		}
615 615
 	}
... ...
@@ -630,12 +619,6 @@ func (container *Container) Restart(seconds int) error {
630 630
 	return container.Start()
631 631
 }
632 632
 
633
-// Wait blocks until the container stops running, then returns its exit code.
634
-func (container *Container) Wait() int {
635
-	<-container.waitLock
636
-	return container.State.GetExitCode()
637
-}
638
-
639 633
 func (container *Container) Resize(h, w int) error {
640 634
 	return container.command.Terminal.Resize(h, w)
641 635
 }
... ...
@@ -678,21 +661,6 @@ func (container *Container) Export() (archive.Archive, error) {
678 678
 		nil
679 679
 }
680 680
 
681
-func (container *Container) WaitTimeout(timeout time.Duration) error {
682
-	done := make(chan bool, 1)
683
-	go func() {
684
-		container.Wait()
685
-		done <- true
686
-	}()
687
-
688
-	select {
689
-	case <-time.After(timeout):
690
-		return fmt.Errorf("Timed Out")
691
-	case <-done:
692
-		return nil
693
-	}
694
-}
695
-
696 681
 func (container *Container) Mount() error {
697 682
 	return container.daemon.Mount(container)
698 683
 }
... ...
@@ -1103,9 +1071,7 @@ func (container *Container) startLoggingToDisk() error {
1103 1103
 }
1104 1104
 
1105 1105
 func (container *Container) waitForStart() error {
1106
-	callbackLock := make(chan struct{})
1107 1106
 	callback := func(command *execdriver.Command) {
1108
-		container.State.SetRunning(command.Pid())
1109 1107
 		if command.Tty {
1110 1108
 			// The callback is called after the process Start()
1111 1109
 			// so we are in the parent process. In TTY mode, stdin/out/err is the PtySlace
... ...
@@ -1117,16 +1083,23 @@ func (container *Container) waitForStart() error {
1117 1117
 		if err := container.ToDisk(); err != nil {
1118 1118
 			utils.Debugf("%s", err)
1119 1119
 		}
1120
-		close(callbackLock)
1120
+		container.State.SetRunning(command.Pid())
1121 1121
 	}
1122 1122
 
1123 1123
 	// We use a callback here instead of a goroutine and an chan for
1124 1124
 	// syncronization purposes
1125 1125
 	cErr := utils.Go(func() error { return container.monitor(callback) })
1126 1126
 
1127
+	waitStart := make(chan struct{})
1128
+
1129
+	go func() {
1130
+		container.State.WaitRunning(-1 * time.Second)
1131
+		close(waitStart)
1132
+	}()
1133
+
1127 1134
 	// Start should not return until the process is actually running
1128 1135
 	select {
1129
-	case <-callbackLock:
1136
+	case <-waitStart:
1130 1137
 	case err := <-cErr:
1131 1138
 		return err
1132 1139
 	}
... ...
@@ -138,7 +138,7 @@ func (daemon *Daemon) containerRoot(id string) string {
138 138
 // Load reads the contents of a container from disk
139 139
 // This is typically done at startup.
140 140
 func (daemon *Daemon) load(id string) (*Container, error) {
141
-	container := &Container{root: daemon.containerRoot(id)}
141
+	container := &Container{root: daemon.containerRoot(id), State: NewState()}
142 142
 	if err := container.FromDisk(); err != nil {
143 143
 		return nil, err
144 144
 	}
... ...
@@ -236,12 +236,6 @@ func (daemon *Daemon) register(container *Container, updateSuffixarray bool, con
236 236
 				}
237 237
 			}
238 238
 		}
239
-	} else {
240
-		// When the container is not running, we still initialize the waitLock
241
-		// chan and close it. Receiving on nil chan blocks whereas receiving on a
242
-		// closed chan does not. In this case we do not want to block.
243
-		container.waitLock = make(chan struct{})
244
-		close(container.waitLock)
245 239
 	}
246 240
 	return nil
247 241
 }
... ...
@@ -588,6 +582,7 @@ func (daemon *Daemon) newContainer(name string, config *runconfig.Config, img *i
588 588
 		Name:            name,
589 589
 		Driver:          daemon.driver.String(),
590 590
 		ExecDriver:      daemon.execDriver.Name(),
591
+		State:           NewState(),
591 592
 	}
592 593
 	container.root = daemon.containerRoot(container.ID)
593 594
 
... ...
@@ -900,7 +895,7 @@ func (daemon *Daemon) shutdown() error {
900 900
 				if err := c.KillSig(15); err != nil {
901 901
 					utils.Debugf("kill 15 error for %s - %s", c.ID, err)
902 902
 				}
903
-				c.Wait()
903
+				c.State.WaitStop(-1 * time.Second)
904 904
 				utils.Debugf("container stopped %s", c.ID)
905 905
 			}()
906 906
 		}
... ...
@@ -75,7 +75,7 @@ func (s *State) WaitRunning(timeout time.Duration) (int, error) {
75 75
 
76 76
 // WaitStop waits until state is stopped. If state already stopped it returns
77 77
 // immediatly. If you want wait forever you must supply negative timeout.
78
-// Returns exit code, that was passed to SetRunning
78
+// Returns exit code, that was passed to SetStopped
79 79
 func (s *State) WaitStop(timeout time.Duration) (int, error) {
80 80
 	s.RLock()
81 81
 	if !s.Running {
... ...
@@ -224,7 +224,7 @@ func TestRunDisconnect(t *testing.T) {
224 224
 	// cause /bin/cat to exit.
225 225
 	setTimeout(t, "Waiting for /bin/cat to exit timed out", 2*time.Second, func() {
226 226
 		container := globalDaemon.List()[0]
227
-		container.Wait()
227
+		container.State.WaitStop(-1 * time.Second)
228 228
 		if container.State.IsRunning() {
229 229
 			t.Fatalf("/bin/cat is still running after closing stdin")
230 230
 		}
... ...
@@ -276,7 +276,7 @@ func TestRunDisconnectTty(t *testing.T) {
276 276
 	// In tty mode, we expect the process to stay alive even after client's stdin closes.
277 277
 
278 278
 	// Give some time to monitor to do his thing
279
-	container.WaitTimeout(500 * time.Millisecond)
279
+	container.State.WaitStop(500 * time.Millisecond)
280 280
 	if !container.State.IsRunning() {
281 281
 		t.Fatalf("/bin/cat should  still be running after closing stdin (tty mode)")
282 282
 	}
... ...
@@ -535,7 +535,7 @@ func TestAttachDisconnect(t *testing.T) {
535 535
 
536 536
 	// We closed stdin, expect /bin/cat to still be running
537 537
 	// Wait a little bit to make sure container.monitor() did his thing
538
-	err := container.WaitTimeout(500 * time.Millisecond)
538
+	_, err := container.State.WaitStop(500 * time.Millisecond)
539 539
 	if err == nil || !container.State.IsRunning() {
540 540
 		t.Fatalf("/bin/cat is not running after closing stdin")
541 541
 	}
... ...
@@ -543,7 +543,7 @@ func TestAttachDisconnect(t *testing.T) {
543 543
 	// Try to avoid the timeout in destroy. Best effort, don't check error
544 544
 	cStdin, _ := container.StdinPipe()
545 545
 	cStdin.Close()
546
-	container.Wait()
546
+	container.State.WaitStop(-1 * time.Second)
547 547
 }
548 548
 
549 549
 // Expected behaviour: container gets deleted automatically after exit
... ...
@@ -2,7 +2,6 @@ package docker
2 2
 
3 3
 import (
4 4
 	"fmt"
5
-	"github.com/dotcloud/docker/runconfig"
6 5
 	"io"
7 6
 	"io/ioutil"
8 7
 	"os"
... ...
@@ -10,6 +9,8 @@ import (
10 10
 	"strings"
11 11
 	"testing"
12 12
 	"time"
13
+
14
+	"github.com/dotcloud/docker/runconfig"
13 15
 )
14 16
 
15 17
 func TestKillDifferentUser(t *testing.T) {
... ...
@@ -60,7 +61,7 @@ func TestKillDifferentUser(t *testing.T) {
60 60
 	if container.State.IsRunning() {
61 61
 		t.Errorf("Container shouldn't be running")
62 62
 	}
63
-	container.Wait()
63
+	container.State.WaitStop(-1 * time.Second)
64 64
 	if container.State.IsRunning() {
65 65
 		t.Errorf("Container shouldn't be running")
66 66
 	}
... ...
@@ -134,7 +135,7 @@ func TestRestartStdin(t *testing.T) {
134 134
 	if err := stdin.Close(); err != nil {
135 135
 		t.Fatal(err)
136 136
 	}
137
-	container.Wait()
137
+	container.State.WaitStop(-1 * time.Second)
138 138
 	output, err := ioutil.ReadAll(stdout)
139 139
 	if err != nil {
140 140
 		t.Fatal(err)
... ...
@@ -164,7 +165,7 @@ func TestRestartStdin(t *testing.T) {
164 164
 	if err := stdin.Close(); err != nil {
165 165
 		t.Fatal(err)
166 166
 	}
167
-	container.Wait()
167
+	container.State.WaitStop(-1 * time.Second)
168 168
 	output, err = ioutil.ReadAll(stdout)
169 169
 	if err != nil {
170 170
 		t.Fatal(err)
... ...
@@ -212,7 +213,7 @@ func TestStdin(t *testing.T) {
212 212
 	if err := stdin.Close(); err != nil {
213 213
 		t.Fatal(err)
214 214
 	}
215
-	container.Wait()
215
+	container.State.WaitStop(-1 * time.Second)
216 216
 	output, err := ioutil.ReadAll(stdout)
217 217
 	if err != nil {
218 218
 		t.Fatal(err)
... ...
@@ -257,7 +258,7 @@ func TestTty(t *testing.T) {
257 257
 	if err := stdin.Close(); err != nil {
258 258
 		t.Fatal(err)
259 259
 	}
260
-	container.Wait()
260
+	container.State.WaitStop(-1 * time.Second)
261 261
 	output, err := ioutil.ReadAll(stdout)
262 262
 	if err != nil {
263 263
 		t.Fatal(err)
... ...
@@ -366,7 +367,7 @@ func BenchmarkRunParallel(b *testing.B) {
366 366
 				complete <- err
367 367
 				return
368 368
 			}
369
-			if err := container.WaitTimeout(15 * time.Second); err != nil {
369
+			if _, err := container.State.WaitStop(15 * time.Second); err != nil {
370 370
 				complete <- err
371 371
 				return
372 372
 			}
... ...
@@ -496,7 +496,7 @@ func startEchoServerContainer(t *testing.T, proto string) (*daemon.Daemon, *daem
496 496
 	})
497 497
 
498 498
 	// Even if the state is running, lets give some time to lxc to spawn the process
499
-	container.WaitTimeout(500 * time.Millisecond)
499
+	container.State.WaitStop(500 * time.Millisecond)
500 500
 
501 501
 	strPort = container.NetworkSettings.Ports[p][0].HostPort
502 502
 	return daemon, container, strPort
... ...
@@ -611,7 +611,7 @@ func TestRestore(t *testing.T) {
611 611
 	// Simulate a crash/manual quit of dockerd: process dies, states stays 'Running'
612 612
 	cStdin, _ := container2.StdinPipe()
613 613
 	cStdin.Close()
614
-	if err := container2.WaitTimeout(2 * time.Second); err != nil {
614
+	if _, err := container2.State.WaitStop(2 * time.Second); err != nil {
615 615
 		t.Fatal(err)
616 616
 	}
617 617
 	container2.State.SetRunning(42)
... ...
@@ -96,11 +96,13 @@ func containerAttach(eng *engine.Engine, id string, t utils.Fataler) (io.WriteCl
96 96
 }
97 97
 
98 98
 func containerWait(eng *engine.Engine, id string, t utils.Fataler) int {
99
-	return getContainer(eng, id, t).Wait()
99
+	ex, _ := getContainer(eng, id, t).State.WaitStop(-1 * time.Second)
100
+	return ex
100 101
 }
101 102
 
102 103
 func containerWaitTimeout(eng *engine.Engine, id string, t utils.Fataler) error {
103
-	return getContainer(eng, id, t).WaitTimeout(500 * time.Millisecond)
104
+	_, err := getContainer(eng, id, t).State.WaitStop(500 * time.Millisecond)
105
+	return err
104 106
 }
105 107
 
106 108
 func containerKill(eng *engine.Engine, id string, t utils.Fataler) {
... ...
@@ -307,7 +309,7 @@ func runContainer(eng *engine.Engine, r *daemon.Daemon, args []string, t *testin
307 307
 		return "", err
308 308
 	}
309 309
 
310
-	container.Wait()
310
+	container.State.WaitStop(-1 * time.Second)
311 311
 	data, err := ioutil.ReadAll(stdout)
312 312
 	if err != nil {
313 313
 		return "", err
... ...
@@ -17,6 +17,7 @@ import (
17 17
 	"sort"
18 18
 	"strings"
19 19
 	"syscall"
20
+	"time"
20 21
 
21 22
 	"github.com/dotcloud/docker/archive"
22 23
 	"github.com/dotcloud/docker/daemon"
... ...
@@ -696,7 +697,7 @@ func (b *buildFile) run(c *daemon.Container) error {
696 696
 	}
697 697
 
698 698
 	// Wait for it to finish
699
-	if ret := c.Wait(); ret != 0 {
699
+	if ret, _ := c.State.WaitStop(-1 * time.Second); ret != 0 {
700 700
 		err := &utils.JSONError{
701 701
 			Message: fmt.Sprintf("The command %v returned a non-zero code: %d", b.config.Cmd, ret),
702 702
 			Code:    ret,
... ...
@@ -2123,7 +2123,7 @@ func (srv *Server) ContainerWait(job *engine.Job) engine.Status {
2123 2123
 	}
2124 2124
 	name := job.Args[0]
2125 2125
 	if container := srv.daemon.Get(name); container != nil {
2126
-		status := container.Wait()
2126
+		status, _ := container.State.WaitStop(-1 * time.Second)
2127 2127
 		job.Printf("%d\n", status)
2128 2128
 		return engine.StatusOK
2129 2129
 	}
... ...
@@ -2336,7 +2336,7 @@ func (srv *Server) ContainerAttach(job *engine.Job) engine.Status {
2336 2336
 		// If we are in stdinonce mode, wait for the process to end
2337 2337
 		// otherwise, simply return
2338 2338
 		if container.Config.StdinOnce && !container.Config.Tty {
2339
-			container.Wait()
2339
+			container.State.WaitStop(-1 * time.Second)
2340 2340
 		}
2341 2341
 	}
2342 2342
 	return engine.StatusOK