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

azurerm_service_fabric_cluster - support for specifying the cluster code version #1945

Merged
merged 17 commits into from
Sep 24, 2018
Merged

azurerm_service_fabric_cluster - support for specifying the cluster code version #1945

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5dab9e0
Merge pull request #1 from terraform-providers/master
Sep 18, 2018
c7c080a
allowed two client certificates to be specified
Sep 18, 2018
16c9c64
added cluster_code_version atribute
Sep 19, 2018
762f6e4
corrected docs
Sep 19, 2018
71672ce
added acceptance tests
Sep 19, 2018
695c118
keep LF on checkout to Windows
Sep 19, 2018
e5991f4
new line added
Sep 19, 2018
26a2db6
removed from this PR
Sep 19, 2018
11f2ba0
added validation
Sep 19, 2018
b20430b
alignment based on PR comment
Sep 19, 2018
3f1f375
Removing a blank space
tombuildsstuff Sep 19, 2018
a15e2f2
fmt
Sep 19, 2018
2b6309a
Merge branch 'feature/cluster_code_version' of https://github.com/ste…
Sep 19, 2018
6cdb272
added empty string condition
Sep 20, 2018
6d13c16
read ClusterCodeVersion when UpgradeMode is Manual
Sep 21, 2018
6f1d1f2
Making the `cluster_code_version` field computed
tombuildsstuff Sep 24, 2018
f06da91
Supporting having `manual` upgrade mode with the version set by Azure
tombuildsstuff Sep 24, 2018
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
13 changes: 12 additions & 1 deletion azurerm/resource_arm_service_fabric_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ func resourceArmServiceFabricCluster() *schema.Resource {
}, false),
},

"cluster_code_version": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.NoZeroValues,
},
steve-hawkins marked this conversation as resolved.
Show resolved Hide resolved

