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.

Sign up for GitHub

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

provider/azure: added a number of storage and networking resources. #2052

Merged
merged 7 commits into from
Jun 12, 2015
Merged

provider/azure: added a number of storage and networking resources. #2052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9670e69
Merge resources unto upstream.
aznashwan Jun 5, 2015
137cb97
Minor fixups, refactors and test updates.
aznashwan Jun 11, 2015
329424c
Added documentation.
aznashwan Jun 11, 2015
ebfbef0
Added Storage Queue resource.
aznashwan Jun 11, 2015
bd371a3
Removed obsolete TODO's.
aznashwan Jun 12, 2015
548a17d
Added documentation for running tests.
aznashwan Jun 12, 2015
82a7f08
Removed redundant casting checks.
aznashwan Jun 12, 2015
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
2 changes: 1 addition & 1 deletion builtin/providers/azure/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"os"
"sync"

"github.com/svanharmelen/azure-sdk-for-go/management"
"github.com/Azure/azure-sdk-for-go/management"
)

// Config is the configuration structure used to instantiate a
Expand Down
48 changes: 48 additions & 0 deletions builtin/providers/azure/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package azure

const (
// terraformAzureLabel is used as the label for the hosted service created
// by Terraform on Azure.
terraformAzureLabel = "terraform-on-azure"

// terraformAzureDescription is the description used for the hosted service
// created by Terraform on Azure.
terraformAzureDescription = "Hosted service automatically created by terraform."
)

// parameterDescriptions holds a list of descriptions for all the available
// parameters of an Azure configuration.
var parameterDescriptions = map[string]string{
// provider descriptions:
"management_url": "The URL of the management API all requests should be sent to.\n" +
"Defaults to 'https://management.core.windows.net/', which is the default Azure API URL.\n" +
"This should be filled in only if you have your own datacenter with its own hosted management API.",
"management_certificate": "The certificate for connecting to the management API specified with 'management_url'",
"subscription_id": "The subscription ID to be used when connecting to the management API.",
"publish_settings_file": "The publish settings file, either created by you or downloaded from 'https://manage.windowsazure.com/publishsettings'",
// general resource descriptions:
"name": "Name of the resource to be created as it will appear in the Azure dashboard.",
"service_name": "Name of the hosted service within Azure. Will have a DNS entry as dns-name.cloudapp.net",
"location": "The Azure location where the resource will be located.\n" +
"A list of Azure locations can be found here: http://azure.microsoft.com/en-us/regions/",
"reverse_dns_fqdn": "The reverse of the fully qualified domain name. Optional.",
"label": "Label by which the resource will be identified by. Optional.",
"description": "Brief description of the resource. Optional.",
// hosted service descriptions:
"ephemeral_contents": "Sets whether the associated contents of this resource should also be\n" +
"deleted upon this resource's deletion. Default is false.",
// instance descriptions:
"image": "The image the new VM will be booted from. Mandatory.",
"size": "The size in GB of the disk to be created. Mandatory.",
"os_type": "The OS type of the VM. Either Windows or Linux. Mandatory.",
"storage_account": "The storage account (pool) name. Mandatory.",
"storage_container": "The storage container name from the storage pool given with 'storage_pool'.",
"user_name": "The user name to be configured on the new VM.",
"user_password": "The user password to be configured on the new VM.",
"default_certificate_thumbprint": "The thumbprint of the WinRM Certificate to be used as a default.",
// local network descriptions:
"vpn_gateway_address": "The IP address of the VPN gateway bridged through this virtual network.",
"address_space_prefixes": "List of address space prefixes in the format '<IP>/netmask'",
// dns descriptions:
"dns_address": "Address of the DNS server. Required.",
}
16 changes: 12 additions & 4 deletions builtin/providers/azure/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,18 @@ func Provider() terraform.ResourceProvider {
},

