From 88b9328432eaadd688d992930dc9e2758b35301b 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. Check for number of consumers during delete integration for an offer. Remove RemoteOffer only when the current integration is the only consumer to the offer. Otherwise use DestroyIntegration that removes only the integration between the calling comsumer and the offer without affecting the relation between other consumers and offer. Fixes: #308 --- internal/juju/offers.go | 12 +- internal/provider/resource_integration.go | 18 ++- .../provider/resource_integration_test.go | 148 ++++++++++++++++++ 3 files changed, 172 insertions(+), 6 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index c448b424..d2001c5f 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -50,11 +50,12 @@ type ReadOfferInput struct { } type ReadOfferResponse struct { - ApplicationName string - Endpoint string - ModelName string - Name string - OfferURL string + ApplicationName string + Endpoint string + ModelName string + Name string + OfferURL string + ConnectionsCount int } type DestroyOfferInput struct { @@ -173,6 +174,7 @@ func (c offersClient) ReadOffer(input *ReadOfferInput) (*ReadOfferResponse, erro response.ApplicationName = result.ApplicationName response.OfferURL = result.OfferURL response.Endpoint = result.Endpoints[0].Name + response.ConnectionsCount = len(result.Connections) //no model name is returned but it can be parsed from the resulting offer URL to ensure parity //TODO: verify if we can fetch information another way diff --git a/internal/provider/resource_integration.go b/internal/provider/resource_integration.go index e3f026cd..9a1d6734 100644 --- a/internal/provider/resource_integration.go +++ b/internal/provider/resource_integration.go @@ -411,8 +411,24 @@ func (r *integrationResource) Delete(ctx context.Context, req resource.DeleteReq return } - //If one of the endpoints is an offer then we need to remove the remote offer rather than destroying the integration + var removeRemoteOffer bool = false + //If one of the endpoints is an offer then we need to remove the remote offer if there is one consumer if offer != nil { + offer_response, err := r.client.Offers.ReadOffer(&juju.ReadOfferInput{ + OfferURL: *offer, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", err.Error()) + return + } + + // If there is only one connection in the offer, remove the remote offer + if offer_response.ConnectionsCount == 1 { + removeRemoteOffer = true + } + } + + if removeRemoteOffer { errs := r.client.Offers.RemoveRemoteOffer(&juju.RemoveRemoteOfferInput{ ModelName: modelName, OfferURL: *offer, diff --git a/internal/provider/resource_integration_test.go b/internal/provider/resource_integration_test.go index 334fc505..a26e69ac 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,150 @@ resource "juju_integration" "a" { } `, srcModelName, dstModelName, viaCIDRs) } + +func TestAcc_ResourceIntegrationWithMultipleConsumers_Stable(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) }, + ExternalProviders: map[string]resource.ExternalProvider{ + "juju": { + VersionConstraint: TestProviderStableVersion, + Source: "juju/juju", + }, + }, + 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" + channel = "14/edge" + } +} + +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) +}