provider/openstack: Updates to openstack_images_image_v2 resource

This commit has a few more fixes to the recently added
openstack_images_image_v2 resource:

* tags were changed to a Set because the OpenStack Image API does
not seem to respect ordering.
* The visibility argument was fixed.
* Acceptance tests for all updatable fields has been implemented.
* Documentation updates, including a new entry in the sidebar.

builtin/providers/openstack/provider_test.go

@@ -60,6 +60,13 @@ func testAccPreCheck(t *testing.T) {
 	}
 }
 
+func testAccPreCheckAdminOnly(t *testing.T) {
+	v := os.Getenv("OS_USERNAME")
+	if v != "admin" {
+		t.Skip("Skipping test because it requires the admin user")
+	}
+}
+
 func TestProvider(t *testing.T) {
 	if err := Provider().(*schema.Provider).InternalValidate(); err != nil {
 		t.Fatalf("err: %s", err)

builtin/providers/openstack/resource_openstack_images_image_v2.go

@@ -141,9 +141,10 @@ func resourceImagesImageV2() *schema.Resource {
 			},
 
 			"tags": &schema.Schema{
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
 				Elem:     &schema.Schema{Type: schema.TypeString},
+				Set:      schema.HashString,
 			},
 
 			"update_at": &schema.Schema{
@@ -156,7 +157,7 @@ func resourceImagesImageV2() *schema.Resource {
 				Optional:     true,
 				ForceNew:     false,
 				ValidateFunc: resourceImagesImageV2ValidateVisibility,
-				Default:      images.ImageVisibilityPrivate,
+				Default:      "private",
 			},
 		},
 	}
@@ -182,11 +183,8 @@ func resourceImagesImageV2Create(d *schema.ResourceData, meta interface{}) error
 	}
 
 	if v, ok := d.GetOk("tags"); ok {
-		var tags []string
+		tags := v.(*schema.Set).List()
-		for _, tag := range v.([]interface{}) {
+		createOpts.Tags = resourceImagesImageV2BuildTags(tags)
-			tags = append(tags, tag.(string))
-		}
-		createOpts.Tags = tags
 	}
 
 	d.Partial(true)
@@ -273,9 +271,8 @@ func resourceImagesImageV2Read(d *schema.ResourceData, meta interface{}) error {
 	d.Set("name", img.Name)
 	d.Set("protected", img.Protected)
 	d.Set("size_bytes", img.SizeBytes)
-	d.Set("tags", resourceImagesImageV2RemoveEmptyTags(img.Tags))
+	d.Set("tags", img.Tags)
 	d.Set("visibility", img.Visibility)
-
 	return nil
 }
 
@@ -289,7 +286,8 @@ func resourceImagesImageV2Update(d *schema.ResourceData, meta interface{}) error
 	updateOpts := make(images.UpdateOpts, 0)
 
 	if d.HasChange("visibility") {
-		v := images.UpdateVisibility{Visibility: d.Get("visibility").(images.ImageVisibility)}
+		visibility := resourceImagesImageV2VisibilityFromString(d.Get("visibility").(string))
+		v := images.UpdateVisibility{Visibility: visibility}
 		updateOpts = append(updateOpts, v)
 	}
 
@@ -299,7 +297,10 @@ func resourceImagesImageV2Update(d *schema.ResourceData, meta interface{}) error
 	}
 
 	if d.HasChange("tags") {
-		v := images.ReplaceImageTags{NewTags: d.Get("tags").([]string)}
+		tags := d.Get("tags").(*schema.Set).List()
+		v := images.ReplaceImageTags{
+			NewTags: resourceImagesImageV2BuildTags(tags),
+		}
 		updateOpts = append(updateOpts, v)
 	}
 
@@ -330,12 +331,22 @@ func resourceImagesImageV2Delete(d *schema.ResourceData, meta interface{}) error
 }
 
 func resourceImagesImageV2ValidateVisibility(v interface{}, k string) (ws []string, errors []error) {
-	value := v.(images.ImageVisibility)
+	value := v.(string)
-	if value == images.ImageVisibilityPublic || value == images.ImageVisibilityPrivate || value == images.ImageVisibilityShared || value == images.ImageVisibilityCommunity {
+	validVisibilities := []string{
-		return
+		"public",
+		"private",
+		"shared",
+		"community",
 	}
 
-	errors = append(errors, fmt.Errorf("%q must be one of %q, %q, %q, %q", k, images.ImageVisibilityPublic, images.ImageVisibilityPrivate, images.ImageVisibilityCommunity, images.ImageVisibilityShared))
+	for _, v := range validVisibilities {
+		if value == v {
+			return
+		}
+	}
+
+	err := fmt.Errorf("%s must be one of %s", k, validVisibilities)
+	errors = append(errors, err)
 	return
 }
 
@@ -476,12 +487,11 @@ func resourceImagesImageV2RefreshFunc(client *gophercloud.ServiceClient, id stri
 	}
 }
 
-func resourceImagesImageV2RemoveEmptyTags(s []string) []string {
+func resourceImagesImageV2BuildTags(v []interface{}) []string {
-	var r []string
+	var tags []string
-	for _, str := range s {
+	for _, tag := range v {
-		if str != "" {
+		tags = append(tags, tag.(string))
-			r = append(r, str)
 	}
-	}
+
-	return r
+	return tags
 }

builtin/providers/openstack/resource_openstack_images_image_v2_test.go

@@ -36,7 +36,7 @@ func TestAccImagesImageV2_basic(t *testing.T) {
 	})
 }
 
