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

Allow specifying accelerators in cluster node_config (#1067) #1115

Merged
merged 3 commits into from
Feb 24, 2018
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
59 changes: 49 additions & 10 deletions google/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ var schemaNodeConfig = &schema.Schema{
ValidateFunc: validation.IntAtLeast(10),
},

"guest_accelerator": &schema.Schema{
Type: schema.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"count": &schema.Schema{
Type: schema.TypeInt,
Required: true,
ForceNew: true,
},
"type": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: linkDiffSuppress,
},
},
},
},

"image_type": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -128,6 +150,22 @@ func expandNodeConfig(v interface{}) *container.NodeConfig {
nc.MachineType = v.(string)
}

if v, ok := nodeConfig["guest_accelerator"]; ok {
accels := v.([]interface{})
guestAccelerators := make([]*container.AcceleratorConfig, 0, len(accels))
for _, raw := range accels {
data := raw.(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you could make accels a []map[string]interface{}, which would mean you'd only have to do one of these casts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making accels a []map[string]interface{} leads to an error:

panic: interface conversion: interface {} is []interface {}, not []map[string]interface {}

goroutine 297 [running]:
github.com/terraform-providers/terraform-provider-google/google.expandNodeConfig(0x16797c0, 0xc4205361c0, 0xb)
	/home/i056593/Code/go/src/github.com/terraform-providers/terraform-provider-google/google/node_config.go:154 +0x12e0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, all right, that makes sense.

if data["count"].(int) == 0 {
continue
}
guestAccelerators = append(guestAccelerators, &container.AcceleratorConfig{
AcceleratorCount: int64(data["count"].(int)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be able to do data["count"].(int64) - why do it this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using data["count"].(int64) leads to an error:

panic: interface conversion: interface {} is int, not int64

goroutine 296 [running]:
github.com/terraform-providers/terraform-provider-google/google.expandNodeConfig(0x16797c0, 0xc42035a040, 0xb)
	/home/i056593/Code/go/src/github.com/terraform-providers/terraform-provider-google/google/node_config.go:162 +0x12b4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AcceleratorType: data["type"].(string),
})
}
nc.Accelerators = guestAccelerators
}

if v, ok := nodeConfig["disk_size_gb"]; ok {
nc.DiskSizeGb = int64(v.(int))
}
Expand Down Expand Up @@ -196,16 +234,17 @@ func flattenNodeConfig(c *container.NodeConfig) []map[string]interface{} {
}

config = append(config, map[string]interface{}{
"machine_type": c.MachineType,
"disk_size_gb": c.DiskSizeGb,
"local_ssd_count": c.LocalSsdCount,
"service_account": c.ServiceAccount,
"metadata": c.Metadata,
"image_type": c.ImageType,
"labels": c.Labels,
"tags": c.Tags,
"preemptible": c.Preemptible,
"min_cpu_platform": c.MinCpuPlatform,
"machine_type": c.MachineType,
"disk_size_gb": c.DiskSizeGb,
"guest_accelerator": c.Accelerators,
"local_ssd_count": c.LocalSsdCount,
"service_account": c.ServiceAccount,
"metadata": c.Metadata,
"image_type": c.ImageType,
"labels": c.Labels,
"tags": c.Tags,
"preemptible": c.Preemptible,
"min_cpu_platform": c.MinCpuPlatform,
})

if len(c.OauthScopes) > 0 {
Expand Down
72 changes: 72 additions & 0 deletions google/resource_container_node_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,28 @@ func TestAccContainerNodePool_withNodeConfig(t *testing.T) {
})
}

func TestAccContainerNodePool_withGPU(t *testing.T) {
t.Parallel()

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckContainerNodePoolDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccContainerNodePool_withGPU(),
Check: resource.ComposeTestCheckFunc(
testAccCheckContainerNodePoolMatches("google_container_node_pool.np_with_gpu"),
),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great - can you stick an ImportState step on there so we know that accelerated node pools can be imported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be the only ImportState in this file - why introduce it for just this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

We've talked to Hashicorp about best practices recently, and so it's a new thing we're doing with new tests.

resource.TestStep{
ResourceName: "google_container_node_pool.np_with_gpu",
ImportState: true,
},
},
})
}

