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 20, 2023
1 parent 452b1ff commit 95e5ac3
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 27 deletions.
12 changes: 7 additions & 5 deletions internal/juju/offers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
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 95e5ac3

Please sign in to comment.