From 5e052b5dbdacdfcedf8d44313ba93279a48ceb46 Mon Sep 17 00:00:00 2001 From: Hemanth Nakkina Date: Tue, 12 Sep 2023 10:38:35 +0530 Subject: [PATCH] Fix remove integration with multiple consumers Currently removing an integration with an offer removes the remote offer. This does not take into consideration of any other consumers integrated with the offer. This leads to other consumers getting relation-broken which is not expected. Remove the integration using DestroyIntegration irrespective of whether the endpoint is an offer or not. Fixes: #308 --- internal/provider/resource_integration.go | 31 ++-- .../provider/resource_integration_test.go | 142 ++++++++++++++++++ 2 files changed, 151 insertions(+), 22 deletions(-) diff --git a/internal/provider/resource_integration.go b/internal/provider/resource_integration.go index e3f026cd..2f31d4db 100644 --- a/internal/provider/resource_integration.go +++ b/internal/provider/resource_integration.go @@ -405,33 +405,20 @@ func (r *integrationResource) Delete(ctx context.Context, req resource.DeleteReq var apps []nestedApplication state.Application.ElementsAs(ctx, &apps, false) - endpoints, offer, _, err := parseEndpoints(apps) + endpoints, _, _, err := parseEndpoints(apps) if err != nil { resp.Diagnostics.AddError("Provider Error", err.Error()) return } - //If one of the endpoints is an offer then we need to remove the remote offer rather than destroying the integration - if offer != nil { - errs := r.client.Offers.RemoveRemoteOffer(&juju.RemoveRemoteOfferInput{ - ModelName: modelName, - OfferURL: *offer, - }) - if len(errs) > 0 { - for _, v := range errs { - resp.Diagnostics.AddError("Client Error", v.Error()) - } - return - } - } else { - err = r.client.Integrations.DestroyIntegration(&juju.IntegrationInput{ - ModelName: modelName, - Endpoints: endpoints, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", err.Error()) - return - } + // Remove the integration + err = r.client.Integrations.DestroyIntegration(&juju.IntegrationInput{ + ModelName: modelName, + Endpoints: endpoints, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", err.Error()) + return } r.trace(fmt.Sprintf("Deleted integration resource: %q", state.ID.ValueString())) } diff --git a/internal/provider/resource_integration_test.go b/internal/provider/resource_integration_test.go index 334fc505..ac360415 100644 --- a/internal/provider/resource_integration_test.go +++ b/internal/provider/resource_integration_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform-plugin-testing/config" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" @@ -250,3 +251,144 @@ resource "juju_integration" "a" { } `, srcModelName, dstModelName, viaCIDRs) } + +func TestAcc_ResourceIntegrationWithMultipleConsumers_Edge(t *testing.T) { + if testingCloud != LXDCloudTesting { + t.Skip(t.Name() + " only runs with LXD") + } + srcModelName := acctest.RandomWithPrefix("tf-test-integration") + dstModelName := acctest.RandomWithPrefix("tf-test-integration-dst") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + CheckDestroy: testAccCheckIntegrationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceIntegrationMultipleConsumers(srcModelName, dstModelName), + ConfigVariables: config.Variables{ + "enable-b1-consumer": config.BoolVariable(true), + "enable-b2-consumer": config.BoolVariable(true), + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("juju_integration.b1.0", "model", dstModelName), + resource.TestCheckResourceAttr("juju_integration.b1.0", "id", fmt.Sprintf("%v:%v:%v", dstModelName, "a:db-admin", "b1:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.b1.0", "application.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.b1.0", "application.*", map[string]string{"name": "b1", "endpoint": "backend-db-admin"}), + resource.TestCheckResourceAttr("juju_integration.b2.0", "model", dstModelName), + resource.TestCheckResourceAttr("juju_integration.b2.0", "id", fmt.Sprintf("%v:%v:%v", dstModelName, "a:db-admin", "b2:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.b2.0", "application.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.b2.0", "application.*", map[string]string{"name": "b2", "endpoint": "backend-db-admin"}), + ), + }, + { + Config: testAccResourceIntegrationMultipleConsumers(srcModelName, dstModelName), + ConfigVariables: config.Variables{ + "enable-b1-consumer": config.BoolVariable(true), + "enable-b2-consumer": config.BoolVariable(false), + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("juju_integration.b1.0", "model", dstModelName), + resource.TestCheckResourceAttr("juju_integration.b1.0", "id", fmt.Sprintf("%v:%v:%v", dstModelName, "a:db-admin", "b1:backend-db-admin")), + resource.TestCheckResourceAttr("juju_integration.b1.0", "application.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs("juju_integration.b1.0", "application.*", map[string]string{"name": "b1", "endpoint": "backend-db-admin"}), + ), + }, + { + Config: testAccResourceIntegrationMultipleConsumers(srcModelName, dstModelName), + ConfigVariables: config.Variables{ + "enable-b1-consumer": config.BoolVariable(false), + "enable-b2-consumer": config.BoolVariable(false), + }, + }, + }, + }) +} + +// testAccResourceIntegrationWithMultipleConusmers generates a plan where a +// two pgbouncer applications relates to postgresql:db-admin offer. +func testAccResourceIntegrationMultipleConsumers(srcModelName string, dstModelName string) string { + return fmt.Sprintf(` +resource "juju_model" "a" { + name = %q +} + +resource "juju_application" "a" { + model = juju_model.a.name + name = "a" + + charm { + name = "postgresql" + series = "jammy" + } +} + +resource "juju_offer" "a" { + model = juju_model.a.name + application_name = juju_application.a.name + endpoint = "db-admin" +} + +resource "juju_model" "b" { + name = %q +} + +resource "juju_application" "b1" { + model = juju_model.b.name + name = "b1" + + charm { + name = "pgbouncer" + series = "focal" + } +} + +resource "juju_integration" "b1" { + count = var.enable-b1-consumer ? 1 : 0 + model = juju_model.b.name + + application { + name = juju_application.b1.name + endpoint = "backend-db-admin" + } + + application { + offer_url = juju_offer.a.url + } +} + +resource "juju_application" "b2" { + model = juju_model.b.name + name = "b2" + + charm { + name = "pgbouncer" + series = "focal" + } +} + +resource "juju_integration" "b2" { + count = var.enable-b2-consumer ? 1 : 0 + model = juju_model.b.name + + application { + name = juju_application.b2.name + endpoint = "backend-db-admin" + } + + application { + offer_url = juju_offer.a.url + } +} + +variable "enable-b1-consumer" { + description = "Enable integration for b1 with offer" + default = false +} + +variable "enable-b2-consumer" { + description = "Enable integration for b2 with offer" + default = false +} +`, srcModelName, dstModelName) +}