diff --git a/CHANGELOG.md b/CHANGELOG.md index b3b42c5c6..e4f437d9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,13 @@ can be found in [Pivotal Documentation](docs.pivotal.io/platform-automation). This ensures that commands are not kept in `bash` history. The environment variable `OM_PASSWORD` will overwrite the password value in `env.yml`. +## 6.4.1 + +### Bug Fixes +- `pending-changes` would always fail if installation incomplete, product unconfigured, or stemcell missing + regardless of whether the `--check` flag (exit 1 if there are pending changes) was set. + This has been fixed so that the implied and intended behavior is reflected in the ouput of the command. + ## 6.4.0 ### Features diff --git a/cmd/main.go b/cmd/main.go index d9d5fd25d..77db89251 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -174,7 +174,7 @@ func Main(sout io.Writer, serr io.Writer, version string, applySleepDurationStri commandSet["installation-log"] = commands.NewInstallationLog(api, stdout) commandSet["installations"] = commands.NewInstallations(api, presenter) commandSet["interpolate"] = commands.NewInterpolate(os.Environ, stdout, os.Stdin) - commandSet["pending-changes"] = commands.NewPendingChanges(presenter, api) + commandSet["pending-changes"] = commands.NewPendingChanges(presenter, api, stderr) commandSet["pre-deploy-check"] = commands.NewPreDeployCheck(presenter, api, stdout) commandSet["product-metadata"] = commands.NewProductMetadata(stdout) commandSet["regenerate-certificates"] = commands.NewRegenerateCertificates(api, stdout) diff --git a/commands/pending_changes.go b/commands/pending_changes.go index 1a53b5ea0..f25ab6a0b 100644 --- a/commands/pending_changes.go +++ b/commands/pending_changes.go @@ -16,6 +16,7 @@ type PendingChanges struct { Check bool `long:"check" description:"Exit 1 if there are any pending changes. Useful for validating that Ops Manager is in a clean state."` Format string `long:"format" short:"f" default:"table" description:"Format to print as (options: table,json)"` } + logger logger } //counterfeiter:generate -o ./fakes/pending_changes_service.go --fake-name PendingChangesService . pendingChangesService @@ -23,10 +24,11 @@ type pendingChangesService interface { ListStagedPendingChanges() (api.PendingChangesOutput, error) } -func NewPendingChanges(presenter presenters.FormattedPresenter, service pendingChangesService) PendingChanges { +func NewPendingChanges(presenter presenters.FormattedPresenter, service pendingChangesService, logger logger) PendingChanges { return PendingChanges{ service: service, presenter: presenter, + logger: logger, } } @@ -60,16 +62,19 @@ func (pc PendingChanges) Execute(args []string) error { } } - if len(errs) > 0 { - return fmt.Errorf("%s\nPlease validate your Ops Manager installation in the UI", strings.Join(errs, ",\n")) + for _, ProductChange := range output.ChangeList { + if ProductChange.Action != "unchanged" { + errs = append(errs, fmt.Sprintf("there are pending changes.\nGo into the Ops Manager UI, unstage changes, and try again")) + break + } } - if pc.Options.Check { - for _, ProductChange := range output.ChangeList { - if ProductChange.Action != "unchanged" { - return fmt.Errorf("there are pending changes.\nGo into the Ops Manager UI, unstage changes, and try again") - } + if len(errs) > 0 { + if pc.Options.Check { + return fmt.Errorf("%s\nPlease validate your Ops Manager installation in the UI", strings.Join(errs, ",\n")) } + + pc.logger.Printf("Warnings:\n%s", strings.Join(errs, ",\n")) } return nil } diff --git a/commands/pending_changes_test.go b/commands/pending_changes_test.go index 258f54f03..5f9dfc1f3 100644 --- a/commands/pending_changes_test.go +++ b/commands/pending_changes_test.go @@ -2,6 +2,8 @@ package commands_test import ( "errors" + "github.com/onsi/gomega/gbytes" + "log" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -16,15 +18,17 @@ var _ = Describe("PendingChanges.Execute", func() { presenter *presenterfakes.FormattedPresenter pcService *fakes.PendingChangesService command commands.PendingChanges + stderr *gbytes.Buffer + logger *log.Logger ) BeforeEach(func() { + stderr = gbytes.NewBuffer() + logger = log.New(stderr, "", 0) + presenter = &presenterfakes.FormattedPresenter{} pcService = &fakes.PendingChangesService{} - command = commands.NewPendingChanges(presenter, pcService) - }) - - BeforeEach(func() { + command = commands.NewPendingChanges(presenter, pcService, logger) pcService.ListStagedPendingChangesReturns(api.PendingChangesOutput{ ChangeList: []api.ProductChange{ { @@ -42,6 +46,11 @@ var _ = Describe("PendingChanges.Execute", func() { PreDelete: "false", }, }, + CompletenessChecks: &api.CompletenessChecks{ + ConfigurationComplete: false, + StemcellPresent: false, + ConfigurablePropertiesValid: false, + }, }, { GUID: "some-product-without-errand", @@ -52,12 +61,17 @@ var _ = Describe("PendingChanges.Execute", func() { }, nil) }) - It("lists the pending changes", func() { + It("lists the pending changes and all warnings", func() { err := command.Execute([]string{}) Expect(err).ToNot(HaveOccurred()) Expect(presenter.SetFormatArgsForCall(0)).To(Equal("table")) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) + + Expect(stderr).To(gbytes.Say("configuration is incomplete for guid")) + Expect(stderr).To(gbytes.Say("stemcell is missing for one or more products for guid")) + Expect(stderr).To(gbytes.Say("one or more properties are invalid for guid")) + Expect(stderr).To(gbytes.Say("there are pending changes")) }) When("the check flag is provided", func() { @@ -140,35 +154,6 @@ var _ = Describe("PendingChanges.Execute", func() { Expect(err).ToNot(HaveOccurred()) }) }) - }) - - When("the format flag is provided", func() { - It("sets the format on the presenter", func() { - err := command.Execute([]string{"--format", "json"}) - Expect(err).ToNot(HaveOccurred()) - - Expect(presenter.SetFormatArgsForCall(0)).To(Equal("json")) - }) - }) - - Describe("failure cases", func() { - When("an unknown flag is passed", func() { - It("returns an error", func() { - err := command.Execute([]string{"--unknown-flag"}) - Expect(err).To(MatchError("could not parse pending-changes flags: flag provided but not defined: -unknown-flag")) - }) - }) - - When("fetching the pending changes fails", func() { - It("returns an error", func() { - command := commands.NewPendingChanges(presenter, pcService) - - pcService.ListStagedPendingChangesReturns(api.PendingChangesOutput{}, errors.New("beep boop")) - - err := command.Execute([]string{}) - Expect(err).To(MatchError("failed to retrieve pending changes beep boop")) - }) - }) Describe("Ops Man 2.5 and earlier", func() { When("completeness_check returns any false values", func() { @@ -187,7 +172,7 @@ var _ = Describe("PendingChanges.Execute", func() { }, }, nil) - err := command.Execute([]string{}) + err := command.Execute(options) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) Expect(err).To(MatchError(ContainSubstring("configuration is incomplete for guid some-product-without-errands"))) Expect(err).To(MatchError(ContainSubstring("Please validate your Ops Manager installation in the UI"))) @@ -208,7 +193,7 @@ var _ = Describe("PendingChanges.Execute", func() { }, }, nil) - err := command.Execute([]string{}) + err := command.Execute(options) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) Expect(err).To(MatchError(ContainSubstring("stemcell is missing for one or more products for guid some-product-without-errands"))) Expect(err).To(MatchError(ContainSubstring("Please validate your Ops Manager installation in the UI"))) @@ -230,7 +215,7 @@ var _ = Describe("PendingChanges.Execute", func() { }, }, nil) - err := command.Execute([]string{}) + err := command.Execute(options) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) Expect(err).To(MatchError(ContainSubstring("one or more properties are invalid for guid some-product-without-errands"))) Expect(err).To(MatchError(ContainSubstring("Please validate your Ops Manager installation in the UI"))) @@ -263,7 +248,7 @@ var _ = Describe("PendingChanges.Execute", func() { }, }, nil) - err := command.Execute([]string{}) + err := command.Execute(options) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) Expect(err).To(MatchError(ContainSubstring("one or more properties are invalid for guid some-product-without-errands"))) Expect(err).To(MatchError(ContainSubstring("stemcell is missing for one or more products for guid some-product-without-errands"))) @@ -292,7 +277,7 @@ var _ = Describe("PendingChanges.Execute", func() { }, }, nil) - err := command.Execute([]string{}) + err := command.Execute(options) Expect(presenter.PresentPendingChangesCallCount()).To(Equal(1)) Expect(err).To(MatchError(ContainSubstring("one or more properties are invalid for guid some-product-without-errands"))) Expect(err).To(MatchError(ContainSubstring("stemcell is missing for one or more products for guid some-product-without-errands"))) @@ -302,9 +287,32 @@ var _ = Describe("PendingChanges.Execute", func() { }) }) }) + }) + + When("the format flag is provided", func() { + It("sets the format on the presenter", func() { + err := command.Execute([]string{"--format", "json"}) + Expect(err).ToNot(HaveOccurred()) + + Expect(presenter.SetFormatArgsForCall(0)).To(Equal("json")) + }) + }) + + When("an unknown flag is passed", func() { + It("returns an error", func() { + err := command.Execute([]string{"--unknown-flag"}) + Expect(err).To(MatchError("could not parse pending-changes flags: flag provided but not defined: -unknown-flag")) + }) + }) + + When("fetching the pending changes fails", func() { + It("returns an error", func() { + command := commands.NewPendingChanges(presenter, pcService, nil) - Describe("Ops Man 2.6 and later", func() { + pcService.ListStagedPendingChangesReturns(api.PendingChangesOutput{}, errors.New("beep boop")) + err := command.Execute([]string{}) + Expect(err).To(MatchError("failed to retrieve pending changes beep boop")) }) }) })