From 28ac386dea28b2bb58209a905c9b774c1be25841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Diego=20Gonz=C3=A1lez?= <25335192+jdgonzaleza@users.noreply.github.com> Date: Wed, 14 Sep 2022 13:15:41 -0500 Subject: [PATCH] Revert "feat: add --wait flag to upgrade-service (#2285)" This reverts commit 41ebecb29f603c571cd0d5b3a4aa448e91007e10. --- actor/v7action/service_instance.go | 16 +-- actor/v7action/service_instance_test.go | 45 +++---- command/v7/actor.go | 2 +- command/v7/upgrade_service_command.go | 25 ++-- command/v7/upgrade_service_command_test.go | 118 +++--------------- command/v7/v7fakes/fake_actor.go | 43 +++---- .../isolated/upgrade_service_command_test.go | 4 +- 7 files changed, 69 insertions(+), 184 deletions(-) diff --git a/actor/v7action/service_instance.go b/actor/v7action/service_instance.go index b1fa9b68e4..7ac5c7bbd5 100644 --- a/actor/v7action/service_instance.go +++ b/actor/v7action/service_instance.go @@ -146,13 +146,10 @@ func (actor Actor) UpdateManagedServiceInstance(params UpdateManagedServiceInsta return stream, Warnings(warnings), err } -func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spaceGUID string) (chan PollJobEvent, Warnings, error) { - var ( - serviceInstance resources.ServiceInstance - servicePlan resources.ServicePlan - jobURL ccv3.JobURL - stream chan PollJobEvent - ) +func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spaceGUID string) (Warnings, error) { + var serviceInstance resources.ServiceInstance + var servicePlan resources.ServicePlan + var jobURL ccv3.JobURL warnings, err := railway.Sequentially( func() (warnings ccv3.Warnings, err error) { @@ -176,12 +173,11 @@ func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spa return }, func() (warnings ccv3.Warnings, err error) { - stream = actor.PollJobToEventStream(jobURL) - return + return actor.CloudControllerClient.PollJobForState(jobURL, constant.JobPolling) }, ) - return stream, Warnings(warnings), err + return Warnings(warnings), err } func (actor Actor) RenameServiceInstance(currentServiceInstanceName, spaceGUID, newServiceInstanceName string) (Warnings, error) { diff --git a/actor/v7action/service_instance_test.go b/actor/v7action/service_instance_test.go index 03a7ca94f6..cb1551a3db 100644 --- a/actor/v7action/service_instance_test.go +++ b/actor/v7action/service_instance_test.go @@ -493,7 +493,7 @@ var _ = Describe("Service Instance Actions", func() { }) It("returns warnings and an error", func() { - Expect(warnings).To(ContainElement("warning from get")) + //Expect(warnings).To(ContainElement("warning from get")) Expect(executeErr).To(MatchError("bang")) }) }) @@ -1282,13 +1282,12 @@ var _ = Describe("Service Instance Actions", func() { ) var ( - executeErr error - warnings Warnings - stream chan PollJobEvent + err error + warnings Warnings ) JustBeforeEach(func() { - stream, warnings, executeErr = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID) + warnings, err = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID) }) It("makes a request to get the service instance", func() { @@ -1314,9 +1313,8 @@ var _ = Describe("Service Instance Actions", func() { }) It("returns the appropriate error", func() { - Expect(stream).To(BeNil()) Expect(warnings).To(ConsistOf("get SI warning")) - Expect(executeErr).To(MatchError(actionerror.ServiceInstanceNotFoundError{ + Expect(err).To(MatchError(actionerror.ServiceInstanceNotFoundError{ Name: fakeServiceInstanceName, })) }) @@ -1344,9 +1342,8 @@ var _ = Describe("Service Instance Actions", func() { }) It("returns the appropriate error", func() { - Expect(stream).To(BeNil()) Expect(warnings).To(ConsistOf("get SI warning")) - Expect(executeErr).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{})) + Expect(err).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{})) }) }) @@ -1383,15 +1380,10 @@ var _ = Describe("Service Instance Actions", func() { ccv3.Warnings{"warning from update"}, nil, ) - - fakeStream := make(chan ccv3.PollJobEvent) - go func() { - fakeStream <- ccv3.PollJobEvent{ - State: constant.JobProcessing, - Warnings: ccv3.Warnings{"stream warning"}, - } - }() - fakeCloudControllerClient.PollJobToEventStreamReturns(fakeStream) + fakeCloudControllerClient.PollJobForStateReturns( + ccv3.Warnings{"warning from poll"}, + nil, + ) }) It("makes the right calls and returns all warnings", func() { @@ -1411,18 +1403,15 @@ var _ = Describe("Service Instance Actions", func() { }) By("polling the job", func() { - Expect(fakeCloudControllerClient.PollJobToEventStreamCallCount()).To(Equal(1)) - Expect(fakeCloudControllerClient.PollJobToEventStreamArgsForCall(0)).To(Equal(jobURL)) + Expect(fakeCloudControllerClient.PollJobForStateCallCount()).To(Equal(1)) + actualURL, actualState := fakeCloudControllerClient.PollJobForStateArgsForCall(0) + Expect(actualURL).To(Equal(jobURL)) + Expect(actualState).To(Equal(constant.JobPolling)) }) - By("returning a stream, warnings and no error", func() { - Eventually(stream).Should(Receive(Equal(PollJobEvent{ - State: JobProcessing, - Warnings: Warnings{"stream warning"}, - }))) - - Expect(executeErr).NotTo(HaveOccurred()) - Expect(warnings).To(ConsistOf("warning from get", "warning from plan", "warning from update")) + By("returning warnings and no error", func() { + Expect(err).NotTo(HaveOccurred()) + Expect(warnings).To(ConsistOf("warning from get", "warning from plan", "warning from update", "warning from poll")) }) }) }) diff --git a/command/v7/actor.go b/command/v7/actor.go index b6173934d6..3bbae507c6 100644 --- a/command/v7/actor.go +++ b/command/v7/actor.go @@ -237,7 +237,7 @@ type Actor interface { UpdateDestination(string, string, string) (v7action.Warnings, error) UpdateDomainLabelsByDomainName(string, map[string]types.NullString) (v7action.Warnings, error) UpdateManagedServiceInstance(params v7action.UpdateManagedServiceInstanceParams) (chan v7action.PollJobEvent, v7action.Warnings, error) - UpgradeManagedServiceInstance(serviceInstanceName, spaceGUID string) (chan v7action.PollJobEvent, v7action.Warnings, error) + UpgradeManagedServiceInstance(serviceInstanceName, spaceGUID string) (v7action.Warnings, error) UpdateOrganizationLabelsByOrganizationName(string, map[string]types.NullString) (v7action.Warnings, error) UpdateOrganizationQuota(quotaName string, newName string, limits v7action.QuotaLimits) (v7action.Warnings, error) UpdateProcessByTypeAndApplication(processType string, appGUID string, updatedProcess resources.Process) (v7action.Warnings, error) diff --git a/command/v7/upgrade_service_command.go b/command/v7/upgrade_service_command.go index 91952e09f6..81bd39a065 100644 --- a/command/v7/upgrade_service_command.go +++ b/command/v7/upgrade_service_command.go @@ -4,7 +4,6 @@ import ( "code.cloudfoundry.org/cli/actor/actionerror" "code.cloudfoundry.org/cli/command/flag" "code.cloudfoundry.org/cli/command/translatableerror" - "code.cloudfoundry.org/cli/command/v7/shared" ) type UpgradeServiceCommand struct { @@ -12,7 +11,6 @@ type UpgradeServiceCommand struct { RequiredArgs flag.ServiceInstance `positional-args:"yes"` Force bool `short:"f" long:"force" description:"Force upgrade without asking for confirmation"` - Wait bool `short:"w" long:"wait" description:"Wait for the operation to complete"` relatedCommands interface{} `related_commands:"services, update-service, update-user-provided-service"` } @@ -40,7 +38,7 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error { serviceInstanceName := string(cmd.RequiredArgs.ServiceInstance) - stream, warnings, actorError := cmd.Actor.UpgradeManagedServiceInstance( + warnings, actorError := cmd.Actor.UpgradeManagedServiceInstance( serviceInstanceName, cmd.Config.TargetedSpace().GUID, ) @@ -48,6 +46,8 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error { switch actorError.(type) { case nil: + cmd.displayUpgradeInProgressMessage() + cmd.UI.DisplayOK() case actionerror.ServiceInstanceUpgradeNotAvailableError: cmd.UI.DisplayText(actorError.Error()) cmd.UI.DisplayOK() @@ -57,17 +57,6 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error { return actorError } - complete, err := shared.WaitForResult(stream, cmd.UI, cmd.Wait) - switch { - case err != nil: - return err - case complete: - cmd.UI.DisplayTextWithFlavor("Upgrade of service instance {{.ServiceInstanceName}} complete.", cmd.serviceInstanceName()) - default: - cmd.UI.DisplayTextWithFlavor("Upgrade in progress. Use 'cf services' or 'cf service {{.ServiceInstanceName}}' to check operation status.", cmd.serviceInstanceName()) - } - - cmd.UI.DisplayOK() return nil } @@ -90,7 +79,6 @@ func (cmd UpgradeServiceCommand) displayEvent() error { "Username": user.Name, }, ) - cmd.UI.DisplayNewline() return nil } @@ -113,6 +101,13 @@ func (cmd UpgradeServiceCommand) displayPrompt() (bool, error) { return upgrade, nil } +func (cmd UpgradeServiceCommand) displayUpgradeInProgressMessage() { + cmd.UI.DisplayTextWithFlavor("Upgrade in progress. Use 'cf services' or 'cf service {{.ServiceInstance}}' to check operation status.", + map[string]interface{}{ + "ServiceInstance": cmd.RequiredArgs.ServiceInstance, + }) +} + func (cmd UpgradeServiceCommand) serviceInstanceName() map[string]interface{} { return map[string]interface{}{ "ServiceInstanceName": cmd.RequiredArgs.ServiceInstance, diff --git a/command/v7/upgrade_service_command_test.go b/command/v7/upgrade_service_command_test.go index 8bbd7d8dcf..bc8aa3f54f 100644 --- a/command/v7/upgrade_service_command_test.go +++ b/command/v7/upgrade_service_command_test.go @@ -76,7 +76,6 @@ var _ = Describe("upgrade-service command", func() { When("the service instance does not exist", func() { BeforeEach(func() { fakeActor.UpgradeManagedServiceInstanceReturns( - nil, v7action.Warnings{"upgrade warning"}, actionerror.ServiceInstanceNotFoundError{Name: serviceInstanceName}, ) @@ -90,126 +89,37 @@ var _ = Describe("upgrade-service command", func() { }) }) - When("the actor returns an unexpected error", func() { + When("the service instance upgrade starts successfully", func() { BeforeEach(func() { fakeActor.UpgradeManagedServiceInstanceReturns( - make(chan v7action.PollJobEvent), v7action.Warnings{"upgrade warning"}, - errors.New("bang"), - ) - }) - - It("fails with warnings", func() { - Expect(executeErr).To(MatchError("bang")) - Expect(testUI.Err).To(Say("upgrade warning")) - Expect(testUI.Out).NotTo(Say("OK")) - }) - }) - - When("stream goes to polling", func() { - BeforeEach(func() { - fakeStream := make(chan v7action.PollJobEvent) - fakeActor.UpgradeManagedServiceInstanceReturns( - fakeStream, - v7action.Warnings{"actor warning"}, nil, ) - - go func() { - fakeStream <- v7action.PollJobEvent{ - State: v7action.JobPolling, - Warnings: v7action.Warnings{"poll warning"}, - } - }() }) - It("prints messages and warnings", func() { + It("succeeds with a message", func() { + Expect(executeErr).NotTo(HaveOccurred()) + Expect(testUI.Err).To(Say("upgrade warning")) Expect(testUI.Out).To(SatisfyAll( - Say(`Upgrading service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username), - Say(`\n`), - Say(`Upgrade in progress. Use 'cf services' or 'cf service %s' to check operation status\.`, serviceInstanceName), - Say(`OK\n`), - )) - - Expect(testUI.Err).To(SatisfyAll( - Say("actor warning"), - Say("poll warning"), + Say("\n"), + Say(`Upgrade in progress. Use 'cf services' or 'cf service %s' to check operation status\.\n`, serviceInstanceName), + Say("OK\n"), )) }) }) - When("error in event stream", func() { - BeforeEach(func() { - setFlag(&cmd, "--wait") - - fakeStream := make(chan v7action.PollJobEvent) - fakeActor.UpgradeManagedServiceInstanceReturns( - fakeStream, - v7action.Warnings{"a warning"}, - nil, - ) - - go func() { - fakeStream <- v7action.PollJobEvent{ - State: v7action.JobPolling, - Warnings: v7action.Warnings{"poll warning"}, - } - fakeStream <- v7action.PollJobEvent{ - State: v7action.JobFailed, - Warnings: v7action.Warnings{"failed warning"}, - Err: errors.New("boom"), - } - }() - }) - - It("returns the error and prints warnings", func() { - Expect(executeErr).To(MatchError("boom")) - Expect(testUI.Err).To(SatisfyAll( - Say("poll warning"), - Say("failed warning"), - )) - }) - }) - - When("--wait flag specified", func() { + When("the actor returns an unexpected error", func() { BeforeEach(func() { - setFlag(&cmd, "--wait") - - fakeStream := make(chan v7action.PollJobEvent) fakeActor.UpgradeManagedServiceInstanceReturns( - fakeStream, - v7action.Warnings{"a warning"}, - nil, + v7action.Warnings{"upgrade warning"}, + errors.New("bang"), ) - - go func() { - fakeStream <- v7action.PollJobEvent{ - State: v7action.JobPolling, - Warnings: v7action.Warnings{"poll warning"}, - } - fakeStream <- v7action.PollJobEvent{ - State: v7action.JobComplete, - Warnings: v7action.Warnings{"failed warning"}, - } - close(fakeStream) - }() }) - It("prints messages and warnings", func() { - Expect(testUI.Out).To(SatisfyAll( - Say(`Upgrading service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username), - Say(`\n`), - Say(`Waiting for the operation to complete\.\.\n`), - Say(`\n`), - Say(`Upgrade of service instance %s complete\.\n`, serviceInstanceName), - Say(`OK\n`), - )) - - Expect(testUI.Err).To(SatisfyAll( - Say("a warning"), - Say("poll warning"), - Say("failed warning"), - )) + It("fails with warnings", func() { + Expect(executeErr).To(MatchError("bang")) + Expect(testUI.Err).To(Say("upgrade warning")) + Expect(testUI.Out).NotTo(Say("OK")) }) }) } diff --git a/command/v7/v7fakes/fake_actor.go b/command/v7/v7fakes/fake_actor.go index 3c1defcdf7..f3cddabd4f 100644 --- a/command/v7/v7fakes/fake_actor.go +++ b/command/v7/v7fakes/fake_actor.go @@ -3536,21 +3536,19 @@ type FakeActor struct { result1 v7action.Warnings result2 error } - UpgradeManagedServiceInstanceStub func(string, string) (chan v7action.PollJobEvent, v7action.Warnings, error) + UpgradeManagedServiceInstanceStub func(string, string) (v7action.Warnings, error) upgradeManagedServiceInstanceMutex sync.RWMutex upgradeManagedServiceInstanceArgsForCall []struct { arg1 string arg2 string } upgradeManagedServiceInstanceReturns struct { - result1 chan v7action.PollJobEvent - result2 v7action.Warnings - result3 error + result1 v7action.Warnings + result2 error } upgradeManagedServiceInstanceReturnsOnCall map[int]struct { - result1 chan v7action.PollJobEvent - result2 v7action.Warnings - result3 error + result1 v7action.Warnings + result2 error } UploadBitsPackageStub func(resources.Package, []sharedaction.V3Resource, io.Reader, int64) (resources.Package, v7action.Warnings, error) uploadBitsPackageMutex sync.RWMutex @@ -18807,7 +18805,7 @@ func (fake *FakeActor) UpdateUserProvidedServiceInstanceReturnsOnCall(i int, res }{result1, result2} } -func (fake *FakeActor) UpgradeManagedServiceInstance(arg1 string, arg2 string) (chan v7action.PollJobEvent, v7action.Warnings, error) { +func (fake *FakeActor) UpgradeManagedServiceInstance(arg1 string, arg2 string) (v7action.Warnings, error) { fake.upgradeManagedServiceInstanceMutex.Lock() ret, specificReturn := fake.upgradeManagedServiceInstanceReturnsOnCall[len(fake.upgradeManagedServiceInstanceArgsForCall)] fake.upgradeManagedServiceInstanceArgsForCall = append(fake.upgradeManagedServiceInstanceArgsForCall, struct { @@ -18820,10 +18818,10 @@ func (fake *FakeActor) UpgradeManagedServiceInstance(arg1 string, arg2 string) ( return fake.UpgradeManagedServiceInstanceStub(arg1, arg2) } if specificReturn { - return ret.result1, ret.result2, ret.result3 + return ret.result1, ret.result2 } fakeReturns := fake.upgradeManagedServiceInstanceReturns - return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 + return fakeReturns.result1, fakeReturns.result2 } func (fake *FakeActor) UpgradeManagedServiceInstanceCallCount() int { @@ -18832,7 +18830,7 @@ func (fake *FakeActor) UpgradeManagedServiceInstanceCallCount() int { return len(fake.upgradeManagedServiceInstanceArgsForCall) } -func (fake *FakeActor) UpgradeManagedServiceInstanceCalls(stub func(string, string) (chan v7action.PollJobEvent, v7action.Warnings, error)) { +func (fake *FakeActor) UpgradeManagedServiceInstanceCalls(stub func(string, string) (v7action.Warnings, error)) { fake.upgradeManagedServiceInstanceMutex.Lock() defer fake.upgradeManagedServiceInstanceMutex.Unlock() fake.UpgradeManagedServiceInstanceStub = stub @@ -18845,33 +18843,30 @@ func (fake *FakeActor) UpgradeManagedServiceInstanceArgsForCall(i int) (string, return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeActor) UpgradeManagedServiceInstanceReturns(result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { +func (fake *FakeActor) UpgradeManagedServiceInstanceReturns(result1 v7action.Warnings, result2 error) { fake.upgradeManagedServiceInstanceMutex.Lock() defer fake.upgradeManagedServiceInstanceMutex.Unlock() fake.UpgradeManagedServiceInstanceStub = nil fake.upgradeManagedServiceInstanceReturns = struct { - result1 chan v7action.PollJobEvent - result2 v7action.Warnings - result3 error - }{result1, result2, result3} + result1 v7action.Warnings + result2 error + }{result1, result2} } -func (fake *FakeActor) UpgradeManagedServiceInstanceReturnsOnCall(i int, result1 chan v7action.PollJobEvent, result2 v7action.Warnings, result3 error) { +func (fake *FakeActor) UpgradeManagedServiceInstanceReturnsOnCall(i int, result1 v7action.Warnings, result2 error) { fake.upgradeManagedServiceInstanceMutex.Lock() defer fake.upgradeManagedServiceInstanceMutex.Unlock() fake.UpgradeManagedServiceInstanceStub = nil if fake.upgradeManagedServiceInstanceReturnsOnCall == nil { fake.upgradeManagedServiceInstanceReturnsOnCall = make(map[int]struct { - result1 chan v7action.PollJobEvent - result2 v7action.Warnings - result3 error + result1 v7action.Warnings + result2 error }) } fake.upgradeManagedServiceInstanceReturnsOnCall[i] = struct { - result1 chan v7action.PollJobEvent - result2 v7action.Warnings - result3 error - }{result1, result2, result3} + result1 v7action.Warnings + result2 error + }{result1, result2} } func (fake *FakeActor) UploadBitsPackage(arg1 resources.Package, arg2 []sharedaction.V3Resource, arg3 io.Reader, arg4 int64) (resources.Package, v7action.Warnings, error) { diff --git a/integration/v7/isolated/upgrade_service_command_test.go b/integration/v7/isolated/upgrade_service_command_test.go index bc1bd08585..41f30093ed 100644 --- a/integration/v7/isolated/upgrade_service_command_test.go +++ b/integration/v7/isolated/upgrade_service_command_test.go @@ -24,7 +24,6 @@ var _ = Describe("upgrade-service command", func() { Say(`\s+cf upgrade-service SERVICE_INSTANCE`), Say(`OPTIONS:`), Say(`\s+--force, -f\s+Force upgrade without asking for confirmation`), - Say(`\s+--wait, -w\s+Wait for the operation to complete\n`), Say(`SEE ALSO:`), Say(`\s+services, update-service, update-user-provided-service`), ) @@ -108,7 +107,7 @@ var _ = Describe("upgrade-service command", func() { }) }) - When("the service instance exists", func() { + When("the service instance exist", func() { var broker *servicebrokerstub.ServiceBrokerStub BeforeEach(func() { @@ -152,6 +151,7 @@ var _ = Describe("upgrade-service command", func() { Expect(session.Out).To(Say("Upgrade in progress")) }) }) + }) }) })