new resource: azurerm_log_analytics_workspace_datasource_linux_syslog #6449

magodo · 2020-04-13T08:48:31Z

This implements one of the log analytics workspace data sources required in #3182

NOTE: There is a dedicated workspace-level resource called linux_syslog_collection, which has only a state indicating whether linux_syslog DS is applied to all the machines in this workspace. Rather than creating a corresponding resource for the "collection", making it a workspace property seems more intuitive in the context of terraform.

Test result

💤 via 🦉 v1.14.1 make testacc TEST=./azurerm/internal/services/loganalytics/tests TESTARGS="-run='(TestAccAzureRMLogAnalyticsWorkspace_update|TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic)'"
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test ./azurerm/internal/services/loganalytics/tests -v -run='(TestAccAzureRMLogAnalyticsWorkspace_update|TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic)' -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic
=== PAUSE TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_update
=== PAUSE TestAccAzureRMLogAnalyticsWorkspace_update
=== CONT  TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic
=== CONT  TestAccAzureRMLogAnalyticsWorkspace_update
--- PASS: TestAccDataSourceAzureRMLogAnalyticsWorkspace_basic (220.81s)
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_update (405.41s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/loganalytics/tests  405.450s

There is a dedicated workspace-level resource called `linux_syslog_collection`, which has only a `state` indicating whether `linux_syslog` DS is applied to all the machines in this workspace. Rather than creating a corresponding resource for the "collection", making it as a workspace property seems more intuitive in context of terraform.

katbyte

Thanks @magodo,

overall this looks good, but i question that the SDK is missing the required objects for this resource. Shouldn't we update the swagger and use the SDK?

katbyte · 2020-04-15T02:03:39Z

website/docs/r/log_analytics_datasource_linux_syslog.html.markdown

+  location            = azurerm_resource_group.example.location
+  resource_group_name = azurerm_resource_group.example.name
+  sku                 = "PerGB2018"
+}


Should we include the toggle in here?

katbyte · 2020-04-15T03:17:59Z

azurerm/internal/services/loganalytics/resource_arm_log_analytics_datasource_linux_syslog.go

+type dataSourceLinuxSyslog struct {
+	Name       string                          `json:"syslogName"`
+	Severities []dataSourceLinuxSyslogSeverity `json:"syslogSeverities"`
+}
+
+type dataSourceLinuxSyslogSeverity struct {
+	Severity string `json:"severity"`
+}


Shouldn't this be added to the sagger and become part of the SDK?

katbyte · 2020-04-15T03:19:15Z

azurerm/internal/services/loganalytics/resource_arm_log_analytics_datasource_linux_syslog.go

+
+	resp, err := client.Get(ctx, resourceGroup, workspaceName, name)
+	if err != nil {
+		return fmt.Errorf("failed to retrieve Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)


replacing error with failed doesn't really change the fact we start the error message with a ERROR or FAILED. maybe we should change to grammar like

Suggested change

return fmt.Errorf("failed to retrieve Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)

return fmt.Errorf("retrieving Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)

katbyte · 2020-04-15T03:19:34Z

azurerm/internal/services/loganalytics/resource_arm_log_analytics_datasource_linux_syslog.go

+		return fmt.Errorf("failed to retrieve Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)
+	}
+	if resp.ID == nil || *resp.ID == "" {
+		return fmt.Errorf("cannot read Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q) ID", name, resourceGroup, workspaceName)


and

Suggested change

return fmt.Errorf("cannot read Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q) ID", name, resourceGroup, workspaceName)

return fmt.Errorf("reading Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q) ID", name, resourceGroup, workspaceName)

as the error or failed is implied? YDYT?

…he toggle

magodo · 2020-04-15T05:53:59Z

@katbyte Thank you for the quick response 👍
I have made the update according to your comment. For the type issue, I have submitted a PR (Azure/azure-rest-api-specs#9072) to azure-api-spec to track and once it's been addressed, then we can switch to the SDK type.

magodo · 2020-04-24T02:54:57Z

@katbyte is it possible to get this merged for now (at least it works)?
Per reply from service team, the issue seems not likely to be addressed in the near future.

tombuildsstuff · 2020-06-26T10:12:23Z

hey @magodo

Unfortunately we need the SDK to define these types else they can change on Azure's side without correctly handling semantic versioning - as such these types need to be present in the Azure SDK/Swagger here. Since this is currently blocked on this upstream API Issue - I'm going to temporarily close this issue for the moment (it's already in the Blocked milestone) - but once the upstream issue has been resolved then we can re-open this and take another look.

Thanks!

ghost · 2020-07-26T14:50:00Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

magodo added 2 commits April 13, 2020 16:45

new resource: azurerm_log_analytics_workspace_datasource_linux_syslog

d36e0ee

ghost added size/XL documentation labels Apr 13, 2020

magodo added new-resource service/log-analytics labels Apr 13, 2020

magodo mentioned this pull request Apr 13, 2020

New Resource: azurerm_log_analytics_workspace_datasource_iis #6378

Closed

katbyte requested changes Apr 15, 2020

View reviewed changes

katbyte added the waiting-response label Apr 15, 2020

comment SDK lack type; modify error message; modify doc/test to add t…

69e913d

…he toggle

katbyte added sdk/requires-swagger-changes Changes need to be made in the Swagger specifications to enable this functionality sdk/requires-upgrade This is dependent upon upgrading an SDK labels Apr 21, 2020

katbyte added this to the Blocked milestone Apr 21, 2020

magodo removed the waiting-response label Apr 26, 2020

magodo requested a review from katbyte April 26, 2020 03:18

magodo mentioned this pull request Jun 22, 2020

new resource: log_analytics_datasource_azure_activity_log_resource #7429

Closed

tombuildsstuff closed this Jun 26, 2020

tombuildsstuff added the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Jun 26, 2020

ghost locked and limited conversation to collaborators Jul 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new resource: azurerm_log_analytics_workspace_datasource_linux_syslog #6449

new resource: azurerm_log_analytics_workspace_datasource_linux_syslog #6449

magodo commented Apr 13, 2020

katbyte left a comment

katbyte Apr 15, 2020

katbyte Apr 15, 2020

katbyte Apr 15, 2020

katbyte Apr 15, 2020

magodo commented Apr 15, 2020

magodo commented Apr 24, 2020

tombuildsstuff commented Jun 26, 2020

ghost commented Jul 26, 2020

	return fmt.Errorf("failed to retrieve Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)
	return fmt.Errorf("retrieving Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q): %+v", name, resourceGroup, workspaceName, err)

	return fmt.Errorf("cannot read Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q) ID", name, resourceGroup, workspaceName)
	return fmt.Errorf("reading Log Analytics Data Source Linux Syslog %q (Resource Group %q / Workspace: %q) ID", name, resourceGroup, workspaceName)

new resource: azurerm_log_analytics_workspace_datasource_linux_syslog #6449

new resource: azurerm_log_analytics_workspace_datasource_linux_syslog #6449

Conversation

magodo commented Apr 13, 2020

Test result

katbyte left a comment

Choose a reason for hiding this comment

katbyte Apr 15, 2020

Choose a reason for hiding this comment

katbyte Apr 15, 2020

Choose a reason for hiding this comment

katbyte Apr 15, 2020

Choose a reason for hiding this comment

katbyte Apr 15, 2020

Choose a reason for hiding this comment

magodo commented Apr 15, 2020

magodo commented Apr 24, 2020

tombuildsstuff commented Jun 26, 2020

ghost commented Jul 26, 2020