convert placement_configs to typeSet

jotruon · jotruon · commit 637d6ece6fa1 · 2020-05-11T11:35:40.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Fixed
 - Add missing attributes for `oci_file_storage_mount_target` import [Github issue #1037](https://github.com/terraform-providers/terraform-provider-oci/issues/1037)
 - Fixed the diff for `whitelisted_ips` arguments order in `oci_database_autonomous_database` resource [Issue #1050](https://github.com/terraform-providers/terraform-provider-oci/issues/1050)
+- Fixed the `placement_configs` order mismatch in `oci_containerengine_node_pool` [GitHub issue #1045](https://github.com/terraform-providers/terraform-provider-oci/issues/1045)
 
 ## 3.74.0 (May 06, 2020)
 
diff --git a/examples/container_engine/regional_subnet/cluster.tf b/examples/container_engine/regional_subnet/cluster.tf
@@ -51,13 +51,13 @@ resource "oci_containerengine_node_pool" "test_node_pool" {
 
   node_config_details {
     placement_configs {
-      availability_domain = "${data.oci_identity_availability_domain.ad1.name}"
-      subnet_id           = "${oci_core_subnet.node_pool_regional_subnet_1.id}"
+      availability_domain = "${data.oci_identity_availability_domain.ad2.name}"
+      subnet_id           = "${oci_core_subnet.node_pool_regional_subnet_2.id}"
     }
 
     placement_configs {
-      availability_domain = "${data.oci_identity_availability_domain.ad2.name}"
-      subnet_id           = "${oci_core_subnet.node_pool_regional_subnet_2.id}"
+      availability_domain = "${data.oci_identity_availability_domain.ad1.name}"
+      subnet_id           = "${oci_core_subnet.node_pool_regional_subnet_1.id}"
     }
 
     size = 4
diff --git a/oci/containerengine_node_pool_data_source.go b/oci/containerengine_node_pool_data_source.go
@@ -89,7 +89,7 @@ func (s *ContainerengineNodePoolDataSourceCrud) SetData() error {
 	}
 
 	if s.Res.NodeConfigDetails != nil {
-		s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails)})
+		s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails, true)})
 	} else {
 		s.D.Set("node_config_details", nil)
 	}
diff --git a/oci/containerengine_node_pool_resource.go b/oci/containerengine_node_pool_resource.go
@@ -3,18 +3,20 @@
 package oci
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"log"
 	"strings"
 	"time"
 
-	"github.com/terraform-providers/terraform-provider-oci/httpreplay"
-
+	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
 
+	"github.com/terraform-providers/terraform-provider-oci/httpreplay"
+
 	oci_common "github.com/oracle/oci-go-sdk/common"
 	oci_containerengine "github.com/oracle/oci-go-sdk/containerengine"
 )
@@ -99,8 +101,9 @@ func ContainerengineNodePoolResource() *schema.Resource {
 					Schema: map[string]*schema.Schema{
 						// Required
 						"placement_configs": {
-							Type:     schema.TypeList,
+							Type:     schema.TypeSet,
 							Required: true,
+							Set:      placementConfigsHashCodeForSets,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									// Required
@@ -742,7 +745,7 @@ func (s *ContainerengineNodePoolResourceCrud) SetData() error {
 	}
 
 	if s.Res.NodeConfigDetails != nil {
-		s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails)})
+		s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails, false)})
 	} else {
 		s.D.Set("node_config_details", nil)
 	}
