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

provider/docker: Add support for a list of pull_triggers within the docker_image resource. #10845

Merged
merged 1 commit into from
Jan 3, 2017
Merged

provider/docker: Add support for a list of pull_triggers within the docker_image resource. #10845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion builtin/providers/docker/resource_docker_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,19 @@ func resourceDockerImage() *schema.Resource {
},

"pull_trigger": &schema.Schema{
Type: schema.TypeString,
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"pull_triggers"},
Deprecated: "Use field pull_triggers instead",
},

"pull_triggers": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
ForceNew: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
},
}
Expand Down
10 changes: 3 additions & 7 deletions builtin/providers/docker/resource_docker_image_funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,11 @@ func findImage(d *schema.ResourceData, client *dc.Client) (*dc.APIImages, error)
return nil, fmt.Errorf("Empty image name is not allowed")
}

foundImage := searchLocalImages(data, imageName)

if foundImage == nil {
if err := pullImage(&data, client, imageName); err != nil {
return nil, fmt.Errorf("Unable to pull image %s: %s", imageName, err)
}
if err := pullImage(&data, client, imageName); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we deleting the searchLocalImages check?

Copy link
Contributor Author

@hmcgonig hmcgonig Jan 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This was removed for two reasons, first because we do not need to care about if the image is found locally or not. A docker pull on an image that you already have just results in a no-op. Second, this is actually incorrect in the case of someone using a docker-swarm. If you search local images in the swarm, it will return any image on any host in the swarm. Because of this behavior, it was previously possible for terraform to try and start a container on a swarm host that did not have the image pulled, which is incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

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

does this pullImage work for both pull_trigger and pull_triggers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this functionality is correct in either case.

return nil, fmt.Errorf("Unable to pull image %s: %s", imageName, err)
}

foundImage = searchLocalImages(data, imageName)
foundImage := searchLocalImages(data, imageName)
if foundImage != nil {
return foundImage, nil
}
Expand Down
28 changes: 27 additions & 1 deletion builtin/providers/docker/resource_docker_image_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@ func TestAccDockerImage_data(t *testing.T) {
})
}

func TestAccDockerImage_data_pull_trigger(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
PreventPostDestroyRefresh: true,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccDockerImageFromDataConfigWithPullTrigger,
Check: resource.ComposeTestCheckFunc(
resource.TestMatchResourceAttr("docker_image.foobarbazoo", "latest", contentDigestRegexp),
),
},
},
})
}

func testAccDockerImageDestroy(s *terraform.State) error {
for _, rs := range s.RootModule().Resources {
if rs.Type != "docker_image" {
Expand Down Expand Up @@ -131,6 +147,16 @@ data "docker_registry_image" "foobarbaz" {
}
resource "docker_image" "foobarbaz" {
name = "${data.docker_registry_image.foobarbaz.name}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful to have 2 tests - 1 for pull_trigger and 1 for pull_triggers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do this, sure.

pull_trigger = "${data.docker_registry_image.foobarbaz.sha256_digest}"
pull_triggers = ["${data.docker_registry_image.foobarbaz.sha256_digest}"]
}
`

const testAccDockerImageFromDataConfigWithPullTrigger = `
data "docker_registry_image" "foobarbazoo" {
name = "alpine:3.1"
}
resource "docker_image" "foobarbazoo" {
name = "${data.docker_registry_image.foobarbazoo.name}"
pull_trigger = "${data.docker_registry_image.foobarbazoo.sha256_digest}"
}
`
2 changes: 1 addition & 1 deletion website/source/docs/providers/docker/d/registry_image.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ data "docker_registry_image" "ubuntu" {

resource "docker_image" "ubuntu" {
name = "${data.docker_registry_image.ubuntu.name}"
pull_trigger = "${data.docker_registry_image.ubuntu.sha256_digest}"
pull_triggers = ["${data.docker_registry_image.ubuntu.sha256_digest}"]
}
```

Expand Down
14 changes: 8 additions & 6 deletions website/source/docs/providers/docker/r/image.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Pulls a Docker image to a given Docker host from a Docker Registry.

This resource will *not* pull new layers of the image automatically unless used in
conjunction with [`docker_registry_image`](/docs/providers/docker/d/registry_image.html)
data source to update the `pull_trigger` field.
data source to update the `pull_triggers` field.

## Example Usage

Expand All @@ -36,7 +36,7 @@ data "docker_registry_image" "ubuntu" {

resource "docker_image" "ubuntu" {
name = "${data.docker_registry_image.ubuntu.name}"
pull_trigger = "${data.docker_registry_image.ubuntu.sha256_digest}"
pull_triggers = ["${data.docker_registry_image.ubuntu.sha256_digest}"]
}
```

Expand All @@ -48,10 +48,12 @@ The following arguments are supported:
* `keep_locally` - (Optional, boolean) If true, then the Docker image won't be
deleted on destroy operation. If this is false, it will delete the image from
the docker local storage on destroy operation.
* `pull_trigger` - (Optional, string) Used to store the image digest from the
registry and will cause an image pull when changed. Needed when using
the `docker_registry_image` [data source](/docs/providers/docker/d/registry_image.html)
to trigger an update of the image.
* `pull_triggers` - (Optional, list of strings) List of values which cause an
image pull when changed. This is used to store the image digest from the
registry when using the `docker_registry_image` [data source](/docs/providers/docker/d/registry_image.html)
to trigger an image update.
* `pull_trigger` - **Deprecated**, use `pull_triggers` instead.


## Attributes Reference

Expand Down