New Resource: `azurerm_iothub` #887

shelleybess · 2018-02-23T22:47:59Z

Add IotHub resource to provider.

As the refresh function ( read lifecyle ) of the iothub resource is only dependent on the id of the resource to genarate the state StatePassthrough method is used

Signed-off-by: shelley.bess <shelley.bess@vertivco.com>

shelleybess · 2018-02-28T23:42:39Z

azurerm/resource_arm_iothub_resource_provider.go

+			return nil
+		}
+
+		return fmt.Errorf("Error waiting for the deletion of IoTHub %q", iotHubName)


There is a timeout issue here. IoTHub is deleted in Azure, but terraform times out with an error destroying resource.

so this timeout is configured in the client initialisation above: https://github.com/terraform-providers/terraform-provider-azurerm/pull/887/files/3e927a6d96d0c91b3dc132d9b9e83f4b1fc96650#diff-848235249455ee48aaf3ea5d431960aeR597

we can actually swap that out for this function:

https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/config.go#L435
which sets a 60m polling timeout, which should solve this issue. There's a separate task to move everything over to that at some point, but any changes to azure/config.go cause merge conflicts, so we're trying to do it piecemeal rather than bit by bit

Hope that helps :)

This is very helpful, thank you!

tombuildsstuff

hey @shelleybess

Thanks for pushing those updates (and for fixing the rebase issues) - I've taken a look through and left some updated comments in-line :)

Thanks!

tombuildsstuff · 2018-03-01T02:03:23Z

azurerm/import_arm_iothub_test.go

+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMSubnetDestroy,


can we update this to be testCheckAzureRMIotHubDestroy?

tombuildsstuff · 2018-03-01T02:04:28Z

azurerm/resource_arm_iothub_resource_provider.go

+		return err
+	}
+
+	if !*res.NameAvailable {


will this fail when updating a resource?

It updates the resource correctly. I want to add an accTest for this as well.

tombuildsstuff · 2018-03-01T02:04:56Z

azurerm/resource_arm_iothub_resource_provider.go

+		desc.Tags = *expandTags(tags)
+	}
+
+	future, err := iothubClient.CreateOrUpdate(ctx, rg, name, desc, updateIoT)


so we should be able to submit an empty string rather than retrieving the eTag here (since Terraform's plan will show the user what will be done)

tombuildsstuff · 2018-03-01T02:06:08Z

azurerm/resource_arm_iothub_resource_provider.go

+		return fmt.Errorf("Error flattening `shared_access_policy` in IoTHub %q: %+v", iothubName, err)
+	}
+
+	d.Set("shared_access_policy", keys)


can we sort the ordering on these four lines, I think this should be:

if err := d.Set("shared_access_policy", keys); err != nil { return fmt.Errorf("...") }

tombuildsstuff · 2018-03-01T02:06:47Z

azurerm/resource_arm_iothub_resource_provider.go

+	d.Set("shared_access_policy", keys)
+	d.Set("hostname", properties.HostName)
+	d.Set("type", desc.Type)
+	flattenAndSetTags(d, &desc.Tags)


can we also set the sku block? This should be returned as part of the properties object

tombuildsstuff · 2018-03-01T02:07:17Z

azurerm/resource_arm_iothub_resource_provider_test.go

