Bug Fix - update the load balancer ports

Author: Terraform Team Automation

internal/integrationtest/core_instance_pool_instance_test.go

@@ -54,14 +54,19 @@ var (
 		"image":                               acctest.Representation{RepType: acctest.Required, Create: `${var.InstanceImageOCID[var.region]}`},
 		"ipxe_script":                         acctest.Representation{RepType: acctest.Optional, Create: `ipxeScript`},
 		"is_pv_encryption_in_transit_enabled": acctest.Representation{RepType: acctest.Optional, Create: `false`},
-		"launch_options":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceLaunchOptionsRepresentation},
+		"launch_options":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceLaunchOptionsRepresentationOnlyNetworkType},
 		"metadata":                            acctest.Representation{RepType: acctest.Optional, Create: map[string]string{"user_data": "abcd"}},
 		"shape_config":                        acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceShapeConfigRepresentation},
 		"source_details":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceSourceDetailsRepresentation},
 		"subnet_id":                           acctest.Representation{RepType: acctest.Required, Create: `${oci_core_subnet.test_subnet.id}`},
 		"state":                               acctest.Representation{RepType: acctest.Required, Create: `RUNNING`},
 	}
 
+	// Because NetworkType is the only supported launchOption
+	CoreInstanceLaunchOptionsRepresentationOnlyNetworkType = map[string]interface{}{
+		"network_type": acctest.Representation{RepType: acctest.Optional, Create: `PARAVIRTUALIZED`},
+	}
+
 	CoreInstancePoolForAttachInstanceRepresentation = map[string]interface{}{
 		"compartment_id":            acctest.Representation{RepType: acctest.Required, Create: `${var.compartment_id}`},
 		"instance_configuration_id": acctest.Representation{RepType: acctest.Required, Create: `${oci_core_instance_configuration.test_instance_configuration.id}`},

internal/integrationtest/core_instance_pool_test.go

@@ -126,7 +126,7 @@ var (
 		"shape":                               acctest.Representation{RepType: acctest.Optional, Create: InstanceConfigurationVmShape},
 		"source_details":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceConfigurationInstanceDetailsLaunchDetailsSourceDetailsRepresentation},
 		"agent_config":                        acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceAgentConfigRepresentation},
-		"launch_options":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceLaunchOptionsRepresentation},
+		"launch_options":                      acctest.RepresentationGroup{RepType: acctest.Optional, Group: CoreInstanceLaunchOptionsRepresentationOnlyNetworkType},
 		"is_pv_encryption_in_transit_enabled": acctest.Representation{RepType: acctest.Optional, Create: `false`},
 		"launch_mode":                         acctest.Representation{RepType: acctest.Optional, Create: `NATIVE`},
 		"preferred_maintenance_action":        acctest.Representation{RepType: acctest.Optional, Create: `LIVE_MIGRATE`},

internal/service/core/core_instance_pool_resource.go

@@ -806,29 +806,53 @@ func (s *CoreInstancePoolResourceCrud) updateCompartment(compartment interface{}
 	return nil
 }
 
-func (s *CoreInstancePoolResourceCrud) oneEditAway(oldLoadBalancers []oci_core.AttachLoadBalancerDetails, newLoadBalancers []oci_core.AttachLoadBalancerDetails) (bool, string, oci_core.AttachLoadBalancerDetails, oci_core.AttachLoadBalancerDetails) {
-	if Abs(len(newLoadBalancers)-len(oldLoadBalancers)) != 1 {
-		return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}
-	}
-	if len(newLoadBalancers) > len(oldLoadBalancers) {
-		for i := 0; i < len(oldLoadBalancers); i++ {
-			if *oldLoadBalancers[i].LoadBalancerId != *newLoadBalancers[i].LoadBalancerId {
-				return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}
-			}
+/**
+   	This method decides if load_balancers update/attach/detach should go through.
+	Only one operation can happen in single request
+	1. update/attach should happen from the end of the lb list
+	2. detach can happen from anywhere in the list.
+*/
+func (s *CoreInstancePoolResourceCrud) oneEditAway(oldLoadBalancers []oci_core.AttachLoadBalancerDetails, newLoadBalancers []oci_core.AttachLoadBalancerDetails) (bool, string, oci_core.AttachLoadBalancerDetails, oci_core.AttachLoadBalancerDetails, string) {
+	newLbsLength := len(newLoadBalancers)
+	oldLbsLength := len(oldLoadBalancers)
+
+	// if old and new lb size is > 1 throw error
+	if Abs(newLbsLength-oldLbsLength) > 1 {
+		return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}, "Error: Failed to update load balancers, only one load balancer can be modified but found more than one"
+	}
+
+	// if old and new lb size is same that means this is an update to lb
+	if Abs(newLbsLength-oldLbsLength) == 0 {
+		deltas, positionToUpdate := s.getLoadBalancerDeltas(oldLoadBalancers, newLoadBalancers)
+		if deltas > 1 {
+			return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}, "Error: Failed to update load balancers details, only one load balancer can be modified but found more than one"
+		} else if deltas == 1 && positionToUpdate == newLbsLength-1 {
+			return true, "update", oldLoadBalancers[positionToUpdate], newLoadBalancers[positionToUpdate], ""
 		}
-		return true, "attach", oci_core.AttachLoadBalancerDetails{}, newLoadBalancers[len(newLoadBalancers)-1]
+		return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}, "Error: Failed to update load balancers, only the load balancer which is at the end of the config can be modified"
 	}