@@ -802,10 +805,11 @@ func (s *ContainerengineNodePoolResourceCrud) mapToCreateNodePoolNodeConfigDetai
 	result := oci_containerengine.CreateNodePoolNodeConfigDetails{}
 
 	if placementConfigs, ok := s.D.GetOkExists(fmt.Sprintf(fieldKeyFormat, "placement_configs")); ok {
-		interfaces := placementConfigs.([]interface{})
+		set := placementConfigs.(*schema.Set)
+		interfaces := set.List()
 		tmp := make([]oci_containerengine.NodePoolPlacementConfigDetails, len(interfaces))
 		for i := range interfaces {
-			stateDataIndex := i
+			stateDataIndex := placementConfigsHashCodeForSets(interfaces[i])
 			fieldKeyFormatNextLevel := fmt.Sprintf("%s.%d.%%s", fmt.Sprintf(fieldKeyFormat, "placement_configs"), stateDataIndex)
 			converted, err := s.mapToNodePoolPlacementConfigDetails(fieldKeyFormatNextLevel)
 			if err != nil {
@@ -826,14 +830,18 @@ func (s *ContainerengineNodePoolResourceCrud) mapToCreateNodePoolNodeConfigDetai
 	return result, nil
 }
 
-func NodePoolNodeConfigDetailsToMap(obj *oci_containerengine.NodePoolNodeConfigDetails) map[string]interface{} {
+func NodePoolNodeConfigDetailsToMap(obj *oci_containerengine.NodePoolNodeConfigDetails, datasource bool) map[string]interface{} {
 	result := map[string]interface{}{}
 
 	placementConfigs := []interface{}{}
 	for _, item := range obj.PlacementConfigs {
 		placementConfigs = append(placementConfigs, NodePoolPlacementConfigDetailsToMap(item))
 	}
-	result["placement_configs"] = placementConfigs
+	if datasource {
+		result["placement_configs"] = placementConfigs
+	} else {
+		result["placement_configs"] = schema.NewSet(placementConfigsHashCodeForSets, placementConfigs)
+	}
 
 	if obj.Size != nil {
 		result["size"] = int(*obj.Size)
@@ -1034,23 +1042,38 @@ func NodeSourceOptionToMap(obj *oci_containerengine.NodeSourceOption) map[string
 	return result
 }
 
+func placementConfigsHashCodeForSets(v interface{}) int {
+	var buf bytes.Buffer
+	m := v.(map[string]interface{})
+	if availabilityDomain, ok := m["availability_domain"]; ok && availabilityDomain != "" {
+		buf.WriteString(fmt.Sprintf("%v-", availabilityDomain))
+	}
+	if subnetId, ok := m["subnet_id"]; ok && subnetId != "" {
+		buf.WriteString(fmt.Sprintf("%v-", subnetId))
+	}
+	return hashcode.String(buf.String())
+}
+
 func (s *ContainerengineNodePoolResourceCrud) mapToUpdateNodePoolNodeConfigDetails(fieldKeyFormat string) (oci_containerengine.UpdateNodePoolNodeConfigDetails, error) {
 	result := oci_containerengine.UpdateNodePoolNodeConfigDetails{}
 
 	result.PlacementConfigs = []oci_containerengine.NodePoolPlacementConfigDetails{}
 	if placementConfigs, ok := s.D.GetOkExists(fmt.Sprintf(fieldKeyFormat, "placement_configs")); ok {
-		interfaces := placementConfigs.([]interface{})
+		set := placementConfigs.(*schema.Set)
+		interfaces := set.List()
 		tmp := make([]oci_containerengine.NodePoolPlacementConfigDetails, len(interfaces))
 		for i := range interfaces {
-			stateDataIndex := i
+			stateDataIndex := placementConfigsHashCodeForSets(interfaces[i])
 			fieldKeyFormatNextLevel := fmt.Sprintf("%s.%d.%%s", fmt.Sprintf(fieldKeyFormat, "placement_configs"), stateDataIndex)
 			converted, err := s.mapToNodePoolPlacementConfigDetails(fieldKeyFormatNextLevel)
 			if err != nil {
 				return result, err
 			}
 			tmp[i] = converted
 		}
-		result.PlacementConfigs = tmp
+		if len(tmp) != 0 || s.D.HasChange(fmt.Sprintf(fieldKeyFormat, "placement_configs")) {
+			result.PlacementConfigs = tmp
+		}
 	}
 
 	if size, ok := s.D.GetOkExists(fmt.Sprintf(fieldKeyFormat, "size")); ok {
diff --git a/oci/containerengine_node_pool_resource_test.go b/oci/containerengine_node_pool_resource_test.go
@@ -164,8 +164,8 @@ func TestResourceContainerengineNodePool_regionalsubnet(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "name", "name"),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.0.placement_configs.#", "1"),
-					resource.TestCheckResourceAttrSet(resourceName, "node_config_details.0.placement_configs.0.availability_domain"),
-					resource.TestCheckResourceAttrSet(resourceName, "node_config_details.0.placement_configs.0.subnet_id"),
+					CheckResourceSetContainsElementWithProperties(resourceName, "node_config_details.0.placement_configs", nil, []string{"availability_domain"}),
+					CheckResourceSetContainsElementWithProperties(resourceName, "node_config_details.0.placement_configs", nil, []string{"subnet_id"}),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.0.size", "2"),
 					resource.TestCheckResourceAttr(resourceName, "node_image_name", "Oracle-Linux-7.4"),
 					resource.TestCheckResourceAttr(resourceName, "node_metadata.%", "1"),
@@ -196,7 +196,7 @@ func TestResourceContainerengineNodePool_regionalsubnet(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "name", "name2"),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.0.placement_configs.#", "1"),
-					resource.TestCheckResourceAttrSet(resourceName, "node_config_details.0.placement_configs.0.subnet_id"),
+					CheckResourceSetContainsElementWithProperties(resourceName, "node_config_details.0.placement_configs", nil, []string{"subnet_id"}),
 					resource.TestCheckResourceAttr(resourceName, "node_config_details.0.size", "4"),
 					resource.TestCheckResourceAttr(resourceName, "node_image_name", "Oracle-Linux-7.4"),
 					resource.TestCheckResourceAttr(resourceName, "node_metadata.%", "1"),
diff --git a/oci/containerengine_node_pools_data_source.go b/oci/containerengine_node_pools_data_source.go
@@ -134,7 +134,7 @@ func (s *ContainerengineNodePoolsDataSourceCrud) SetData() error {
 		}
 
 		if r.NodeConfigDetails != nil {
-			nodePool["node_config_details"] = []interface{}{NodePoolNodeConfigDetailsToMap(r.NodeConfigDetails)}
+			nodePool["node_config_details"] = []interface{}{NodePoolNodeConfigDetailsToMap(r.NodeConfigDetails, true)}
 		} else {
 			nodePool["node_config_details"] = nil
 		}

Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ func (s *ContainerengineNodePoolDataSourceCrud) SetData() error {`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`if s.Res.NodeConfigDetails != nil {`
`92`		`- s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails)})`
	`92`	`+ s.D.Set("node_config_details", []interface{}{NodePoolNodeConfigDetailsToMap(s.Res.NodeConfigDetails, true)})`
`93`	`93`	`} else {`
`94`	`94`	`s.D.Set("node_config_details", nil)`
`95`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ func (s *ContainerengineNodePoolsDataSourceCrud) SetData() error {`
`134`	`134`	`}`
`135`	`135`
`136`	`136`	`if r.NodeConfigDetails != nil {`
`137`		`- nodePool["node_config_details"] = []interface{}{NodePoolNodeConfigDetailsToMap(r.NodeConfigDetails)}`
	`137`	`+ nodePool["node_config_details"] = []interface{}{NodePoolNodeConfigDetailsToMap(r.NodeConfigDetails, true)}`
`138`	`138`	`} else {`
`139`	`139`	`nodePool["node_config_details"] = nil`
`140`	`140`	`}`