fix failures in pending-changes that occurred when the --check flag w…

…as not passed

* Resolves #496
* [#175105770]

Signed-off-by: Kira Boyle <kboyle@vmware.com>

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

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)

commands/pending_changes.go

@@ -16,17 +16,19 @@ 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
 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
 }

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"))
 		})
 	})
 })