-	for i := 0; i < len(newLoadBalancers); i++ {
-		if *oldLoadBalancers[i].LoadBalancerId != *newLoadBalancers[i].LoadBalancerId {
-			for j := i + 1; j < len(newLoadBalancers); j++ {
-				if *oldLoadBalancers[j].LoadBalancerId != *newLoadBalancers[j-1].LoadBalancerId {
-					return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}
+
+	// If the new lbs length is > old one that means an attach of an lb
+	if newLbsLength > oldLbsLength {
+		deltas, _ := s.getLoadBalancerDeltas(oldLoadBalancers, newLoadBalancers)
+		if deltas != 0 {
+			return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}, "Error: Failed to attach load balancer, only the load balancer which is at the end of the config can be attached"
+		}
+		return true, "attach", oci_core.AttachLoadBalancerDetails{}, newLoadBalancers[newLbsLength-1], ""
+	}
+
+	// Remaining case does detach
+	for i := 0; i < newLbsLength; i++ {
+		if lbHasChanges(oldLoadBalancers[i], newLoadBalancers[i]) /**oldLoadBalancers[i].LoadBalancerId != *newLoadBalancers[i].LoadBalancerId*/ {
+			for j := i + 1; j < newLbsLength; j++ {
+				if lbHasChanges(oldLoadBalancers[j], newLoadBalancers[j-1]) /**oldLoadBalancers[j].LoadBalancerId != *newLoadBalancers[j-1].LoadBalancerId*/ {
+					return false, "", oci_core.AttachLoadBalancerDetails{}, oci_core.AttachLoadBalancerDetails{}, "Error: Failed to detach load balancer, only one load balancer can be detached but found more than one"
 				}
 			}
-			return true, "detach", oldLoadBalancers[i], oci_core.AttachLoadBalancerDetails{}
+			return true, "detach", oldLoadBalancers[i], oci_core.AttachLoadBalancerDetails{}, ""
 		}
 	}
-	return true, "detach", oldLoadBalancers[len(oldLoadBalancers)-1], oci_core.AttachLoadBalancerDetails{}
+	return true, "detach", oldLoadBalancers[oldLbsLength-1], oci_core.AttachLoadBalancerDetails{}, ""
 }
 
 func (s *CoreInstancePoolResourceCrud) updateLoadBalancers(oldRaw interface{}, newRaw interface{}) error {
@@ -845,48 +869,49 @@ func (s *CoreInstancePoolResourceCrud) updateLoadBalancers(oldRaw interface{}, n
 		converted := mapToAttachLoadBalancerDetails(item.(map[string]interface{}))
 		newBalancers[i] = converted
 	}
-	canEdit, operation, oldLoadbalancer, newLoadBalancer := s.oneEditAway(oldBalancers, newBalancers)
+	canEdit, operation, oldLoadbalancer, newLoadBalancer, errorMsg := s.oneEditAway(oldBalancers, newBalancers)
 	if !canEdit {
-		return fmt.Errorf("only one add/remove is allowed at once, new LoadBalancer must be added at the end of list")
+		return fmt.Errorf(errorMsg)
 	}
 	id := s.D.Id()
 
-	if operation == "attach" {
-		attachLoadBalancerRequest := oci_core.AttachLoadBalancerRequest{}
-		attachLoadBalancerRequest.InstancePoolId = &id
-		attachLoadBalancerRequest.RequestMetadata.RetryPolicy = tfresource.GetRetryPolicy(s.DisableNotFoundRetries, "core")
-		attachLoadBalancerRequest.AttachLoadBalancerDetails = newLoadBalancer
-		_, err := s.Client.AttachLoadBalancer(context.Background(), attachLoadBalancerRequest)
+	if operation == "detach" || operation == "update" {
+		detachLoadBalancerRequest := oci_core.DetachLoadBalancerRequest{}
+		detachLoadBalancerRequest.LoadBalancerId = oldLoadbalancer.LoadBalancerId
+		detachLoadBalancerRequest.RequestMetadata.RetryPolicy = tfresource.GetRetryPolicy(s.DisableNotFoundRetries, "core")
+		detachLoadBalancerRequest.InstancePoolId = &id
+		detachLoadBalancerRequest.BackendSetName = oldLoadbalancer.BackendSetName
+		_, err := s.Client.DetachLoadBalancer(context.Background(), detachLoadBalancerRequest)
 
 		if err != nil {
 			return err
 		}
 
-		_, err = s.pollForLbOperationCompletion(&id, &attachLoadBalancerRequest.AttachLoadBalancerDetails)
+		_, err = s.pollForLbOperationCompletion(&id, &oldLoadbalancer)
 
 		if err != nil {
 			return err
 		}
 	}
 
-	if operation == "detach" {
-		detachLoadBalancerRequest := oci_core.DetachLoadBalancerRequest{}
-		detachLoadBalancerRequest.LoadBalancerId = oldLoadbalancer.LoadBalancerId
-		detachLoadBalancerRequest.RequestMetadata.RetryPolicy = tfresource.GetRetryPolicy(s.DisableNotFoundRetries, "core")
-		detachLoadBalancerRequest.InstancePoolId = &id
-		detachLoadBalancerRequest.BackendSetName = oldLoadbalancer.BackendSetName
-		_, err := s.Client.DetachLoadBalancer(context.Background(), detachLoadBalancerRequest)
+	if operation == "attach" || operation == "update" {
+		attachLoadBalancerRequest := oci_core.AttachLoadBalancerRequest{}
+		attachLoadBalancerRequest.InstancePoolId = &id
+		attachLoadBalancerRequest.RequestMetadata.RetryPolicy = tfresource.GetRetryPolicy(s.DisableNotFoundRetries, "core")
+		attachLoadBalancerRequest.AttachLoadBalancerDetails = newLoadBalancer
+		_, err := s.Client.AttachLoadBalancer(context.Background(), attachLoadBalancerRequest)
 
 		if err != nil {
 			return err
 		}
 
-		_, err = s.pollForLbOperationCompletion(&id, &oldLoadbalancer)
+		_, err = s.pollForLbOperationCompletion(&id, &attachLoadBalancerRequest.AttachLoadBalancerDetails)
 
 		if err != nil {
 			return err
 		}
 	}
+
 	return nil
 }
 
@@ -946,3 +971,23 @@ func (s *CoreInstancePoolResourceCrud) pollForLbOperationCompletion(poolId *stri
 
 	return &response.InstancePool, nil
 }
+
+func (s *CoreInstancePoolResourceCrud) getLoadBalancerDeltas(balancers []oci_core.AttachLoadBalancerDetails, balancers2 []oci_core.AttachLoadBalancerDetails) (int, int) {
+	lbLength := len(balancers)
+	deltas := 0
+	positionToUpdate := -1
+	for i := 0; i < lbLength; i++ {
+		if lbHasChanges(balancers[i], balancers2[i]) {
+			deltas++
+			positionToUpdate = i
+		}
+	}
+	return deltas, positionToUpdate
+}
+
+func lbHasChanges(details oci_core.AttachLoadBalancerDetails, details2 oci_core.AttachLoadBalancerDetails) bool {
+	return !(*details.BackendSetName == *details2.BackendSetName &&
+		*details.LoadBalancerId == *details2.LoadBalancerId &&
+		*details.Port == *details2.Port &&
+		*details.VnicSelection == *details2.VnicSelection)
+}

internal/tfresource/crud_helpers.go

@@ -518,29 +518,10 @@ func MySqlVersionDiffSuppress(key string, old string, new string, d *schema.Reso
 func LoadBalancersSuppressDiff(key string, old string, new string, d *schema.ResourceData) bool {
 	return loadBalancersSuppressDiff(d)
 }
-func loadBalancersSuppressDiff(d schemaResourceData) bool {
-	if !d.HasChange("load_balancers") {
-		return true
-	}
-	oldRaw, newRaw := d.GetChange("load_balancers")
-	interfaces := oldRaw.([]interface{})
-	oldBalancers := make(map[string]bool, len(interfaces))
-	for _, item := range interfaces {
-		oldBalancers[item.(map[string]interface{})["load_balancer_id"].(string)] = true
-	}
 
-	interfaces = newRaw.([]interface{})
-	if len(interfaces) != len(oldBalancers) {
-		return false
-	}
-
-	for _, item := range interfaces {
-		converted := item.(map[string]interface{})["load_balancer_id"].(string)
-		if _, ok := oldBalancers[converted]; !ok {
-			return false
-		}
-	}
-	return true
+// Removed custom logic for diffs because we should update the lb details for nay change on the existing lb.
+func loadBalancersSuppressDiff(d schemaResourceData) bool {
+	return !d.HasChange("load_balancers")
 }
 
 func EqualIgnoreCaseSuppressDiff(key string, old string, new string, d *schema.ResourceData) bool {

internal/tfresource/crud_helpers_test.go

@@ -894,7 +894,7 @@ func TestUnitloadBalancersSuppressDiff(t *testing.T) {
 		{
 			name:   "Test HasChange() is true and output is true",
 			args:   args{d: changeReqResourceData("4")},
-			output: true,
+			output: false,
 		},
 	}
 	for _, test := range tests {