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

Add support for using labels on compute_instance #150

Merged
merged 1 commit into from
Jun 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 50 additions & 0 deletions google/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,18 @@ func resourceComputeInstance() *schema.Resource {
Computed: true,
},

"labels": &schema.Schema{
Type: schema.TypeMap,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},

"label_fingerprint": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},

"create_timeout": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Expand Down Expand Up @@ -676,6 +688,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
Name: d.Get("name").(string),
NetworkInterfaces: networkInterfaces,
Tags: resourceInstanceTags(d),
Labels: resourceInstanceLabels(d),
ServiceAccounts: serviceAccounts,
Scheduling: scheduling,
}
Expand Down Expand Up @@ -845,6 +858,14 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
d.Set("tags_fingerprint", instance.Tags.Fingerprint)
}

if len(instance.Labels) > 0 {
d.Set("labels", instance.Labels)
}

if instance.LabelFingerprint != "" {
d.Set("label_fingerprint", instance.LabelFingerprint)
}

disksCount := d.Get("disk.#").(int)
attachedDisksCount := d.Get("attached_disk.#").(int)
disks := make([]map[string]interface{}, 0, disksCount)
Expand Down Expand Up @@ -977,6 +998,24 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
d.SetPartial("tags")
}

if d.HasChange("labels") {
labels := resourceInstanceLabels(d)
labelFingerprint := d.Get("label_fingerprint").(string)
req := compute.InstancesSetLabelsRequest{Labels: labels, LabelFingerprint: labelFingerprint}

op, err := config.clientCompute.Instances.SetLabels(project, zone, d.Id(), &req).Do()
if err != nil {
return fmt.Errorf("Error updating labels: %s", err)
}

opErr := computeOperationWaitZone(config, op, project, zone, "labels to update")
if opErr != nil {
return opErr
}

d.SetPartial("labels")
}

if d.HasChange("scheduling") {
prefix := "scheduling.0"
scheduling := &compute.Scheduling{}
Expand Down Expand Up @@ -1127,6 +1166,17 @@ func resourceInstanceMetadata(d *schema.ResourceData) (*compute.Metadata, error)
return m, nil
}

func resourceInstanceLabels(d *schema.ResourceData) map[string]string {
labels := map[string]string{}
if v, ok := d.GetOk("labels"); ok {
labelMap := v.(map[string]interface{})
for k, v := range labelMap {
labels[k] = v.(string)
}
}
return labels
}

func resourceInstanceTags(d *schema.ResourceData) *compute.Tags {
// Calculate the tags
var tags *compute.Tags
Expand Down
42 changes: 38 additions & 4 deletions google/resource_compute_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func TestAccComputeInstance_basic1(t *testing.T) {
testAccCheckComputeInstanceExists(
"google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceTag(&instance, "foo"),
testAccCheckComputeInstanceLabel(&instance, "my_key", "my_value"),
testAccCheckComputeInstanceMetadata(&instance, "foo", "bar"),
testAccCheckComputeInstanceMetadata(&instance, "baz", "qux"),
testAccCheckComputeInstanceDisk(&instance, instanceName, true, true),
Expand Down Expand Up @@ -385,6 +386,7 @@ func TestAccComputeInstance_update(t *testing.T) {
"google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceMetadata(
&instance, "bar", "baz"),
testAccCheckComputeInstanceLabel(&instance, "only_me", "nothing_else"),
testAccCheckComputeInstanceTag(&instance, "baz"),
testAccCheckComputeInstanceAccessConfig(&instance),
),
Expand Down Expand Up @@ -795,6 +797,24 @@ func testAccCheckComputeInstanceTag(instance *compute.Instance, n string) resour
}
}

func testAccCheckComputeInstanceLabel(instance *compute.Instance, key string, value string) resource.TestCheckFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this checks that a single label exists but doesn't check that there aren't labels we don't expect to be there. Do you think this is sufficient or should it take the key/value map and compare that?

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 could add that but I'm not sure it's something that could happen... Although it might make the test syntax cleaner later down the road.

At some point I think our tests would benefit from building a terraform config up programmatically via a builder or something similar; this would end up making the tests a lot more explicit (currently a lot of information is hidden in the 'config' function). If we had that, then testing the entire map would be a lot easier...

Something like:

	resource.Test(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testAccCheckComputeInstanceDestroy,
		Steps: []resource.TestStep{
			resource.TestStep{
				Config: NewInstanceBuilder(instanceName).WithLabels(myLabels).Build(),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckComputeInstanceExists(
						"google_compute_instance.foobar", &instance),
					testAccCheckComputeInstanceHasLabels(&instance, myLabels),
				),
			},
		},

return func(s *terraform.State) error {
if instance.Labels == nil {
return fmt.Errorf("no labels found on instance %s", instance.Name)
}

v, ok := instance.Labels[key]
if !ok {
return fmt.Errorf("No label found with key %s on instance %s", key, instance.Name)
}
if v != value {
return fmt.Errorf("Expected value '%s' but found value '%s' for label '%s' on instance %s", value, v, key, instance.Name)
}

return nil
}
}

func testAccCheckComputeInstanceServiceAccount(instance *compute.Instance, scope string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if count := len(instance.ServiceAccounts); count != 1 {
Expand Down Expand Up @@ -919,6 +939,11 @@ resource "google_compute_instance" "foobar" {
create_timeout = 5

metadata_startup_script = "echo Hello"

labels {
my_key = "my_value"
my_other_key = "my_other_value"
}
}
`, instance)
}
Expand Down Expand Up @@ -1050,10 +1075,11 @@ resource "google_compute_instance" "foobar" {
func testAccComputeInstance_update(instance string) string {
return fmt.Sprintf(`
resource "google_compute_instance" "foobar" {
name = "%s"
machine_type = "n1-standard-1"
zone = "us-central1-a"
tags = ["baz"]
name = "%s"
machine_type = "n1-standard-1"
zone = "us-central1-a"
can_ip_forward = false
tags = ["baz"]

disk {
image = "debian-8-jessie-v20160803"
Expand All @@ -1067,6 +1093,14 @@ resource "google_compute_instance" "foobar" {
metadata {
bar = "baz"
}

create_timeout = 5

metadata_startup_script = "echo Hello"

labels {
only_me = "nothing_else"
}
}
`, instance)
}
Expand Down
4 changes: 4 additions & 0 deletions website/docs/r/compute_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ The following arguments are supported:

* `tags` - (Optional) A list of tags to attach to the instance.

* `labels` - (Optional) A set of key/value label pairs to assign to the instance.

* `create_timeout` - (Optional) Configurable timeout in minutes for creating instances. Default is 4 minutes.
Changing this forces a new resource to be created.

Expand Down Expand Up @@ -208,6 +210,8 @@ exported:

* `tags_fingerprint` - The unique fingerprint of the tags.

* `label_fingerprint` - The unique fingerprint of the labels.

* `network_interface.0.address` - The internal ip address of the instance, either manually or dynamically assigned.

* `network_interface.0.access_config.0.assigned_nat_ip` - If the instance has an access config, either the given external ip (in the `nat_ip` field) or the ephemeral (generated) ip (if you didn't provide one).
Expand Down