From 2e01e0635b667d1603aad5dbb0c5ed43fd60cdcc Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 12 Jun 2015 18:44:37 +0000 Subject: [PATCH 1/3] When linking to other containers, introduce a slight delay; this lets the Docker API get those containers running. Otherwise when you try to start a container linking to them, the start command will fail, leading to an error. --- .../docker/resource_docker_container_funcs.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/builtin/providers/docker/resource_docker_container_funcs.go b/builtin/providers/docker/resource_docker_container_funcs.go index 9408d8a6c..d355b898c 100644 --- a/builtin/providers/docker/resource_docker_container_funcs.go +++ b/builtin/providers/docker/resource_docker_container_funcs.go @@ -5,6 +5,7 @@ import ( "fmt" "strconv" "strings" + "time" dc "github.com/fsouza/go-dockerclient" "github.com/hashicorp/terraform/helper/schema" @@ -19,6 +20,8 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err return err } + delayStart := false + image := d.Get("image").(string) if _, ok := data.DockerImages[image]; !ok { if _, ok := data.DockerImages[image+":latest"]; !ok { @@ -106,6 +109,13 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err if v, ok := d.GetOk("links"); ok { hostConfig.Links = stringSetToStringSlice(v.(*schema.Set)) + delayStart = true + } + + // For instance, Docker will fail to start conatiners with links + // to other containers if the containers haven't started yet + if delayStart { + time.Sleep(3 * time.Second) } if err := client.StartContainer(retContainer.ID, hostConfig); err != nil { From 56cfba2509bbfe574d4c569e96e4201673ac2253 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 12 Jun 2015 21:01:36 +0000 Subject: [PATCH 2/3] Fix a serious problem when using links. Links cause there to be more than one name for a container to be returned. As a result, only looking at the first element of the container names could cause a container to not be found, leading Terraform to remove it from state and attempt to recreate it. --- .../docker/resource_docker_container_funcs.go | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/builtin/providers/docker/resource_docker_container_funcs.go b/builtin/providers/docker/resource_docker_container_funcs.go index d355b898c..779ce1d17 100644 --- a/builtin/providers/docker/resource_docker_container_funcs.go +++ b/builtin/providers/docker/resource_docker_container_funcs.go @@ -211,15 +211,17 @@ func fetchDockerContainer(name string, client *dc.Client) (*dc.APIContainers, er // Sometimes the Docker API prefixes container names with / // like it does in these commands. But if there's no // set name, it just uses the ID without a /...ugh. - var dockerContainerName string - if len(apiContainer.Names) > 0 { - dockerContainerName = strings.TrimLeft(apiContainer.Names[0], "/") - } else { - dockerContainerName = apiContainer.ID - } - - if dockerContainerName == name { - return &apiContainer, nil + switch len(apiContainer.Names) { + case 0: + if apiContainer.ID == name { + return &apiContainer, nil + } + default: + for _, containerName := range apiContainer.Names { + if strings.TrimLeft(containerName, "/") == name { + return &apiContainer, nil + } + } } } From edbc5783162e5ea4496d7aa888b7d6334f199e06 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 25 Jun 2015 14:38:56 +0000 Subject: [PATCH 3/3] As discussed on the issue, remove the hard-coded delay on startup in favor of attempting to detect if the initial container ever enters running state, and erroring out if not. It will re-check the container once every 500ms for 15 seconds total; future work could make that configurable. --- .../docker/resource_docker_container_funcs.go | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/builtin/providers/docker/resource_docker_container_funcs.go b/builtin/providers/docker/resource_docker_container_funcs.go index 779ce1d17..058a4411b 100644 --- a/builtin/providers/docker/resource_docker_container_funcs.go +++ b/builtin/providers/docker/resource_docker_container_funcs.go @@ -11,6 +11,10 @@ import ( "github.com/hashicorp/terraform/helper/schema" ) +var ( + creationTime time.Time +) + func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) error { var err error client := meta.(*dc.Client) @@ -20,15 +24,12 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err return err } - delayStart := false - image := d.Get("image").(string) if _, ok := data.DockerImages[image]; !ok { if _, ok := data.DockerImages[image+":latest"]; !ok { return fmt.Errorf("Unable to find image %s", image) - } else { - image = image + ":latest" } + image = image + ":latest" } // The awesome, wonderful, splendiferous, sensical @@ -109,15 +110,9 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err if v, ok := d.GetOk("links"); ok { hostConfig.Links = stringSetToStringSlice(v.(*schema.Set)) - delayStart = true - } - - // For instance, Docker will fail to start conatiners with links - // to other containers if the containers haven't started yet - if delayStart { - time.Sleep(3 * time.Second) } + creationTime = time.Now() if err := client.StartContainer(retContainer.ID, hostConfig); err != nil { return fmt.Errorf("Unable to start container: %s", err) } @@ -132,21 +127,49 @@ func resourceDockerContainerRead(d *schema.ResourceData, meta interface{}) error if err != nil { return err } - if apiContainer == nil { // This container doesn't exist anymore d.SetId("") - return nil } - container, err := client.InspectContainer(apiContainer.ID) - if err != nil { - return fmt.Errorf("Error inspecting container %s: %s", apiContainer.ID, err) + var container *dc.Container + + loops := 1 // if it hasn't just been created, don't delay + if !creationTime.IsZero() { + loops = 30 // with 500ms spacing, 15 seconds; ought to be plenty + } + sleepTime := 500 * time.Millisecond + + for i := loops; i > 0; i-- { + container, err = client.InspectContainer(apiContainer.ID) + if err != nil { + return fmt.Errorf("Error inspecting container %s: %s", apiContainer.ID, err) + } + + if container.State.Running || + (!container.State.Running && !d.Get("must_run").(bool)) { + break + } + + if creationTime.IsZero() { // We didn't just create it, so don't wait around + return resourceDockerContainerDelete(d, meta) + } + + if container.State.FinishedAt.After(creationTime) { + // It exited immediately, so error out so dependent containers + // aren't started + resourceDockerContainerDelete(d, meta) + return fmt.Errorf("Container %s exited after creation, error was: %s", apiContainer.ID, container.State.Error) + } + + time.Sleep(sleepTime) } - if d.Get("must_run").(bool) && !container.State.Running { - return resourceDockerContainerDelete(d, meta) + // Handle the case of the for loop above running its course + if !container.State.Running && d.Get("must_run").(bool) { + resourceDockerContainerDelete(d, meta) + return fmt.Errorf("Container %s failed to be in running state", apiContainer.ID) } // Read Network Settings