Skip to content

Commit

Permalink
test: reproduce failed cleanups
Browse files Browse the repository at this point in the history
We have an issue where we fail to clean up if there is an issue with the
Terraform state. Here we introduce some tests that capture that
behaviour. By reproducing the error in a test, we can test-drive a
solution and have better confidence that we have fixed the issue.

[#185835561](https://www.pivotaltracker.com/story/show/185835561)
  • Loading branch information
blgm committed Feb 15, 2024
1 parent e09d09c commit 6ee2551
Show file tree
Hide file tree
Showing 13 changed files with 268 additions and 7 deletions.
71 changes: 71 additions & 0 deletions integrationtest/binding_cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package integrationtest_test

import (
"encoding/json"
"fmt"

"github.com/cloudfoundry/cloud-service-broker/dbservice/models"
"github.com/cloudfoundry/cloud-service-broker/integrationtest/packer"
"github.com/cloudfoundry/cloud-service-broker/internal/testdrive"
"github.com/cloudfoundry/cloud-service-broker/pkg/providers/tf/workspace"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Binding Cleanup", func() {
const (
badServiceOfferingGUID = "81b4ebaa-cc08-11ee-bd34-0f8547e428e9"
badServicePlanGUID = "9ff671e2-cc08-11ee-bb95-3facf049ac9d"
goodServiceOfferingGUID = "7779a92a-cc0b-11ee-85c4-4b4aa590c58a"
goodServicePlanGUID = "911ce91e-cc0b-11ee-a5e8-33dbc3f841a1"
)

var (
brokerpak string
broker *testdrive.Broker
)

BeforeEach(func() {
brokerpak = must(packer.BuildBrokerpak(csb, fixtures("binding-cleanup")))
broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter)))

DeferCleanup(func() {
Expect(broker.Stop()).To(Succeed())
cleanup(brokerpak)
})
})

It("does not need to clean up after a binding failed cleanly", func() {
By("provisioning successfully")
instance := must(broker.Provision(badServiceOfferingGUID, badServicePlanGUID))

By("failing to bind")
binding, err := broker.CreateBinding(instance)
Expect(err).To(MatchError(ContainSubstring("error performing bind: error waiting for result: bind failed: Error: Missing required argument")))

By("seeing an HTTP 410 Gone error")
Expect(broker.DeleteBinding(instance, binding.GUID)).To(MatchError(ContainSubstring("unexpected status code 410")))
})

// This test captures an incorrect behavior that we want to fix
It("fails to clean up after a corrupted binding", func() {
By("provisioning successfully")
instance := must(broker.Provision(goodServiceOfferingGUID, goodServicePlanGUID))

By("binding successfully")
binding := must(broker.CreateBinding(instance))

By("corrupting the state as if terraform had been killed")
invalidWorkspace := must(json.Marshal(workspace.TerraformWorkspace{State: []byte(`{"foo`)})) // Base64-encoded truncated JSON
Expect(
dbConn.Model(&models.TerraformDeployment{}).
Where("id = ?", fmt.Sprintf("tf:%s:%s", instance.GUID, binding.GUID)).
Update("workspace", invalidWorkspace).
Error,
).To(Succeed())

By("failing to clean up the binding")
Expect(broker.DeleteBinding(instance, binding.GUID)).To(MatchError(ContainSubstring("failed to unbind: invalid workspace state unexpected end of JSON input")))
})
})
4 changes: 4 additions & 0 deletions integrationtest/fixtures/binding-cleanup/fake-bad-bind.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Fails as required "length" parameter is missing
resource "random_string" "random" {}

output bind_output { value = random_string.random.result }
27 changes: 27 additions & 0 deletions integrationtest/fixtures/binding-cleanup/fake-bad-service.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version: 1
name: fake-bad-service
id: 81b4ebaa-cc08-11ee-bd34-0f8547e428e9
description: description
display_name: Fake
image_url: https://example.com/icon.jpg
documentation_url: https://example.com
support_url: https://example.com/support.html
plans:
- name: standard
id: 9ff671e2-cc08-11ee-bb95-3facf049ac9d
description: Standard plan
display_name: Standard
provision:
template_refs:
main: fake-provision.tf
outputs:
- field_name: provision_output
type: string
details: provision output
bind:
template_refs:
main: fake-bad-bind.tf
outputs:
- field_name: bind_output
type: string
details: bind output
5 changes: 5 additions & 0 deletions integrationtest/fixtures/binding-cleanup/fake-good-bind.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resource "random_string" "random" {
length = 10
}

output bind_output { value = random_string.random.result }
27 changes: 27 additions & 0 deletions integrationtest/fixtures/binding-cleanup/fake-good-service.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version: 1
name: fake-good-service
id: 7779a92a-cc0b-11ee-85c4-4b4aa590c58a
description: description
display_name: Fake
image_url: https://example.com/icon.jpg
documentation_url: https://example.com
support_url: https://example.com/support.html
plans:
- name: standard
id: 911ce91e-cc0b-11ee-a5e8-33dbc3f841a1
description: Standard plan
display_name: Standard
provision:
template_refs:
main: fake-provision.tf
outputs:
- field_name: provision_output
type: string
details: provision output
bind:
template_refs:
main: fake-good-bind.tf
outputs:
- field_name: bind_output
type: string
details: bind output
3 changes: 3 additions & 0 deletions integrationtest/fixtures/binding-cleanup/fake-provision.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
resource "random_uuid" "random" {}