+func testCheckAzureRMIotHubDestroy(s *terraform.State) error {
+	ctx := testAccProvider.Meta().(*ArmClient).StopContext
+
+	conn := testAccProvider.Meta().(*ArmClient).expressRouteCircuitClient


I think this wants to be using iothubResourceClient?

tombuildsstuff · 2018-03-01T02:07:40Z

azurerm/resource_arm_iothub_resource_provider_test.go

+			"purpose" = "testing"
+		}
+}
+`, rInt, location, rInt)


really minor could we sort the spacing out here?

tombuildsstuff · 2018-03-01T02:09:02Z

website/azurerm.erb

@@ -359,6 +359,17 @@
                </ul>
            </li>

+            <li<%= sidebar_current("docs-azurerm-resource-iothub") %>>
+              <a href="#">IotHub Resources</a>


🤔 do you think it'd make sense for this to live under the Messaging Resources block?

I agree. It shouldn't have its own block.

tombuildsstuff · 2018-03-01T02:09:28Z

website/docs/r/iothub.html.markdown

+
+* `id` - The IotHub ID.
+
+* `etag` - The etag of IotHub Resource.


minor could we remove this since it's now no longer being exposed

tombuildsstuff

hey @shelleybess

Thanks for pushing those updates - I've taken a look through and left a few more comments in-line, but this is really close now. The two main things left to fix are ensuring we set all of the fields, and re-vendoring v12.5.0-beta of the Azure SDK for Go - otherwise this is looking pretty good :)

Thanks!

tombuildsstuff · 2018-03-02T22:09:59Z

azurerm/resource_arm_iothub_resource_provider.go

+	}
+
+	keysResp, err := iothubClient.ListKeys(ctx, id.ResourceGroup, iothubName)
+	keyList := keysResp.Response()


can we check the error returned here?

tombuildsstuff · 2018-03-08T22:00:08Z

azurerm/resource_arm_iothub_resource_provider.go

+
+	future, err := iothubClient.CreateOrUpdate(ctx, rg, name, desc, "")
+	if err != nil {
+		return err


can we wrap this error to be more descriptive with what went wrong:

return fmt.Errorf("Error creating IoTHub %q (Resource Group %q): %+v", err)

tombuildsstuff · 2018-03-08T22:00:55Z

azurerm/resource_arm_iothub_resource_provider.go

+
+		keyMap["permissions"] = string(key.Rights)
+		keys = append(keys, keyMap)
+	}


can we pull this out into a separate function flattenAzureIotSharedAccessPolicy?

tombuildsstuff · 2018-03-08T22:07:14Z

azurerm/resource_arm_iothub_resource_provider.go

+	}
+
+	d.Set("type", desc.Type)
+	flattenAndSetTags(d, &desc.Tags)


so the Import test (TestAccAzureRMIotHub_importBasic ) currently fails due to the following fields not being set - can we set them:

location

name

resource_group_name

sku

capacity

name

tier

on that note - could we pull this out into a new flatten function? I believe this should be what's needed to flatten the SKU block:

func flattenArmIoTHubSku(input *devices.IotHubSkuInfo) []interface { sku := make(map[string]interface{}, 0) if capacity := input.Capacity; capacity != nil { sku["capacity"] = *capacity } if name := input.Name; name != nil { sku["name"] = *name } if tier := input.Tier; tier != nil { sku["tier"] = *tier } return []interface{}{result} }

tombuildsstuff · 2018-03-08T22:09:35Z

azurerm/resource_arm_iothub_resource_provider_test.go

+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestAccAzureRMIotHub_basicStandard(t *testing.T) {


minor can we split this into two tests - one _basic without the SKU block, and one _complete with it?

tombuildsstuff · 2018-03-08T22:10:37Z

azurerm/config.go

+	setUserAgent(&ihrc.Client)
+	ihrc.Authorizer = auth
+	ihrc.Sender = sender
+	ihrc.SkipResourceProviderRegistration = c.skipProviderRegistration


rather than these 4 lines:

setUserAgent(&ihrc.Client) ihrc.Authorizer = auth ihrc.Sender = sender ihrc.SkipResourceProviderRegistration = c.skipProviderRegistration

can we swap this over to using the newer style registration function? e.g.

c.configureClient(&ihrc.Client, auth)

tombuildsstuff · 2018-03-08T22:13:30Z

vendor/vendor.json

-			"revision": "8c58b4788dedd95779efe0ac2055bb6a1b9b8e01",
-			"revisionTime": "2018-01-06T16:29:27Z",
+			"revision": "1e3943ee722538420f21945f4bc820347abe433d",
+			"revisionTime": "2018-02-15T17:42:55Z",
 			"version": "v9.7.0",
 			"versionExact": "v9.7.0"


taking a look into this - this is because SDKv14 is vendored in, downgrading to v12.5 should fix this for the moment (we'll upgrade in the near future)

tombuildsstuff · 2018-03-08T22:14:00Z

vendor/vendor.json

+			"checksumSHA1": "l4BaDOG5showWuLiGBjsY4eVhtk=",
+			"path": "github.com/Azure/azure-sdk-for-go/services/iothub/mgmt/2017-07-01/devices",
+			"revision": "1e334c402ea1460704b0263e5d188f28ad946ce1",
+			"revisionTime": "2018-02-16T17:41:56Z"


can we vendor this using the v12.5.0-beta tag? I believe that should mean we can downgrade to v10 of AutoRest too - you can do this via:

govendor update github.com/Azure/azure-sdk-for-go/services/iothub/mgmt/2017-07-01/devices@=v12.5.0-beta

tombuildsstuff · 2018-03-08T22:15:03Z

website/docs/r/iothub.html.markdown

+		"purpose" = "testing"
+	    }
+	}
+


minor could we make the spacing consistent here by switching to use spaces instead of tabs?

tombuildsstuff · 2018-03-08T22:15:30Z

website/docs/r/iothub.html.markdown

+
+* `name` - (Required) The name of the sku. Supports `F1`, `S1`, `S2`, and `S3`.
+
+* `tier` - (Required) The billing tier for the IoT Hub. `Free` or `Standard`.


can we make this "Possible values are free or Standard"

tombuildsstuff · 2018-03-14T17:34:22Z

hey @shelleybess

Thanks for pushing the latest set of updates - just to let you know that I'm going to push a few commits to fix the issues outlined above so that we can ship this as part of the v1.3.0 release of the AzureRM Provider; I hope you don't mind!

Thanks!

shelleybess · 2018-03-14T19:39:56Z

@tombuildsstuff

That's great, thank you! I don't mind at all. I'm sorry I haven't had time to wrap that up. The deletion of IoTHub is still timing out. I have not made any changes to that yet.

Thank you :)

tombuildsstuff · 2018-03-14T21:41:54Z

hey @shelleybess

That's great, thank you! I don't mind at all. I'm sorry I haven't had time to wrap that up.

No worries at all :)

The deletion of IoTHub is still timing out. I have not made any changes to that yet.

Thanks for confirming that - taking a look into this this appears to be a bug in the Azure SDK, which is coming from a badly defined Swagger document (since the Go SDK is generated) - as such I've opened this issue to get that fixed: Azure/azure-rest-api-specs#2661

In the interim we should be able to work around this by replacing the Delete waiter with a custom waiter (here's an example in the Template Deployment resource](https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/resource_arm_template_deployment.go#L266)) - I'll push a commit to add one (since I've got the branch checked out)

Thanks!

tombuildsstuff · 2018-03-15T03:31:47Z

Tests pass:

tombuildsstuff

LGTM with the latest changes 👍 - thanks for this!

ghost · 2020-03-31T15:21:36Z

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 👉 hashibot-feedback@hashicorp.com. Thanks!

girishramnani and others added 30 commits September 28, 2017 11:49

added iothub dependency

e5c5c62

added the config and cleaned up the schema iothub resource schema

ee1bf60

added validation and params in the schemas

bc22b50

added minimal creation method

f9bf64d

added resource to the provider list

531319c

cleaned up the schema a bit and added read

12bd9e0

working read and create method on iothub resource

293ab2c

added lifecycle methods for iothub

bbd1481

added tags

7fda639

basic integration tests added

e58ffde

Added validations for the name and followed naming convention

b569ba9

updated the iothub vendor dependency

95b677a

Added keys to the lifecycle and schema

6d8a41d

resolved a keys struct related bug

5dd8cef

Added documentation for iothub

677794f

Added importer in the iothub resource group

f2acf20

As the refresh function ( read lifecyle ) of the iothub resource is only dependent on the id of the resource to genarate the state StatePassthrough method is used

Iothub consumer group initial commit

0a1b3fc

Signed-off-by: shelley.bess <shelley.bess@vertivco.com>

added iothub dependency

ffac304

added the config and cleaned up the schema iothub resource schema

3d76c7a

added validation and params in the schemas

1ebedec

added minimal creation method

838271c

added resource to the provider list

f378215

cleaned up the schema a bit and added read

77c92ff

working read and create method on iothub resource

14aaec9

added lifecycle methods for iothub

ff6e44d

added tags

c7d4aab

basic integration tests added

9b6a7dc

Added validations for the name and followed naming convention

93c67b3

updated the iothub vendor dependency

d6f58e7

Added keys to the lifecycle and schema

96bf5f6

IotHub import test

3e927a6

shelleybess commented Feb 28, 2018

View reviewed changes

tombuildsstuff reviewed Mar 1, 2018

View reviewed changes

tombuildsstuff added the waiting-response label Mar 1, 2018

shelley.bess added 3 commits February 28, 2018 20:51

Modify IotHub docs

2ea2cfc

Config changes, remove un-needed eTag

b68afc6

Correct primary_key in iothub

4d9bfc8

tombuildsstuff reviewed Mar 8, 2018

View reviewed changes

Downgrading to v12.5.0-beta of the Azure SDK for Go/v9.7 of AutoRest

4114e23

tombuildsstuff self-assigned this Mar 14, 2018

tombuildsstuff added 3 commits March 14, 2018 12:24

Refactoring / issues identified in the PR

ae9b604

Renaming to match the convention

48cfcd7

Fixing the spacing identified in the PR

fe759b8

tombuildsstuff changed the title ~~WIP: New resource azurerm_iothub~~ New resource azurerm_iothub Mar 14, 2018

Adding back in resourceGroup to the Parameters body

422862d

tombuildsstuff changed the title ~~New resource azurerm_iothub~~ New Resource: azurerm_iothub Mar 14, 2018

tombuildsstuff mentioned this pull request Mar 14, 2018

IoTHub doesn't handle deletion correctly Azure/azure-rest-api-specs#2661

Open

Adding a custom waiter to the delete method

d55a1b9

tombuildsstuff removed the waiting-response label Mar 15, 2018

tombuildsstuff modified the milestones: 1.3.0, 1.3.1 Mar 15, 2018

tombuildsstuff approved these changes Mar 15, 2018

View reviewed changes

tombuildsstuff merged commit 10309b8 into hashicorp:master Mar 15, 2018

tombuildsstuff added a commit that referenced this pull request Mar 15, 2018

Updating to include #887

e8bb072

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_iothub` #887