ResourcesMap: map[string]*schema.Resource{
"azure_data_disk": resourceAzureDataDisk(),
"azure_instance": resourceAzureInstance(),
"azure_security_group": resourceAzureSecurityGroup(),
"azure_virtual_network": resourceAzureVirtualNetwork(),
"azure_instance": resourceAzureInstance(),
"azure_data_disk": resourceAzureDataDisk(),
"azure_hosted_service": resourceAzureHostedService(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought about this again, but did you actually test with your current approach? Because I don't think this will work as you might expect. So how would you share a hosted service between multiple instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Azure has this notion of deployment slots per service.
The most straightforward example I can think of is a web app where you have a 'production' machine serving the current version of your app, and a 'dev' machine on which you add new features etc... In such a case; Azure gives you the huge convenience to 'swap' (it's more or less just switching dns refs) your production machine with the dev one whenever you want to roll out a new version of your app, rollback options etc...

Unfortunately, as of right now, the sdk assumes you wish to use the 'Production' slot all the time; however adding a slot decision option there is trivial.
When such a slot mechanism will be added to the sdk; its use could easily be added to the hostedservice resource as a list of TypeString's and users will have the slots options if they so desire it.

Copy link
Contributor

Choose a reason for hiding this comment

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

That (the SDK currently only using the Production slot) was one of the main reasons why I thought to just start with a single resource combining all the needed parts. At least until after the SDK is extend, tested and updated with the needed options to enable such use cases, as otherwise we would be offering resources that do not work as people knowing Azure would probably expect.

"azure_storage_service": resourceAzureStorageService(),
"azure_storage_container": resourceAzureStorageContainer(),
"azure_storage_blob": resourceAzureStorageBlob(),
"azure_storage_queue": resourceAzureStorageQueue(),
"azure_virtual_network": resourceAzureVirtualNetwork(),
"azure_dns_server": resourceAzureDnsServer(),
"azure_local_network_connection": resourceAzureLocalNetworkConnection(),
"azure_security_group": resourceAzureSecurityGroup(),
"azure_security_group_rule": resourceAzureSecurityGroupRule(),
},

ConfigureFunc: providerConfigure,
Expand Down
12 changes: 12 additions & 0 deletions builtin/providers/azure/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ import (
var testAccProviders map[string]terraform.ResourceProvider
var testAccProvider *schema.Provider

const testAccSecurityGroupName = "terraform-security-group"

// testAccStorageServiceName is used as the name for the Storage Service
// created in all storage-related tests.
// It is much more convenient to provide a Storage Service which
// has been created beforehand as the creation of one takes a lot
// and would greatly impede the multitude of tests which rely on one.
// NOTE: the storage container should be located in `West US`.
var testAccStorageServiceName = os.Getenv("AZURE_STORAGE")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the behavior when this is not set?

In other places we've:

  • Had the provider's setup fail fast with a message when it's not sert.
  • Had the tests that rely on the value being set either skip or fail fast.

From what I can tell the value just ends up empty right now, would be nice to do something more explicit 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.

Currently, all of the tests run the PreCheck function present below; which fails if the appropriate combination of environment variables used for the creation of the Provider and AZURE_STORAGE is not set.
That function is ran in each test; so absolutely no test will be run without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if I like the consts in this file as normally they (terraform-security-group for example) are just part of the test configs.

Having just a single call to get the AZURE_STORAGE from the env and use the var testAccStorageServiceName everywhere looks like a nice improvement! And indeed I see no problem with this approach as it's indeed tested to exist before getting to this point...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The constants are a bit dodgy; yes, but I placed them there for a couple of reasons:

  • it may seem superficial, but they aid the changing of these values in the rare cases where one might like to run the tests but somehow already has a terraform-security-group security group up there (it actually happened to me).
  • they greatly aid practicality in tests of 'sub-resources' that all stem from the same super-resource and thus there's no point in fetching that particular value for each when it's always the same for all of them (the storage elements example).


const testAccStorageContainerName = "terraform-testing-container"

func init() {
testAccProvider = Provider().(*schema.Provider)
testAccProviders = map[string]terraform.ResourceProvider{
Expand Down
10 changes: 5 additions & 5 deletions builtin/providers/azure/resource_azure_data_disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"log"
"time"

"github.com/Azure/azure-sdk-for-go/management"
"github.com/Azure/azure-sdk-for-go/management/virtualmachinedisk"
"github.com/hashicorp/terraform/helper/schema"
"github.com/svanharmelen/azure-sdk-for-go/management"
"github.com/svanharmelen/azure-sdk-for-go/management/virtualmachinedisk"
)

const dataDiskBlobStorageURL = "http://%s.blob.core.windows.net/disks/%s.vhd"
Expand Down Expand Up @@ -50,7 +50,7 @@ func resourceAzureDataDisk() *schema.Resource {
Default: "None",
},

"storage": &schema.Schema{
"storage_service_name": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Expand Down Expand Up @@ -321,7 +321,7 @@ func mediaLink(d *schema.ResourceData) string {
name = fmt.Sprintf("%s-%d", d.Get("virtual_machine").(string), d.Get("lun").(int))
}

return fmt.Sprintf(dataDiskBlobStorageURL, d.Get("storage").(string), name.(string))
return fmt.Sprintf(dataDiskBlobStorageURL, d.Get("storage_service_name").(string), name.(string))
}

func verifyDataDiskParameters(d *schema.ResourceData) error {
Expand All @@ -332,7 +332,7 @@ func verifyDataDiskParameters(d *schema.ResourceData) error {
}

if _, ok := d.GetOk("media_link"); !ok {
if _, ok := d.GetOk("storage"); !ok {
if _, ok := d.GetOk("storage_service_name"); !ok {
return fmt.Errorf("If not supplying 'media_link', you must supply 'storage'.")
}
}
Expand Down
25 changes: 12 additions & 13 deletions builtin/providers/azure/resource_azure_data_disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package azure

import (
"fmt"
"os"
"strconv"
"testing"

"github.com/Azure/azure-sdk-for-go/management"
"github.com/Azure/azure-sdk-for-go/management/virtualmachinedisk"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
"github.com/svanharmelen/azure-sdk-for-go/management"
"github.com/svanharmelen/azure-sdk-for-go/management/virtualmachinedisk"
)

func TestAccAzureDataDisk_basic(t *testing.T) {
Expand Down Expand Up @@ -174,7 +173,7 @@ resource "azure_instance" "foo" {
name = "terraform-test"
image = "Ubuntu Server 14.04 LTS"
size = "Basic_A1"
storage = "%s"
storage_service_name = "%s"
location = "West US"
username = "terraform"
password = "Pass!admin123"
Expand All @@ -183,16 +182,16 @@ resource "azure_instance" "foo" {
resource "azure_data_disk" "foo" {
lun = 0
size = 10
storage = "${azure_instance.foo.storage}"
storage_service_name = "${azure_instance.foo.storage_service_name}"
virtual_machine = "${azure_instance.foo.id}"
}`, os.Getenv("AZURE_STORAGE"))
}`, testAccStorageServiceName)

var testAccAzureDataDisk_advanced = fmt.Sprintf(`
resource "azure_instance" "foo" {
name = "terraform-test1"
image = "Ubuntu Server 14.04 LTS"
size = "Basic_A1"
storage = "%s"
storage_service_name = "%s"
location = "West US"
username = "terraform"
password = "Pass!admin123"
Expand All @@ -202,16 +201,16 @@ resource "azure_data_disk" "foo" {
lun = 1
size = 10
caching = "ReadOnly"
storage = "${azure_instance.foo.storage}"
storage_service_name = "${azure_instance.foo.storage_service_name}"
virtual_machine = "${azure_instance.foo.id}"
}`, os.Getenv("AZURE_STORAGE"))
}`, testAccStorageServiceName)

var testAccAzureDataDisk_update = fmt.Sprintf(`
resource "azure_instance" "foo" {
name = "terraform-test1"
image = "Ubuntu Server 14.04 LTS"
size = "Basic_A1"
storage = "%s"
storage_service_name = "%s"
location = "West US"
username = "terraform"
password = "Pass!admin123"
Expand All @@ -221,7 +220,7 @@ resource "azure_instance" "bar" {
name = "terraform-test2"
image = "Ubuntu Server 14.04 LTS"
size = "Basic_A1"
storage = "${azure_instance.foo.storage}"
storage_service_name = "${azure_instance.foo.storage_service_name}"
location = "West US"
username = "terraform"
password = "Pass!admin123"
Expand All @@ -231,6 +230,6 @@ resource "azure_data_disk" "foo" {
lun = 2
size = 20
caching = "ReadWrite"
storage = "${azure_instance.bar.storage}"
storage_service_name = "${azure_instance.bar.storage_service_name}"
virtual_machine = "${azure_instance.bar.id}"
}`, os.Getenv("AZURE_STORAGE"))
}`, testAccStorageServiceName)
Loading