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.

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

Fixed bug where docker_container resource was not working properly with swarm #3364

Conversation

josephholsten
Copy link
Contributor

Just a rebase of: #1990

@josephholsten
Copy link
Contributor Author

oh, there was a stray space causing a go vet error. fixed nao.

@peterbourgon
Copy link

Can has merge?

@tomwilkie
Copy link
Contributor

I believe there was an error rebasing this, it be:

diff --git a/builtin/providers/docker/resource_docker_container_funcs.go b/builtin/providers/docker/resource_docker_container_funcs.go
index aa74a4e..45897cc 100644
--- a/builtin/providers/docker/resource_docker_container_funcs.go
+++ b/builtin/providers/docker/resource_docker_container_funcs.go
@@ -4,7 +4,6 @@ import (
        "errors"
        "fmt"
        "strconv"
-       "strings"
        "time"

        dc "github.com/fsouza/go-dockerclient"
@@ -123,7 +122,7 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err
 func resourceDockerContainerRead(d *schema.ResourceData, meta interface{}) error {
        client := meta.(*dc.Client)

-       apiContainer, err := fetchDockerContainer(d.Get("name").(string), client)
+       apiContainer, err := fetchDockerContainer(d.Id(), client)
        if err != nil {
                return err
        }
@@ -223,28 +222,15 @@ func stringSetToStringSlice(stringSet *schema.Set) []string {
        return ret
 }

-func fetchDockerContainer(name string, client *dc.Client) (*dc.APIContainers, error) {
+func fetchDockerContainer(ID string, client *dc.Client) (*dc.APIContainers, error) {
        apiContainers, err := client.ListContainers(dc.ListContainersOptions{All: true})
-
        if err != nil {
                return nil, fmt.Errorf("Error fetching container information from Docker: %s\n", err)
        }

        for _, apiContainer := range apiContainers {
-               // 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.
-               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
-                               }
-                       }
+               if apiContainer.ID == ID {
+                       return &apiContainer, nil
                }
        }

@tomwilkie
Copy link
Contributor

Above fix (and rebase) is here: https://github.com/tomwilkie/terraform/tree/swarm-fix

@jen20
Copy link
Contributor

jen20 commented Dec 2, 2015

Looks good to me - we should merge this following #3761 (in order that we catch all the changes which need making). I'll come back to this tomorrow when the 0.6.8 release window is closed.

jen20 added a commit that referenced this pull request Dec 2, 2015
This reapplies the patch mentioned in #3364 - for an unknown reason the
diff there was incorrect.
@jen20
Copy link
Contributor

jen20 commented Dec 2, 2015

As @tomwilkie points out there was likely an issue rebasing here, I have manually applied the patch and opened up a new pull request at #4148. I'll go ahead and close this PR in favour of that one.

@jen20 jen20 closed this Dec 2, 2015
@ghost
Copy link

ghost commented Apr 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants