From efb9bbf9af18193c014d635fdf5e14e7adc70bcc Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Thu, 24 Jan 2019 18:32:45 -0800 Subject: [PATCH 1/7] Init work to add log analytics for ACI. --- azurerm/resource_arm_container_group.go | 112 ++++++++++++++++++- azurerm/resource_arm_container_group_test.go | 65 ++++++++++- 2 files changed, 173 insertions(+), 4 deletions(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 0bdf5bd2dfaf..7f0343c2bd21 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -279,6 +279,44 @@ func resourceArmContainerGroup() *schema.Resource { }, }, + "log_analytics": { + Type: schema.TypeList, + Optional: true, + ForceNew: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "workspace_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, + }, + "workspace_key": { + Type: schema.TypeString, + Required: true, + Sensitive: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, + }, + "log_type": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{ + string(containerinstance.ContainerInsights), + string(containerinstance.ContainerInstanceLogs), + }, true), + }, + "metadata": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + }, + }, + }, + }, + "ip_address": { Type: schema.TypeString, Computed: true, @@ -340,8 +378,17 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e containerGroup.ContainerGroupProperties.IPAddress.DNSNameLabel = &dnsNameLabel } - if _, err := client.CreateOrUpdate(ctx, resGroup, name, containerGroup); err != nil { - return err + containerGroup.Diagnostics = &containerinstance.ContainerGroupDiagnostics{ + LogAnalytics: expandContainerLogAnalytics(d), + } + + future, err := client.CreateOrUpdate(ctx, resGroup, name, containerGroup) + if err != nil { + return fmt.Errorf("Error creating/updating container group %q (Resource Group %q): %+v", name, resGroup, err) + } + + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("Error waiting for completion of container group %q (Resource Group %q): %+v", name, resGroup, err) } read, err := client.Get(ctx, resGroup, name) @@ -406,7 +453,14 @@ func resourceArmContainerGroupRead(d *schema.ResourceData, meta interface{}) err d.Set("restart_policy", string(props.RestartPolicy)) d.Set("os_type", string(props.OsType)) + + if diag := props.Diagnostics; diag != nil { + if err := d.Set("log_analytics", flattenContainerLogAnalytics(diag.LogAnalytics)); err != nil { + return fmt.Errorf("Error setting `log_analytics`: %+v", err) + } + } } + flattenAndSetTags(d, resp.Tags) return nil @@ -647,6 +701,33 @@ func expandContainerVolumes(input interface{}) (*[]containerinstance.VolumeMount return &volumeMounts, &containerGroupVolumes } +func expandContainerLogAnalytics(d *schema.ResourceData) *containerinstance.LogAnalytics { + logList := d.Get("log_analytics").([]interface{}) + + if len(logList) <= 0 { + return nil + } + + log := logList[0].(map[string]interface{}) + ws_id := log["workspace_id"].(string) + ws_key := log["workspace_key"].(string) + log_type := log["log_type"].(string) + metadataMap := log["metadata"].(map[string]interface{}) + metadata := make(map[string]*string) + for k, v := range metadataMap { + // TODO: + strValue := v.(string) + metadata[k] = &strValue + } + + return &containerinstance.LogAnalytics{ + WorkspaceID: &ws_id, + WorkspaceKey: &ws_key, + LogType: containerinstance.LogAnalyticsLogType(log_type), + Metadata: metadata, + } +} + func flattenContainerImageRegistryCredentials(d *schema.ResourceData, input *[]containerinstance.ImageRegistryCredential) []interface{} { if input == nil { return nil @@ -876,6 +957,33 @@ func flattenContainerVolumes(volumeMounts *[]containerinstance.VolumeMount, cont return volumeConfigs } +func flattenContainerLogAnalytics(input *containerinstance.LogAnalytics) []interface{} { + if input == nil { + return []interface{}{} + } + + output := make(map[string]interface{}) + + if input.WorkspaceID != nil { + output["workspace_id"] = *input.WorkspaceID + } + + if input.WorkspaceKey != nil { + output["workspace_key"] = *input.WorkspaceKey + } + + output["log_type"] = containerinstance.LogAnalyticsLogType(input.LogType) + + // TODO: string ? + metadata := make(map[string]interface{}) + for k, v := range input.Metadata { + metadata[k] = *v + } + output["metadata"] = metadata + + return []interface{}{output} +} + func resourceArmContainerGroupPortsHash(v interface{}) int { var buf bytes.Buffer diff --git a/azurerm/resource_arm_container_group_test.go b/azurerm/resource_arm_container_group_test.go index 17a71c477841..9190e62201da 100644 --- a/azurerm/resource_arm_container_group_test.go +++ b/azurerm/resource_arm_container_group_test.go @@ -217,6 +217,8 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "container.0.volume.0.read_only", "false"), resource.TestCheckResourceAttr(resourceName, "os_type", "Linux"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "OnFailure"), + resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), + resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", "ContainerInsights"), ), }, { @@ -293,6 +295,7 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "container.0.secure_environment_variables.secureFoo1", "secureBar1"), resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "Never"), + resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), ), }, { @@ -543,6 +546,26 @@ resource "azurerm_resource_group" "test" { location = "%s" } +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + sku = "PerGB2018" +} + +resource "azurerm_log_analytics_solution" "test" { + solution_name = "ContainerInsights" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + workspace_resource_id = "${azurerm_log_analytics_workspace.test.id}" + workspace_name = "${azurerm_log_analytics_workspace.test.name}" + + plan { + publisher = "Microsoft" + product = "OMSGallery/ContainerInsights" + } +} + resource "azurerm_container_group" "test" { name = "acctestcontainergroup-%d" location = "${azurerm_resource_group.test.location}" @@ -575,11 +598,20 @@ resource "azurerm_container_group" "test" { commands = ["cmd.exe", "echo", "hi"] } + log_analytics { + workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" + workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" + log_type = "ContainerInsights" + metadata { + "node-name" = "acctestContainerGroup" + } + } + tags { environment = "Testing" } } -`, ri, location, ri, ri) +`, ri, location, ri, ri, ri) } func testAccAzureRMContainerGroup_linuxComplete(ri int, location string) string { @@ -589,6 +621,26 @@ resource "azurerm_resource_group" "test" { location = "%s" } +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + sku = "PerGB2018" +} + +resource "azurerm_log_analytics_solution" "test" { + solution_name = "ContainerInsights" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + workspace_resource_id = "${azurerm_log_analytics_workspace.test.id}" + workspace_name = "${azurerm_log_analytics_workspace.test.name}" + + plan { + publisher = "Microsoft" + product = "OMSGallery/ContainerInsights" + } +} + resource "azurerm_storage_account" "test" { name = "accsa%d" resource_group_name = "${azurerm_resource_group.test.name}" @@ -649,11 +701,20 @@ resource "azurerm_container_group" "test" { commands = ["/bin/bash", "-c", "ls"] } + log_analytics { + workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" + workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" + log_type = "ContainerInsights" + metadata { + "node-name" = "acctestContainerGroup" + } + } + tags { environment = "Testing" } } -`, ri, location, ri, ri, ri, ri) +`, ri, location, ri, ri, ri, ri, ri) } func testCheckAzureRMContainerGroupExists(resourceName string) resource.TestCheckFunc { From a0c25fed65695b7fbb4b98f25f5fcc404572ca2a Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Thu, 24 Jan 2019 21:30:06 -0800 Subject: [PATCH 2/7] Update the documentation, test and dev code for ACI Log Analytics. --- azurerm/resource_arm_container_group.go | 53 ++++++++++++++++---- azurerm/resource_arm_container_group_test.go | 1 - website/docs/r/container_group.html.markdown | 26 +++++++++- 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 7f0343c2bd21..c29a6f9f1f01 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -280,7 +280,7 @@ func resourceArmContainerGroup() *schema.Resource { }, "log_analytics": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, ForceNew: true, MaxItems: 1, @@ -312,9 +312,13 @@ func resourceArmContainerGroup() *schema.Resource { Type: schema.TypeMap, Optional: true, ForceNew: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, }, }, }, + Set: containerGroupLogAnalyticsHash, }, "ip_address": { @@ -702,7 +706,7 @@ func expandContainerVolumes(input interface{}) (*[]containerinstance.VolumeMount } func expandContainerLogAnalytics(d *schema.ResourceData) *containerinstance.LogAnalytics { - logList := d.Get("log_analytics").([]interface{}) + logList := d.Get("log_analytics").(*schema.Set).List() if len(logList) <= 0 { return nil @@ -957,31 +961,35 @@ func flattenContainerVolumes(volumeMounts *[]containerinstance.VolumeMount, cont return volumeConfigs } -func flattenContainerLogAnalytics(input *containerinstance.LogAnalytics) []interface{} { +func flattenContainerLogAnalytics(input *containerinstance.LogAnalytics) *schema.Set { if input == nil { - return []interface{}{} + return nil } - output := make(map[string]interface{}) - + log := make(map[string]interface{}) + logSet := schema.Set{ + F: containerGroupLogAnalyticsHash, + } if input.WorkspaceID != nil { - output["workspace_id"] = *input.WorkspaceID + log["workspace_id"] = *input.WorkspaceID } if input.WorkspaceKey != nil { - output["workspace_key"] = *input.WorkspaceKey + log["workspace_key"] = *input.WorkspaceKey } - output["log_type"] = containerinstance.LogAnalyticsLogType(input.LogType) + log["log_type"] = containerinstance.LogAnalyticsLogType(input.LogType) // TODO: string ? metadata := make(map[string]interface{}) for k, v := range input.Metadata { metadata[k] = *v } - output["metadata"] = metadata + log["metadata"] = metadata + + logSet.Add(log) - return []interface{}{output} + return &logSet } func resourceArmContainerGroupPortsHash(v interface{}) int { @@ -994,3 +1002,26 @@ func resourceArmContainerGroupPortsHash(v interface{}) int { return hashcode.String(buf.String()) } + +func containerGroupLogAnalyticsHash(v interface{}) int { + var buf bytes.Buffer + + if m, ok := v.(map[string]interface{}); ok { + buf.WriteString(fmt.Sprintf("%s-", m["workspace_id"].(string))) + + if v, ok := m["log_type"].(containerinstance.LogAnalyticsLogType); ok { + buf.WriteString(fmt.Sprintf("%s-", string(v))) + } + if v, ok := m["log_type"].(string); ok { + buf.WriteString(fmt.Sprintf("%s-", v)) + } + + metadata := m["metadata"].(map[string]interface{}) + for k, v := range metadata { + buf.WriteString(fmt.Sprintf("%s-", k)) + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + } + + return hashcode.String(buf.String()) +} diff --git a/azurerm/resource_arm_container_group_test.go b/azurerm/resource_arm_container_group_test.go index 9190e62201da..fe0976d8851a 100644 --- a/azurerm/resource_arm_container_group_test.go +++ b/azurerm/resource_arm_container_group_test.go @@ -218,7 +218,6 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "os_type", "Linux"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "OnFailure"), resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), - resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", "ContainerInsights"), ), }, { diff --git a/website/docs/r/container_group.html.markdown b/website/docs/r/container_group.html.markdown index 6ab0bf58e4c8..307dc9be1642 100644 --- a/website/docs/r/container_group.html.markdown +++ b/website/docs/r/container_group.html.markdown @@ -110,14 +110,18 @@ The following arguments are supported: * `restart_policy` - (Optional) Restart policy for the container group. Allowed values are `Always`, `Never`, `OnFailure`. Defaults to `Always`. -* `image_registry_credential` - (Optional) Set image registry credentials for the group as documented in the `image_registry_credential` block below +* `image_registry_credential` - (Optional) Set image registry credentials for the group as documented in the `image_registry_credential` block below. * `container` - (Required) The definition of a container that is part of the group as documented in the `container` block below. Changing this forces a new resource to be created. +* `log_analytics` - (Optional) The information of Log Analytics for the group as documented in the `log_analytics` block below. Changing this forces a new resource to be created. + ~> **Note:** if `os_type` is set to `Windows` currently only a single `container` block is supported. * `tags` - (Optional) A mapping of tags to assign to the resource. +--- + The `container` block supports: * `name` - (Required) Specifies the name of the Container. Changing this forces a new resource to be created. @@ -142,6 +146,8 @@ The `container` block supports: * `volume` - (Optional) The definition of a volume mount for this container as documented in the `volume` block below. Changing this forces a new resource to be created. +--- + The `volume` block supports: * `name` - (Required) The name of the volume mount. Changing this forces a new resource to be created. @@ -156,6 +162,8 @@ The `volume` block supports: * `share_name` - (Required) The Azure storage share that is to be mounted as a volume. This must be created on the storage account specified as above. Changing this forces a new resource to be created. +--- + The `image_registry_credential` block supports: * `username` - (Required) The username with which to connect to the registry. @@ -164,12 +172,28 @@ The `image_registry_credential` block supports: * `server` - (Required) The address to use to connect to the registry without protocol ("https"/"http"). For example: "myacr.acr.io" +--- + The `ports` block supports: * `port` - (Required) The port number the container will expose. * `protocol` - (Required) The network protocol associated with port. Possible values are `TCP` & `UDP`. +--- + +The `log_analytics` block supports: + +* `workspace_id` - (Required) The workspace id for Log Analytics. + +~> **NOTE:** This field is `workspace_id` of `azurerm_log_analytics_workspace.test.workspace_id`, NOT `id`. + +* `workspace_key` - (Required) The workspace key for Log Analytics. + +* `log_type` - (Required) The log type to be used. Possible values include: `ContainerInsights` and `ContainerInstanceLogs`. + +* `metadata` - (Optional) The metadata for Log Analytics. + ## Attributes Reference The following attributes are exported: From aed5999888f8b658a8fe9917e10ad60d850e2dab Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Thu, 24 Jan 2019 22:10:57 -0800 Subject: [PATCH 3/7] Fix: removed unnecessary conversion. --- azurerm/resource_arm_container_group.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index c29a6f9f1f01..395b9a308f96 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -978,7 +978,7 @@ func flattenContainerLogAnalytics(input *containerinstance.LogAnalytics) *schema log["workspace_key"] = *input.WorkspaceKey } - log["log_type"] = containerinstance.LogAnalyticsLogType(input.LogType) + log["log_type"] = input.LogType // TODO: string ? metadata := make(map[string]interface{}) From d2cc2db26ce810016d8bab8c5e414867b1e984a6 Mon Sep 17 00:00:00 2001 From: Junyi Yi Date: Wed, 30 Jan 2019 18:23:55 -0800 Subject: [PATCH 4/7] Update azurerm/resource_arm_container_group.go Co-Authored-By: metacpp <1684739+metacpp@users.noreply.github.com> --- azurerm/resource_arm_container_group.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 395b9a308f96..9472209c40d5 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -306,7 +306,7 @@ func resourceArmContainerGroup() *schema.Resource { ValidateFunc: validation.StringInSlice([]string{ string(containerinstance.ContainerInsights), string(containerinstance.ContainerInstanceLogs), - }, true), + }, false), }, "metadata": { Type: schema.TypeMap, From 7c53b056148c0cee4492baaa56fc528c93a363ab Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Wed, 30 Jan 2019 18:31:31 -0800 Subject: [PATCH 5/7] Add more validations in test. --- azurerm/resource_arm_container_group.go | 36 +++++++++----------- azurerm/resource_arm_container_group_test.go | 9 +++++ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 395b9a308f96..c766931eaa20 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -18,6 +18,10 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) +const ( + LogAnalyticsWorkSpaceKeyPath = "log_analytics.0.workspace_key" +) + func resourceArmContainerGroup() *schema.Resource { return &schema.Resource{ Create: resourceArmContainerGroupCreate, @@ -280,7 +284,7 @@ func resourceArmContainerGroup() *schema.Resource { }, "log_analytics": { - Type: schema.TypeSet, + Type: schema.TypeList, Optional: true, ForceNew: true, MaxItems: 1, @@ -290,7 +294,7 @@ func resourceArmContainerGroup() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: validate.UUID, }, "workspace_key": { Type: schema.TypeString, @@ -318,7 +322,6 @@ func resourceArmContainerGroup() *schema.Resource { }, }, }, - Set: containerGroupLogAnalyticsHash, }, "ip_address": { @@ -382,8 +385,9 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e containerGroup.ContainerGroupProperties.IPAddress.DNSNameLabel = &dnsNameLabel } + logList := d.Get("log_analytics").([]interface{}) containerGroup.Diagnostics = &containerinstance.ContainerGroupDiagnostics{ - LogAnalytics: expandContainerLogAnalytics(d), + LogAnalytics: expandContainerLogAnalytics(logList), } future, err := client.CreateOrUpdate(ctx, resGroup, name, containerGroup) @@ -459,7 +463,7 @@ func resourceArmContainerGroupRead(d *schema.ResourceData, meta interface{}) err d.Set("os_type", string(props.OsType)) if diag := props.Diagnostics; diag != nil { - if err := d.Set("log_analytics", flattenContainerLogAnalytics(diag.LogAnalytics)); err != nil { + if err := d.Set("log_analytics", flattenContainerLogAnalytics(d, diag.LogAnalytics)); err != nil { return fmt.Errorf("Error setting `log_analytics`: %+v", err) } } @@ -705,9 +709,7 @@ func expandContainerVolumes(input interface{}) (*[]containerinstance.VolumeMount return &volumeMounts, &containerGroupVolumes } -func expandContainerLogAnalytics(d *schema.ResourceData) *containerinstance.LogAnalytics { - logList := d.Get("log_analytics").(*schema.Set).List() - +func expandContainerLogAnalytics(logList []interface{}) *containerinstance.LogAnalytics { if len(logList) <= 0 { return nil } @@ -719,7 +721,6 @@ func expandContainerLogAnalytics(d *schema.ResourceData) *containerinstance.LogA metadataMap := log["metadata"].(map[string]interface{}) metadata := make(map[string]*string) for k, v := range metadataMap { - // TODO: strValue := v.(string) metadata[k] = &strValue } @@ -961,35 +962,30 @@ func flattenContainerVolumes(volumeMounts *[]containerinstance.VolumeMount, cont return volumeConfigs } -func flattenContainerLogAnalytics(input *containerinstance.LogAnalytics) *schema.Set { +func flattenContainerLogAnalytics(d *schema.ResourceData, input *containerinstance.LogAnalytics) []interface{} { if input == nil { - return nil + return []interface{}{} } log := make(map[string]interface{}) - logSet := schema.Set{ - F: containerGroupLogAnalyticsHash, - } + if input.WorkspaceID != nil { log["workspace_id"] = *input.WorkspaceID } - if input.WorkspaceKey != nil { - log["workspace_key"] = *input.WorkspaceKey + if v, ok := d.GetOk(LogAnalyticsWorkSpaceKeyPath); ok { + log["workspace_key"] = v } log["log_type"] = input.LogType - // TODO: string ? metadata := make(map[string]interface{}) for k, v := range input.Metadata { metadata[k] = *v } log["metadata"] = metadata - logSet.Add(log) - - return &logSet + return []interface{}{log} } func resourceArmContainerGroupPortsHash(v interface{}) int { diff --git a/azurerm/resource_arm_container_group_test.go b/azurerm/resource_arm_container_group_test.go index fe0976d8851a..d8952718a00d 100644 --- a/azurerm/resource_arm_container_group_test.go +++ b/azurerm/resource_arm_container_group_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" @@ -218,6 +219,10 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "os_type", "Linux"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "OnFailure"), resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), + resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", string(containerinstance.ContainerInsights)), + resource.TestCheckResourceAttr(resourceName, "log_analytics.0.metadata.%", "1"), + resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_id"), + resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_key"), ), }, { @@ -295,6 +300,10 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "Never"), resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), + resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", string(containerinstance.ContainerInsights)), + resource.TestCheckResourceAttr(resourceName, "log_analytics.0.metadata.%", "1"), + resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_id"), + resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_key"), ), }, { From 4911eedd295393c513322665333fe17ca14c5368 Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Wed, 30 Jan 2019 18:46:04 -0800 Subject: [PATCH 6/7] Remove useless hash function and add workspace_key into ignoring list while verifying import state. --- azurerm/resource_arm_container_group.go | 23 -------------------- azurerm/resource_arm_container_group_test.go | 2 ++ 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 8a1b0d76dc8d..1613b8da9331 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -998,26 +998,3 @@ func resourceArmContainerGroupPortsHash(v interface{}) int { return hashcode.String(buf.String()) } - -func containerGroupLogAnalyticsHash(v interface{}) int { - var buf bytes.Buffer - - if m, ok := v.(map[string]interface{}); ok { - buf.WriteString(fmt.Sprintf("%s-", m["workspace_id"].(string))) - - if v, ok := m["log_type"].(containerinstance.LogAnalyticsLogType); ok { - buf.WriteString(fmt.Sprintf("%s-", string(v))) - } - if v, ok := m["log_type"].(string); ok { - buf.WriteString(fmt.Sprintf("%s-", v)) - } - - metadata := m["metadata"].(map[string]interface{}) - for k, v := range metadata { - buf.WriteString(fmt.Sprintf("%s-", k)) - buf.WriteString(fmt.Sprintf("%s-", v.(string))) - } - } - - return hashcode.String(buf.String()) -} diff --git a/azurerm/resource_arm_container_group_test.go b/azurerm/resource_arm_container_group_test.go index d8952718a00d..714ab1c58597 100644 --- a/azurerm/resource_arm_container_group_test.go +++ b/azurerm/resource_arm_container_group_test.go @@ -234,6 +234,7 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { "container.0.secure_environment_variables.%", "container.0.secure_environment_variables.secureFoo", "container.0.secure_environment_variables.secureFoo1", + "log_analytics.0.workspace_key", }, }, }, @@ -314,6 +315,7 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { "container.0.secure_environment_variables.%", "container.0.secure_environment_variables.secureFoo", "container.0.secure_environment_variables.secureFoo1", + "log_analytics.0.workspace_key", }, }, }, From 15f3cc269fe853a0ae4b92f84095ff4371faf87a Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 6 Mar 2019 16:49:58 +0100 Subject: [PATCH 7/7] fixing issues raised in code review --- azurerm/resource_arm_container_group.go | 190 +++++++++++-------- azurerm/resource_arm_container_group_test.go | 51 ++--- website/docs/r/container_group.html.markdown | 58 +++--- 3 files changed, 170 insertions(+), 129 deletions(-) diff --git a/azurerm/resource_arm_container_group.go b/azurerm/resource_arm_container_group.go index 1613b8da9331..35bcae9cbaa9 100644 --- a/azurerm/resource_arm_container_group.go +++ b/azurerm/resource_arm_container_group.go @@ -18,10 +18,6 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) -const ( - LogAnalyticsWorkSpaceKeyPath = "log_analytics.0.workspace_key" -) - func resourceArmContainerGroup() *schema.Resource { return &schema.Resource{ Create: resourceArmContainerGroupCreate, @@ -283,41 +279,54 @@ func resourceArmContainerGroup() *schema.Resource { }, }, - "log_analytics": { + "diagnostics": { Type: schema.TypeList, Optional: true, ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "workspace_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validate.UUID, - }, - "workspace_key": { - Type: schema.TypeString, - Required: true, - Sensitive: true, - ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, - }, - "log_type": { - Type: schema.TypeString, + "log_analytics": { + Type: schema.TypeList, Required: true, ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{ - string(containerinstance.ContainerInsights), - string(containerinstance.ContainerInstanceLogs), - }, false), - }, - "metadata": { - Type: schema.TypeMap, - Optional: true, - ForceNew: true, - Elem: &schema.Schema{ - Type: schema.TypeString, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "workspace_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.UUID, + }, + + "workspace_key": { + Type: schema.TypeString, + Required: true, + Sensitive: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, + }, + + "log_type": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{ + string(containerinstance.ContainerInsights), + string(containerinstance.ContainerInstanceLogs), + }, false), + }, + + "metadata": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, }, }, }, @@ -363,6 +372,9 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e tags := d.Get("tags").(map[string]interface{}) restartPolicy := d.Get("restart_policy").(string) + diagnosticsRaw := d.Get("diagnostics").([]interface{}) + diagnostics := expandContainerGroupDiagnostics(diagnosticsRaw) + containers, containerGroupPorts, containerGroupVolumes := expandContainerGroupContainers(d) containerGroup := containerinstance.ContainerGroup{ Name: &name, @@ -370,6 +382,7 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e Tags: expandTags(tags), ContainerGroupProperties: &containerinstance.ContainerGroupProperties{ Containers: containers, + Diagnostics: diagnostics, RestartPolicy: containerinstance.ContainerGroupRestartPolicy(restartPolicy), IPAddress: &containerinstance.IPAddress{ Type: containerinstance.ContainerGroupIPAddressType(IPAddressType), @@ -385,11 +398,6 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e containerGroup.ContainerGroupProperties.IPAddress.DNSNameLabel = &dnsNameLabel } - logList := d.Get("log_analytics").([]interface{}) - containerGroup.Diagnostics = &containerinstance.ContainerGroupDiagnostics{ - LogAnalytics: expandContainerLogAnalytics(logList), - } - future, err := client.CreateOrUpdate(ctx, resGroup, name, containerGroup) if err != nil { return fmt.Errorf("Error creating/updating container group %q (Resource Group %q): %+v", name, resGroup, err) @@ -449,7 +457,7 @@ func resourceArmContainerGroupRead(d *schema.ResourceData, meta interface{}) err } if err := d.Set("image_registry_credential", flattenContainerImageRegistryCredentials(d, props.ImageRegistryCredentials)); err != nil { - return fmt.Errorf("Error setting `capabilities`: %+v", err) + return fmt.Errorf("Error setting `image_registry_credential`: %+v", err) } if address := props.IPAddress; address != nil { @@ -462,10 +470,8 @@ func resourceArmContainerGroupRead(d *schema.ResourceData, meta interface{}) err d.Set("restart_policy", string(props.RestartPolicy)) d.Set("os_type", string(props.OsType)) - if diag := props.Diagnostics; diag != nil { - if err := d.Set("log_analytics", flattenContainerLogAnalytics(d, diag.LogAnalytics)); err != nil { - return fmt.Errorf("Error setting `log_analytics`: %+v", err) - } + if err := d.Set("diagnostics", flattenContainerGroupDiagnostics(d, props.Diagnostics)); err != nil { + return fmt.Errorf("Error setting `diagnostics`: %+v", err) } } @@ -709,30 +715,6 @@ func expandContainerVolumes(input interface{}) (*[]containerinstance.VolumeMount return &volumeMounts, &containerGroupVolumes } -func expandContainerLogAnalytics(logList []interface{}) *containerinstance.LogAnalytics { - if len(logList) <= 0 { - return nil - } - - log := logList[0].(map[string]interface{}) - ws_id := log["workspace_id"].(string) - ws_key := log["workspace_key"].(string) - log_type := log["log_type"].(string) - metadataMap := log["metadata"].(map[string]interface{}) - metadata := make(map[string]*string) - for k, v := range metadataMap { - strValue := v.(string) - metadata[k] = &strValue - } - - return &containerinstance.LogAnalytics{ - WorkspaceID: &ws_id, - WorkspaceKey: &ws_key, - LogType: containerinstance.LogAnalyticsLogType(log_type), - Metadata: metadata, - } -} - func flattenContainerImageRegistryCredentials(d *schema.ResourceData, input *[]containerinstance.ImageRegistryCredential) []interface{} { if input == nil { return nil @@ -962,30 +944,80 @@ func flattenContainerVolumes(volumeMounts *[]containerinstance.VolumeMount, cont return volumeConfigs } -func flattenContainerLogAnalytics(d *schema.ResourceData, input *containerinstance.LogAnalytics) []interface{} { - if input == nil { - return []interface{}{} +func expandContainerGroupDiagnostics(input []interface{}) *containerinstance.ContainerGroupDiagnostics { + if len(input) == 0 { + return nil } - log := make(map[string]interface{}) + vs := input[0].(map[string]interface{}) + + analyticsVs := vs["log_analytics"].([]interface{}) + analyticsV := analyticsVs[0].(map[string]interface{}) + + workspaceId := analyticsV["workspace_id"].(string) + workspaceKey := analyticsV["workspace_key"].(string) + logType := containerinstance.LogAnalyticsLogType(analyticsV["log_type"].(string)) + + metadataMap := analyticsV["metadata"].(map[string]interface{}) + metadata := make(map[string]*string) + for k, v := range metadataMap { + strValue := v.(string) + metadata[k] = &strValue + } - if input.WorkspaceID != nil { - log["workspace_id"] = *input.WorkspaceID + return &containerinstance.ContainerGroupDiagnostics{ + LogAnalytics: &containerinstance.LogAnalytics{ + WorkspaceID: utils.String(workspaceId), + WorkspaceKey: utils.String(workspaceKey), + LogType: logType, + Metadata: metadata, + }, } +} - if v, ok := d.GetOk(LogAnalyticsWorkSpaceKeyPath); ok { - log["workspace_key"] = v +func flattenContainerGroupDiagnostics(d *schema.ResourceData, input *containerinstance.ContainerGroupDiagnostics) []interface{} { + if input == nil { + return []interface{}{} } - log["log_type"] = input.LogType + logAnalytics := make([]interface{}, 0) + + if la := input.LogAnalytics; la != nil { + output := make(map[string]interface{}) + + output["log_type"] = string(la.LogType) - metadata := make(map[string]interface{}) - for k, v := range input.Metadata { - metadata[k] = *v + metadata := make(map[string]interface{}) + for k, v := range la.Metadata { + metadata[k] = *v + } + output["metadata"] = metadata + + if la.WorkspaceID != nil { + output["workspace_id"] = *la.WorkspaceID + } + + // the existing config may not exist at Import time, protect against it. + workspaceKey := "" + if existingDiags := d.Get("diagnostics").([]interface{}); len(existingDiags) > 0 { + existingDiag := existingDiags[0].(map[string]interface{}) + if existingLA := existingDiag["log_analytics"].([]interface{}); len(existingLA) > 0 { + vs := existingLA[0].(map[string]interface{}) + if key := vs["workspace_key"]; key != nil && key.(string) != "" { + workspaceKey = key.(string) + } + } + } + output["workspace_key"] = workspaceKey + + logAnalytics = append(logAnalytics, output) } - log["metadata"] = metadata - return []interface{}{log} + return []interface{}{ + map[string]interface{}{ + "log_analytics": logAnalytics, + }, + } } func resourceArmContainerGroupPortsHash(v interface{}) int { diff --git a/azurerm/resource_arm_container_group_test.go b/azurerm/resource_arm_container_group_test.go index e35c5ccb1349..e16fe7c26b25 100644 --- a/azurerm/resource_arm_container_group_test.go +++ b/azurerm/resource_arm_container_group_test.go @@ -5,7 +5,6 @@ import ( "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" @@ -218,11 +217,11 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "container.0.volume.0.read_only", "false"), resource.TestCheckResourceAttr(resourceName, "os_type", "Linux"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "OnFailure"), - resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), - resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", string(containerinstance.ContainerInsights)), - resource.TestCheckResourceAttr(resourceName, "log_analytics.0.metadata.%", "1"), - resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_id"), - resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_key"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.#", "1"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.0.log_type", "ContainerInsights"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.0.metadata.%", "1"), + resource.TestCheckResourceAttrSet(resourceName, "diagnostics.0.log_analytics.0.workspace_id"), + resource.TestCheckResourceAttrSet(resourceName, "diagnostics.0.log_analytics.0.workspace_key"), ), }, { @@ -234,7 +233,7 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) { "container.0.secure_environment_variables.%", "container.0.secure_environment_variables.secureFoo", "container.0.secure_environment_variables.secureFoo1", - "log_analytics.0.workspace_key", + "diagnostics.0.log_analytics.0.workspace_key", }, }, }, @@ -300,11 +299,11 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "container.0.secure_environment_variables.secureFoo1", "secureBar1"), resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"), resource.TestCheckResourceAttr(resourceName, "restart_policy", "Never"), - resource.TestCheckResourceAttr(resourceName, "log_analytics.#", "1"), - resource.TestCheckResourceAttr(resourceName, "log_analytics.0.log_type", string(containerinstance.ContainerInsights)), - resource.TestCheckResourceAttr(resourceName, "log_analytics.0.metadata.%", "1"), - resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_id"), - resource.TestCheckResourceAttrSet(resourceName, "log_analytics.0.workspace_key"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.#", "1"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.0.log_type", "ContainerInsights"), + resource.TestCheckResourceAttr(resourceName, "diagnostics.0.log_analytics.0.metadata.%", "1"), + resource.TestCheckResourceAttrSet(resourceName, "diagnostics.0.log_analytics.0.workspace_id"), + resource.TestCheckResourceAttrSet(resourceName, "diagnostics.0.log_analytics.0.workspace_key"), ), }, { @@ -315,7 +314,7 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { "container.0.secure_environment_variables.%", "container.0.secure_environment_variables.secureFoo", "container.0.secure_environment_variables.secureFoo1", - "log_analytics.0.workspace_key", + "diagnostics.0.log_analytics.0.workspace_key", }, }, }, @@ -608,12 +607,14 @@ resource "azurerm_container_group" "test" { commands = ["cmd.exe", "echo", "hi"] } - log_analytics { - workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" - workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" - log_type = "ContainerInsights" - metadata { - "node-name" = "acctestContainerGroup" + diagnostics { + log_analytics { + workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" + workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" + log_type = "ContainerInsights" + metadata { + "node-name" = "acctestContainerGroup" + } } } @@ -711,12 +712,14 @@ resource "azurerm_container_group" "test" { commands = ["/bin/bash", "-c", "ls"] } + diagnostics { log_analytics { - workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" - workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" - log_type = "ContainerInsights" - metadata { - "node-name" = "acctestContainerGroup" + workspace_id = "${azurerm_log_analytics_workspace.test.workspace_id}" + workspace_key = "${azurerm_log_analytics_workspace.test.primary_shared_key}" + log_type = "ContainerInsights" + metadata { + "node-name" = "acctestContainerGroup" + } } } diff --git a/website/docs/r/container_group.html.markdown b/website/docs/r/container_group.html.markdown index 2fc1f45a407f..fa6627863522 100644 --- a/website/docs/r/container_group.html.markdown +++ b/website/docs/r/container_group.html.markdown @@ -102,27 +102,29 @@ The following arguments are supported: * `location` - (Required) Specifies the supported Azure location where the resource exists. Changing this forces a new resource to be created. -* `ip_address_type` - (Optional) Specifies the ip address type of the container. `Public` is the only acceptable value at this time. Changing this forces a new resource to be created. +* `container` - (Required) The definition of a container that is part of the group as documented in the `container` block below. Changing this forces a new resource to be created. -* `dns_name_label` - (Optional) The DNS label/name for the container groups IP. +~> **Note:** if `os_type` is set to `Windows` currently only a single `container` block is supported. * `os_type` - (Required) The OS for the container group. Allowed values are `Linux` and `Windows`. Changing this forces a new resource to be created. -* `restart_policy` - (Optional) Restart policy for the container group. Allowed values are `Always`, `Never`, `OnFailure`. Defaults to `Always`. +--- -* `image_registry_credential` - (Optional) Set image registry credentials for the group as documented in the `image_registry_credential` block below. +* `diagnostics` - (Optional) A `diagnostics` block as documented below. -* `container` - (Required) The definition of a container that is part of the group as documented in the `container` block below. Changing this forces a new resource to be created. +* `dns_name_label` - (Optional) The DNS label/name for the container groups IP. -* `log_analytics` - (Optional) The information of Log Analytics for the group as documented in the `log_analytics` block below. Changing this forces a new resource to be created. +* `ip_address_type` - (Optional) Specifies the ip address type of the container. `Public` is the only acceptable value at this time. Changing this forces a new resource to be created. -~> **Note:** if `os_type` is set to `Windows` currently only a single `container` block is supported. +* `image_registry_credential` - (Optional) A `image_registry_credential` block as documented below. + +* `restart_policy` - (Optional) Restart policy for the container group. Allowed values are `Always`, `Never`, `OnFailure`. Defaults to `Always`. * `tags` - (Optional) A mapping of tags to assign to the resource. --- -The `container` block supports: +A `container` block supports: * `name` - (Required) Specifies the name of the Container. Changing this forces a new resource to be created. @@ -148,33 +150,35 @@ The `container` block supports: --- -The `volume` block supports: +A `diagnostics` block supports: -* `name` - (Required) The name of the volume mount. Changing this forces a new resource to be created. +* `log_analytics` - (Required) A `log_analytics` block as defined below. -* `mount_path` - (Required) The path on which this volume is to be mounted. Changing this forces a new resource to be created. +--- -* `read_only` - (Optional) Specify if the volume is to be mounted as read only or not. The default value is `false`. Changing this forces a new resource to be created. +A `image_registry_credential` block supports: -* `storage_account_name` - (Required) The Azure storage account from which the volume is to be mounted. Changing this forces a new resource to be created. +* `username` - (Required) The username with which to connect to the registry. -* `storage_account_key` - (Required) The access key for the Azure Storage account specified as above. Changing this forces a new resource to be created. +* `password` - (Required) The password with which to connect to the registry. -* `share_name` - (Required) The Azure storage share that is to be mounted as a volume. This must be created on the storage account specified as above. Changing this forces a new resource to be created. +* `server` - (Required) The address to use to connect to the registry without protocol ("https"/"http"). For example: "myacr.acr.io" --- -The `image_registry_credential` block supports: +A `log_analytics` block supports: -* `username` - (Required) The username with which to connect to the registry. +* `log_type` - (Required) The log type which should be used. Possible values are `ContainerInsights` and `ContainerInstanceLogs`. -* `password` - (Required) The password with which to connect to the registry. +* `workspace_id` - (Required) The Workspace ID of the Log Analytics Workspace. -* `server` - (Required) The address to use to connect to the registry without protocol ("https"/"http"). For example: "myacr.acr.io" +* `workspace_key` - (Required) The Workspace Key of the Log Analytics Workspace. + +* `metadata` - (Optional) Any metadata required for Log Analytics. --- -The `ports` block supports: +A `ports` block supports: * `port` - (Required) The port number the container will expose. @@ -182,17 +186,19 @@ The `ports` block supports: --- -The `log_analytics` block supports: +A `volume` block supports: + +* `name` - (Required) The name of the volume mount. Changing this forces a new resource to be created. -* `workspace_id` - (Required) The workspace id for Log Analytics. +* `mount_path` - (Required) The path on which this volume is to be mounted. Changing this forces a new resource to be created. -~> **NOTE:** This field is `workspace_id` of `azurerm_log_analytics_workspace.test.workspace_id`, NOT `id`. +* `read_only` - (Optional) Specify if the volume is to be mounted as read only or not. The default value is `false`. Changing this forces a new resource to be created. -* `workspace_key` - (Required) The workspace key for Log Analytics. +* `storage_account_name` - (Required) The Azure storage account from which the volume is to be mounted. Changing this forces a new resource to be created. -* `log_type` - (Required) The log type to be used. Possible values include: `ContainerInsights` and `ContainerInstanceLogs`. +* `storage_account_key` - (Required) The access key for the Azure Storage account specified as above. Changing this forces a new resource to be created. -* `metadata` - (Optional) The metadata for Log Analytics. +* `share_name` - (Required) The Azure storage share that is to be mounted as a volume. This must be created on the storage account specified as above. Changing this forces a new resource to be created. ## Attributes Reference