diff --git a/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-bind.tf b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-bind.tf new file mode 100644 index 000000000..1e780259e --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-bind.tf @@ -0,0 +1,16 @@ +terraform { + required_providers { + random = { + source = "registry.terraform.io/hashicorp/random" + } + } +} + +resource "random_integer" "priority" { + min = 3 + max = 4 +} + +output "provision_output" { + value = random_integer.priority.result +} \ No newline at end of file diff --git a/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-provision.tf b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-provision.tf new file mode 100644 index 000000000..8b8e961f3 --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-provision.tf @@ -0,0 +1,18 @@ +terraform { + required_providers { + random = { + source = "registry.terraform.io/hashicorp/random" + } + } +} + +variable "max" {type = number} + +resource "random_integer" "priority" { + min = 3 + max = var.max - 4 +} + +output "provision_output" { + value = random_integer.priority.result +} \ No newline at end of file diff --git a/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-service.yml b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-service.yml new file mode 100644 index 000000000..2d0eba164 --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure-updated/fake-service.yml @@ -0,0 +1,31 @@ +version: 1 +name: fake-service +id: 10557d15-dd47-40e6-ab4f-53fbe81e3022 +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: first + id: e328fdae-d97c-43b1-a1a7-0f8e961d1d0c + description: First plan + display_name: First +provision: + template_refs: + main: fake-provision.tf + user_inputs: + - field_name: max + type: number + details: provision input + outputs: + - field_name: provision_output + type: integer + details: provision output +bind: + template_refs: + main: fake-bind.tf + outputs: + - field_name: provision_output + type: integer + details: provision output diff --git a/integrationtest/fixtures/terraform-upgrade-failure-updated/manifest.yml b/integrationtest/fixtures/terraform-upgrade-failure-updated/manifest.yml new file mode 100644 index 000000000..e1fbf511d --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure-updated/manifest.yml @@ -0,0 +1,21 @@ +packversion: 1 +name: fake-brokerpak +version: 0.1.0 +metadata: + author: noone@nowhere.com +platforms: +- os: linux + arch: amd64 +- os: darwin + arch: amd64 +terraform_binaries: +- name: tofu + version: 1.6.2 + source: https://github.com/opentofu/opentofu/archive/refs/tags/v1.6.2.zip + default: true +- name: terraform-provider-random + version: 3.5.1 +terraform_upgrade_path: +- version: 1.6.2 +service_definitions: +- fake-service.yml diff --git a/integrationtest/fixtures/terraform-upgrade-failure/fake-bind.tf b/integrationtest/fixtures/terraform-upgrade-failure/fake-bind.tf new file mode 100644 index 000000000..0ef2520a4 --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure/fake-bind.tf @@ -0,0 +1,16 @@ +terraform { + required_providers { + random = { + source = "registry.terraform.io/hashicorp/random" + } + } +} + +resource "random_integer" "priority" { + min = 1 + max = 2 +} + +output "provision_output" { + value = random_integer.priority.result +} \ No newline at end of file diff --git a/integrationtest/fixtures/terraform-upgrade-failure/fake-provision.tf b/integrationtest/fixtures/terraform-upgrade-failure/fake-provision.tf new file mode 100644 index 000000000..3dc518cee --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure/fake-provision.tf @@ -0,0 +1,18 @@ +terraform { + required_providers { + random = { + source = "registry.terraform.io/hashicorp/random" + } + } +} + +variable "max" {type = number} + +resource "random_integer" "priority" { + min = 1 + max = var.max +} + +output "provision_output" { + value = random_integer.priority.result +} \ No newline at end of file diff --git a/integrationtest/fixtures/terraform-upgrade-failure/fake-service.yml b/integrationtest/fixtures/terraform-upgrade-failure/fake-service.yml new file mode 100644 index 000000000..2d0eba164 --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure/fake-service.yml @@ -0,0 +1,31 @@ +version: 1 +name: fake-service +id: 10557d15-dd47-40e6-ab4f-53fbe81e3022 +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: first + id: e328fdae-d97c-43b1-a1a7-0f8e961d1d0c + description: First plan + display_name: First +provision: + template_refs: + main: fake-provision.tf + user_inputs: + - field_name: max + type: number + details: provision input + outputs: + - field_name: provision_output + type: integer + details: provision output +bind: + template_refs: + main: fake-bind.tf + outputs: + - field_name: provision_output + type: integer + details: provision output diff --git a/integrationtest/fixtures/terraform-upgrade-failure/manifest.yml b/integrationtest/fixtures/terraform-upgrade-failure/manifest.yml new file mode 100644 index 000000000..e4f9e338a --- /dev/null +++ b/integrationtest/fixtures/terraform-upgrade-failure/manifest.yml @@ -0,0 +1,18 @@ +packversion: 1 +name: fake-brokerpak +version: 0.1.0 +metadata: + author: noone@nowhere.com +platforms: +- os: linux + arch: amd64 +- os: darwin + arch: amd64 +terraform_binaries: +- name: tofu + version: 1.6.0 + source: https://github.com/opentofu/opentofu/archive/refs/tags/v1.6.0.zip +- name: terraform-provider-random + version: 3.5.1 +service_definitions: +- fake-service.yml diff --git a/integrationtest/terraform_upgrade_failure_test.go b/integrationtest/terraform_upgrade_failure_test.go new file mode 100644 index 000000000..4bbe200b2 --- /dev/null +++ b/integrationtest/terraform_upgrade_failure_test.go @@ -0,0 +1,127 @@ +package integrationtest_test + +import ( + "encoding/json" + "fmt" + + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" + "github.com/pivotal-cf/brokerapi/v12/domain" + + "github.com/cloudfoundry/cloud-service-broker/v2/integrationtest/packer" + "github.com/cloudfoundry/cloud-service-broker/v2/internal/testdrive" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("TF Upgrade Failure", func() { + const ( + serviceOfferingGUID = "10557d15-dd47-40e6-ab4f-53fbe81e3022" + servicePlanGUID = "e328fdae-d97c-43b1-a1a7-0f8e961d1d0c" + startingVersion = "1.6.0" + endingVersion = "1.6.2" + ) + + var ( + brokerpak string + broker *testdrive.Broker + ) + + tfStateOutputValue := func(deploymentID, outputName string) any { + var tfDeploymentReceiver models.TerraformDeployment + Expect(dbConn.Where("id = ?", deploymentID).First(&tfDeploymentReceiver).Error).NotTo(HaveOccurred()) + var workspaceReceiver struct { + State []byte `json:"tfstate"` + } + Expect(json.Unmarshal(tfDeploymentReceiver.Workspace, &workspaceReceiver)).NotTo(HaveOccurred()) + var stateReceiver struct { + Outputs map[string]struct { + Type string `json:"type"` + Value any `json:"value"` + } `json:"outputs"` + } + Expect(json.Unmarshal(workspaceReceiver.State, &stateReceiver)).NotTo(HaveOccurred()) + Expect(stateReceiver.Outputs).To(HaveKey(outputName), "could not find output with this name") + return stateReceiver.Outputs[outputName].Value + } + + instanceTFStateVersion := func(serviceInstanceGUID string) string { + return terraformStateVersion(fmt.Sprintf("tf:%s:", serviceInstanceGUID)) + } + + bindingTFStateVersion := func(serviceInstanceGUID, bindingGUID string) string { + return terraformStateVersion(fmt.Sprintf("tf:%s:%s", serviceInstanceGUID, bindingGUID)) + } + + instanceTFStateOutputValue := func(serviceInstanceGUID string) int { + val := tfStateOutputValue(fmt.Sprintf("tf:%s:", serviceInstanceGUID), "provision_output") + return int(val.(float64)) + } + + bindingTFStateOutputValue := func(serviceInstanceGUID, bindingGUID string) int { + val := tfStateOutputValue(fmt.Sprintf("tf:%s:%s", serviceInstanceGUID, bindingGUID), "provision_output") + return int(val.(float64)) + } + + BeforeEach(func() { + brokerpak = must(packer.BuildBrokerpak(csb, fixtures("terraform-upgrade-failure"))) + broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter))) + + DeferCleanup(func() { + Expect(broker.Stop()).To(Succeed()) + cleanup(brokerpak) + }) + }) + + It("can recover an upgrade after a failure", func() { + By("provisioning a service instance at 1.6.0") + serviceInstance := must(broker.Provision(serviceOfferingGUID, servicePlanGUID, testdrive.WithProvisionParams(map[string]any{"max": 2}))) + Expect(instanceTFStateVersion(serviceInstance.GUID)).To(Equal(startingVersion)) + + By("creating a service binding") + binding := must(broker.CreateBinding(serviceInstance)) + + By("updating the brokerpak and restarting the broker") + Expect(broker.Stop()).To(Succeed()) + must(packer.BuildBrokerpak(csb, fixtures("terraform-upgrade-failure-updated"), packer.WithDirectory(brokerpak))) + + broker = must(testdrive.StartBroker( + csb, brokerpak, database, + testdrive.WithEnv("TERRAFORM_UPGRADES_ENABLED=true"), + testdrive.WithOutputs(GinkgoWriter, GinkgoWriter), + )) + + By("validating the service instance and binding are at the starting TF version") + Expect(instanceTFStateVersion(serviceInstance.GUID)).To(Equal(startingVersion)) + Expect(bindingTFStateVersion(serviceInstance.GUID, binding.GUID)).To(Equal(startingVersion)) + + By("observing the instance and binding outputs are in the correct range") + Expect(instanceTFStateOutputValue(serviceInstance.GUID)).To(BeElementOf(1, 2)) + Expect(bindingTFStateOutputValue(serviceInstance.GUID, binding.GUID)).To(BeElementOf(1, 2)) + + // This fails because it tries to generate a random number whose max value is lower than its min value + By("running 'cf upgrade-service' and getting a failure") + Expect(broker.UpgradeService(serviceInstance, endingVersion, testdrive.WithUpgradePreviousValues(domain.PreviousValues{PlanID: servicePlanGUID}))).To(MatchError("update failed with state: failed")) + + By("observing that the instance TF state file has been updated to the latest version, but not the binding") + Expect(instanceTFStateVersion(serviceInstance.GUID)).To(Equal(endingVersion)) + Expect(bindingTFStateVersion(serviceInstance.GUID, binding.GUID)).To(Equal(startingVersion)) + + By("observing the instance and binding outputs have not changed") + Expect(instanceTFStateOutputValue(serviceInstance.GUID)).To(BeElementOf(1, 2)) + Expect(bindingTFStateOutputValue(serviceInstance.GUID, binding.GUID)).To(BeElementOf(1, 2)) + + By("hacking the database so that the next upgrade will succeed") + Expect(dbConn.Model(&models.ProvisionRequestDetails{}).Where("service_instance_id = ?", serviceInstance.GUID).Update("request_details", `{"max":8}`).Error).To(Succeed()) + + By("running 'cf upgrade-service' again and succeeding") + Expect(broker.UpgradeService(serviceInstance, endingVersion, testdrive.WithUpgradePreviousValues(domain.PreviousValues{PlanID: servicePlanGUID}))).To(Succeed()) + + By("observing that the instance TF state file has been updated to the latest version for both the instance and the binding") + Expect(instanceTFStateVersion(serviceInstance.GUID)).To(Equal(endingVersion)) + Expect(bindingTFStateVersion(serviceInstance.GUID, binding.GUID)).To(Equal(endingVersion)) + + By("observing the instance and binding output are in the updated range") + Expect(instanceTFStateOutputValue(serviceInstance.GUID)).To(BeElementOf(3, 4)) + Expect(bindingTFStateOutputValue(serviceInstance.GUID, binding.GUID)).To(BeElementOf(3, 4)) + }) +}) diff --git a/pkg/providers/tf/upgrade.go b/pkg/providers/tf/upgrade.go index c763185f2..b9dc388ef 100644 --- a/pkg/providers/tf/upgrade.go +++ b/pkg/providers/tf/upgrade.go @@ -104,14 +104,19 @@ func (provider *TerraformProvider) performTerraformUpgrade(ctx context.Context, return err } + // Because an upgrade can fail, and when it fails we still record the higher TF version in the state, we + // need to perform the upgrade to the current TF version before performing the upgrade on higher versions. + // This allows failures to be re-tried. We could add code to try and keep track of the failures, but + // because TF is idempotent, it's cleaner to just run Apply at the current version, which will be a + // no-op for success cases. if currentTfVersion.LessThan(version.Must(version.NewVersion("1.5.0"))) { return errors.New("upgrade only supported for Terraform versions >= 1.5.0") - } else if currentTfVersion.LessThan(provider.tfBinContext.DefaultTfVersion) { + } else if currentTfVersion.LessThanOrEqual(provider.tfBinContext.DefaultTfVersion) { if len(provider.tfBinContext.TfUpgradePath) == 0 { return errors.New("tofu version mismatch and no upgrade path specified") } for _, targetTfVersion := range provider.tfBinContext.TfUpgradePath { - if currentTfVersion.LessThan(targetTfVersion) { + if currentTfVersion.LessThanOrEqual(targetTfVersion) { err = provider.VersionedInvoker(targetTfVersion).Apply(ctx, workspace) if err != nil { return err diff --git a/pkg/providers/tf/upgrade_test.go b/pkg/providers/tf/upgrade_test.go index 7b394586d..0616b14a3 100644 --- a/pkg/providers/tf/upgrade_test.go +++ b/pkg/providers/tf/upgrade_test.go @@ -72,6 +72,7 @@ var _ = Describe("Upgrade", func() { BeforeEach(func() { fakeInvoker1 = &tffakes.FakeTerraformInvoker{} fakeInvoker2 = &tffakes.FakeTerraformInvoker{} + fakeDefaultInvoker = &tffakes.FakeTerraformInvoker{} instanceTFDeployment.Workspace = fakeWorkspace fakeDeploymentManager.GetTerraformDeploymentReturns(instanceTFDeployment, nil) @@ -105,9 +106,13 @@ var _ = Describe("Upgrade", func() { Expect(fakeInvoker2.ApplyCallCount()).To(Equal(1)) Expect(getWorkspace(fakeInvoker2, 0)).To(Equal(fakeWorkspace)) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerCallCount()).To(Equal(2)) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(0)).To(Equal(newVersion("3.0.0"))) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(1)).To(Equal(newVersion("4.0.0"))) + Expect(fakeDefaultInvoker.ApplyCallCount()).To(Equal(1)) + Expect(getWorkspace(fakeDefaultInvoker, 0)).To(Equal(fakeWorkspace)) + + Expect(fakeInvokerBuilder.VersionedTerraformInvokerCallCount()).To(Equal(3)) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(0)).To(Equal(newVersion("2.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(1)).To(Equal(newVersion("3.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(2)).To(Equal(newVersion("4.0.0"))) Expect(fakeDeploymentManager.UpdateWorkspaceHCLCallCount()).To(Equal(1)) actualDeploymentID, actualAction, actualUpgradeContext := fakeDeploymentManager.UpdateWorkspaceHCLArgsForCall(0) @@ -250,29 +255,50 @@ var _ = Describe("Upgrade", func() { }) Describe("UpgradeBindings", func() { + const ( + firstBindingID = "firstBindingID" + secondBindingID = "secondBindingID" + ) + var ( - firstBindingID = "firstBindingID" - secondBindingID = "secondBindingID" - firstBindingWorkspace = &workspacefakes.FakeWorkspace{} - firstBindingDeployment = storage.TerraformDeployment{ID: instanceDeploymentID + firstBindingID, Workspace: firstBindingWorkspace} - secondBindingWorkspace = &workspacefakes.FakeWorkspace{} + firstBindingWorkspace *workspacefakes.FakeWorkspace + firstBindingDeployment storage.TerraformDeployment + secondBindingWorkspace *workspacefakes.FakeWorkspace + secondBindingDeployment storage.TerraformDeployment + + bindingDeployments []storage.TerraformDeployment + firstBindingVars map[string]any + secondBindingVars map[string]any + bindingsVarContexts []*varcontext.VarContext + + fakeInvoker1 *tffakes.FakeTerraformInvoker + fakeInvoker2 *tffakes.FakeTerraformInvoker + fakeInvoker3 *tffakes.FakeTerraformInvoker + fakeInvoker4 *tffakes.FakeTerraformInvoker + fakeInvoker5 *tffakes.FakeTerraformInvoker + fakeInvoker6 *tffakes.FakeTerraformInvoker + ) + + BeforeEach(func() { + firstBindingWorkspace = &workspacefakes.FakeWorkspace{} + firstBindingDeployment = storage.TerraformDeployment{ID: instanceDeploymentID + firstBindingID, Workspace: firstBindingWorkspace} + secondBindingWorkspace = &workspacefakes.FakeWorkspace{} secondBindingDeployment = storage.TerraformDeployment{ID: instanceDeploymentID + secondBindingID, Workspace: secondBindingWorkspace} bindingDeployments = []storage.TerraformDeployment{ firstBindingDeployment, secondBindingDeployment, } - firstBindingVars = map[string]any{"tf_id": instanceDeploymentID + firstBindingID, "first-binding-var": "first-binding-value"} - secondBindingVars = map[string]any{"tf_id": instanceDeploymentID + secondBindingID, "second-binding-var": "second-binding-value"} - bindingsVarContexts []*varcontext.VarContext + firstBindingVars = map[string]any{"tf_id": instanceDeploymentID + firstBindingID, "first-binding-var": "first-binding-value"} + secondBindingVars = map[string]any{"tf_id": instanceDeploymentID + secondBindingID, "second-binding-var": "second-binding-value"} fakeInvoker1 = &tffakes.FakeTerraformInvoker{} fakeInvoker2 = &tffakes.FakeTerraformInvoker{} fakeInvoker3 = &tffakes.FakeTerraformInvoker{} fakeInvoker4 = &tffakes.FakeTerraformInvoker{} - ) + fakeInvoker5 = &tffakes.FakeTerraformInvoker{} + fakeInvoker6 = &tffakes.FakeTerraformInvoker{} - BeforeEach(func() { instanceTFDeployment.Workspace = fakeWorkspace fakeDeploymentManager.GetTerraformDeploymentReturns(instanceTFDeployment, nil) @@ -284,6 +310,8 @@ var _ = Describe("Upgrade", func() { fakeInvokerBuilder.VersionedTerraformInvokerReturnsOnCall(1, fakeInvoker2) fakeInvokerBuilder.VersionedTerraformInvokerReturnsOnCall(2, fakeInvoker3) fakeInvokerBuilder.VersionedTerraformInvokerReturnsOnCall(3, fakeInvoker4) + fakeInvokerBuilder.VersionedTerraformInvokerReturnsOnCall(4, fakeInvoker5) + fakeInvokerBuilder.VersionedTerraformInvokerReturnsOnCall(5, fakeInvoker6) firstBindingWorkspace.StateTFVersionReturns(newVersion("2.0.0"), nil) firstBindingWorkspace.ModuleInstancesReturns([]workspace.ModuleInstance{{ModuleName: "first-binding-moduleName"}}) @@ -327,7 +355,6 @@ var _ = Describe("Upgrade", func() { actualSecondBindingDeployment, _ := fakeDeploymentManager.MarkOperationFinishedArgsForCall(1) Expect(actualSecondBindingDeployment.ID).To(Equal(secondBindingDeployment.ID)) - By("checking the invoker was called for the service instance with correct workspace") By("checking the invoker was called for the first binding with correct workspace") Expect(fakeInvoker1.ApplyCallCount()).To(Equal(1)) Expect(getWorkspace(fakeInvoker1, 0)).To(Equal(firstBindingWorkspace)) @@ -335,18 +362,26 @@ var _ = Describe("Upgrade", func() { Expect(fakeInvoker2.ApplyCallCount()).To(Equal(1)) Expect(getWorkspace(fakeInvoker2, 0)).To(Equal(firstBindingWorkspace)) - By("checking the invoker was called for the second binding with correct workspace") Expect(fakeInvoker3.ApplyCallCount()).To(Equal(1)) - Expect(getWorkspace(fakeInvoker3, 0)).To(Equal(secondBindingWorkspace)) + Expect(getWorkspace(fakeInvoker3, 0)).To(Equal(firstBindingWorkspace)) + By("checking the invoker was called for the second binding with correct workspace") Expect(fakeInvoker4.ApplyCallCount()).To(Equal(1)) Expect(getWorkspace(fakeInvoker4, 0)).To(Equal(secondBindingWorkspace)) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerCallCount()).To(Equal(4)) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(0)).To(Equal(newVersion("3.0.0"))) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(1)).To(Equal(newVersion("4.0.0"))) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(2)).To(Equal(newVersion("3.0.0"))) - Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(3)).To(Equal(newVersion("4.0.0"))) + Expect(fakeInvoker5.ApplyCallCount()).To(Equal(1)) + Expect(getWorkspace(fakeInvoker5, 0)).To(Equal(secondBindingWorkspace)) + + Expect(fakeInvoker6.ApplyCallCount()).To(Equal(1)) + Expect(getWorkspace(fakeInvoker6, 0)).To(Equal(secondBindingWorkspace)) + + Expect(fakeInvokerBuilder.VersionedTerraformInvokerCallCount()).To(Equal(6)) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(0)).To(Equal(newVersion("2.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(1)).To(Equal(newVersion("3.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(2)).To(Equal(newVersion("4.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(3)).To(Equal(newVersion("2.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(4)).To(Equal(newVersion("3.0.0"))) + Expect(fakeInvokerBuilder.VersionedTerraformInvokerArgsForCall(5)).To(Equal(newVersion("4.0.0"))) By("ensuring the HCL was updated") Expect(fakeDeploymentManager.UpdateWorkspaceHCLCallCount()).To(Equal(2))