Skip to content

Commit

Permalink
Fix remove integration with multiple consumers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hemanthnakkina committed Sep 21, 2023
1 parent 7c1971d commit ab12b1f
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 22 deletions.
31 changes: 9 additions & 22 deletions internal/provider/resource_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
142 changes: 142 additions & 0 deletions internal/provider/resource_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

0 comments on commit ab12b1f

Please sign in to comment.