"management_endpoint": {
Type: schema.TypeString,
Required: true,
Expand Down Expand Up @@ -97,7 +103,7 @@ func resourceArmServiceFabricCluster() *schema.Resource {
"client_certificate_thumbprint": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
MaxItems: 2,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"thumbprint": {
Expand Down Expand Up @@ -275,6 +281,7 @@ func resourceArmServiceFabricClusterCreate(d *schema.ResourceData, meta interfac
reliabilityLevel := d.Get("reliability_level").(string)
managementEndpoint := d.Get("management_endpoint").(string)
upgradeMode := d.Get("upgrade_mode").(string)
clusterCodeVersion := d.Get("cluster_code_version").(string)
vmImage := d.Get("vm_image").(string)
tags := d.Get("tags").(map[string]interface{})

Expand Down Expand Up @@ -309,6 +316,7 @@ func resourceArmServiceFabricClusterCreate(d *schema.ResourceData, meta interfac
NodeTypes: nodeTypes,
ReliabilityLevel: servicefabric.ReliabilityLevel(reliabilityLevel),
UpgradeMode: servicefabric.UpgradeMode(upgradeMode),
ClusterCodeVersion: utils.String(clusterCodeVersion),
VMImage: utils.String(vmImage),
},
}
Expand Down Expand Up @@ -346,6 +354,7 @@ func resourceArmServiceFabricClusterUpdate(d *schema.ResourceData, meta interfac
name := d.Get("name").(string)
reliabilityLevel := d.Get("reliability_level").(string)
upgradeMode := d.Get("upgrade_mode").(string)
clusterCodeVersion := d.Get("cluster_code_version").(string)
tags := d.Get("tags").(map[string]interface{})

addOnFeaturesRaw := d.Get("add_on_features").(*schema.Set).List()
Expand All @@ -372,6 +381,7 @@ func resourceArmServiceFabricClusterUpdate(d *schema.ResourceData, meta interfac
NodeTypes: nodeTypes,
ReliabilityLevel: servicefabric.ReliabilityLevel1(reliabilityLevel),
UpgradeMode: servicefabric.UpgradeMode1(upgradeMode),
ClusterCodeVersion: utils.String(clusterCodeVersion),
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 run the test suite here and this fails because the ClusterCodeVersion is an empty string when it's not specified (so this fails on the API Validation):

* azurerm_service_fabric_cluster.test: Error creating Service Fabric Cluster "acctest-9043244035322992792" (Resource Group "acctestRG-9043244035322992792"): servicefabric.ClustersClient#Create: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="InvalidRequestFormat" Message="Cannot parse the request." Details=[{"code":"InvalidRequestFormat","message":"Error parsing version string:. Path 'properties.clusterCodeVersion', line 1, position 115."}]

could we update this to only set the field if it's got a value? e.g.

if clusterCodeVersion != "" {
  parameters.ClusterPropertiesUpdateParameters.ClusterCodeVersion = utils.String(clusterCodeVersion)
}

(we also need to update the Create above to do the same thing)

},
Tags: expandTags(tags),
}
Expand Down Expand Up @@ -423,6 +433,7 @@ func resourceArmServiceFabricClusterRead(d *schema.ResourceData, meta interface{
d.Set("management_endpoint", props.ManagementEndpoint)
d.Set("reliability_level", string(props.ReliabilityLevel))
d.Set("upgrade_mode", string(props.UpgradeMode))
d.Set("cluster_code_version ", props.ClusterCodeVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the test failure here is because there's a space at the end of this string (so the d.Set can't find a matching property in the schema called cluster_code_version [note the space] - so the Set fails)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@steve-hawkins since this PR otherwise LGTM - I hope you don't mind but I've pushed a commit to remove the extra space (and will kick off the tests now)

d.Set("vm_image", props.VMImage)

addOnFeatures := flattenServiceFabricClusterAddOnFeatures(props.AddOnFeatures)
Expand Down
63 changes: 63 additions & 0 deletions azurerm/resource_arm_service_fabric_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,41 @@ func TestAccAzureRMServiceFabricCluster_basic(t *testing.T) {
})
}

func TestAccAzureRMServiceFabricCluster_manualClusterCodeVersion(t *testing.T) {
resourceName := "azurerm_service_fabric_cluster.test"
ri := acctest.RandInt()
location := testLocation()

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMServiceFabricClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccAzureRMServiceFabricCluster_manualClusterCodeVersion(ri, location, "6.3.162.9494"),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMServiceFabricClusterExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "upgrade_mode", "Manual"),
resource.TestCheckResourceAttr(resourceName, "cluster_code_version", "6.3.162.9494"),
),
},
{
Config: testAccAzureRMServiceFabricCluster_manualClusterCodeVersion(ri, location, "6.3.176.9494"),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMServiceFabricClusterExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "upgrade_mode", "Manual"),
resource.TestCheckResourceAttr(resourceName, "cluster_code_version", "6.3.176.9494"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAzureRMServiceFabricCluster_addOnFeatures(t *testing.T) {
resourceName := "azurerm_service_fabric_cluster.test"
ri := acctest.RandInt()
Expand Down Expand Up @@ -432,6 +467,34 @@ resource "azurerm_service_fabric_cluster" "test" {
`, rInt, location, rInt, count)
}

func testAccAzureRMServiceFabricCluster_manualClusterCodeVersion(rInt int, location, clusterCodeVersion string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-%[1]d"
location = "%[2]s"
}

resource "azurerm_service_fabric_cluster" "test" {
name = "acctest-%[1]d"
resource_group_name = "${azurerm_resource_group.test.name}"
location = "${azurerm_resource_group.test.location}"
reliability_level = "Bronze"
upgrade_mode = "Manual"
cluster_code_version = "%[3]s"
vm_image = "Windows"
management_endpoint = "http://example:80"

node_type {
name = "first"
instance_count = 3
is_primary = true
client_endpoint_port = 2020
http_endpoint_port = 80
}
}
`, rInt, location, clusterCodeVersion)
}

func testAccAzureRMServiceFabricCluster_addOnFeatures(rInt int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
Expand Down
17 changes: 10 additions & 7 deletions website/docs/r/service_fabric_cluster.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ resource "azurerm_resource_group" "test" {
}

resource "azurerm_service_fabric_cluster" "test" {
name = "example-servicefabric"
resource_group_name = "${azurerm_resource_group.test.name}"
location = "${azurerm_resource_group.test.location}"
reliability_level = "Bronze"
upgrade_mode = "Automatic"
vm_image = "Windows"
management_endpoint = "https://example:80"
name = "example-servicefabric"
resource_group_name = "${azurerm_resource_group.test.name}"
location = "${azurerm_resource_group.test.location}"
reliability_level = "Bronze"
upgrade_mode = "Manual"
cluster_code_version = "6.3.176.9494"
vm_image = "Windows"
management_endpoint = "https://example:80"

node_type {
name = "first"
Expand Down Expand Up @@ -60,6 +61,8 @@ The following arguments are supported:

---

* `cluster_code_version` - (Optional) Required if Upgrade Mode set to `Manual`, Specifies the Version of the Cluster Code of the cluster.

* `add_on_features` - (Optional) A List of one or more features which should be enabled, such as `DnsService`.

* `certificate` - (Optional) A `certificate` block as defined below.
Expand Down