output provision_output { value = random_uuid.random.result }
19 changes: 19 additions & 0 deletions integrationtest/fixtures/binding-cleanup/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
packversion: 1
name: fake-brokerpak
version: 0.1.0
metadata:
author: [email protected]
platforms:
- os: linux
arch: amd64
- os: darwin
arch: amd64
terraform_binaries:
- name: terraform
version: 1.5.7
source: https://github.com/hashicorp/terraform/archive/v1.5.7.zip
- name: terraform-provider-random
version: 3.1.0
service_definitions:
- fake-good-service.yml
- fake-bad-service.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Fails as required "length" parameter is missing
resource "random_string" "random" {}

output provision_output { value = random_string.random.result }
20 changes: 20 additions & 0 deletions integrationtest/fixtures/provision-cleanup/fake-string-service.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
version: 1
name: fake-string-service
id: cfeda8d0-cbf3-11ee-be53-73f17d1c612b
description: description
display_name: Fake
image_url: https://example.com/icon.jpg
documentation_url: https://example.com
support_url: https://example.com/support.html
plans:
- name: standard
id: d8fbab66-cbf3-11ee-ab90-d7299e1fcf96
description: Standard plan
display_name: Standard
provision:
template_refs:
main: fake-string-provision.tf
outputs:
- field_name: provision_output
type: string
details: provision output
18 changes: 18 additions & 0 deletions integrationtest/fixtures/provision-cleanup/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
packversion: 1
name: fake-brokerpak
version: 0.1.0
metadata:
author: [email protected]
platforms:
- os: linux
arch: amd64
- os: darwin
arch: amd64
terraform_binaries:
- name: terraform
version: 1.5.7
source: https://github.com/hashicorp/terraform/archive/v1.5.7.zip
- name: terraform-provider-random
version: 3.1.0
service_definitions:
- fake-string-service.yml
65 changes: 65 additions & 0 deletions integrationtest/provision_cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package integrationtest_test

import (
"encoding/json"
"fmt"

"github.com/cloudfoundry/cloud-service-broker/dbservice/models"
"github.com/cloudfoundry/cloud-service-broker/integrationtest/packer"
"github.com/cloudfoundry/cloud-service-broker/internal/testdrive"
"github.com/cloudfoundry/cloud-service-broker/pkg/providers/tf/workspace"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Provision Cleanup", func() {
const (
serviceOfferingGUID = "cfeda8d0-cbf3-11ee-be53-73f17d1c612b"
servicePlanGUID = "d8fbab66-cbf3-11ee-ab90-d7299e1fcf96"
)

var (
brokerpak string
broker *testdrive.Broker
)

BeforeEach(func() {
brokerpak = must(packer.BuildBrokerpak(csb, fixtures("provision-cleanup")))
broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter)))

DeferCleanup(func() {
Expect(broker.Stop()).To(Succeed())
cleanup(brokerpak)
})
})

// This test captures an incorrect behavior that we want to fix
It("fails to clean up after a provision failed with empty state", func() {
By("failing to provision")
instance, err := broker.Provision(serviceOfferingGUID, servicePlanGUID)
Expect(err).To(MatchError("provision failed with state: failed"))

By("failing to clean up")
Expect(broker.Deprovision(instance)).To(MatchError(ContainSubstring("failed to delete: workspace state not generated")))
})

// This test captures an incorrect behavior that we want to fix
It("fails to clean up after a provision failed with corrupted state", func() {
By("failing to provision")
instance, err := broker.Provision(serviceOfferingGUID, servicePlanGUID)
Expect(err).To(MatchError("provision failed with state: failed"))

By("corrupting the state as if terraform had been killed")
invalidWorkspace := must(json.Marshal(workspace.TerraformWorkspace{State: []byte(`{"foo`)})) // Base64-encoded truncated JSON
Expect(
dbConn.Model(&models.TerraformDeployment{}).
Where("id = ?", fmt.Sprintf("tf:%s:", instance.GUID)).
Update("workspace", invalidWorkspace).
Error,
).To(Succeed())

By("failing to clean up")
Expect(broker.Deprovision(instance)).To(MatchError(ContainSubstring("failed to delete: invalid workspace state unexpected end of JSON input")))
})
})
4 changes: 2 additions & 2 deletions internal/testdrive/broker_create_binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func (b *Broker) CreateBinding(s ServiceInstance, opts ...CreateBindingOption) (
bindResponse = b.Client.Bind(s.GUID, cfg.guid, s.ServiceOfferingGUID, s.ServicePlanGUID, uuid.New(), cfg.params)
switch {
case bindResponse.Error != nil:
return ServiceBinding{}, bindResponse.Error
return ServiceBinding{GUID: cfg.guid}, bindResponse.Error
case bindResponse.StatusCode != http.StatusCreated:
return ServiceBinding{}, &UnexpectedStatusError{StatusCode: bindResponse.StatusCode, ResponseBody: bindResponse.ResponseBody}
return ServiceBinding{GUID: cfg.guid}, &UnexpectedStatusError{StatusCode: bindResponse.StatusCode, ResponseBody: bindResponse.ResponseBody}
default:
return ServiceBinding{
GUID: cfg.guid,
Expand Down
8 changes: 3 additions & 5 deletions internal/testdrive/broker_provision.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,19 @@ func (b *Broker) Provision(serviceOfferingGUID, servicePlanGUID string, opts ...
case err != nil:
return err
case state != domain.Succeeded:
return fmt.Errorf("provison failed with state: %s", state)
return fmt.Errorf("provision failed with state: %s", state)
default:
return nil
}
},
)
if err != nil {
return ServiceInstance{}, err
}

// If it fails, we still return the GUIDs for cleanup
return ServiceInstance{
GUID: cfg.guid,
ServicePlanGUID: servicePlanGUID,
ServiceOfferingGUID: serviceOfferingGUID,
}, nil
}, err
}

func WithProvisionParams(params any) ProvisionOption {
Expand Down

0 comments on commit 6ee2551

Please sign in to comment.