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.

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
  • Loading branch information
hemanthnakkina committed Sep 18, 2023
1 parent 452b1ff commit 88b9328
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 6 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
18 changes: 17 additions & 1 deletion internal/provider/resource_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
148 changes: 148 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,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)
}

0 comments on commit 88b9328

Please sign in to comment.