-func TestAccImagesImageV2_with_tags(t *testing.T) {
+func TestAccImagesImageV2_name(t *testing.T) {
 	var image images.Image
 
 	resource.Test(t, resource.TestCase{
@@ -45,7 +45,35 @@ func TestAccImagesImageV2_with_tags(t *testing.T) {
 		CheckDestroy: testAccCheckImagesImageV2Destroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccImagesImageV2_with_tags,
+				Config: testAccImagesImageV2_name_1,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					resource.TestCheckResourceAttr(
+						"openstack_images_image_v2.image_1", "name", "Rancher TerraformAccTest"),
+				),
+			},
+			resource.TestStep{
+				Config: testAccImagesImageV2_name_2,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					resource.TestCheckResourceAttr(
+						"openstack_images_image_v2.image_1", "name", "TerraformAccTest Rancher"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccImagesImageV2_tags(t *testing.T) {
+	var image images.Image
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckImagesImageV2Destroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccImagesImageV2_tags_1,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
 					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "foo"),
@@ -53,6 +81,56 @@ func TestAccImagesImageV2_with_tags(t *testing.T) {
 					testAccCheckImagesImageV2TagCount("openstack_images_image_v2.image_1", 2),
 				),
 			},
+			resource.TestStep{
+				Config: testAccImagesImageV2_tags_2,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "foo"),
+					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "bar"),
+					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "baz"),
+					testAccCheckImagesImageV2TagCount("openstack_images_image_v2.image_1", 3),
+				),
+			},
+			resource.TestStep{
+				Config: testAccImagesImageV2_tags_3,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "foo"),
+					testAccCheckImagesImageV2HasTag("openstack_images_image_v2.image_1", "baz"),
+					testAccCheckImagesImageV2TagCount("openstack_images_image_v2.image_1", 2),
+				),
+			},
+		},
+	})
+}
+
+func TestAccImagesImageV2_visibility(t *testing.T) {
+	var image images.Image
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+			testAccPreCheckAdminOnly(t)
+		},
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckImagesImageV2Destroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccImagesImageV2_visibility_1,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					resource.TestCheckResourceAttr(
+						"openstack_images_image_v2.image_1", "visibility", "private"),
+				),
+			},
+			resource.TestStep{
+				Config: testAccImagesImageV2_visibility_2,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckImagesImageV2Exists("openstack_images_image_v2.image_1", &image),
+					resource.TestCheckResourceAttr(
+						"openstack_images_image_v2.image_1", "visibility", "public"),
+				),
+			},
 		},
 	})
 }
@@ -188,7 +266,23 @@ var testAccImagesImageV2_basic = `
       disk_format = "qcow2"
   }`
 
-var testAccImagesImageV2_with_tags = `
+var testAccImagesImageV2_name_1 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "Rancher TerraformAccTest"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+  }`
+
+var testAccImagesImageV2_name_2 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "TerraformAccTest Rancher"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+  }`
+
+var testAccImagesImageV2_tags_1 = `
   resource "openstack_images_image_v2" "image_1" {
       name   = "Rancher TerraformAccTest"
       image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
@@ -196,3 +290,39 @@ var testAccImagesImageV2_with_tags = `
       disk_format = "qcow2"
       tags = ["foo","bar"]
   }`
+
+var testAccImagesImageV2_tags_2 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "Rancher TerraformAccTest"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+      tags = ["foo","bar","baz"]
+  }`
+
+var testAccImagesImageV2_tags_3 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "Rancher TerraformAccTest"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+      tags = ["foo","baz"]
+  }`
+
+var testAccImagesImageV2_visibility_1 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "Rancher TerraformAccTest"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+      visibility = "private"
+  }`
+
+var testAccImagesImageV2_visibility_2 = `
+  resource "openstack_images_image_v2" "image_1" {
+      name   = "Rancher TerraformAccTest"
+      image_source_url = "https://releases.rancher.com/os/latest/rancheros-openstack.img"
+      container_format = "bare"
+      disk_format = "qcow2"
+      visibility = "public"
+  }`

website/source/docs/providers/openstack/r/images_image_v2.html.markdown

@@ -61,9 +61,11 @@ The following arguments are supported:
     is used. Changing this creates a new Image.
 
 * `tags` - (Optional) The tags of the image. It must be a list of strings.
+    At this time, it is not possible to delete all tags of an image.
 
 * `visibility` - (Optional) The visibility of the image. Must be one of
-   "public", "private", "community", or "shared". 
+   "public", "private", "community", or "shared". The ability to set the
+   visibility depends upon the configuration of the OpenStack cloud.
 
 Note: The `properties` attribute handling in the gophercloud library is currently buggy
 and needs to be fixed before being implemented in this resource.

website/source/layouts/openstack.erb

@@ -49,6 +49,15 @@
           </ul>
         </li>
 
+        <li<%= sidebar_current(/^docs-openstack-resource-images/) %>>
+          <a href="#">Images Resources</a>
+          <ul class="nav nav-visible">
+            <li<%= sidebar_current("docs-openstack-resource-images-image-v2") %>>
+              <a href="/docs/providers/openstack/r/images_image_v2.html">openstack_images_image_v2</a>
+            </li>
+          </ul>
+        </li>
+
         <li<%= sidebar_current(/^docs-openstack-resource-networking/) %>>
           <a href="#">Networking Resources</a>
           <ul class="nav nav-visible">