func TestAccContainerNodePool_withManagement(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -315,6 +337,18 @@ func testAccCheckContainerNodePoolMatches(n string) resource.TestCheckFunc {

}

tfGA := attributes["node_config.0.guest_accelerator.#"] == "1"
if gcpGA := nodepool.Config.Accelerators != nil && len(nodepool.Config.Accelerators) == 1; tfGA != gcpGA {
if tf := attributes["node_config.0.guest_accelerator.0.type"]; nodepool.Config.Accelerators[0].AcceleratorType != tf {
return fmt.Errorf("Mismatched NodeConfig.Accelerators type. TF State: %s. GCP State: %s",
tf, nodepool.Config.Accelerators[0].AcceleratorType)
}
if tf := attributes["node_config.0.guest_accelerator.0.count"]; strconv.FormatInt(nodepool.Config.Accelerators[0].AcceleratorCount, 10) != tf {
return fmt.Errorf("Mismatched NodeConfig.Accelerators count. TF State: %s. GCP State: %d",
tf, nodepool.Config.Accelerators[0].AcceleratorCount)
}
}

return nil
}
}
Expand Down Expand Up @@ -583,6 +617,44 @@ resource "google_container_node_pool" "np_with_node_config" {
}`, acctest.RandString(10), acctest.RandString(10))
}

func testAccContainerNodePool_withGPU() string {
return fmt.Sprintf(`
resource "google_container_cluster" "cluster" {
name = "tf-cluster-nodepool-test-%s"
zone = "us-central1-c"
initial_node_count = 1
node_version = "1.9.2-gke.1"
min_master_version = "1.9.2-gke.1"
}
resource "google_container_node_pool" "np_with_gpu" {
name = "tf-nodepool-test-%s"
zone = "us-central1-c"
cluster = "${google_container_cluster.cluster.name}"
initial_node_count = 1
node_config {
machine_type = "n1-standard-1"
disk_size_gb = 10
oauth_scopes = [
"https://www.googleapis.com/auth/devstorage.read_only",
"https://www.googleapis.com/auth/logging.write",
"https://www.googleapis.com/auth/monitoring",
"https://www.googleapis.com/auth/service.management.readonly",
"https://www.googleapis.com/auth/servicecontrol",
"https://www.googleapis.com/auth/trace.append"
]
preemptible = true
service_account = "default"
image_type = "COS"
guest_accelerator = [
{
type = "nvidia-tesla-k80"
count = 1
}
]
}
}`, acctest.RandString(10), acctest.RandString(10))
}

func testAccContainerNodePool_withNodeConfigScopeAlias() string {
return fmt.Sprintf(`
resource "google_container_cluster" "cluster" {
Expand Down
9 changes: 9 additions & 0 deletions website/docs/r/container_cluster.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ The `node_config` block supports:
* `disk_size_gb` - (Optional) Size of the disk attached to each node, specified
in GB. The smallest allowed disk size is 10GB. Defaults to 100GB.

* `guest_accelerator` - (Optional) List of the type and count of accelerator cards attached to the instance.
Structure documented below.

* `image_type` - (Optional) The image type to use for this node.

* `labels` - (Optional) The Kubernetes labels (key/value pairs) to be applied to each node.
Expand Down Expand Up @@ -276,6 +279,12 @@ The `node_config` block supports:
* `tags` - (Optional) The list of instance tags applied to all nodes. Tags are used to identify
valid sources or targets for network firewalls.

The `guest_accelerator` block supports:

* `type` (Required) - The accelerator type resource to expose to this instance. E.g. `nvidia-tesla-k80`.

* `count` (Required) - The number of the guest accelerator cards exposed to this instance.

## Attributes Reference

In addition to the arguments listed above, the following computed attributes are
Expand Down
4 changes: 4 additions & 0 deletions website/docs/r/container_node_pool.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ resource "google_container_cluster" "primary" {
"https://www.googleapis.com/auth/logging.write",
"https://www.googleapis.com/auth/monitoring",
]
guest_accelerator = [{
type="nvidia-tesla-k80"
count=1
}]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to add the details of this structure to the documentation below here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
```
Expand Down