Fix remove integration with multiple consumers

[hemanthnakkina]

For an offer with multiple consumers, if the integration for one consumer is removed, the offer is removed on consumer/remote side. This leads to removal of integrations for rest of the consumers.
Use DestroyIntegration even for relations with offers as endpoint instead of deletion of remoteoffer.

Fixes: #308

Description

If integration having endpoint of type offer is removed, the remote-consumer offer is removed. This resulted in break of other applications that are consuming the offer.

Fixes: #308

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Environment

• Juju controller version: 3.1.5

• Terraform version: v1.5.7

QA steps

Manual QA steps should be done to test this PR.

Steps are mentioned in #308

Additional notes

[hmlanigan]

Thank you for your PR, it's greatly appreciated.

Please update the commit message with why the change is being made.

It'd be helpful to have an AC test as well for this scenario.

[hemanthnakkina]

Thank you for your PR, it's greatly appreciated.

Please update the commit message with why the change is being made.

It'd be helpful to have an AC test as well for this scenario.

@hmlanigan Updated the commit message, implemented the suggestions. Also added AC test.
Can you let me know if there is a way to check if the resource does not exist (OR) shall i implement a function to check this.
(In the current PR, I have not kept checks to verify if resource does not exist.)

[hmlanigan]

@hemanthnakkina I'm not sure about checking for other resources, and if it's possible. What do you need that data for?

To use an offer, it must be created in the other model. So RemoveRemoteOffer is removing it from the consuming side, not the creation. I don't see a need to check current terraform resources.

[hmlanigan]

Running make simplify will resolve the formatting failure.

[hemanthnakkina]

@hemanthnakkina I'm not sure about checking for other resources, and if it's possible. What do you need that data for?

To use an offer, it must be created in the other model. So RemoveRemoteOffer is removing it from the consuming side, not the creation. I don't see a need to check current terraform resources.

@hmlanigan I thought i will add an explicit check if integration is removed or not here [1] but that may not be required.
Also please see my inline comments on RemoveRemoteOffer

[1] https://github.com/juju/terraform-provider-juju/pull/309/files#diff-a5a2fd15620981d3654ab6a96ad049f36da99525e179914a1acc21a246c436fbR295

[hmlanigan]

I'd approve, but there is an error found during the test runs

=== RUN   TestAcc_ResourceIntegrationWithMultipleConsumers_Edge
    resource_integration_test.go:262: Step 1/3 error: Error running apply: exit status 1
        
        Error: Client Error
        
          with juju_integration.b2[0],
          on terraform_plugin_test.tf line 60, in resource "juju_integration" "b2":
          60: resource "juju_integration" "b2" {
        
        Unable to consume remote offer, got error: cannot add saas application "a":
        saas application already exists

[hemanthnakkina]

I'd approve, but there is an error found during the test runs

=== RUN   TestAcc_ResourceIntegrationWithMultipleConsumers_Edge
    resource_integration_test.go:262: Step 1/3 error: Error running apply: exit status 1
        
        Error: Client Error
        
          with juju_integration.b2[0],
          on terraform_plugin_test.tf line 60, in resource "juju_integration" "b2":
          60: resource "juju_integration" "b2" {
        
        Unable to consume remote offer, got error: cannot add saas application "a":
        saas application already exists

@hmlanigan I have tested the acceptance tests on juju 3.1/stable and juju 2.9/stable using the below command and the tests are successful. Not sure why they are failing here. The terraform version used in my local tests are v1.5.7.
go test -v -timeout 50m -cover ./internal/provider

I have tested manually as well with terraform plan and its working on my local.
Not sure what exactly is the difference in CI and my local?

On another note, I increased the integration tests timeout value to 40 min as this new test requires 5 minutes to execute.

[hmlanigan]

I have tested manually as well with terraform plan and its working on my local. Not sure what exactly is the difference in CI and my local?

Usually these failures are a timing issue. I haven't been able to reproduce locally either, but we need clean test runs to merge. Doing a bit of investigation.

[hemanthnakkina]

@hmlanigan I have added commit to handle error message saas application already exists.
Can you trigger the Checks.

Also at some point of time, we need to look at using some other app in integration tests instead of PostgreSQL as it is taking 5 minutes to destroy due to errors in the charm.

[hmlanigan]

@hemanthnakkina Well that's annoying, apparently the juju error types aren't working for the AlreadyExists in the API. Let's put it back and please add a TODO to understand why the jujuerrors.AlreadyExists error isn't coming thru. Thank you for your patience with this. There might be a translation bug in the juju api layers.

[hemanthnakkina]

@hemanthnakkina Well that's annoying, apparently the juju error types aren't working for the AlreadyExists in the API. Let's put it back and please add a TODO to understand why the jujuerrors.AlreadyExists error isn't coming thru. Thank you for your patience with this. There might be a translation bug in the juju api layers.

@hmlanigan Changed back to using errors, thanks! Ready for CI check and review.