New Resource: `azurerm_iothub` #887

shelleybess commented Feb 23, 2018

shelleybess Feb 28, 2018

tombuildsstuff Mar 1, 2018

shelleybess Mar 1, 2018

tombuildsstuff left a comment

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

shelleybess Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff Mar 1, 2018

shelleybess Mar 1, 2018

tombuildsstuff Mar 1, 2018

tombuildsstuff left a comment

tombuildsstuff Mar 2, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff Mar 8, 2018

tombuildsstuff commented Mar 14, 2018

shelleybess commented Mar 14, 2018

tombuildsstuff commented Mar 14, 2018

tombuildsstuff commented Mar 15, 2018

tombuildsstuff left a comment

ghost commented Mar 31, 2020


		* `id` - The IotHub ID.

		* `etag` - The etag of IotHub Resource.


		* `name` - (Required) The name of the sku. Supports `F1`, `S1`, `S2`, and `S3`.

		* `tier` - (Required) The billing tier for the IoT Hub. `Free` or `Standard`.

New Resource: azurerm_iothub #887

New Resource: azurerm_iothub #887

Conversation

shelleybess commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented Mar 14, 2018

shelleybess commented Mar 14, 2018

tombuildsstuff commented Mar 14, 2018

tombuildsstuff commented Mar 15, 2018

tombuildsstuff left a comment

Choose a reason for hiding this comment

ghost commented Mar 31, 2020

New Resource: `azurerm_iothub` #887

New Resource: `azurerm_iothub` #887