From 3c5be946525200e6d5d6c1b64c3c3f876fd31a1f Mon Sep 17 00:00:00 2001 From: nam Date: Wed, 6 Nov 2019 12:38:55 +0100 Subject: [PATCH 01/13] list and show commit command: initial command addition --- api/modelgeneration/modelgeneration.go | 30 +++++++ cmd/juju/model/listcommits.go | 111 +++++++++++++++++++++++++ cmd/juju/model/showcommit.go | 108 ++++++++++++++++++++++++ 3 files changed, 249 insertions(+) create mode 100644 cmd/juju/model/listcommits.go create mode 100644 cmd/juju/model/showcommit.go diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index ebbaf46b064..2fbdc18f6a6 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -69,6 +69,36 @@ func (c *Client) CommitBranch(branchName string) (int, error) { return result.Result, nil } +// CommitBranch commits the branch with the input name to the model, +// effectively completing it and applying all branch changes across the model. +// The new generation ID of the model is returned. +func (c *Client) ListCommits() (int, error) { + var result params.IntResult + err := c.facade.FacadeCall("ListCommits", nil, &result) + if err != nil { + return 0, errors.Trace(err) + } + if result.Error != nil { + return 0, errors.Trace(result.Error) + } + return result.Result, nil +} + +// CommitBranch commits the branch with the input name to the model, +// effectively completing it and applying all branch changes across the model. +// The new generation ID of the model is returned. +func (c *Client) ShowCommit() (int, error) { + var result params.IntResult + err := c.facade.FacadeCall("ShowCommit", nil, &result) + if err != nil { + return 0, errors.Trace(err) + } + if result.Error != nil { + return 0, errors.Trace(result.Error) + } + return result.Result, nil +} + // TrackBranch sets the input units and/or applications // to track changes made under the input branch name. func (c *Client) TrackBranch(branchName string, entities []string, numUnits int) error { diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go new file mode 100644 index 00000000000..6764326db6a --- /dev/null +++ b/cmd/juju/model/listcommits.go @@ -0,0 +1,111 @@ +// Copyright 2019 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package model + +import ( + "github.com/juju/cmd" + "github.com/juju/errors" + "github.com/juju/gnuflag" + + "github.com/juju/juju/api/modelgeneration" + jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/modelcmd" +) + +const ( + listCommitsSummary = "Lists commits history" + listCommitsDoc = ` +List commits shows the timeline of changes to the model that occurred through branching. +It does not take into account other changes to the model that did not occur through a managed branch. + +Examples: + juju list-commits + +See also: + commits + show-commit + add-branch + track + branch + abort + diff +` +) + +//TODO: instead of diffing, i just show the content of config +//gen-id is unique corresponds to commit it to show + +// NewCommitCommand wraps listCommitsCommand with sane model settings. +func NewListCommitCommand() cmd.Command { + return modelcmd.Wrap(&listCommitsCommand{}) +} + +// listCommitsCommand supplies the "commit" CLI command used to commit changes made +// under a branch, to the model. +type listCommitsCommand struct { + modelcmd.ModelCommandBase + + api ListCommitsCommandAPI +} + +// ListCommitsCommandAPI defines an API interface to be used during testing. +//go:generate mockgen -package mocks -destination ./mocks/commit_mock.go github.com/juju/juju/cmd/juju/model ListCommitsCommandAPI +type ListCommitsCommandAPI interface { + Close() error + + // ListCommitsBranch commits the branch with the input name to the model, + // effectively completing it and applying + // all branch changes across the model. + // The new generation ID of the model is returned. + ListCommits() (int, error) +} + +// Info implements part of the cmd.Command interface. +func (c *listCommitsCommand) Info() *cmd.Info { + info := &cmd.Info{ + Name: "list-commits", + Purpose: listCommitsSummary, + Doc: listCommitsDoc, + } + return jujucmd.Info(info) +} + +// SetFlags implements part of the cmd.Command interface. +func (c *listCommitsCommand) SetFlags(f *gnuflag.FlagSet) { + c.ModelCommandBase.SetFlags(f) +} + +// Init implements part of the cmd.Command interface. +func (c *listCommitsCommand) Init(args []string) error { + return nil +} + +// getAPI returns the API. This allows passing in a test CommitCommandAPI +// implementation. +func (c *listCommitsCommand) getAPI() (ListCommitsCommandAPI, error) { + if c.api != nil { + return c.api, nil + } + api, err := c.NewAPIRoot() + if err != nil { + return nil, errors.Annotate(err, "opening API connection") + } + client := modelgeneration.NewClient(api) + return client, nil +} + +// Run implements the meaty part of the cmd.Command interface. +func (c *listCommitsCommand) Run(ctx *cmd.Context) error { + client, err := c.getAPI() + if err != nil { + return err + } + defer func() { _ = client.Close() }() + + _, err = client.ListCommits() + if err != nil { + return err + } + return err +} diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go new file mode 100644 index 00000000000..8a4f76e8280 --- /dev/null +++ b/cmd/juju/model/showcommit.go @@ -0,0 +1,108 @@ +// Copyright 2019 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package model + +import ( + "github.com/juju/cmd" + "github.com/juju/errors" + "github.com/juju/gnuflag" + + "github.com/juju/juju/api/modelgeneration" + jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/modelcmd" +) + +const ( + showCommitsSummary = "Shows the commit content" + showCommitsDoc = ` +show-commit shows the effective change from a branch to the master. + +Examples: + juju show-commits 3 + +See also: + list-commits + add-branch + track + branch + abort + diff +` +) + +//TODO: instead of diffing, i just show the content of config +//gen-id is unique corresponds to commit it to show + +// NewCommitCommand wraps listCommitsCommand with sane model settings. +func NewShowCommitCommand() cmd.Command { + return modelcmd.Wrap(&ShowCommitCommand{}) +} + +// ShowCommitCommand supplies the "show-commit" CLI command used to show commits +type ShowCommitCommand struct { + modelcmd.ModelCommandBase + + api ShowCommitCommandAPI +} + +// ShowCommitCommandAPI defines an API interface to be used during testing. +//go:generate mockgen -package mocks -destination ./mocks/commit_mock.go github.com/juju/juju/cmd/juju/model ShowCommitCommandAPI +type ShowCommitCommandAPI interface { + Close() error + + // ListCommitsBranch commits the branch with the input name to the model, + // effectively completing it and applying + // all branch changes across the model. + // The new generation ID of the model is returned. + ShowCommit() (int, error) +} + +// Info implements part of the cmd.Command interface. +func (c *ShowCommitCommand) Info() *cmd.Info { + info := &cmd.Info{ + Name: "list-commits", + Purpose: listCommitsSummary, + Doc: listCommitsDoc, + } + return jujucmd.Info(info) +} + +// SetFlags implements part of the cmd.Command interface. +func (c *ShowCommitCommand) SetFlags(f *gnuflag.FlagSet) { + c.ModelCommandBase.SetFlags(f) +} + +// Init implements part of the cmd.Command interface. +func (c *ShowCommitCommand) Init(args []string) error { + return nil +} + +// getAPI returns the API. This allows passing in a test CommitCommandAPI +// implementation. +func (c *ShowCommitCommand) getAPI() (ShowCommitCommandAPI, error) { + if c.api != nil { + return c.api, nil + } + api, err := c.NewAPIRoot() + if err != nil { + return nil, errors.Annotate(err, "opening API connection") + } + client := modelgeneration.NewClient(api) + return client, nil +} + +// Run implements the meaty part of the cmd.Command interface. +func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { + client, err := c.getAPI() + if err != nil { + return err + } + defer func() { _ = client.Close() }() + + _, err = client.ShowCommit() + if err != nil { + return err + } + return err +} From 53648c3a5b6849a5475f4d993284264339569e07 Mon Sep 17 00:00:00 2001 From: nam Date: Wed, 6 Nov 2019 15:37:41 +0100 Subject: [PATCH 02/13] list and show commit apiserver : initial command addition on the apiserver part --- api/modelgeneration/modelgeneration.go | 23 +++-- .../client/modelgeneration/interface.go | 1 + .../client/modelgeneration/modelgeneration.go | 82 +++++++++++++++++- apiserver/facades/schema.json | 13 ++- apiserver/params/params.go | 14 +++- cmd/juju/model/export_test.go | 8 ++ cmd/juju/model/listcommits.go | 79 +++++++++++------ cmd/juju/model/listcommits_test.go | 84 +++++++++++++++++++ cmd/juju/model/mocks/diff_mock.go | 4 + core/model/generation.go | 18 ++++ 10 files changed, 292 insertions(+), 34 deletions(-) create mode 100644 cmd/juju/model/listcommits_test.go diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index 2fbdc18f6a6..acef95a2f41 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -72,16 +72,16 @@ func (c *Client) CommitBranch(branchName string) (int, error) { // CommitBranch commits the branch with the input name to the model, // effectively completing it and applying all branch changes across the model. // The new generation ID of the model is returned. -func (c *Client) ListCommits() (int, error) { - var result params.IntResult +func (c *Client) ListCommits(formatTime func(time.Time) string) (model.GenerationCommits, error) { + var result params.GenerationResults err := c.facade.FacadeCall("ListCommits", nil, &result) if err != nil { - return 0, errors.Trace(err) + return nil, errors.Trace(err) } if result.Error != nil { - return 0, errors.Trace(result.Error) + return nil, errors.Trace(result.Error) } - return result.Result, nil + return generationCommitsFromResult(result, formatTime), nil } // CommitBranch commits the branch with the input name to the model, @@ -204,3 +204,16 @@ func generationInfoFromResult( } return summaries } + +func generationCommitsFromResult(results params.GenerationResults, formatTime func(time.Time) string) model.GenerationCommits { + commits := make(model.GenerationCommits, len(results.Generations)) + for i, gen := range results.Generations { + commits[i] = model.GenerationCommit{ + CommitNumber: gen.GenerationId, + Created: formatTime(time.Unix(gen.Created, 0)), + CreatedBy: gen.CreatedBy, + BranchName: gen.BranchName, + } + } + return commits +} diff --git a/apiserver/facades/client/modelgeneration/interface.go b/apiserver/facades/client/modelgeneration/interface.go index 9065886622e..5b042e27350 100644 --- a/apiserver/facades/client/modelgeneration/interface.go +++ b/apiserver/facades/client/modelgeneration/interface.go @@ -45,6 +45,7 @@ type Generation interface { Commit(string) (int, error) Abort(string) error Config() map[string]settings.ItemChanges + GenerationId() int } // Application describes application state used by the model generation API. diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index 291ab0396a4..c40798443a8 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -238,7 +238,8 @@ func (api *API) AbortBranch(arg params.BranchArg) (params.ErrorResult, error) { // including units on the branch and the configuration disjoint with the // master generation. // An error is returned if no in-flight branch matching in input is found. -func (api *API) BranchInfo(args params.BranchInfoArgs) (params.GenerationResults, error) { +func (api *API) BranchInfo( + args params.BranchInfoArgs) (params.GenerationResults, error) { result := params.GenerationResults{} isModelAdmin, err := api.hasAdminAccess() @@ -276,6 +277,80 @@ func (api *API) BranchInfo(args params.BranchInfoArgs) (params.GenerationResults return result, nil } +// ListCommits will return the commits, hence only branches with generation_id higher than 0 +// Keyed by the generation_id +func (api *API) ListCommits() (params.GenerationResults, error) { + result := params.GenerationResults{} + + isModelAdmin, err := api.hasAdminAccess() + if err != nil { + return result, errors.Trace(err) + } + if !isModelAdmin && !api.isControllerAdmin { + return result, common.ErrPerm + } + + var branches []Generation + if branches, err = api.model.Branches(); err != nil { + return generationResultsError(err) + } + + results := make([]params.Generation, len(branches)) + for i, b := range branches { + generation, err := api.oneBranchInfo(b, false) + if err != nil { + return generationResultsError(err) + } + if generation.GenerationId > 0 { + results[i] = generation + } + } + + result.Generations = results + return result, nil +} + +// ShowCommit will return details a commit given by its generationId +// An error is returned if no commit can be found corresponding to a branch. +func (api *API) ShowCommit(generationId int) (params.GenerationResult, error) { + result := params.GenerationResult{} + + isModelAdmin, err := api.hasAdminAccess() + if err != nil { + return result, errors.Trace(err) + } + if !isModelAdmin && !api.isControllerAdmin { + return result, common.ErrPerm + } + if generationId < 1 { + err := errors.Errorf("supplied generation id has to be higher than 0") + return generationResultError(err) + } + + branches, err := api.model.Branches() + if err != nil { + return generationResultError(err) + } + + found := false + for _, b := range branches { + // TODO: needs to be of type GenerationApplication + generation, err := api.oneBranchInfo(b, false) + if err != nil { + return generationResultError(err) + } + if generation.GenerationId > 0 && generation.GenerationId == generationId { + result.Generation = generation + found = true + } + } + if !found { + err := errors.Errorf("did not find the given generationid %q", generationId) + return generationResultError(err) + } + return result, nil +} + func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generation, error) { deltas := branch.Config() @@ -320,6 +395,7 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati BranchName: branch.BranchName(), Created: branch.Created(), CreatedBy: branch.CreatedBy(), + GenerationId: branch.GenerationId(), Applications: apps, }, nil } @@ -352,6 +428,10 @@ func generationResultsError(err error) (params.GenerationResults, error) { return params.GenerationResults{Error: common.ServerError(err)}, nil } +func generationResultError(err error) (params.GenerationResult, error) { + return params.GenerationResult{Error: common.ServerError(err)}, nil +} + func intResultsError(err error) (params.IntResult, error) { return params.IntResult{Error: common.ServerError(err)}, nil } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index ec3ee662beb..d1ccd8d12cc 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23096,6 +23096,14 @@ } } }, + "ListCommits": { + "type": "object", + "properties": { + "Result": { + "$ref": "#/definitions/GenerationResults" + } + } + }, "TrackBranch": { "type": "object", "properties": { @@ -23255,6 +23263,9 @@ }, "created-by": { "type": "string" + }, + "generation-id": { + "type": "integer" } }, "additionalProperties": false, @@ -23262,7 +23273,7 @@ "branch", "created", "created-by", - "applications" + "generation-id" ] }, "GenerationApplication": { diff --git a/apiserver/params/params.go b/apiserver/params/params.go index ee977f1bba8..27075d3a2d3 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -1233,9 +1233,12 @@ type Generation struct { // Created is the user who created the generation. CreatedBy string `json:"created-by"` + // GenerationId is the id . + GenerationId int `json:"generation-id"` + // Applications holds the collection of application changes // made under this generation. - Applications []GenerationApplication `json:"applications"` + Applications []GenerationApplication `json:"applications,omitempty"` } // GenerationResults transports a collection of generation details. @@ -1247,6 +1250,15 @@ type GenerationResults struct { Error *Error `json:"error,omitempty"` } +// GenerationResult transports a generation detail. +type GenerationResult struct { + // Generation holds the details of the requested generation. + Generation Generation `json:"generation"` + + // Error holds the value of any error that occurred processing the request. + Error *Error `json:"error,omitempty"` +} + // CharmProfilingInfoResult contains the result based on ProfileInfoArg values // to update profiles on a machine. type CharmProfilingInfoResult struct { diff --git a/cmd/juju/model/export_test.go b/cmd/juju/model/export_test.go index b229eff22df..8c5791664c9 100644 --- a/cmd/juju/model/export_test.go +++ b/cmd/juju/model/export_test.go @@ -228,3 +228,11 @@ func NewDiffCommandForTest(api DiffCommandAPI, store jujuclient.ClientStore) cmd cmd.SetClientStore(store) return modelcmd.Wrap(cmd) } + +func NewListCommitsCommandForTest(api CommitsCommandAPI, store jujuclient.ClientStore) cmd.Command { + cmd := &CommitsCommand{ + api: api, + } + cmd.SetClientStore(store) + return modelcmd.Wrap(cmd) +} diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index 6764326db6a..ace52b1c320 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -7,6 +7,12 @@ import ( "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" + "github.com/juju/juju/cmd/juju/common" + "github.com/juju/juju/core/model" + "github.com/juju/juju/juju/osenv" + "os" + "strconv" + "time" "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" @@ -16,11 +22,11 @@ import ( const ( listCommitsSummary = "Lists commits history" listCommitsDoc = ` -List commits shows the timeline of changes to the model that occurred through branching. +commits shows the timeline of changes to the model that occurred through branching. It does not take into account other changes to the model that did not occur through a managed branch. Examples: - juju list-commits + juju commits See also: commits @@ -33,57 +39,74 @@ See also: ` ) -//TODO: instead of diffing, i just show the content of config -//gen-id is unique corresponds to commit it to show - -// NewCommitCommand wraps listCommitsCommand with sane model settings. -func NewListCommitCommand() cmd.Command { - return modelcmd.Wrap(&listCommitsCommand{}) -} - -// listCommitsCommand supplies the "commit" CLI command used to commit changes made +// CommitsCommand supplies the "commit" CLI command used to commit changes made // under a branch, to the model. -type listCommitsCommand struct { +type CommitsCommand struct { modelcmd.ModelCommandBase - api ListCommitsCommandAPI + api CommitsCommandAPI + out cmd.Output + + isoTime bool } -// ListCommitsCommandAPI defines an API interface to be used during testing. -//go:generate mockgen -package mocks -destination ./mocks/commit_mock.go github.com/juju/juju/cmd/juju/model ListCommitsCommandAPI -type ListCommitsCommandAPI interface { +// CommitsCommandAPI defines an API interface to be used during testing. +//go:generate mockgen -package mocks -destination ./mocks/commits_mock.go github.com/juju/juju/cmd/juju/model CommitsCommandAPI +type CommitsCommandAPI interface { Close() error // ListCommitsBranch commits the branch with the input name to the model, // effectively completing it and applying // all branch changes across the model. // The new generation ID of the model is returned. - ListCommits() (int, error) + ListCommits(func(time.Time) string) (model.GenerationCommits, error) +} + +// NewCommitCommand wraps CommitsCommand with sane model settings. +func NewCommitsCommand() cmd.Command { + return modelcmd.Wrap(&CommitsCommand{}) } // Info implements part of the cmd.Command interface. -func (c *listCommitsCommand) Info() *cmd.Info { +func (c *CommitsCommand) Info() *cmd.Info { info := &cmd.Info{ - Name: "list-commits", + Name: "commits", Purpose: listCommitsSummary, Doc: listCommitsDoc, + Aliases: []string{"list-commits"}, } return jujucmd.Info(info) } // SetFlags implements part of the cmd.Command interface. -func (c *listCommitsCommand) SetFlags(f *gnuflag.FlagSet) { +func (c *CommitsCommand) SetFlags(f *gnuflag.FlagSet) { c.ModelCommandBase.SetFlags(f) + f.BoolVar(&c.isoTime, "utc", false, "Display time as UTC in RFC3339 format") } // Init implements part of the cmd.Command interface. -func (c *listCommitsCommand) Init(args []string) error { +func (c *CommitsCommand) Init(args []string) error { + lArgs := len(args) + if lArgs > 0 { + return errors.Errorf("expected no arguments, but got %v", lArgs) + } + + // If use of ISO time not specified on command line, check env var. + if !c.isoTime { + var err error + envVarValue := os.Getenv(osenv.JujuStatusIsoTimeEnvKey) + if envVarValue != "" { + if c.isoTime, err = strconv.ParseBool(envVarValue); err != nil { + return errors.Annotatef(err, "invalid %s env var, expected true|false", osenv.JujuStatusIsoTimeEnvKey) + } + } + } return nil } // getAPI returns the API. This allows passing in a test CommitCommandAPI // implementation. -func (c *listCommitsCommand) getAPI() (ListCommitsCommandAPI, error) { +func (c *CommitsCommand) getAPI() (CommitsCommandAPI, error) { if c.api != nil { return c.api, nil } @@ -96,16 +119,20 @@ func (c *listCommitsCommand) getAPI() (ListCommitsCommandAPI, error) { } // Run implements the meaty part of the cmd.Command interface. -func (c *listCommitsCommand) Run(ctx *cmd.Context) error { +func (c *CommitsCommand) Run(ctx *cmd.Context) error { client, err := c.getAPI() if err != nil { return err } defer func() { _ = client.Close() }() + // Partially apply our time format + formatTime := func(t time.Time) string { + return common.FormatTime(&t, c.isoTime) + } - _, err = client.ListCommits() + commits, err := client.ListCommits(formatTime) if err != nil { - return err + return errors.Trace(err) } - return err + return errors.Trace(c.out.Write(ctx, commits)) } diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go new file mode 100644 index 00000000000..5fe378dd068 --- /dev/null +++ b/cmd/juju/model/listcommits_test.go @@ -0,0 +1,84 @@ +// Copyright 2019 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package model_test + +import ( + "fmt" + "github.com/golang/mock/gomock" + "github.com/juju/cmd" + "github.com/juju/cmd/cmdtesting" + jc "github.com/juju/testing/checkers" + "github.com/pkg/errors" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/cmd/juju/model" + "github.com/juju/juju/cmd/juju/model/mocks" + coremodel "github.com/juju/juju/core/model" +) + +type commitsSuite struct { + generationBaseSuite + + api *mocks.MockListCommitsCommandAPI +} + +var _ = gc.Suite(&commitsSuite{}) + +func (s *commitsSuite) TestInitNoArg(c *gc.C) { + err := s.runInit() + c.Assert(err, jc.ErrorIsNil) +} + +func (s *commitsSuite) TestInitOneArg(c *gc.C) { + err := s.runInit(s.branchName) + c.Assert(err, gc.ErrorMatches, `expected no arguments, but got 1`) +} + +func (s *commitsSuite) TestRunCommandNextGenExists(c *gc.C) { + defer s.setup(c).Finish() + + result := []coremodel.GenerationCommit{ + { + Created: "0001-01-01 00:00:00Z", + CreatedBy: "test-user", + CommitNumber: 1, + BranchName: "bla", + }, + { + Created: "0001-02-02 00:00:00Z", + CreatedBy: "test-user", + CommitNumber: 2, + BranchName: "test", + }, + } + s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) + + ctx, err := s.runCommand(c) + c.Assert(err, jc.ErrorIsNil) + fmt.Println(cmdtesting.Stdout(ctx)) +} + +func (s *commitsSuite) TestRunCommandAPIError(c *gc.C) { + defer s.setup(c).Finish() + + s.api.EXPECT().ListCommits(gomock.Any()).Return(nil, errors.New("boom")) + + _, err := s.runCommand(c) + c.Assert(err, gc.ErrorMatches, "boom") +} + +func (s *commitsSuite) runInit(args ...string) error { + return cmdtesting.InitCommand(model.NewListCommitsCommandForTest(nil, s.store), args) +} + +func (s *commitsSuite) runCommand(c *gc.C) (*cmd.Context, error) { + return cmdtesting.RunCommand(c, model.NewListCommitsCommandForTest(s.api, s.store)) +} + +func (s *commitsSuite) setup(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.api = mocks.NewMockListCommitsCommandAPI(ctrl) + s.api.EXPECT().Close() + return ctrl +} diff --git a/cmd/juju/model/mocks/diff_mock.go b/cmd/juju/model/mocks/diff_mock.go index 670efcebaba..6cdfac2ddda 100644 --- a/cmd/juju/model/mocks/diff_mock.go +++ b/cmd/juju/model/mocks/diff_mock.go @@ -36,6 +36,7 @@ func (m *MockDiffCommandAPI) EXPECT() *MockDiffCommandAPIMockRecorder { // BranchInfo mocks base method func (m *MockDiffCommandAPI) BranchInfo(arg0 string, arg1 bool, arg2 func(time.Time) string) (map[string]model.Generation, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "BranchInfo", arg0, arg1, arg2) ret0, _ := ret[0].(map[string]model.Generation) ret1, _ := ret[1].(error) @@ -44,11 +45,13 @@ func (m *MockDiffCommandAPI) BranchInfo(arg0 string, arg1 bool, arg2 func(time.T // BranchInfo indicates an expected call of BranchInfo func (mr *MockDiffCommandAPIMockRecorder) BranchInfo(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BranchInfo", reflect.TypeOf((*MockDiffCommandAPI)(nil).BranchInfo), arg0, arg1, arg2) } // Close mocks base method func (m *MockDiffCommandAPI) Close() error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Close") ret0, _ := ret[0].(error) return ret0 @@ -56,5 +59,6 @@ func (m *MockDiffCommandAPI) Close() error { // Close indicates an expected call of Close func (mr *MockDiffCommandAPIMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockDiffCommandAPI)(nil).Close)) } diff --git a/core/model/generation.go b/core/model/generation.go index e0c6591d929..38a6fa5de17 100644 --- a/core/model/generation.go +++ b/core/model/generation.go @@ -70,6 +70,24 @@ type Generation struct { Applications []GenerationApplication `yaml:"applications"` } +// Generation represents detail of a model generation including config changes. +type GenerationCommit struct { + // Created is the formatted time at generation creation. + Created string `yaml:"created"` + + // Created is the user who created the generation. + CreatedBy string `yaml:"created-by"` + + BranchName string `yaml:"branch-name"` + + // CommitNumber is the generation-id + CommitNumber int `yaml:"generation-id"` +} + // GenerationSummaries is a type alias for a representation // of changes-by-generation. type GenerationSummaries = map[string]Generation + +// GenerationCommits is a type alias for a representation of each commit. +// Keyed by the generation id +type GenerationCommits = []GenerationCommit From e012d8047bd6b8594fabdeeebf319c76ece98834 Mon Sep 17 00:00:00 2001 From: nam Date: Wed, 6 Nov 2019 18:29:42 +0100 Subject: [PATCH 03/13] list command : add formatting options and tests --- api/modelgeneration/modelgeneration.go | 4 +- .../client/modelgeneration/modelgeneration.go | 17 ++-- cmd/juju/commands/main.go | 1 + cmd/juju/model/listcommits.go | 55 ++++++++++++- cmd/juju/model/listcommits_test.go | 82 ++++++++++++++++--- cmd/juju/model/mocks/commits_mock.go | 64 +++++++++++++++ 6 files changed, 202 insertions(+), 21 deletions(-) create mode 100644 cmd/juju/model/mocks/commits_mock.go diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index acef95a2f41..2c441640895 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -69,9 +69,7 @@ func (c *Client) CommitBranch(branchName string) (int, error) { return result.Result, nil } -// CommitBranch commits the branch with the input name to the model, -// effectively completing it and applying all branch changes across the model. -// The new generation ID of the model is returned. +// ListCommits lists the committed branches commits, func (c *Client) ListCommits(formatTime func(time.Time) string) (model.GenerationCommits, error) { var result params.GenerationResults err := c.facade.FacadeCall("ListCommits", nil, &result) diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index c40798443a8..c47b3b4e295 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -294,17 +294,22 @@ func (api *API) ListCommits() (params.GenerationResults, error) { if branches, err = api.model.Branches(); err != nil { return generationResultsError(err) } + logger.Errorf("branches found %q", branches) + //TODO: check whether branches usage is correct results := make([]params.Generation, len(branches)) for i, b := range branches { - generation, err := api.oneBranchInfo(b, false) - if err != nil { - return generationResultsError(err) - } - if generation.GenerationId > 0 { - results[i] = generation + if b.GenerationId() > 0 { + gen := params.Generation{ + BranchName: b.BranchName(), + Created: b.Created(), + CreatedBy: b.CreatedBy(), + GenerationId: b.GenerationId(), + } + results[i] = gen } } + logger.Errorf("results found %q", results) result.Generations = results return result, nil diff --git a/cmd/juju/commands/main.go b/cmd/juju/commands/main.go index 494d144ddd4..19016c2860f 100644 --- a/cmd/juju/commands/main.go +++ b/cmd/juju/commands/main.go @@ -372,6 +372,7 @@ func registerCommands(r commandRegistry, ctx *cmd.Context) { r.Register(model.NewBranchCommand()) r.Register(model.NewDiffCommand()) r.Register(model.NewAbortCommand()) + r.Register(model.NewCommitsCommand()) } r.Register(newMigrateCommand()) diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index ace52b1c320..43f9a7bdf7c 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -4,12 +4,15 @@ package model import ( + "fmt" + "github.com/gosuri/uitable" "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" "github.com/juju/juju/cmd/juju/common" "github.com/juju/juju/core/model" "github.com/juju/juju/juju/osenv" + "io" "os" "strconv" "time" @@ -82,6 +85,11 @@ func (c *CommitsCommand) Info() *cmd.Info { func (c *CommitsCommand) SetFlags(f *gnuflag.FlagSet) { c.ModelCommandBase.SetFlags(f) f.BoolVar(&c.isoTime, "utc", false, "Display time as UTC in RFC3339 format") + c.out.AddFlags(f, "tabular", map[string]cmd.Formatter{ + "yaml": cmd.FormatYaml, + "json": cmd.FormatJson, + "tabular": c.printTabular, + }) } // Init implements part of the cmd.Command interface. @@ -134,5 +142,50 @@ func (c *CommitsCommand) Run(ctx *cmd.Context) error { if err != nil { return errors.Trace(err) } - return errors.Trace(c.out.Write(ctx, commits)) + tabular := constructYaml(commits) + return errors.Trace(c.out.Write(ctx, tabular)) +} + +// printTabular prints the list of actions in tabular format +func (c *CommitsCommand) printTabular(writer io.Writer, value interface{}) error { + list, ok := value.(formattedCommitList) + if !ok { + return errors.New("unexpected value") + } + + table := uitable.New() + table.MaxColWidth = 50 + table.Wrap = true + + table.AddRow("Commit", "Committed at", "Committed by", "Branch name") + for _, c := range list.Commits { + table.AddRow(c.CommitId, c.CommittedAt, c.CommittedBy, c.BranchName) + } + _, _ = fmt.Fprint(writer, table) + return nil +} + +func constructYaml(gen model.GenerationCommits) formattedCommitList { + result := formattedCommitList{} + for _, gen := range gen { + fmc := formattedCommit{ + CommitId: gen.CommitNumber, + BranchName: gen.BranchName, + CommittedAt: gen.Created, + CommittedBy: gen.CreatedBy, + } + result.Commits = append(result.Commits, fmc) + } + return result +} + +type formattedCommit struct { + CommitId int `json:"id" yaml:"id"` + BranchName string `json:"branch-name" yaml:"branch-name"` + CommittedAt string `json:"committed-at" yaml:"committed-at"` + CommittedBy string `json:"committed-by" yaml:"committed-by"` +} + +type formattedCommitList struct { + Commits []formattedCommit `json:"commits" yaml:"commits"` } diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index 5fe378dd068..60b87a53367 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -11,6 +11,7 @@ import ( jc "github.com/juju/testing/checkers" "github.com/pkg/errors" gc "gopkg.in/check.v1" + "regexp" "github.com/juju/juju/cmd/juju/model" "github.com/juju/juju/cmd/juju/model/mocks" @@ -20,7 +21,7 @@ import ( type commitsSuite struct { generationBaseSuite - api *mocks.MockListCommitsCommandAPI + api *mocks.MockCommitsCommandAPI } var _ = gc.Suite(&commitsSuite{}) @@ -34,29 +35,88 @@ func (s *commitsSuite) TestInitOneArg(c *gc.C) { err := s.runInit(s.branchName) c.Assert(err, gc.ErrorMatches, `expected no arguments, but got 1`) } - -func (s *commitsSuite) TestRunCommandNextGenExists(c *gc.C) { - defer s.setup(c).Finish() - - result := []coremodel.GenerationCommit{ +func (s *commitsSuite) getMockValues() []coremodel.GenerationCommit { + values := []coremodel.GenerationCommit{ { - Created: "0001-01-01 00:00:00Z", + Created: "0001-01-01", CreatedBy: "test-user", CommitNumber: 1, BranchName: "bla", }, { - Created: "0001-02-02 00:00:00Z", + Created: "0001-02-02", CreatedBy: "test-user", CommitNumber: 2, BranchName: "test", }, } + return values +} + +func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { + defer s.setup(c).Finish() + result := s.getMockValues() + expected := + "Commit\tCommitted at \tCommitted by\tBranch name" + + "\n1 \t0001-01-01\ttest-user \tbla " + + "\n2 \t0001-02-02\ttest-user \ttest \n" s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) ctx, err := s.runCommand(c) c.Assert(err, jc.ErrorIsNil) + c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) +} + +func (s *commitsSuite) TestRunCommandJsonOutput(c *gc.C) { + defer s.setup(c).Finish() + result := s.getMockValues() + unwrap := regexp.MustCompile(`[\s+\n]`) + expected := unwrap.ReplaceAllLiteralString(` +{ + "commits": [ + { + "id": 1, + "branch-name": "bla", + "committed-at": "0001-01-01", + "committed-by": "test-user" + }, + { + "id": 2, + "branch-name": "test", + "committed-at": "0001-02-02", + "committed-by": "test-user" + } + ] +} +`, "") + expected = expected + "\n" + s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) + + ctx, err := s.runCommand(c, "--format=json") + c.Assert(err, jc.ErrorIsNil) + c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) +} + +func (s *commitsSuite) TestRunCommandYamlOutput(c *gc.C) { + defer s.setup(c).Finish() + result := s.getMockValues() + expected := ` +commits: +- id: 1 + branch-name: bla + committed-at: "0001-01-01" + committed-by: test-user +- id: 2 + branch-name: test + committed-at: "0001-02-02" + committed-by: test-user +`[1:] + s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) + + ctx, err := s.runCommand(c, "--format=yaml") + c.Assert(err, jc.ErrorIsNil) fmt.Println(cmdtesting.Stdout(ctx)) + c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) } func (s *commitsSuite) TestRunCommandAPIError(c *gc.C) { @@ -72,13 +132,13 @@ func (s *commitsSuite) runInit(args ...string) error { return cmdtesting.InitCommand(model.NewListCommitsCommandForTest(nil, s.store), args) } -func (s *commitsSuite) runCommand(c *gc.C) (*cmd.Context, error) { - return cmdtesting.RunCommand(c, model.NewListCommitsCommandForTest(s.api, s.store)) +func (s *commitsSuite) runCommand(c *gc.C, args ...string) (*cmd.Context, error) { + return cmdtesting.RunCommand(c, model.NewListCommitsCommandForTest(s.api, s.store), args...) } func (s *commitsSuite) setup(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.api = mocks.NewMockListCommitsCommandAPI(ctrl) + s.api = mocks.NewMockCommitsCommandAPI(ctrl) s.api.EXPECT().Close() return ctrl } diff --git a/cmd/juju/model/mocks/commits_mock.go b/cmd/juju/model/mocks/commits_mock.go new file mode 100644 index 00000000000..0f21de96460 --- /dev/null +++ b/cmd/juju/model/mocks/commits_mock.go @@ -0,0 +1,64 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/cmd/juju/model (interfaces: CommitsCommandAPI) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + gomock "github.com/golang/mock/gomock" + model "github.com/juju/juju/core/model" + reflect "reflect" + time "time" +) + +// MockCommitsCommandAPI is a mock of CommitsCommandAPI interface +type MockCommitsCommandAPI struct { + ctrl *gomock.Controller + recorder *MockCommitsCommandAPIMockRecorder +} + +// MockCommitsCommandAPIMockRecorder is the mock recorder for MockCommitsCommandAPI +type MockCommitsCommandAPIMockRecorder struct { + mock *MockCommitsCommandAPI +} + +// NewMockCommitsCommandAPI creates a new mock instance +func NewMockCommitsCommandAPI(ctrl *gomock.Controller) *MockCommitsCommandAPI { + mock := &MockCommitsCommandAPI{ctrl: ctrl} + mock.recorder = &MockCommitsCommandAPIMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockCommitsCommandAPI) EXPECT() *MockCommitsCommandAPIMockRecorder { + return m.recorder +} + +// Close mocks base method +func (m *MockCommitsCommandAPI) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close +func (mr *MockCommitsCommandAPIMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockCommitsCommandAPI)(nil).Close)) +} + +// ListCommits mocks base method +func (m *MockCommitsCommandAPI) ListCommits(arg0 func(time.Time) string) ([]model.GenerationCommit, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListCommits", arg0) + ret0, _ := ret[0].([]model.GenerationCommit) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListCommits indicates an expected call of ListCommits +func (mr *MockCommitsCommandAPIMockRecorder) ListCommits(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCommits", reflect.TypeOf((*MockCommitsCommandAPI)(nil).ListCommits), arg0) +} From 63656b12c9575de0153d93121c731947832a80a6 Mon Sep 17 00:00:00 2001 From: nam Date: Thu, 7 Nov 2019 12:12:48 +0100 Subject: [PATCH 04/13] list and show command : get branch by id, renaming structs --- api/modelgeneration/modelgeneration.go | 49 ++++++--- .../client/modelgeneration/interface.go | 4 + .../modelgeneration/mocks/package_mock.go | 84 +++++++++++++++ .../client/modelgeneration/modelgeneration.go | 96 ++++++++--------- .../facades/client/modelgeneration/shim.go | 22 ++++ apiserver/facades/schema.json | 100 ++++++++++++++++-- apiserver/params/params.go | 45 ++++++++ cmd/juju/model/listcommits.go | 10 +- cmd/juju/model/listcommits_test.go | 21 ++-- cmd/juju/model/mocks/showcommit_mock.go | 64 +++++++++++ cmd/juju/model/showcommit.go | 29 +++-- core/model/generation.go | 25 +++-- state/modelgeneration.go | 68 ++++++++++++ 13 files changed, 518 insertions(+), 99 deletions(-) create mode 100644 cmd/juju/model/mocks/showcommit_mock.go diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index 2c441640895..ea020c3cbe8 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -71,7 +71,7 @@ func (c *Client) CommitBranch(branchName string) (int, error) { // ListCommits lists the committed branches commits, func (c *Client) ListCommits(formatTime func(time.Time) string) (model.GenerationCommits, error) { - var result params.GenerationResults + var result params.GenerationCommitResults err := c.facade.FacadeCall("ListCommits", nil, &result) if err != nil { return nil, errors.Trace(err) @@ -79,22 +79,22 @@ func (c *Client) ListCommits(formatTime func(time.Time) string) (model.Generatio if result.Error != nil { return nil, errors.Trace(result.Error) } - return generationCommitsFromResult(result, formatTime), nil + return generationCommitsFromResults(result, formatTime), nil } // CommitBranch commits the branch with the input name to the model, // effectively completing it and applying all branch changes across the model. // The new generation ID of the model is returned. -func (c *Client) ShowCommit() (int, error) { - var result params.IntResult +func (c *Client) ShowCommit(formatTime func(time.Time) string) (model.GenerationCommit, error) { + var result params.GenerationCommitResult err := c.facade.FacadeCall("ShowCommit", nil, &result) if err != nil { - return 0, errors.Trace(err) + return model.GenerationCommit{}, errors.Trace(err) } if result.Error != nil { - return 0, errors.Trace(result.Error) + return model.GenerationCommit{}, errors.Trace(result.Error) } - return result.Result, nil + return generationCommitFromResult(result, formatTime), nil } // TrackBranch sets the input units and/or applications @@ -203,15 +203,38 @@ func generationInfoFromResult( return summaries } -func generationCommitsFromResult(results params.GenerationResults, formatTime func(time.Time) string) model.GenerationCommits { - commits := make(model.GenerationCommits, len(results.Generations)) - for i, gen := range results.Generations { +func generationCommitsFromResults(results params.GenerationCommitResults, formatTime func(time.Time) string) model.GenerationCommits { + commits := make(model.GenerationCommits, len(results.GenerationCommits)) + for i, gen := range results.GenerationCommits { commits[i] = model.GenerationCommit{ - CommitNumber: gen.GenerationId, - Created: formatTime(time.Unix(gen.Created, 0)), - CreatedBy: gen.CreatedBy, + GenerationId: gen.GenerationId, + Completed: formatTime(time.Unix(gen.Completed, 0)), + CompletedBy: gen.CompletedBy, BranchName: gen.BranchName, } } return commits } + +func generationCommitFromResult(result params.GenerationCommitResult, formatTime func(time.Time) string) model.GenerationCommit { + genCommit := result.GenerationCommit + appChanges := make([]model.GenerationApplication, len(genCommit.Applications)) + for i, a := range genCommit.Applications { + app := model.GenerationApplication{ + ApplicationName: a.ApplicationName, + UnitProgress: a.UnitProgress, + ConfigChanges: a.ConfigChanges, + } + appChanges[i] = app + } + modelCommit := model.GenerationCommit{ + BranchName: genCommit.BranchName, + Completed: formatTime(time.Unix(genCommit.Completed, 0)), + CompletedBy: genCommit.CompletedBy, + Created: formatTime(time.Unix(genCommit.Created, 0)), + CreatedBy: genCommit.CreatedBy, + GenerationId: genCommit.GenerationId, + Applications: appChanges, + } + return modelCommit +} diff --git a/apiserver/facades/client/modelgeneration/interface.go b/apiserver/facades/client/modelgeneration/interface.go index 5b042e27350..2bda20d6942 100644 --- a/apiserver/facades/client/modelgeneration/interface.go +++ b/apiserver/facades/client/modelgeneration/interface.go @@ -26,6 +26,8 @@ type Model interface { AddBranch(string, string) error Branch(string) (Generation, error) Branches() ([]Generation, error) + CommittedBranch(int) (Generation, error) + CommittedBranches() ([]Generation, error) } // ModelCache describes a cached model used by the model generation API. @@ -38,6 +40,8 @@ type Generation interface { BranchName() string Created() int64 CreatedBy() string + Completed() int64 + CompletedBy() string AssignAllUnits(string) error AssignUnits(string, int) error AssignUnit(string) error diff --git a/apiserver/facades/client/modelgeneration/mocks/package_mock.go b/apiserver/facades/client/modelgeneration/mocks/package_mock.go index 8b8d14517fe..34e35ff80b8 100644 --- a/apiserver/facades/client/modelgeneration/mocks/package_mock.go +++ b/apiserver/facades/client/modelgeneration/mocks/package_mock.go @@ -39,6 +39,7 @@ func (m *MockState) EXPECT() *MockStateMockRecorder { // Application mocks base method func (m *MockState) Application(arg0 string) (modelgeneration.Application, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Application", arg0) ret0, _ := ret[0].(modelgeneration.Application) ret1, _ := ret[1].(error) @@ -47,11 +48,13 @@ func (m *MockState) Application(arg0 string) (modelgeneration.Application, error // Application indicates an expected call of Application func (mr *MockStateMockRecorder) Application(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockState)(nil).Application), arg0) } // ControllerTag mocks base method func (m *MockState) ControllerTag() names_v3.ControllerTag { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ControllerTag") ret0, _ := ret[0].(names_v3.ControllerTag) return ret0 @@ -59,11 +62,13 @@ func (m *MockState) ControllerTag() names_v3.ControllerTag { // ControllerTag indicates an expected call of ControllerTag func (mr *MockStateMockRecorder) ControllerTag() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerTag", reflect.TypeOf((*MockState)(nil).ControllerTag)) } // Model mocks base method func (m *MockState) Model() (modelgeneration.Model, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Model") ret0, _ := ret[0].(modelgeneration.Model) ret1, _ := ret[1].(error) @@ -72,6 +77,7 @@ func (m *MockState) Model() (modelgeneration.Model, error) { // Model indicates an expected call of Model func (mr *MockStateMockRecorder) Model() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Model", reflect.TypeOf((*MockState)(nil).Model)) } @@ -100,6 +106,7 @@ func (m *MockModel) EXPECT() *MockModelMockRecorder { // AddBranch mocks base method func (m *MockModel) AddBranch(arg0, arg1 string) error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddBranch", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -107,11 +114,13 @@ func (m *MockModel) AddBranch(arg0, arg1 string) error { // AddBranch indicates an expected call of AddBranch func (mr *MockModelMockRecorder) AddBranch(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddBranch", reflect.TypeOf((*MockModel)(nil).AddBranch), arg0, arg1) } // Branch mocks base method func (m *MockModel) Branch(arg0 string) (modelgeneration.Generation, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Branch", arg0) ret0, _ := ret[0].(modelgeneration.Generation) ret1, _ := ret[1].(error) @@ -120,11 +129,13 @@ func (m *MockModel) Branch(arg0 string) (modelgeneration.Generation, error) { // Branch indicates an expected call of Branch func (mr *MockModelMockRecorder) Branch(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Branch", reflect.TypeOf((*MockModel)(nil).Branch), arg0) } // Branches mocks base method func (m *MockModel) Branches() ([]modelgeneration.Generation, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Branches") ret0, _ := ret[0].([]modelgeneration.Generation) ret1, _ := ret[1].(error) @@ -133,11 +144,43 @@ func (m *MockModel) Branches() ([]modelgeneration.Generation, error) { // Branches indicates an expected call of Branches func (mr *MockModelMockRecorder) Branches() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Branches", reflect.TypeOf((*MockModel)(nil).Branches)) } +// CommittedBranch mocks base method +func (m *MockModel) CommittedBranch(arg0 int) (modelgeneration.Generation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CommittedBranch", arg0) + ret0, _ := ret[0].(modelgeneration.Generation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CommittedBranch indicates an expected call of CommittedBranch +func (mr *MockModelMockRecorder) CommittedBranch(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CommittedBranch", reflect.TypeOf((*MockModel)(nil).CommittedBranch), arg0) +} + +// CommittedBranches mocks base method +func (m *MockModel) CommittedBranches() ([]modelgeneration.Generation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CommittedBranches") + ret0, _ := ret[0].([]modelgeneration.Generation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CommittedBranches indicates an expected call of CommittedBranches +func (mr *MockModelMockRecorder) CommittedBranches() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CommittedBranches", reflect.TypeOf((*MockModel)(nil).CommittedBranches)) +} + // ModelTag mocks base method func (m *MockModel) ModelTag() names_v3.ModelTag { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ModelTag") ret0, _ := ret[0].(names_v3.ModelTag) return ret0 @@ -145,6 +188,7 @@ func (m *MockModel) ModelTag() names_v3.ModelTag { // ModelTag indicates an expected call of ModelTag func (mr *MockModelMockRecorder) ModelTag() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModelTag", reflect.TypeOf((*MockModel)(nil).ModelTag)) } @@ -173,6 +217,7 @@ func (m *MockGeneration) EXPECT() *MockGenerationMockRecorder { // Abort mocks base method func (m *MockGeneration) Abort(arg0 string) error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Abort", arg0) ret0, _ := ret[0].(error) return ret0 @@ -180,11 +225,13 @@ func (m *MockGeneration) Abort(arg0 string) error { // Abort indicates an expected call of Abort func (mr *MockGenerationMockRecorder) Abort(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Abort", reflect.TypeOf((*MockGeneration)(nil).Abort), arg0) } // AssignAllUnits mocks base method func (m *MockGeneration) AssignAllUnits(arg0 string) error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AssignAllUnits", arg0) ret0, _ := ret[0].(error) return ret0 @@ -192,11 +239,13 @@ func (m *MockGeneration) AssignAllUnits(arg0 string) error { // AssignAllUnits indicates an expected call of AssignAllUnits func (mr *MockGenerationMockRecorder) AssignAllUnits(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignAllUnits", reflect.TypeOf((*MockGeneration)(nil).AssignAllUnits), arg0) } // AssignUnit mocks base method func (m *MockGeneration) AssignUnit(arg0 string) error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AssignUnit", arg0) ret0, _ := ret[0].(error) return ret0 @@ -204,11 +253,13 @@ func (m *MockGeneration) AssignUnit(arg0 string) error { // AssignUnit indicates an expected call of AssignUnit func (mr *MockGenerationMockRecorder) AssignUnit(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignUnit", reflect.TypeOf((*MockGeneration)(nil).AssignUnit), arg0) } // AssignUnits mocks base method func (m *MockGeneration) AssignUnits(arg0 string, arg1 int) error { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AssignUnits", arg0, arg1) ret0, _ := ret[0].(error) return ret0 @@ -216,11 +267,13 @@ func (m *MockGeneration) AssignUnits(arg0 string, arg1 int) error { // AssignUnits indicates an expected call of AssignUnits func (mr *MockGenerationMockRecorder) AssignUnits(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignUnits", reflect.TypeOf((*MockGeneration)(nil).AssignUnits), arg0, arg1) } // AssignedUnits mocks base method func (m *MockGeneration) AssignedUnits() map[string][]string { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AssignedUnits") ret0, _ := ret[0].(map[string][]string) return ret0 @@ -228,11 +281,13 @@ func (m *MockGeneration) AssignedUnits() map[string][]string { // AssignedUnits indicates an expected call of AssignedUnits func (mr *MockGenerationMockRecorder) AssignedUnits() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignedUnits", reflect.TypeOf((*MockGeneration)(nil).AssignedUnits)) } // BranchName mocks base method func (m *MockGeneration) BranchName() string { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "BranchName") ret0, _ := ret[0].(string) return ret0 @@ -240,11 +295,13 @@ func (m *MockGeneration) BranchName() string { // BranchName indicates an expected call of BranchName func (mr *MockGenerationMockRecorder) BranchName() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BranchName", reflect.TypeOf((*MockGeneration)(nil).BranchName)) } // Commit mocks base method func (m *MockGeneration) Commit(arg0 string) (int, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Commit", arg0) ret0, _ := ret[0].(int) ret1, _ := ret[1].(error) @@ -253,11 +310,13 @@ func (m *MockGeneration) Commit(arg0 string) (int, error) { // Commit indicates an expected call of Commit func (mr *MockGenerationMockRecorder) Commit(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Commit", reflect.TypeOf((*MockGeneration)(nil).Commit), arg0) } // Config mocks base method func (m *MockGeneration) Config() map[string]settings.ItemChanges { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Config") ret0, _ := ret[0].(map[string]settings.ItemChanges) return ret0 @@ -265,11 +324,13 @@ func (m *MockGeneration) Config() map[string]settings.ItemChanges { // Config indicates an expected call of Config func (mr *MockGenerationMockRecorder) Config() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Config", reflect.TypeOf((*MockGeneration)(nil).Config)) } // Created mocks base method func (m *MockGeneration) Created() int64 { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Created") ret0, _ := ret[0].(int64) return ret0 @@ -277,11 +338,13 @@ func (m *MockGeneration) Created() int64 { // Created indicates an expected call of Created func (mr *MockGenerationMockRecorder) Created() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Created", reflect.TypeOf((*MockGeneration)(nil).Created)) } // CreatedBy mocks base method func (m *MockGeneration) CreatedBy() string { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreatedBy") ret0, _ := ret[0].(string) return ret0 @@ -289,9 +352,24 @@ func (m *MockGeneration) CreatedBy() string { // CreatedBy indicates an expected call of CreatedBy func (mr *MockGenerationMockRecorder) CreatedBy() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreatedBy", reflect.TypeOf((*MockGeneration)(nil).CreatedBy)) } +// GenerationId mocks base method +func (m *MockGeneration) GenerationId() int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GenerationId") + ret0, _ := ret[0].(int) + return ret0 +} + +// GenerationId indicates an expected call of GenerationId +func (mr *MockGenerationMockRecorder) GenerationId() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerationId", reflect.TypeOf((*MockGeneration)(nil).GenerationId)) +} + // MockApplication is a mock of Application interface type MockApplication struct { ctrl *gomock.Controller @@ -317,6 +395,7 @@ func (m *MockApplication) EXPECT() *MockApplicationMockRecorder { // DefaultCharmConfig mocks base method func (m *MockApplication) DefaultCharmConfig() (charm_v6.Settings, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DefaultCharmConfig") ret0, _ := ret[0].(charm_v6.Settings) ret1, _ := ret[1].(error) @@ -325,11 +404,13 @@ func (m *MockApplication) DefaultCharmConfig() (charm_v6.Settings, error) { // DefaultCharmConfig indicates an expected call of DefaultCharmConfig func (mr *MockApplicationMockRecorder) DefaultCharmConfig() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DefaultCharmConfig", reflect.TypeOf((*MockApplication)(nil).DefaultCharmConfig)) } // UnitNames mocks base method func (m *MockApplication) UnitNames() ([]string, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UnitNames") ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) @@ -338,6 +419,7 @@ func (m *MockApplication) UnitNames() ([]string, error) { // UnitNames indicates an expected call of UnitNames func (mr *MockApplicationMockRecorder) UnitNames() *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnitNames", reflect.TypeOf((*MockApplication)(nil).UnitNames)) } @@ -366,6 +448,7 @@ func (m *MockModelCache) EXPECT() *MockModelCacheMockRecorder { // Branch mocks base method func (m *MockModelCache) Branch(arg0 string) (cache.Branch, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Branch", arg0) ret0, _ := ret[0].(cache.Branch) ret1, _ := ret[1].(error) @@ -374,5 +457,6 @@ func (m *MockModelCache) Branch(arg0 string) (cache.Branch, error) { // Branch indicates an expected call of Branch func (mr *MockModelCacheMockRecorder) Branch(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Branch", reflect.TypeOf((*MockModelCache)(nil).Branch), arg0) } diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index c47b3b4e295..31fe68ae77a 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -277,10 +277,10 @@ func (api *API) BranchInfo( return result, nil } -// ListCommits will return the commits, hence only branches with generation_id higher than 0 -// Keyed by the generation_id -func (api *API) ListCommits() (params.GenerationResults, error) { - result := params.GenerationResults{} +// ShowCommit will return details a commit given by its generationId +// An error is returned if no commit can be found corresponding to a branch. +func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { + result := params.GenerationCommitResult{} isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -289,36 +289,31 @@ func (api *API) ListCommits() (params.GenerationResults, error) { if !isModelAdmin && !api.isControllerAdmin { return result, common.ErrPerm } + if arg.GenerationId < 1 { + err := errors.Errorf("supplied generation id has to be higher than 0") + return generationCommitResultError(err) + } - var branches []Generation - if branches, err = api.model.Branches(); err != nil { - return generationResultsError(err) + branch, err := api.model.CommittedBranch(arg.GenerationId) + if err != nil { + result.Error = common.ServerError(err) + return result, nil } - logger.Errorf("branches found %q", branches) - //TODO: check whether branches usage is correct - results := make([]params.Generation, len(branches)) - for i, b := range branches { - if b.GenerationId() > 0 { - gen := params.Generation{ - BranchName: b.BranchName(), - Created: b.Created(), - CreatedBy: b.CreatedBy(), - GenerationId: b.GenerationId(), - } - results[i] = gen - } + //TODO: convert them + _, err = api.oneBranchInfo(branch, true) + if err != nil { + return generationCommitResultError(err) } - logger.Errorf("results found %q", results) - result.Generations = results + //result.GenerationCommit. = generation.Applications + return result, nil } -// ShowCommit will return details a commit given by its generationId -// An error is returned if no commit can be found corresponding to a branch. -func (api *API) ShowCommit(generationId int) (params.GenerationResult, error) { - result := params.GenerationResult{} +// ListCommits will return the commits, hence only branches with generation_id higher than 0 +func (api *API) ListCommits() (params.GenerationCommitResults, error) { + result := params.GenerationCommitResults{} isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -327,32 +322,26 @@ func (api *API) ShowCommit(generationId int) (params.GenerationResult, error) { if !isModelAdmin && !api.isControllerAdmin { return result, common.ErrPerm } - if generationId < 1 { - err := errors.Errorf("supplied generation id has to be higher than 0") - return generationResultError(err) - } - branches, err := api.model.Branches() - if err != nil { - return generationResultError(err) + var branches []Generation + if branches, err = api.model.CommittedBranches(); err != nil { + return generationCommitResultsError(err) } + logger.Errorf("branches found %q", branches) - found := false - for _, b := range branches { - // TODO: needs to be of type GenerationApplication - generation, err := api.oneBranchInfo(b, false) - if err != nil { - return generationResultError(err) - } - if generation.GenerationId > 0 && generation.GenerationId == generationId { - result.Generation = generation - found = true + results := make([]params.GenerationCommit, len(branches)) + for i, b := range branches { + gen := params.GenerationCommit{ + BranchName: b.BranchName(), + Completed: b.Completed(), + CompletedBy: b.CompletedBy(), + GenerationId: b.GenerationId(), } + results[i] = gen } - if !found { - err := errors.Errorf("did not find the given generationid %q", generationId) - return generationResultError(err) - } + logger.Errorf("results found %q", results) + + result.GenerationCommits = results return result, nil } @@ -400,7 +389,6 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati BranchName: branch.BranchName(), Created: branch.Created(), CreatedBy: branch.CreatedBy(), - GenerationId: branch.GenerationId(), Applications: apps, }, nil } @@ -429,12 +417,20 @@ func (api *API) HasActiveBranch(arg params.BranchArg) (params.BoolResult, error) return result, nil } +func convertGenerationsToGenerationsCommit(generation Generation) params.GenerationCommit { + return params.GenerationCommit{} +} + func generationResultsError(err error) (params.GenerationResults, error) { return params.GenerationResults{Error: common.ServerError(err)}, nil } -func generationResultError(err error) (params.GenerationResult, error) { - return params.GenerationResult{Error: common.ServerError(err)}, nil +func generationCommitResultsError(err error) (params.GenerationCommitResults, error) { + return params.GenerationCommitResults{Error: common.ServerError(err)}, nil +} + +func generationCommitResultError(err error) (params.GenerationCommitResult, error) { + return params.GenerationCommitResult{Error: common.ServerError(err)}, nil } func intResultsError(err error) (params.IntResult, error) { diff --git a/apiserver/facades/client/modelgeneration/shim.go b/apiserver/facades/client/modelgeneration/shim.go index 4edd02e8722..83e48caebbd 100644 --- a/apiserver/facades/client/modelgeneration/shim.go +++ b/apiserver/facades/client/modelgeneration/shim.go @@ -37,6 +37,28 @@ func (g *modelShim) Branches() ([]Generation, error) { return res, nil } +// CommittedBranch wraps the state model CommittedBranch method, +// returning the locally defined Generation interface. +func (g *modelShim) CommittedBranch(id int) (Generation, error) { + m, err := g.Model.CommittedBranch(id) + return m, errors.Trace(err) +} + +// Branches wraps the state model CommittedBranches method, +// returning a collection of the Generation interface. +func (g *modelShim) CommittedBranches() ([]Generation, error) { + branches, err := g.Model.CommittedBranches() + if err != nil { + return nil, errors.Trace(err) + } + + res := make([]Generation, len(branches)) + for i, b := range branches { + res[i] = b + } + return res, nil +} + type applicationShim struct { *state.Application } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index d1ccd8d12cc..c37cce5d8c8 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23100,7 +23100,18 @@ "type": "object", "properties": { "Result": { - "$ref": "#/definitions/GenerationResults" + "$ref": "#/definitions/GenerationCommitResults" + } + } + }, + "ShowCommit": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GenerationId" + }, + "Result": { + "$ref": "#/definitions/GenerationCommitResult" } } }, @@ -23263,17 +23274,13 @@ }, "created-by": { "type": "string" - }, - "generation-id": { - "type": "integer" } }, "additionalProperties": false, "required": [ "branch", "created", - "created-by", - "generation-id" + "created-by" ] }, "GenerationApplication": { @@ -23314,6 +23321,87 @@ "config" ] }, + "GenerationCommit": { + "type": "object", + "properties": { + "applications": { + "type": "array", + "items": { + "$ref": "#/definitions/GenerationApplication" + } + }, + "branch": { + "type": "string" + }, + "completed": { + "type": "integer" + }, + "completed-by": { + "type": "string" + }, + "created": { + "type": "integer" + }, + "created-by": { + "type": "string" + }, + "generation-id": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "branch", + "completed", + "completed-by", + "generation-id" + ] + }, + "GenerationCommitResult": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "generation-commit": { + "$ref": "#/definitions/GenerationCommit" + } + }, + "additionalProperties": false, + "required": [ + "generation-commit" + ] + }, + "GenerationCommitResults": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "generation-commit": { + "type": "array", + "items": { + "$ref": "#/definitions/GenerationCommit" + } + } + }, + "additionalProperties": false, + "required": [ + "generation-commit" + ] + }, + "GenerationId": { + "type": "object", + "properties": { + "generation-id": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "generation-id" + ] + }, "GenerationResults": { "type": "object", "properties": { diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 27075d3a2d3..19c0abbabcd 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -1182,6 +1182,11 @@ type BranchArg struct { BranchName string `json:"branch"` } +// GenerationId represents an in-flight branch via its model and branch name. +type GenerationId struct { + GenerationId int `json:"generation-id"` +} + // BranchInfoArgs transports arguments to the BranchInfo method type BranchInfoArgs struct { // BranchNames is the names of branches for which info is being requested. @@ -1233,9 +1238,31 @@ type Generation struct { // Created is the user who created the generation. CreatedBy string `json:"created-by"` + // Applications holds the collection of application changes + // made under this generation. + Applications []GenerationApplication `json:"applications,omitempty"` +} + +// GenerationCommits represents a model generation's commit details. +type GenerationCommit struct { + // BranchName uniquely identifies a branch *amongst in-flight branches*. + BranchName string `json:"branch"` + + // Created is the Unix timestamp at generation creation. + Completed int64 `json:"completed"` + + // Created is the user who created the generation. + CompletedBy string `json:"completed-by"` + // GenerationId is the id . GenerationId int `json:"generation-id"` + // Created is the Unix timestamp at generation creation. + Created int64 `json:"created,omitempty"` + + // Created is the user who created the generation. + CreatedBy string `json:"created-by,omitempty"` + // Applications holds the collection of application changes // made under this generation. Applications []GenerationApplication `json:"applications,omitempty"` @@ -1259,6 +1286,24 @@ type GenerationResult struct { Error *Error `json:"error,omitempty"` } +// GenerationResult transports a generation detail. +type GenerationCommitResults struct { + // Generation holds the details of the requested generation. + GenerationCommits []GenerationCommit `json:"generation-commit"` + + // Error holds the value of any error that occurred processing the request. + Error *Error `json:"error,omitempty"` +} + +// GenerationResult transports a generation detail. +type GenerationCommitResult struct { + // Generation holds the details of the requested generation. + GenerationCommit GenerationCommit `json:"generation-commit"` + + // Error holds the value of any error that occurred processing the request. + Error *Error `json:"error,omitempty"` +} + // CharmProfilingInfoResult contains the result based on ProfileInfoArg values // to update profiles on a machine. type CharmProfilingInfoResult struct { diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index 43f9a7bdf7c..f659bf6a5a8 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -142,7 +142,7 @@ func (c *CommitsCommand) Run(ctx *cmd.Context) error { if err != nil { return errors.Trace(err) } - tabular := constructYaml(commits) + tabular := constructYamlJson(commits) return errors.Trace(c.out.Write(ctx, tabular)) } @@ -165,14 +165,14 @@ func (c *CommitsCommand) printTabular(writer io.Writer, value interface{}) error return nil } -func constructYaml(gen model.GenerationCommits) formattedCommitList { +func constructYamlJson(gen model.GenerationCommits) formattedCommitList { result := formattedCommitList{} for _, gen := range gen { fmc := formattedCommit{ - CommitId: gen.CommitNumber, + CommitId: gen.GenerationId, BranchName: gen.BranchName, - CommittedAt: gen.Created, - CommittedBy: gen.CreatedBy, + CommittedAt: gen.Completed, + CommittedBy: gen.CompletedBy, } result.Commits = append(result.Commits, fmc) } diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index 60b87a53367..ccd764df7ec 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -38,15 +38,15 @@ func (s *commitsSuite) TestInitOneArg(c *gc.C) { func (s *commitsSuite) getMockValues() []coremodel.GenerationCommit { values := []coremodel.GenerationCommit{ { - Created: "0001-01-01", - CreatedBy: "test-user", - CommitNumber: 1, + Completed: "0001-01-01", + CompletedBy: "test-user", + GenerationId: 1, BranchName: "bla", }, { - Created: "0001-02-02", - CreatedBy: "test-user", - CommitNumber: 2, + Completed: "0001-02-02", + CompletedBy: "test-user", + GenerationId: 2, BranchName: "test", }, } @@ -57,9 +57,10 @@ func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { defer s.setup(c).Finish() result := s.getMockValues() expected := - "Commit\tCommitted at \tCommitted by\tBranch name" + - "\n1 \t0001-01-01\ttest-user \tbla " + - "\n2 \t0001-02-02\ttest-user \ttest \n" + `Commit Committed at Committed by Branch name +1 0001-01-01 test-user bla +2 0001-02-02 test-user test +` s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) ctx, err := s.runCommand(c) @@ -94,7 +95,7 @@ func (s *commitsSuite) TestRunCommandJsonOutput(c *gc.C) { ctx, err := s.runCommand(c, "--format=json") c.Assert(err, jc.ErrorIsNil) - c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) + c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) } func (s *commitsSuite) TestRunCommandYamlOutput(c *gc.C) { diff --git a/cmd/juju/model/mocks/showcommit_mock.go b/cmd/juju/model/mocks/showcommit_mock.go new file mode 100644 index 00000000000..063dce67828 --- /dev/null +++ b/cmd/juju/model/mocks/showcommit_mock.go @@ -0,0 +1,64 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/cmd/juju/model (interfaces: ShowCommitCommandAPI) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + gomock "github.com/golang/mock/gomock" + model "github.com/juju/juju/core/model" + reflect "reflect" + time "time" +) + +// MockShowCommitCommandAPI is a mock of ShowCommitCommandAPI interface +type MockShowCommitCommandAPI struct { + ctrl *gomock.Controller + recorder *MockShowCommitCommandAPIMockRecorder +} + +// MockShowCommitCommandAPIMockRecorder is the mock recorder for MockShowCommitCommandAPI +type MockShowCommitCommandAPIMockRecorder struct { + mock *MockShowCommitCommandAPI +} + +// NewMockShowCommitCommandAPI creates a new mock instance +func NewMockShowCommitCommandAPI(ctrl *gomock.Controller) *MockShowCommitCommandAPI { + mock := &MockShowCommitCommandAPI{ctrl: ctrl} + mock.recorder = &MockShowCommitCommandAPIMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockShowCommitCommandAPI) EXPECT() *MockShowCommitCommandAPIMockRecorder { + return m.recorder +} + +// Close mocks base method +func (m *MockShowCommitCommandAPI) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close +func (mr *MockShowCommitCommandAPIMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).Close)) +} + +// ShowCommit mocks base method +func (m *MockShowCommitCommandAPI) ShowCommit(arg0 func(time.Time) string) (model.GenerationCommit, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ShowCommit", arg0) + ret0, _ := ret[0].(model.GenerationCommit) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ShowCommit indicates an expected call of ShowCommit +func (mr *MockShowCommitCommandAPIMockRecorder) ShowCommit(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShowCommit", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).ShowCommit), arg0) +} diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index 8a4f76e8280..e5db54cc35f 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -7,6 +7,9 @@ import ( "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" + "github.com/juju/juju/cmd/juju/common" + "github.com/juju/juju/core/model" + "time" "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" @@ -32,7 +35,6 @@ See also: ) //TODO: instead of diffing, i just show the content of config -//gen-id is unique corresponds to commit it to show // NewCommitCommand wraps listCommitsCommand with sane model settings. func NewShowCommitCommand() cmd.Command { @@ -44,10 +46,13 @@ type ShowCommitCommand struct { modelcmd.ModelCommandBase api ShowCommitCommandAPI + out cmd.Output + + isoTime bool } // ShowCommitCommandAPI defines an API interface to be used during testing. -//go:generate mockgen -package mocks -destination ./mocks/commit_mock.go github.com/juju/juju/cmd/juju/model ShowCommitCommandAPI +//go:generate mockgen -package mocks -destination ./mocks/showcommit_mock.go github.com/juju/juju/cmd/juju/model ShowCommitCommandAPI type ShowCommitCommandAPI interface { Close() error @@ -55,15 +60,15 @@ type ShowCommitCommandAPI interface { // effectively completing it and applying // all branch changes across the model. // The new generation ID of the model is returned. - ShowCommit() (int, error) + ShowCommit(func(time.Time) string) (model.GenerationCommit, error) } // Info implements part of the cmd.Command interface. func (c *ShowCommitCommand) Info() *cmd.Info { info := &cmd.Info{ - Name: "list-commits", - Purpose: listCommitsSummary, - Doc: listCommitsDoc, + Name: "show-commit", + Purpose: showCommitsDoc, + Doc: showCommitsSummary, } return jujucmd.Info(info) } @@ -71,6 +76,11 @@ func (c *ShowCommitCommand) Info() *cmd.Info { // SetFlags implements part of the cmd.Command interface. func (c *ShowCommitCommand) SetFlags(f *gnuflag.FlagSet) { c.ModelCommandBase.SetFlags(f) + f.BoolVar(&c.isoTime, "utc", false, "Display time as UTC in RFC3339 format") + c.out.AddFlags(f, "yaml", map[string]cmd.Formatter{ + "yaml": cmd.FormatYaml, + "json": cmd.FormatJson, + }) } // Init implements part of the cmd.Command interface. @@ -100,9 +110,12 @@ func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { } defer func() { _ = client.Close() }() - _, err = client.ShowCommit() + formatTime := func(t time.Time) string { + return common.FormatTime(&t, c.isoTime) + } + cmt, err := client.ShowCommit(formatTime) if err != nil { return err } - return err + return c.out.Write(ctx, cmt) } diff --git a/core/model/generation.go b/core/model/generation.go index 38a6fa5de17..75fff28e502 100644 --- a/core/model/generation.go +++ b/core/model/generation.go @@ -70,18 +70,29 @@ type Generation struct { Applications []GenerationApplication `yaml:"applications"` } -// Generation represents detail of a model generation including config changes. +// GenerationCommits represents a model generation's commit details. type GenerationCommit struct { - // Created is the formatted time at generation creation. - Created string `yaml:"created"` + // BranchName uniquely identifies a branch *amongst in-flight branches*. + BranchName string `json:"branch"` + + // Created is the Unix timestamp at generation creation. + Completed string `json:"completed"` // Created is the user who created the generation. - CreatedBy string `yaml:"created-by"` + CompletedBy string `json:"completed-by"` + + // Created is the Unix timestamp at generation creation. + Created string `json:"created,omitempty"` + + // Created is the user who created the generation. + CreatedBy string `json:"created-by,omitempty"` - BranchName string `yaml:"branch-name"` + // GenerationId is the id . + GenerationId int `json:"generation-id,omitempty"` - // CommitNumber is the generation-id - CommitNumber int `yaml:"generation-id"` + // Applications holds the collection of application changes + // made under this generation. + Applications []GenerationApplication `json:"applications,omitempty"` } // GenerationSummaries is a type alias for a representation diff --git a/state/modelgeneration.go b/state/modelgeneration.go index f3b890965c4..79a5cf290c3 100644 --- a/state/modelgeneration.go +++ b/state/modelgeneration.go @@ -144,6 +144,11 @@ func (g *Generation) IsCompleted() bool { return g.doc.Completed > 0 } +// Completed returns the Unix timestamp at generation completion. +func (g *Generation) Completed() int64 { + return g.doc.Completed +} + // CompletedBy returns the user who committed the generation. func (g *Generation) CompletedBy() string { return g.doc.CompletedBy @@ -687,6 +692,12 @@ func insertGenerationTxnOps(id, branchName, userName string, now *time.Time) []t } } +// CommittedBranches returns all committed branches. +func (m *Model) CommittedBranches() ([]*Generation, error) { + b, err := m.st.CommittedBranches() + return b, errors.Trace(err) +} + // Branches returns all "in-flight" branches for the model. func (m *Model) Branches() ([]*Generation, error) { b, err := m.st.Branches() @@ -703,6 +714,25 @@ func (st *State) Branches() ([]*Generation, error) { return nil, errors.Trace(err) } + branches := make([]*Generation, len(docs)) + for i, d := range docs { + branches[i] = newGeneration(st, &d) + } + return branches, nil + +} + +// CommittedBranches returns all committed branches. +func (st *State) CommittedBranches() ([]*Generation, error) { + col, closer := st.db().GetCollection(generationsC) + defer closer() + + var docs []generationDoc + query := bson.M{"completed": bson.M{"$gte": 1}} + if err := col.Find(query).All(&docs); err != nil { + return nil, errors.Trace(err) + } + branches := make([]*Generation, len(docs)) for i, d := range docs { branches[i] = newGeneration(st, &d) @@ -717,6 +747,13 @@ func (m *Model) Branch(name string) (*Generation, error) { return gen, errors.Trace(err) } +// CommittedBranch retrieves the generation with the the input generation_id from the +// collection of completed generations. +func (m *Model) CommittedBranch(id int) (*Generation, error) { + gen, err := m.st.CommittedBranch(id) + return gen, errors.Trace(err) +} + func (m *Model) applicationBranches(appName string) ([]*Generation, error) { branches, err := m.Branches() if err != nil { @@ -745,6 +782,16 @@ func (st *State) Branch(name string) (*Generation, error) { return newGeneration(st, doc), nil } +// CommittedBranch retrieves the generation with the the input id from the +// collection of completed generations. +func (st *State) CommittedBranch(id int) (*Generation, error) { + doc, err := st.getCommittedBranchDoc(id) + if err != nil { + return nil, errors.Trace(err) + } + return newGeneration(st, doc), nil +} + func (st *State) getBranchDoc(name string) (*generationDoc, error) { col, closer := st.db().GetCollection(generationsC) defer closer() @@ -767,6 +814,27 @@ func (st *State) getBranchDoc(name string) (*generationDoc, error) { } } +func (st *State) getCommittedBranchDoc(id int) (*generationDoc, error) { + col, closer := st.db().GetCollection(generationsC) + defer closer() + + doc := &generationDoc{} + err := col.Find(bson.M{ + "generation-id": id, + }).One(doc) + + switch err { + case nil: + return doc, nil + case mgo.ErrNotFound: + mod, _ := st.modelName() + return nil, errors.NotFoundf("generation_id %q in model %q", id, mod) + default: + mod, _ := st.modelName() + return nil, errors.Annotatef(err, "retrieving generation_id %q in model %q", id, mod) + } +} + func (m *Model) unitBranch(unitName string) (*Generation, error) { // NOTE (hml) 2019-07-02 // Currently a unit may only be tracked in a single generation. From c02427f5c7d61d941ba2817561538c92f9665160 Mon Sep 17 00:00:00 2001 From: nam Date: Thu, 7 Nov 2019 15:54:09 +0100 Subject: [PATCH 05/13] bump facade version and rename command descriptions --- api/facadeversions.go | 2 +- apiserver/allfacades.go | 1 + .../client/modelgeneration/modelgeneration.go | 26 +++++--- apiserver/facades/schema.json | 2 +- apiserver/params/params.go | 2 +- cmd/juju/model/listcommits.go | 23 ++++--- cmd/juju/model/showcommit.go | 61 ++++++++++++++----- 7 files changed, 83 insertions(+), 34 deletions(-) diff --git a/api/facadeversions.go b/api/facadeversions.go index 2d080fce9c3..d4d53c9ca6c 100644 --- a/api/facadeversions.go +++ b/api/facadeversions.go @@ -75,7 +75,7 @@ var facadeVersions = map[string]int{ "MigrationStatusWatcher": 1, "MigrationTarget": 1, "ModelConfig": 2, - "ModelGeneration": 3, + "ModelGeneration": 4, "ModelManager": 8, "ModelUpgrader": 1, "NotifyWatcher": 1, diff --git a/apiserver/allfacades.go b/apiserver/allfacades.go index 088cfb89b07..4da1cbb1aaa 100644 --- a/apiserver/allfacades.go +++ b/apiserver/allfacades.go @@ -252,6 +252,7 @@ func AllFacades() *facade.Registry { reg("ModelGeneration", 1, modelgeneration.NewModelGenerationFacade) reg("ModelGeneration", 2, modelgeneration.NewModelGenerationFacadeV2) reg("ModelGeneration", 3, modelgeneration.NewModelGenerationFacadeV3) + reg("ModelGeneration", 4, modelgeneration.NewModelGenerationFacadeV4) reg("ModelManager", 2, modelmanager.NewFacadeV2) reg("ModelManager", 3, modelmanager.NewFacadeV3) reg("ModelManager", 4, modelmanager.NewFacadeV4) diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index 31fe68ae77a..72d5dc5a8d9 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -31,16 +31,20 @@ type API struct { modelCache ModelCache } -type APIV2 struct { +type APIV3 struct { *API } +type APIV2 struct { + *APIV3 +} + type APIV1 struct { *APIV2 } -// NewModelGenerationFacadeV3 provides the signature required for facade registration. -func NewModelGenerationFacadeV3(ctx facade.Context) (*API, error) { +// NewModelGenerationFacadeV4 provides the signature required for facade registration. +func NewModelGenerationFacadeV4(ctx facade.Context) (*API, error) { authorizer := ctx.Auth() st := &stateShim{State: ctx.State()} m, err := st.Model() @@ -54,7 +58,15 @@ func NewModelGenerationFacadeV3(ctx facade.Context) (*API, error) { return NewModelGenerationAPI(st, authorizer, m, &modelCacheShim{Model: mc}) } -// NewModelGenerationFacadeV2 provides the signature required for facade registration. +// NewModelGenerationFacadeV3 provides the signature required for facade registration. +func NewModelGenerationFacadeV3(ctx facade.Context) (*APIV3, error) { + v4, err := NewModelGenerationFacadeV4(ctx) + if err != nil { + return nil, err + } + return &APIV3{v4}, nil + +} // NewModelGenerationFacadeV2 provides the signature required for facade registration. func NewModelGenerationFacadeV2(ctx facade.Context) (*APIV2, error) { v3, err := NewModelGenerationFacadeV3(ctx) if err != nil { @@ -279,7 +291,7 @@ func (api *API) BranchInfo( // ShowCommit will return details a commit given by its generationId // An error is returned if no commit can be found corresponding to a branch. -func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { +func (api *APIV3) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { result := params.GenerationCommitResult{} isModelAdmin, err := api.hasAdminAccess() @@ -312,7 +324,7 @@ func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResu } // ListCommits will return the commits, hence only branches with generation_id higher than 0 -func (api *API) ListCommits() (params.GenerationCommitResults, error) { +func (api *APIV3) ListCommits() (params.GenerationCommitResults, error) { result := params.GenerationCommitResults{} isModelAdmin, err := api.hasAdminAccess() @@ -327,7 +339,6 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) { if branches, err = api.model.CommittedBranches(); err != nil { return generationCommitResultsError(err) } - logger.Errorf("branches found %q", branches) results := make([]params.GenerationCommit, len(branches)) for i, b := range branches { @@ -339,7 +350,6 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) { } results[i] = gen } - logger.Errorf("results found %q", results) result.GenerationCommits = results return result, nil diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index c37cce5d8c8..02ede8ce804 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23037,7 +23037,7 @@ }, { "Name": "ModelGeneration", - "Version": 3, + "Version": 4, "Schema": { "type": "object", "properties": { diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 19c0abbabcd..03e0380ca9f 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -1240,7 +1240,7 @@ type Generation struct { // Applications holds the collection of application changes // made under this generation. - Applications []GenerationApplication `json:"applications,omitempty"` + Applications []GenerationApplication `json:"applications"` } // GenerationCommits represents a model generation's commit details. diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index f659bf6a5a8..d49dbf1f772 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -5,31 +5,38 @@ package model import ( "fmt" - "github.com/gosuri/uitable" - "github.com/juju/cmd" - "github.com/juju/errors" - "github.com/juju/gnuflag" - "github.com/juju/juju/cmd/juju/common" - "github.com/juju/juju/core/model" - "github.com/juju/juju/juju/osenv" "io" "os" "strconv" "time" + "github.com/gosuri/uitable" + "github.com/juju/cmd" + "github.com/juju/errors" + "github.com/juju/gnuflag" + "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/juju/common" "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/core/model" + "github.com/juju/juju/juju/osenv" ) const ( listCommitsSummary = "Lists commits history" listCommitsDoc = ` -commits shows the timeline of changes to the model that occurred through branching. +Commits shows the timeline of changes to the model that occurred through branching. It does not take into account other changes to the model that did not occur through a managed branch. +Lists consists of: +- the commit number +- user who committed the branch +- when the branch was committed +- the branch name Examples: juju commits + juju commits --utc See also: commits diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index e5db54cc35f..bcb0037c317 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -4,25 +4,38 @@ package model import ( + "os" + "strconv" + "time" + "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" - "github.com/juju/juju/cmd/juju/common" - "github.com/juju/juju/core/model" - "time" "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" + "github.com/juju/juju/cmd/juju/common" "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/cmd/output" + "github.com/juju/juju/core/model" + "github.com/juju/juju/juju/osenv" ) const ( - showCommitsSummary = "Shows the commit content" + showCommitsSummary = "Displays details of the commit" showCommitsDoc = ` -show-commit shows the effective change from a branch to the master. +Show-commit shows the committed branches to the model. +Details displayed include: +- user who committed the branch +- when the branch was committed +- user who created the branch +- when the branch was created +- configuration made under the branch for each application +- a summary of how many units are tracking the branch Examples: juju show-commits 3 + juju show-commits 3 --utc See also: list-commits @@ -36,11 +49,6 @@ See also: //TODO: instead of diffing, i just show the content of config -// NewCommitCommand wraps listCommitsCommand with sane model settings. -func NewShowCommitCommand() cmd.Command { - return modelcmd.Wrap(&ShowCommitCommand{}) -} - // ShowCommitCommand supplies the "show-commit" CLI command used to show commits type ShowCommitCommand struct { modelcmd.ModelCommandBase @@ -48,7 +56,8 @@ type ShowCommitCommand struct { api ShowCommitCommandAPI out cmd.Output - isoTime bool + generationId int + isoTime bool } // ShowCommitCommandAPI defines an API interface to be used during testing. @@ -63,6 +72,11 @@ type ShowCommitCommandAPI interface { ShowCommit(func(time.Time) string) (model.GenerationCommit, error) } +// NewCommitCommand wraps listCommitsCommand with sane model settings. +func NewShowCommitCommand() cmd.Command { + return modelcmd.Wrap(&ShowCommitCommand{}) +} + // Info implements part of the cmd.Command interface. func (c *ShowCommitCommand) Info() *cmd.Info { info := &cmd.Info{ @@ -77,14 +91,31 @@ func (c *ShowCommitCommand) Info() *cmd.Info { func (c *ShowCommitCommand) SetFlags(f *gnuflag.FlagSet) { c.ModelCommandBase.SetFlags(f) f.BoolVar(&c.isoTime, "utc", false, "Display time as UTC in RFC3339 format") - c.out.AddFlags(f, "yaml", map[string]cmd.Formatter{ - "yaml": cmd.FormatYaml, - "json": cmd.FormatJson, - }) + c.out.AddFlags(f, "yaml", output.DefaultFormatters) } // Init implements part of the cmd.Command interface. func (c *ShowCommitCommand) Init(args []string) error { + lArgs := len(args) + if lArgs != 1 { + return errors.Errorf("expected exactly 1 commit id, got %d arguments", lArgs) + } + id, err := strconv.Atoi(args[0]) + if err != nil { + return errors.Errorf("encountered problem trying to parse %q into an int", args[0]) + } + c.generationId = id + + // If use of ISO time not specified on command line, check env var. + if !c.isoTime { + var err error + envVarValue := os.Getenv(osenv.JujuStatusIsoTimeEnvKey) + if envVarValue != "" { + if c.isoTime, err = strconv.ParseBool(envVarValue); err != nil { + return errors.Annotatef(err, "invalid %s env var, expected true|false", osenv.JujuStatusIsoTimeEnvKey) + } + } + } return nil } From 4ebeca53941ef83836529b904f48bef5fac2fc98 Mon Sep 17 00:00:00 2001 From: nam Date: Thu, 7 Nov 2019 17:06:09 +0100 Subject: [PATCH 06/13] showcommit command: add showcommit command and it's tests --- api/modelgeneration/modelgeneration.go | 5 +- .../modelgeneration/mocks/package_mock.go | 28 +++ .../client/modelgeneration/modelgeneration.go | 29 +++- apiserver/facades/schema.json | 103 +---------- cmd/juju/commands/main.go | 1 + cmd/juju/model/export_test.go | 8 + cmd/juju/model/showcommit.go | 50 ++++-- cmd/juju/model/showcommit_test.go | 162 ++++++++++++++++++ 8 files changed, 261 insertions(+), 125 deletions(-) create mode 100644 cmd/juju/model/showcommit_test.go diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index ea020c3cbe8..864989649a9 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -85,9 +85,10 @@ func (c *Client) ListCommits(formatTime func(time.Time) string) (model.Generatio // CommitBranch commits the branch with the input name to the model, // effectively completing it and applying all branch changes across the model. // The new generation ID of the model is returned. -func (c *Client) ShowCommit(formatTime func(time.Time) string) (model.GenerationCommit, error) { +func (c *Client) ShowCommit(formatTime func(time.Time) string, generationId int) (model.GenerationCommit, error) { var result params.GenerationCommitResult - err := c.facade.FacadeCall("ShowCommit", nil, &result) + arg := params.GenerationId{GenerationId: generationId} + err := c.facade.FacadeCall("ShowCommit", arg, &result) if err != nil { return model.GenerationCommit{}, errors.Trace(err) } diff --git a/apiserver/facades/client/modelgeneration/mocks/package_mock.go b/apiserver/facades/client/modelgeneration/mocks/package_mock.go index 34e35ff80b8..83f285248bf 100644 --- a/apiserver/facades/client/modelgeneration/mocks/package_mock.go +++ b/apiserver/facades/client/modelgeneration/mocks/package_mock.go @@ -314,6 +314,34 @@ func (mr *MockGenerationMockRecorder) Commit(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Commit", reflect.TypeOf((*MockGeneration)(nil).Commit), arg0) } +// Completed mocks base method +func (m *MockGeneration) Completed() int64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Completed") + ret0, _ := ret[0].(int64) + return ret0 +} + +// Completed indicates an expected call of Completed +func (mr *MockGenerationMockRecorder) Completed() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Completed", reflect.TypeOf((*MockGeneration)(nil).Completed)) +} + +// CompletedBy mocks base method +func (m *MockGeneration) CompletedBy() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CompletedBy") + ret0, _ := ret[0].(string) + return ret0 +} + +// CompletedBy indicates an expected call of CompletedBy +func (mr *MockGenerationMockRecorder) CompletedBy() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CompletedBy", reflect.TypeOf((*MockGeneration)(nil).CompletedBy)) +} + // Config mocks base method func (m *MockGeneration) Config() map[string]settings.ItemChanges { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index 72d5dc5a8d9..480ebdf56d1 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -290,7 +290,8 @@ func (api *API) BranchInfo( } // ShowCommit will return details a commit given by its generationId -// An error is returned if no commit can be found corresponding to a branch. +// An error is returned if either no branch can be found corresponding to the generation id. +// Or the generation id given is below 1. func (api *APIV3) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { result := params.GenerationCommitResult{} @@ -312,13 +313,12 @@ func (api *APIV3) ShowCommit(arg params.GenerationId) (params.GenerationCommitRe return result, nil } - //TODO: convert them - _, err = api.oneBranchInfo(branch, true) + generationCommit, err := api.getGenerationCommit(branch) if err != nil { return generationCommitResultError(err) } - //result.GenerationCommit. = generation.Applications + result.GenerationCommit = generationCommit return result, nil } @@ -403,6 +403,23 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati }, nil } +func (api *APIV3) getGenerationCommit(branch Generation) (params.GenerationCommit, error) { + + generation, err := api.oneBranchInfo(branch, true) + if err != nil { + return params.GenerationCommit{}, errors.Trace(err) + } + return params.GenerationCommit{ + BranchName: branch.BranchName(), + Completed: branch.Completed(), + CompletedBy: branch.CompletedBy(), + GenerationId: branch.GenerationId(), + Created: branch.Created(), + CreatedBy: branch.CreatedBy(), + Applications: generation.Applications, + }, nil +} + // HasActiveBranch returns a true result if the input model has an "in-flight" // branch matching the input name. func (api *API) HasActiveBranch(arg params.BranchArg) (params.BoolResult, error) { @@ -427,10 +444,6 @@ func (api *API) HasActiveBranch(arg params.BranchArg) (params.BoolResult, error) return result, nil } -func convertGenerationsToGenerationsCommit(generation Generation) params.GenerationCommit { - return params.GenerationCommit{} -} - func generationResultsError(err error) (params.GenerationResults, error) { return params.GenerationResults{Error: common.ServerError(err)}, nil } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 02ede8ce804..e5667500c18 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23096,25 +23096,6 @@ } } }, - "ListCommits": { - "type": "object", - "properties": { - "Result": { - "$ref": "#/definitions/GenerationCommitResults" - } - } - }, - "ShowCommit": { - "type": "object", - "properties": { - "Params": { - "$ref": "#/definitions/GenerationId" - }, - "Result": { - "$ref": "#/definitions/GenerationCommitResult" - } - } - }, "TrackBranch": { "type": "object", "properties": { @@ -23280,7 +23261,8 @@ "required": [ "branch", "created", - "created-by" + "created-by", + "applications" ] }, "GenerationApplication": { @@ -23321,87 +23303,6 @@ "config" ] }, - "GenerationCommit": { - "type": "object", - "properties": { - "applications": { - "type": "array", - "items": { - "$ref": "#/definitions/GenerationApplication" - } - }, - "branch": { - "type": "string" - }, - "completed": { - "type": "integer" - }, - "completed-by": { - "type": "string" - }, - "created": { - "type": "integer" - }, - "created-by": { - "type": "string" - }, - "generation-id": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "branch", - "completed", - "completed-by", - "generation-id" - ] - }, - "GenerationCommitResult": { - "type": "object", - "properties": { - "error": { - "$ref": "#/definitions/Error" - }, - "generation-commit": { - "$ref": "#/definitions/GenerationCommit" - } - }, - "additionalProperties": false, - "required": [ - "generation-commit" - ] - }, - "GenerationCommitResults": { - "type": "object", - "properties": { - "error": { - "$ref": "#/definitions/Error" - }, - "generation-commit": { - "type": "array", - "items": { - "$ref": "#/definitions/GenerationCommit" - } - } - }, - "additionalProperties": false, - "required": [ - "generation-commit" - ] - }, - "GenerationId": { - "type": "object", - "properties": { - "generation-id": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "generation-id" - ] - }, "GenerationResults": { "type": "object", "properties": { diff --git a/cmd/juju/commands/main.go b/cmd/juju/commands/main.go index 19016c2860f..ab560ca87df 100644 --- a/cmd/juju/commands/main.go +++ b/cmd/juju/commands/main.go @@ -373,6 +373,7 @@ func registerCommands(r commandRegistry, ctx *cmd.Context) { r.Register(model.NewDiffCommand()) r.Register(model.NewAbortCommand()) r.Register(model.NewCommitsCommand()) + r.Register(model.NewShowCommitCommand()) } r.Register(newMigrateCommand()) diff --git a/cmd/juju/model/export_test.go b/cmd/juju/model/export_test.go index 8c5791664c9..ed462660026 100644 --- a/cmd/juju/model/export_test.go +++ b/cmd/juju/model/export_test.go @@ -236,3 +236,11 @@ func NewListCommitsCommandForTest(api CommitsCommandAPI, store jujuclient.Client cmd.SetClientStore(store) return modelcmd.Wrap(cmd) } + +func NewShowCommitCommandForTest(api ShowCommitCommandAPI, store jujuclient.ClientStore) cmd.Command { + cmd := &ShowCommitCommand{ + api: api, + } + cmd.SetClientStore(store) + return modelcmd.Wrap(cmd) +} diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index bcb0037c317..b1aa87a30f7 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -22,8 +22,8 @@ import ( ) const ( - showCommitsSummary = "Displays details of the commit" - showCommitsDoc = ` + showCommitSummary = "Displays details of the commit" + showCommitDoc = ` Show-commit shows the committed branches to the model. Details displayed include: - user who committed the branch @@ -34,8 +34,8 @@ Details displayed include: - a summary of how many units are tracking the branch Examples: - juju show-commits 3 - juju show-commits 3 --utc + juju show-commit 3 + juju show-commit 3 --utc See also: list-commits @@ -65,14 +65,11 @@ type ShowCommitCommand struct { type ShowCommitCommandAPI interface { Close() error - // ListCommitsBranch commits the branch with the input name to the model, - // effectively completing it and applying - // all branch changes across the model. - // The new generation ID of the model is returned. - ShowCommit(func(time.Time) string) (model.GenerationCommit, error) + // ShowCommit shows the branches which were committed + ShowCommit(func(time.Time) string, int) (model.GenerationCommit, error) } -// NewCommitCommand wraps listCommitsCommand with sane model settings. +// NewShowCommitCommand wraps NewShowCommitCommand with sane model settings. func NewShowCommitCommand() cmd.Command { return modelcmd.Wrap(&ShowCommitCommand{}) } @@ -81,8 +78,8 @@ func NewShowCommitCommand() cmd.Command { func (c *ShowCommitCommand) Info() *cmd.Info { info := &cmd.Info{ Name: "show-commit", - Purpose: showCommitsDoc, - Doc: showCommitsSummary, + Purpose: showCommitDoc, + Doc: showCommitSummary, } return jujucmd.Info(info) } @@ -144,9 +141,34 @@ func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { formatTime := func(t time.Time) string { return common.FormatTime(&t, c.isoTime) } - cmt, err := client.ShowCommit(formatTime) + cmt, err := client.ShowCommit(formatTime, c.generationId) if err != nil { return err } - return c.out.Write(ctx, cmt) + return errors.Trace(c.out.Write(ctx, c.getFormattedOutput(cmt))) +} + +// Run implements the meaty part of the cmd.Command interface. +func (c *ShowCommitCommand) getFormattedOutput(gcm model.GenerationCommit) formattedShowCommit { + applications := map[string]formattedShowCommitApplications{gcm.BranchName: {gcm.Applications}} + commit := formattedShowCommit{ + Branch: applications, + CommittedAt: gcm.Completed, + CommittedBy: gcm.CompletedBy, + Created: gcm.Created, + CreatedBy: gcm.CreatedBy, + } + return commit +} + +type formattedShowCommit struct { + Branch map[string]formattedShowCommitApplications `json:"branch" yaml:"branch"` + CommittedAt string `json:"committed-at" yaml:"committed-at"` + CommittedBy string `json:"committed-by" yaml:"committed-by"` + Created string `json:"created" yaml:"created"` + CreatedBy string `json:"created-by" yaml:"created-by"` +} + +type formattedShowCommitApplications struct { + Applications []model.GenerationApplication `json:"applications" yaml:"applications"` } diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go new file mode 100644 index 00000000000..c9fdc66fc14 --- /dev/null +++ b/cmd/juju/model/showcommit_test.go @@ -0,0 +1,162 @@ +// Copyright 2019 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package model_test + +import ( + "fmt" + "github.com/golang/mock/gomock" + "github.com/juju/cmd" + "github.com/juju/cmd/cmdtesting" + jc "github.com/juju/testing/checkers" + "github.com/pkg/errors" + gc "gopkg.in/check.v1" + "regexp" + + "github.com/juju/juju/cmd/juju/model" + "github.com/juju/juju/cmd/juju/model/mocks" + coremodel "github.com/juju/juju/core/model" +) + +type showCommitsSuite struct { + generationBaseSuite + + api *mocks.MockShowCommitCommandAPI +} + +var _ = gc.Suite(&showCommitsSuite{}) + +func (s *showCommitsSuite) TestInitNoArg(c *gc.C) { + err := s.runInit() + c.Assert(err, gc.ErrorMatches, "expected exactly 1 commit id, got 0 arguments") +} + +func (s *showCommitsSuite) TestInitOneArg(c *gc.C) { + err := s.runInit("1") + c.Assert(err, jc.ErrorIsNil) +} + +func (s *showCommitsSuite) TestInitNotInt(c *gc.C) { + err := s.runInit("something") + c.Assert(err, gc.ErrorMatches, `encountered problem trying to parse "something" into an int`) +} + +func (s *showCommitsSuite) TestInitMoreArgs(c *gc.C) { + args := []string{"1", "2", "3"} + err := s.runInit(args...) + c.Assert(err, gc.ErrorMatches, "expected exactly 1 commit id, got 3 arguments") +} +func (s *showCommitsSuite) getMockValues() coremodel.GenerationCommit { + values := coremodel.GenerationCommit{ + Completed: "0001-01-01", + CompletedBy: "test-user", + Created: "0001-01-00", + CreatedBy: "test-user", + GenerationId: 1, + BranchName: "bla", + Applications: []coremodel.GenerationApplication{{ + ApplicationName: "redis", + UnitProgress: "1/2", + UnitDetail: &coremodel.GenerationUnits{ + UnitsTracking: []string{"redis/0"}, + UnitsPending: []string{"redis/1"}, + }, + ConfigChanges: map[string]interface{}{"databases": 8}, + }}, + } + return values +} + +func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { + defer s.setup(c).Finish() + result := s.getMockValues() + unwrap := regexp.MustCompile(`[\s+\n]`) + expected := unwrap.ReplaceAllLiteralString(` +{ + "branch": { + "bla": { + "applications": [ + { + "ApplicationName": "redis", + "UnitProgress": "1/2", + "UnitDetail": { + "UnitsTracking": [ + "redis/0" + ], + "UnitsPending": [ + "redis/1" + ] + }, + "ConfigChanges": { + "databases": 8 + } + } + ] + } + }, + "committed-at": "0001-01-01", + "committed-by": "test-user", + "created": "0001-01-00", + "created-by": "test-user" +} +`, "") + expected = expected + "\n" + s.api.EXPECT().ShowCommit(gomock.Any()).Return(result, nil) + ctx, err := s.runCommand(c, "1", "--format=json") + c.Assert(err, jc.ErrorIsNil) + output := cmdtesting.Stdout(ctx) + fmt.Println(output) + c.Assert(output, gc.Equals, expected) +} + +func (s *showCommitsSuite) TestRunCommandYamlOutput(c *gc.C) { + defer s.setup(c).Finish() + result := s.getMockValues() + expected := ` +branch: + bla: + applications: + - application: redis + progress: 1/2 + units: + tracking: + - redis/0 + incomplete: + - redis/1 + config: + databases: 8 +committed-at: "0001-01-01" +committed-by: test-user +created: 0001-01-00 +created-by: test-user +`[1:] + s.api.EXPECT().ShowCommit(gomock.Any()).Return(result, nil) + + ctx, err := s.runCommand(c, "1", "--format=yaml") + c.Assert(err, jc.ErrorIsNil) + c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) +} + +func (s *showCommitsSuite) TestRunCommandAPIError(c *gc.C) { + defer s.setup(c).Finish() + + s.api.EXPECT().ShowCommit(gomock.Any()).Return(nil, errors.New("boom")) + + _, err := s.runCommand(c) + c.Assert(err, gc.ErrorMatches, "boom") +} + +func (s *showCommitsSuite) runInit(args ...string) error { + return cmdtesting.InitCommand(model.NewShowCommitCommandForTest(nil, s.store), args) +} + +func (s *showCommitsSuite) runCommand(c *gc.C, args ...string) (*cmd.Context, error) { + return cmdtesting.RunCommand(c, model.NewShowCommitCommandForTest(s.api, s.store), args...) +} + +func (s *showCommitsSuite) setup(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.api = mocks.NewMockShowCommitCommandAPI(ctrl) + s.api.EXPECT().Close() + return ctrl +} From 9ceaa83c2f81436bdbab9f31c1fc401e29c6b264 Mon Sep 17 00:00:00 2001 From: nam Date: Thu, 7 Nov 2019 18:13:05 +0100 Subject: [PATCH 07/13] showcommit command: add showcommit command and it's tests --- cmd/juju/model/mocks/showcommit_mock.go | 8 ++++---- cmd/juju/model/showcommit_test.go | 9 +++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/cmd/juju/model/mocks/showcommit_mock.go b/cmd/juju/model/mocks/showcommit_mock.go index 063dce67828..293b5b31666 100644 --- a/cmd/juju/model/mocks/showcommit_mock.go +++ b/cmd/juju/model/mocks/showcommit_mock.go @@ -49,16 +49,16 @@ func (mr *MockShowCommitCommandAPIMockRecorder) Close() *gomock.Call { } // ShowCommit mocks base method -func (m *MockShowCommitCommandAPI) ShowCommit(arg0 func(time.Time) string) (model.GenerationCommit, error) { +func (m *MockShowCommitCommandAPI) ShowCommit(arg0 func(time.Time) string, arg1 int) (model.GenerationCommit, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ShowCommit", arg0) + ret := m.ctrl.Call(m, "ShowCommit", arg0, arg1) ret0, _ := ret[0].(model.GenerationCommit) ret1, _ := ret[1].(error) return ret0, ret1 } // ShowCommit indicates an expected call of ShowCommit -func (mr *MockShowCommitCommandAPIMockRecorder) ShowCommit(arg0 interface{}) *gomock.Call { +func (mr *MockShowCommitCommandAPIMockRecorder) ShowCommit(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShowCommit", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).ShowCommit), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShowCommit", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).ShowCommit), arg0, arg1) } diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go index c9fdc66fc14..431d8c52add 100644 --- a/cmd/juju/model/showcommit_test.go +++ b/cmd/juju/model/showcommit_test.go @@ -4,7 +4,6 @@ package model_test import ( - "fmt" "github.com/golang/mock/gomock" "github.com/juju/cmd" "github.com/juju/cmd/cmdtesting" @@ -101,11 +100,10 @@ func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { } `, "") expected = expected + "\n" - s.api.EXPECT().ShowCommit(gomock.Any()).Return(result, nil) + s.api.EXPECT().ShowCommit(gomock.Any(), 1).Return(result, nil) ctx, err := s.runCommand(c, "1", "--format=json") c.Assert(err, jc.ErrorIsNil) output := cmdtesting.Stdout(ctx) - fmt.Println(output) c.Assert(output, gc.Equals, expected) } @@ -130,8 +128,7 @@ committed-by: test-user created: 0001-01-00 created-by: test-user `[1:] - s.api.EXPECT().ShowCommit(gomock.Any()).Return(result, nil) - + s.api.EXPECT().ShowCommit(gomock.Any(), 1).Return(result, nil) ctx, err := s.runCommand(c, "1", "--format=yaml") c.Assert(err, jc.ErrorIsNil) c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) @@ -140,7 +137,7 @@ created-by: test-user func (s *showCommitsSuite) TestRunCommandAPIError(c *gc.C) { defer s.setup(c).Finish() - s.api.EXPECT().ShowCommit(gomock.Any()).Return(nil, errors.New("boom")) + s.api.EXPECT().ShowCommit(gomock.Any(), gomock.Any()).Return(nil, errors.New("boom")) _, err := s.runCommand(c) c.Assert(err, gc.ErrorMatches, "boom") From 51c45f314f50d40edfb1226b8ff6c3e8f3aea881 Mon Sep 17 00:00:00 2001 From: nam Date: Fri, 8 Nov 2019 10:52:18 +0100 Subject: [PATCH 08/13] showcommit command: add showcommit own structs to ensure compability --- api/modelgeneration/modelgeneration.go | 6 +- .../client/modelgeneration/modelgeneration.go | 6 +- apiserver/facades/schema.json | 100 ++++++++++++++++++ cmd/juju/model/listcommits_test.go | 9 +- cmd/juju/model/showcommit.go | 2 +- cmd/juju/model/showcommit_test.go | 42 +++----- core/model/generation.go | 27 +++-- state/modelgeneration.go | 2 +- 8 files changed, 147 insertions(+), 47 deletions(-) diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index 864989649a9..c4babbaa62f 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -219,12 +219,12 @@ func generationCommitsFromResults(results params.GenerationCommitResults, format func generationCommitFromResult(result params.GenerationCommitResult, formatTime func(time.Time) string) model.GenerationCommit { genCommit := result.GenerationCommit - appChanges := make([]model.GenerationApplication, len(genCommit.Applications)) + appChanges := make([]model.GenerationCommitApplication, len(genCommit.Applications)) for i, a := range genCommit.Applications { - app := model.GenerationApplication{ + app := model.GenerationCommitApplication{ ApplicationName: a.ApplicationName, - UnitProgress: a.UnitProgress, ConfigChanges: a.ConfigChanges, + UnitsTracking: a.UnitsTracking, } appChanges[i] = app } diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index 480ebdf56d1..f362205e2f6 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -292,7 +292,7 @@ func (api *API) BranchInfo( // ShowCommit will return details a commit given by its generationId // An error is returned if either no branch can be found corresponding to the generation id. // Or the generation id given is below 1. -func (api *APIV3) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { +func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { result := params.GenerationCommitResult{} isModelAdmin, err := api.hasAdminAccess() @@ -324,7 +324,7 @@ func (api *APIV3) ShowCommit(arg params.GenerationId) (params.GenerationCommitRe } // ListCommits will return the commits, hence only branches with generation_id higher than 0 -func (api *APIV3) ListCommits() (params.GenerationCommitResults, error) { +func (api *API) ListCommits() (params.GenerationCommitResults, error) { result := params.GenerationCommitResults{} isModelAdmin, err := api.hasAdminAccess() @@ -403,7 +403,7 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati }, nil } -func (api *APIV3) getGenerationCommit(branch Generation) (params.GenerationCommit, error) { +func (api *API) getGenerationCommit(branch Generation) (params.GenerationCommit, error) { generation, err := api.oneBranchInfo(branch, true) if err != nil { diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index e5667500c18..db36a033dbd 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23096,6 +23096,25 @@ } } }, + "ListCommits": { + "type": "object", + "properties": { + "Result": { + "$ref": "#/definitions/GenerationCommitResults" + } + } + }, + "ShowCommit": { + "type": "object", + "properties": { + "Params": { + "$ref": "#/definitions/GenerationId" + }, + "Result": { + "$ref": "#/definitions/GenerationCommitResult" + } + } + }, "TrackBranch": { "type": "object", "properties": { @@ -23303,6 +23322,87 @@ "config" ] }, + "GenerationCommit": { + "type": "object", + "properties": { + "applications": { + "type": "array", + "items": { + "$ref": "#/definitions/GenerationApplication" + } + }, + "branch": { + "type": "string" + }, + "completed": { + "type": "integer" + }, + "completed-by": { + "type": "string" + }, + "created": { + "type": "integer" + }, + "created-by": { + "type": "string" + }, + "generation-id": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "branch", + "completed", + "completed-by", + "generation-id" + ] + }, + "GenerationCommitResult": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "generation-commit": { + "$ref": "#/definitions/GenerationCommit" + } + }, + "additionalProperties": false, + "required": [ + "generation-commit" + ] + }, + "GenerationCommitResults": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "generation-commit": { + "type": "array", + "items": { + "$ref": "#/definitions/GenerationCommit" + } + } + }, + "additionalProperties": false, + "required": [ + "generation-commit" + ] + }, + "GenerationId": { + "type": "object", + "properties": { + "generation-id": { + "type": "integer" + } + }, + "additionalProperties": false, + "required": [ + "generation-id" + ] + }, "GenerationResults": { "type": "object", "properties": { diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index ccd764df7ec..928b26d0252 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -4,15 +4,15 @@ package model_test import ( - "fmt" "github.com/golang/mock/gomock" - "github.com/juju/cmd" - "github.com/juju/cmd/cmdtesting" - jc "github.com/juju/testing/checkers" "github.com/pkg/errors" gc "gopkg.in/check.v1" "regexp" + "github.com/juju/cmd" + "github.com/juju/cmd/cmdtesting" + jc "github.com/juju/testing/checkers" + "github.com/juju/juju/cmd/juju/model" "github.com/juju/juju/cmd/juju/model/mocks" coremodel "github.com/juju/juju/core/model" @@ -116,7 +116,6 @@ commits: ctx, err := s.runCommand(c, "--format=yaml") c.Assert(err, jc.ErrorIsNil) - fmt.Println(cmdtesting.Stdout(ctx)) c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) } diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index b1aa87a30f7..8f82853c76d 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -170,5 +170,5 @@ type formattedShowCommit struct { } type formattedShowCommitApplications struct { - Applications []model.GenerationApplication `json:"applications" yaml:"applications"` + Applications []model.GenerationCommitApplication `json:"applications" yaml:"applications"` } diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go index 431d8c52add..3d330cb022b 100644 --- a/cmd/juju/model/showcommit_test.go +++ b/cmd/juju/model/showcommit_test.go @@ -5,13 +5,14 @@ package model_test import ( "github.com/golang/mock/gomock" - "github.com/juju/cmd" - "github.com/juju/cmd/cmdtesting" - jc "github.com/juju/testing/checkers" "github.com/pkg/errors" gc "gopkg.in/check.v1" "regexp" + "github.com/juju/cmd" + "github.com/juju/cmd/cmdtesting" + jc "github.com/juju/testing/checkers" + "github.com/juju/juju/cmd/juju/model" "github.com/juju/juju/cmd/juju/model/mocks" coremodel "github.com/juju/juju/core/model" @@ -53,14 +54,10 @@ func (s *showCommitsSuite) getMockValues() coremodel.GenerationCommit { CreatedBy: "test-user", GenerationId: 1, BranchName: "bla", - Applications: []coremodel.GenerationApplication{{ + Applications: []coremodel.GenerationCommitApplication{{ ApplicationName: "redis", - UnitProgress: "1/2", - UnitDetail: &coremodel.GenerationUnits{ - UnitsTracking: []string{"redis/0"}, - UnitsPending: []string{"redis/1"}, - }, - ConfigChanges: map[string]interface{}{"databases": 8}, + UnitsTracking: []string{"redis/0"}, + ConfigChanges: map[string]interface{}{"databases": 8}, }}, } return values @@ -77,15 +74,9 @@ func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { "applications": [ { "ApplicationName": "redis", - "UnitProgress": "1/2", - "UnitDetail": { - "UnitsTracking": [ - "redis/0" - ], - "UnitsPending": [ - "redis/1" - ] - }, + "UnitsTracking": [ + "redis/0" + ], "ConfigChanges": { "databases": 8 } @@ -115,12 +106,8 @@ branch: bla: applications: - application: redis - progress: 1/2 units: - tracking: - - redis/0 - incomplete: - - redis/1 + - redis/0 config: databases: 8 committed-at: "0001-01-01" @@ -131,15 +118,16 @@ created-by: test-user s.api.EXPECT().ShowCommit(gomock.Any(), 1).Return(result, nil) ctx, err := s.runCommand(c, "1", "--format=yaml") c.Assert(err, jc.ErrorIsNil) - c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) + output := cmdtesting.Stdout(ctx) + c.Assert(output, gc.Matches, expected) } func (s *showCommitsSuite) TestRunCommandAPIError(c *gc.C) { defer s.setup(c).Finish() - s.api.EXPECT().ShowCommit(gomock.Any(), gomock.Any()).Return(nil, errors.New("boom")) + s.api.EXPECT().ShowCommit(gomock.Any(), gomock.Any()).Return(coremodel.GenerationCommit{}, errors.New("boom")) - _, err := s.runCommand(c) + _, err := s.runCommand(c, "1") c.Assert(err, gc.ErrorMatches, "boom") } diff --git a/core/model/generation.go b/core/model/generation.go index 75fff28e502..daf1bff2c71 100644 --- a/core/model/generation.go +++ b/core/model/generation.go @@ -57,6 +57,19 @@ type GenerationApplication struct { ConfigChanges map[string]interface{} `yaml:"config"` } +// GenerationCommitApplication represents committed changes to an application +// made under a generation. +type GenerationCommitApplication struct { + // ApplicationsName is the name of the application. + ApplicationName string `yaml:"application"` + + // UnitDetail specifies which units are and are not tracking the branch. + UnitsTracking []string `yaml:"units,omitempty"` + + // Config changes are the committed changes from the generation + ConfigChanges map[string]interface{} `yaml:"config"` +} + // Generation represents detail of a model generation including config changes. type Generation struct { // Created is the formatted time at generation creation. @@ -73,26 +86,26 @@ type Generation struct { // GenerationCommits represents a model generation's commit details. type GenerationCommit struct { // BranchName uniquely identifies a branch *amongst in-flight branches*. - BranchName string `json:"branch"` + BranchName string `yaml:"branch"` // Created is the Unix timestamp at generation creation. - Completed string `json:"completed"` + Completed string `yaml:"completed"` // Created is the user who created the generation. - CompletedBy string `json:"completed-by"` + CompletedBy string `yaml:"completed-by"` // Created is the Unix timestamp at generation creation. - Created string `json:"created,omitempty"` + Created string `yaml:"created,omitempty"` // Created is the user who created the generation. - CreatedBy string `json:"created-by,omitempty"` + CreatedBy string `yaml:"created-by,omitempty"` // GenerationId is the id . - GenerationId int `json:"generation-id,omitempty"` + GenerationId int `yaml:"generation-id,omitempty"` // Applications holds the collection of application changes // made under this generation. - Applications []GenerationApplication `json:"applications,omitempty"` + Applications []GenerationCommitApplication `yaml:"applications,omitempty"` } // GenerationSummaries is a type alias for a representation diff --git a/state/modelgeneration.go b/state/modelgeneration.go index 79a5cf290c3..8ac69ae1563 100644 --- a/state/modelgeneration.go +++ b/state/modelgeneration.go @@ -828,7 +828,7 @@ func (st *State) getCommittedBranchDoc(id int) (*generationDoc, error) { return doc, nil case mgo.ErrNotFound: mod, _ := st.modelName() - return nil, errors.NotFoundf("generation_id %q in model %q", id, mod) + return nil, errors.NotFoundf("generation_id %d in model %q", id, mod) default: mod, _ := st.modelName() return nil, errors.Annotatef(err, "retrieving generation_id %q in model %q", id, mod) From d2a3917948d08e4a329f7466501e42a6224d4eda Mon Sep 17 00:00:00 2001 From: nam Date: Fri, 8 Nov 2019 17:14:48 +0100 Subject: [PATCH 09/13] fix query for correct generation id --- state/modelgeneration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/state/modelgeneration.go b/state/modelgeneration.go index 8ac69ae1563..7d4930f6fd5 100644 --- a/state/modelgeneration.go +++ b/state/modelgeneration.go @@ -728,7 +728,7 @@ func (st *State) CommittedBranches() ([]*Generation, error) { defer closer() var docs []generationDoc - query := bson.M{"completed": bson.M{"$gte": 1}} + query := bson.M{"generation-id": bson.M{"$gte": 1}} if err := col.Find(query).All(&docs); err != nil { return nil, errors.Trace(err) } From 2f14d2f6895455c3b9b847059f05c1bc194d058d Mon Sep 17 00:00:00 2001 From: nam Date: Fri, 8 Nov 2019 17:31:38 +0100 Subject: [PATCH 10/13] incorporate review feedback from simon. mostly related refactoring to naming and spacing --- .../client/modelgeneration/modelgeneration.go | 3 +-- apiserver/params/params.go | 12 ++++++------ cmd/juju/model/listcommits.go | 4 ++-- cmd/juju/model/listcommits_test.go | 8 ++++---- cmd/juju/model/showcommit.go | 4 +--- cmd/juju/model/showcommit_test.go | 6 +++--- state/modelgeneration.go | 1 - 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index f362205e2f6..26176708c5a 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -325,7 +325,7 @@ func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResu // ListCommits will return the commits, hence only branches with generation_id higher than 0 func (api *API) ListCommits() (params.GenerationCommitResults, error) { - result := params.GenerationCommitResults{} + var result params.GenerationCommitResults isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -404,7 +404,6 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati } func (api *API) getGenerationCommit(branch Generation) (params.GenerationCommit, error) { - generation, err := api.oneBranchInfo(branch, true) if err != nil { return params.GenerationCommit{}, errors.Trace(err) diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 03e0380ca9f..517d9256a07 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -1182,7 +1182,7 @@ type BranchArg struct { BranchName string `json:"branch"` } -// GenerationId represents an in-flight branch via its model and branch name. +// GenerationId represents an GenerationId from a branch. type GenerationId struct { GenerationId int `json:"generation-id"` } @@ -1248,10 +1248,10 @@ type GenerationCommit struct { // BranchName uniquely identifies a branch *amongst in-flight branches*. BranchName string `json:"branch"` - // Created is the Unix timestamp at generation creation. + // Completed is the Unix timestamp at generation completion/commit. Completed int64 `json:"completed"` - // Created is the user who created the generation. + // CompletedBy is the user who committed/completed the generation. CompletedBy string `json:"completed-by"` // GenerationId is the id . @@ -1260,7 +1260,7 @@ type GenerationCommit struct { // Created is the Unix timestamp at generation creation. Created int64 `json:"created,omitempty"` - // Created is the user who created the generation. + // CreatedBy is the user who created the generation. CreatedBy string `json:"created-by,omitempty"` // Applications holds the collection of application changes @@ -1286,7 +1286,7 @@ type GenerationResult struct { Error *Error `json:"error,omitempty"` } -// GenerationResult transports a generation detail. +// GenerationCommitResults transports a collection of GenerationCommit detail. type GenerationCommitResults struct { // Generation holds the details of the requested generation. GenerationCommits []GenerationCommit `json:"generation-commit"` @@ -1295,7 +1295,7 @@ type GenerationCommitResults struct { Error *Error `json:"error,omitempty"` } -// GenerationResult transports a generation detail. +// GenerationCommitResult transports a GenerationCommit detail. type GenerationCommitResult struct { // Generation holds the details of the requested generation. GenerationCommit GenerationCommit `json:"generation-commit"` diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index d49dbf1f772..9cba16cba8b 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -108,9 +108,9 @@ func (c *CommitsCommand) Init(args []string) error { // If use of ISO time not specified on command line, check env var. if !c.isoTime { - var err error envVarValue := os.Getenv(osenv.JujuStatusIsoTimeEnvKey) if envVarValue != "" { + var err error if c.isoTime, err = strconv.ParseBool(envVarValue); err != nil { return errors.Annotatef(err, "invalid %s env var, expected true|false", osenv.JujuStatusIsoTimeEnvKey) } @@ -173,7 +173,7 @@ func (c *CommitsCommand) printTabular(writer io.Writer, value interface{}) error } func constructYamlJson(gen model.GenerationCommits) formattedCommitList { - result := formattedCommitList{} + var result formattedCommitList for _, gen := range gen { fmc := formattedCommit{ CommitId: gen.GenerationId, diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index 928b26d0252..c09e38d468d 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -35,7 +35,7 @@ func (s *commitsSuite) TestInitOneArg(c *gc.C) { err := s.runInit(s.branchName) c.Assert(err, gc.ErrorMatches, `expected no arguments, but got 1`) } -func (s *commitsSuite) getMockValues() []coremodel.GenerationCommit { +func (s *commitsSuite) getGenerationCommitValues() []coremodel.GenerationCommit { values := []coremodel.GenerationCommit{ { Completed: "0001-01-01", @@ -55,7 +55,7 @@ func (s *commitsSuite) getMockValues() []coremodel.GenerationCommit { func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { defer s.setup(c).Finish() - result := s.getMockValues() + result := s.getGenerationCommitValues() expected := `Commit Committed at Committed by Branch name 1 0001-01-01 test-user bla @@ -70,7 +70,7 @@ func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { func (s *commitsSuite) TestRunCommandJsonOutput(c *gc.C) { defer s.setup(c).Finish() - result := s.getMockValues() + result := s.getGenerationCommitValues() unwrap := regexp.MustCompile(`[\s+\n]`) expected := unwrap.ReplaceAllLiteralString(` { @@ -100,7 +100,7 @@ func (s *commitsSuite) TestRunCommandJsonOutput(c *gc.C) { func (s *commitsSuite) TestRunCommandYamlOutput(c *gc.C) { defer s.setup(c).Finish() - result := s.getMockValues() + result := s.getGenerationCommitValues() expected := ` commits: - id: 1 diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index 8f82853c76d..128c8486991 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -39,7 +39,7 @@ Examples: See also: list-commits - add-branch + add-branch track branch abort @@ -47,8 +47,6 @@ See also: ` ) -//TODO: instead of diffing, i just show the content of config - // ShowCommitCommand supplies the "show-commit" CLI command used to show commits type ShowCommitCommand struct { modelcmd.ModelCommandBase diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go index 3d330cb022b..221f1f234f8 100644 --- a/cmd/juju/model/showcommit_test.go +++ b/cmd/juju/model/showcommit_test.go @@ -46,7 +46,7 @@ func (s *showCommitsSuite) TestInitMoreArgs(c *gc.C) { err := s.runInit(args...) c.Assert(err, gc.ErrorMatches, "expected exactly 1 commit id, got 3 arguments") } -func (s *showCommitsSuite) getMockValues() coremodel.GenerationCommit { +func (s *showCommitsSuite) getGenerationCommitValue() coremodel.GenerationCommit { values := coremodel.GenerationCommit{ Completed: "0001-01-01", CompletedBy: "test-user", @@ -65,7 +65,7 @@ func (s *showCommitsSuite) getMockValues() coremodel.GenerationCommit { func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { defer s.setup(c).Finish() - result := s.getMockValues() + result := s.getGenerationCommitValue() unwrap := regexp.MustCompile(`[\s+\n]`) expected := unwrap.ReplaceAllLiteralString(` { @@ -100,7 +100,7 @@ func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { func (s *showCommitsSuite) TestRunCommandYamlOutput(c *gc.C) { defer s.setup(c).Finish() - result := s.getMockValues() + result := s.getGenerationCommitValue() expected := ` branch: bla: diff --git a/state/modelgeneration.go b/state/modelgeneration.go index 7d4930f6fd5..b2ff0318d30 100644 --- a/state/modelgeneration.go +++ b/state/modelgeneration.go @@ -719,7 +719,6 @@ func (st *State) Branches() ([]*Generation, error) { branches[i] = newGeneration(st, &d) } return branches, nil - } // CommittedBranches returns all committed branches. From 2fd73af537af3882d708fd8bf5f07a240d5dd30a Mon Sep 17 00:00:00 2001 From: nam Date: Mon, 11 Nov 2019 11:29:55 +0100 Subject: [PATCH 11/13] add no commits output --- cmd/juju/model/listcommits.go | 4 ++++ cmd/juju/model/listcommits_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index 9cba16cba8b..fe5e39c07b5 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -149,6 +149,10 @@ func (c *CommitsCommand) Run(ctx *cmd.Context) error { if err != nil { return errors.Trace(err) } + if len(commits) == 0 && c.out.Name() == "tabular" { + ctx.Infof("no commits to display") + return nil + } tabular := constructYamlJson(commits) return errors.Trace(c.out.Write(ctx, tabular)) } diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index c09e38d468d..b87ca8d983d 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -53,6 +53,25 @@ func (s *commitsSuite) getGenerationCommitValues() []coremodel.GenerationCommit return values } +func (s *commitsSuite) getGenerationEmptyCommitValues() []coremodel.GenerationCommit { + values := []coremodel.GenerationCommit{ + { + Completed: "0001-01-01", + CompletedBy: "test-user", + GenerationId: 1, + BranchName: "bla", + }, + { + Completed: "0001-02-02", + CompletedBy: "test-user", + GenerationId: 2, + BranchName: "test", + }, + } + return values +} + + func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { defer s.setup(c).Finish() result := s.getGenerationCommitValues() From ff32067866004a0a04d5dba8e281dea7857f33ea Mon Sep 17 00:00:00 2001 From: nam Date: Tue, 12 Nov 2019 11:34:33 +0100 Subject: [PATCH 12/13] list-commit show-commit: change timeformat depending on output, sort output, commits feedback featureflags: rename flag from generations to branches --- api/modelgeneration/modelgeneration.go | 18 +-- .../facades/client/client/status_test.go | 2 +- cmd/juju/application/config.go | 6 +- cmd/juju/application/config_test.go | 4 +- cmd/juju/commands/bootstrap.go | 2 +- cmd/juju/commands/main.go | 2 +- cmd/juju/controller/addmodel.go | 2 +- cmd/juju/controller/addmodel_test.go | 6 +- cmd/juju/model/listcommits.go | 39 +++--- cmd/juju/model/listcommits_test.go | 127 ++++-------------- cmd/juju/model/mocks/commits_mock.go | 9 +- cmd/juju/model/mocks/showcommit_mock.go | 9 +- cmd/juju/model/package_test.go | 2 +- cmd/juju/model/showcommit.go | 18 +-- cmd/juju/model/showcommit_test.go | 54 ++------ cmd/juju/status/status_internal_test.go | 2 +- core/model/generation.go | 5 +- environs/bootstrap/prepare.go | 2 +- feature/flags.go | 5 +- jujuclient/models.go | 2 +- jujuclient/modelsfile_test.go | 2 +- 21 files changed, 108 insertions(+), 210 deletions(-) diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index c4babbaa62f..cf5305de96c 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -70,7 +70,7 @@ func (c *Client) CommitBranch(branchName string) (int, error) { } // ListCommits lists the committed branches commits, -func (c *Client) ListCommits(formatTime func(time.Time) string) (model.GenerationCommits, error) { +func (c *Client) ListCommits() (model.GenerationCommits, error) { var result params.GenerationCommitResults err := c.facade.FacadeCall("ListCommits", nil, &result) if err != nil { @@ -79,13 +79,13 @@ func (c *Client) ListCommits(formatTime func(time.Time) string) (model.Generatio if result.Error != nil { return nil, errors.Trace(result.Error) } - return generationCommitsFromResults(result, formatTime), nil + return generationCommitsFromResults(result), nil } // CommitBranch commits the branch with the input name to the model, // effectively completing it and applying all branch changes across the model. // The new generation ID of the model is returned. -func (c *Client) ShowCommit(formatTime func(time.Time) string, generationId int) (model.GenerationCommit, error) { +func (c *Client) ShowCommit(generationId int) (model.GenerationCommit, error) { var result params.GenerationCommitResult arg := params.GenerationId{GenerationId: generationId} err := c.facade.FacadeCall("ShowCommit", arg, &result) @@ -95,7 +95,7 @@ func (c *Client) ShowCommit(formatTime func(time.Time) string, generationId int) if result.Error != nil { return model.GenerationCommit{}, errors.Trace(result.Error) } - return generationCommitFromResult(result, formatTime), nil + return generationCommitFromResult(result), nil } // TrackBranch sets the input units and/or applications @@ -204,12 +204,12 @@ func generationInfoFromResult( return summaries } -func generationCommitsFromResults(results params.GenerationCommitResults, formatTime func(time.Time) string) model.GenerationCommits { +func generationCommitsFromResults(results params.GenerationCommitResults) model.GenerationCommits { commits := make(model.GenerationCommits, len(results.GenerationCommits)) for i, gen := range results.GenerationCommits { commits[i] = model.GenerationCommit{ GenerationId: gen.GenerationId, - Completed: formatTime(time.Unix(gen.Completed, 0)), + Completed: time.Unix(gen.Completed, 0), CompletedBy: gen.CompletedBy, BranchName: gen.BranchName, } @@ -217,7 +217,7 @@ func generationCommitsFromResults(results params.GenerationCommitResults, format return commits } -func generationCommitFromResult(result params.GenerationCommitResult, formatTime func(time.Time) string) model.GenerationCommit { +func generationCommitFromResult(result params.GenerationCommitResult) model.GenerationCommit { genCommit := result.GenerationCommit appChanges := make([]model.GenerationCommitApplication, len(genCommit.Applications)) for i, a := range genCommit.Applications { @@ -230,9 +230,9 @@ func generationCommitFromResult(result params.GenerationCommitResult, formatTime } modelCommit := model.GenerationCommit{ BranchName: genCommit.BranchName, - Completed: formatTime(time.Unix(genCommit.Completed, 0)), + Completed: time.Unix(genCommit.Completed, 0), CompletedBy: genCommit.CompletedBy, - Created: formatTime(time.Unix(genCommit.Created, 0)), + Created: time.Unix(genCommit.Created, 0), CreatedBy: genCommit.CreatedBy, GenerationId: genCommit.GenerationId, Applications: appChanges, diff --git a/apiserver/facades/client/client/status_test.go b/apiserver/facades/client/client/status_test.go index c195b99a0a0..d7f095d3ae1 100644 --- a/apiserver/facades/client/client/status_test.go +++ b/apiserver/facades/client/client/status_test.go @@ -1032,7 +1032,7 @@ func (m mockLeadershipReader) Leaders() (map[string]string, error) { func setGenerationsControllerConfig(c *gc.C, st *state.State) { err := st.UpdateControllerConfig(map[string]interface{}{ - "features": []interface{}{feature.Generations}, + "features": []interface{}{feature.Branches}, }, nil) c.Assert(err, jc.ErrorIsNil) } diff --git a/cmd/juju/application/config.go b/cmd/juju/application/config.go index a16641d666c..5db37f2e26b 100644 --- a/cmd/juju/application/config.go +++ b/cmd/juju/application/config.go @@ -126,7 +126,7 @@ func (c *configCommand) SetFlags(f *gnuflag.FlagSet) { f.Var(&c.configFile, "file", "path to yaml-formatted application config") f.Var(cmd.NewAppendStringsValue(&c.reset), "reset", "Reset the provided comma delimited keys") - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { f.StringVar(&c.branchName, "branch", "", "Specifically target config for the supplied branch") } } @@ -190,7 +190,7 @@ func (c *configCommand) validateGeneration() error { // during development. When we remove the flag, there will be tests // (particularly feature tests) that will need to accommodate a value // for branch in the local store. - if !featureflag.Enabled(feature.Generations) && c.branchName == "" { + if !featureflag.Enabled(feature.Branches) && c.branchName == "" { c.branchName = model.GenerationMaster } @@ -429,7 +429,7 @@ func (c *configCommand) getConfig(client applicationAPI, ctx *cmd.Context) error err = c.out.Write(ctx, resultsMap) - if featureflag.Enabled(feature.Generations) && err == nil { + if featureflag.Enabled(feature.Branches) && err == nil { var gen string gen, err = c.ActiveBranch() if err == nil { diff --git a/cmd/juju/application/config_test.go b/cmd/juju/application/config_test.go index cafabd5ebcf..2f90d55b96f 100644 --- a/cmd/juju/application/config_test.go +++ b/cmd/juju/application/config_test.go @@ -106,7 +106,7 @@ var getTests = []struct { func (s *configCommandSuite) SetUpTest(c *gc.C) { s.FakeJujuXDGDataHomeSuite.SetUpTest(c) - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) s.defaultCharmValues = map[string]interface{}{ "title": "Nearly There", @@ -164,7 +164,7 @@ func (s *configCommandSuite) TestGetCommandInitWithGeneration(c *gc.C) { } func (s *configCommandSuite) TestGetConfig(c *gc.C) { - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) for _, t := range getTests { if !t.useAppConfig { s.fake.appValues = nil diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go index 194d682dd4f..f59fc28db1a 100644 --- a/cmd/juju/commands/bootstrap.go +++ b/cmd/juju/commands/bootstrap.go @@ -502,7 +502,7 @@ func (c *bootstrapCommand) initializeHostedModel( ModelType: hostedModelType, } - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { modelDetails.ActiveBranch = model.GenerationMaster } diff --git a/cmd/juju/commands/main.go b/cmd/juju/commands/main.go index ab560ca87df..8de18363a15 100644 --- a/cmd/juju/commands/main.go +++ b/cmd/juju/commands/main.go @@ -365,7 +365,7 @@ func registerCommands(r commandRegistry, ctx *cmd.Context) { r.Register(model.NewRevokeCommand()) r.Register(model.NewShowCommand()) r.Register(model.NewModelCredentialCommand()) - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { r.Register(model.NewAddBranchCommand()) r.Register(model.NewCommitCommand()) r.Register(model.NewTrackBranchCommand()) diff --git a/cmd/juju/controller/addmodel.go b/cmd/juju/controller/addmodel.go index d847dbd80bb..0b289b3bb64 100644 --- a/cmd/juju/controller/addmodel.go +++ b/cmd/juju/controller/addmodel.go @@ -255,7 +255,7 @@ func (c *addModelCommand) Run(ctx *cmd.Context) error { ModelUUID: model.UUID, ModelType: model.Type, } - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { // Default target is the master branch. details.ActiveBranch = coremodel.GenerationMaster } diff --git a/cmd/juju/controller/addmodel_test.go b/cmd/juju/controller/addmodel_test.go index cd335a4d384..e30703b2037 100644 --- a/cmd/juju/controller/addmodel_test.go +++ b/cmd/juju/controller/addmodel_test.go @@ -196,7 +196,7 @@ func (s *AddModelSuite) TestInit(c *gc.C) { } func (s *AddModelSuite) TestAddExistingName(c *gc.C) { - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) // If there's any model details existing, we just overwrite them. The // controller will error out if the model already exists. Overwriting // means we'll replace any stale details from an previously existing @@ -600,7 +600,7 @@ func (s *AddModelSuite) TestAddErrorRemoveConfigstoreInfo(c *gc.C) { } func (s *AddModelSuite) TestAddStoresValues(c *gc.C) { - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) const controllerName = "test-master" _, err := s.run(c, "test") @@ -621,7 +621,7 @@ func (s *AddModelSuite) TestAddStoresValues(c *gc.C) { } func (s *AddModelSuite) TestNoSwitch(c *gc.C) { - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) const controllerName = "test-master" checkNoModelSelected := func() { _, err := s.store.CurrentModel(controllerName) diff --git a/cmd/juju/model/listcommits.go b/cmd/juju/model/listcommits.go index fe5e39c07b5..df5da13c8b9 100644 --- a/cmd/juju/model/listcommits.go +++ b/cmd/juju/model/listcommits.go @@ -5,15 +5,15 @@ package model import ( "fmt" - "io" - "os" - "strconv" - "time" - "github.com/gosuri/uitable" "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" + "io" + "os" + "sort" + "strconv" + "time" "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" @@ -69,7 +69,7 @@ type CommitsCommandAPI interface { // effectively completing it and applying // all branch changes across the model. // The new generation ID of the model is returned. - ListCommits(func(time.Time) string) (model.GenerationCommits, error) + ListCommits() (model.GenerationCommits, error) } // NewCommitCommand wraps CommitsCommand with sane model settings. @@ -140,20 +140,16 @@ func (c *CommitsCommand) Run(ctx *cmd.Context) error { return err } defer func() { _ = client.Close() }() - // Partially apply our time format - formatTime := func(t time.Time) string { - return common.FormatTime(&t, c.isoTime) - } - commits, err := client.ListCommits(formatTime) + commits, err := client.ListCommits() if err != nil { return errors.Trace(err) } if len(commits) == 0 && c.out.Name() == "tabular" { - ctx.Infof("no commits to display") + ctx.Infof("No commits to list") return nil } - tabular := constructYamlJson(commits) + tabular := c.constructYamlJson(commits) return errors.Trace(c.out.Write(ctx, tabular)) } @@ -176,13 +172,24 @@ func (c *CommitsCommand) printTabular(writer io.Writer, value interface{}) error return nil } -func constructYamlJson(gen model.GenerationCommits) formattedCommitList { +func (c *CommitsCommand) constructYamlJson(generations model.GenerationCommits) formattedCommitList { + + sort.Slice(generations, func(i, j int) bool { + return generations[i].GenerationId > generations[j].GenerationId + }) + var result formattedCommitList - for _, gen := range gen { + for _, gen := range generations { + var formattedDate string + if c.out.Name() == "tabular" { + formattedDate = common.UserFriendlyDuration(gen.Completed, time.Now()) + } else { + formattedDate = common.FormatTime(&gen.Completed, c.isoTime) + } fmc := formattedCommit{ CommitId: gen.GenerationId, BranchName: gen.BranchName, - CommittedAt: gen.Completed, + CommittedAt: formattedDate, CommittedBy: gen.CompletedBy, } result.Commits = append(result.Commits, fmc) diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index b87ca8d983d..83bd14c1dcb 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -4,10 +4,11 @@ package model_test import ( + "fmt" "github.com/golang/mock/gomock" "github.com/pkg/errors" gc "gopkg.in/check.v1" - "regexp" + "time" "github.com/juju/cmd" "github.com/juju/cmd/cmdtesting" @@ -26,25 +27,25 @@ type commitsSuite struct { var _ = gc.Suite(&commitsSuite{}) -func (s *commitsSuite) TestInitNoArg(c *gc.C) { - err := s.runInit() +func (cs *commitsSuite) TestInitNoArg(c *gc.C) { + err := cs.runInit() c.Assert(err, jc.ErrorIsNil) } -func (s *commitsSuite) TestInitOneArg(c *gc.C) { - err := s.runInit(s.branchName) +func (cs *commitsSuite) TestInitOneArg(c *gc.C) { + err := cs.runInit(cs.branchName) c.Assert(err, gc.ErrorMatches, `expected no arguments, but got 1`) } -func (s *commitsSuite) getGenerationCommitValues() []coremodel.GenerationCommit { +func (cs *commitsSuite) getGenerationCommitValues() []coremodel.GenerationCommit { values := []coremodel.GenerationCommit{ { - Completed: "0001-01-01", + Completed: time.Unix(12345, 0), CompletedBy: "test-user", GenerationId: 1, BranchName: "bla", }, { - Completed: "0001-02-02", + Completed: time.Unix(12345, 0), CompletedBy: "test-user", GenerationId: 2, BranchName: "test", @@ -53,111 +54,41 @@ func (s *commitsSuite) getGenerationCommitValues() []coremodel.GenerationCommit return values } -func (s *commitsSuite) getGenerationEmptyCommitValues() []coremodel.GenerationCommit { - values := []coremodel.GenerationCommit{ - { - Completed: "0001-01-01", - CompletedBy: "test-user", - GenerationId: 1, - BranchName: "bla", - }, - { - Completed: "0001-02-02", - CompletedBy: "test-user", - GenerationId: 2, - BranchName: "test", - }, - } - return values -} - - -func (s *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { - defer s.setup(c).Finish() - result := s.getGenerationCommitValues() - expected := - `Commit Committed at Committed by Branch name -1 0001-01-01 test-user bla -2 0001-02-02 test-user test -` - s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) - - ctx, err := s.runCommand(c) - c.Assert(err, jc.ErrorIsNil) - c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) -} - -func (s *commitsSuite) TestRunCommandJsonOutput(c *gc.C) { - defer s.setup(c).Finish() - result := s.getGenerationCommitValues() - unwrap := regexp.MustCompile(`[\s+\n]`) - expected := unwrap.ReplaceAllLiteralString(` -{ - "commits": [ - { - "id": 1, - "branch-name": "bla", - "committed-at": "0001-01-01", - "committed-by": "test-user" - }, - { - "id": 2, - "branch-name": "test", - "committed-at": "0001-02-02", - "committed-by": "test-user" - } - ] -} -`, "") - expected = expected + "\n" - s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) - - ctx, err := s.runCommand(c, "--format=json") - c.Assert(err, jc.ErrorIsNil) - c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) -} - -func (s *commitsSuite) TestRunCommandYamlOutput(c *gc.C) { - defer s.setup(c).Finish() - result := s.getGenerationCommitValues() +func (cs *commitsSuite) TestRunCommandTabularOutput(c *gc.C) { + defer cs.setup(c).Finish() + result := cs.getGenerationCommitValues() expected := ` -commits: -- id: 1 - branch-name: bla - committed-at: "0001-01-01" - committed-by: test-user -- id: 2 - branch-name: test - committed-at: "0001-02-02" - committed-by: test-user +Commit Committed at Committed by Branch name +2 1970-01-01 test-user test +1 1970-01-01 test-user bla `[1:] - s.api.EXPECT().ListCommits(gomock.Any()).Return(result, nil) + cs.api.EXPECT().ListCommits().Return(result, nil) - ctx, err := s.runCommand(c, "--format=yaml") + ctx, err := cs.runCommand(c) c.Assert(err, jc.ErrorIsNil) - c.Assert(cmdtesting.Stdout(ctx), gc.Matches, expected) + c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) } -func (s *commitsSuite) TestRunCommandAPIError(c *gc.C) { - defer s.setup(c).Finish() +func (cs *commitsSuite) TestRunCommandAPIError(c *gc.C) { + defer cs.setup(c).Finish() - s.api.EXPECT().ListCommits(gomock.Any()).Return(nil, errors.New("boom")) + cs.api.EXPECT().ListCommits().Return(nil, errors.New("boom")) - _, err := s.runCommand(c) + _, err := cs.runCommand(c) c.Assert(err, gc.ErrorMatches, "boom") } -func (s *commitsSuite) runInit(args ...string) error { - return cmdtesting.InitCommand(model.NewListCommitsCommandForTest(nil, s.store), args) +func (cs *commitsSuite) runInit(args ...string) error { + return cmdtesting.InitCommand(model.NewListCommitsCommandForTest(nil, cs.store), args) } -func (s *commitsSuite) runCommand(c *gc.C, args ...string) (*cmd.Context, error) { - return cmdtesting.RunCommand(c, model.NewListCommitsCommandForTest(s.api, s.store), args...) +func (cs *commitsSuite) runCommand(c *gc.C, args ...string) (*cmd.Context, error) { + return cmdtesting.RunCommand(c, model.NewListCommitsCommandForTest(cs.api, cs.store), args...) } -func (s *commitsSuite) setup(c *gc.C) *gomock.Controller { +func (cs *commitsSuite) setup(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.api = mocks.NewMockCommitsCommandAPI(ctrl) - s.api.EXPECT().Close() + cs.api = mocks.NewMockCommitsCommandAPI(ctrl) + cs.api.EXPECT().Close() return ctrl } diff --git a/cmd/juju/model/mocks/commits_mock.go b/cmd/juju/model/mocks/commits_mock.go index 0f21de96460..0c8c2d7b68b 100644 --- a/cmd/juju/model/mocks/commits_mock.go +++ b/cmd/juju/model/mocks/commits_mock.go @@ -8,7 +8,6 @@ import ( gomock "github.com/golang/mock/gomock" model "github.com/juju/juju/core/model" reflect "reflect" - time "time" ) // MockCommitsCommandAPI is a mock of CommitsCommandAPI interface @@ -49,16 +48,16 @@ func (mr *MockCommitsCommandAPIMockRecorder) Close() *gomock.Call { } // ListCommits mocks base method -func (m *MockCommitsCommandAPI) ListCommits(arg0 func(time.Time) string) ([]model.GenerationCommit, error) { +func (m *MockCommitsCommandAPI) ListCommits() ([]model.GenerationCommit, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListCommits", arg0) + ret := m.ctrl.Call(m, "ListCommits") ret0, _ := ret[0].([]model.GenerationCommit) ret1, _ := ret[1].(error) return ret0, ret1 } // ListCommits indicates an expected call of ListCommits -func (mr *MockCommitsCommandAPIMockRecorder) ListCommits(arg0 interface{}) *gomock.Call { +func (mr *MockCommitsCommandAPIMockRecorder) ListCommits() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCommits", reflect.TypeOf((*MockCommitsCommandAPI)(nil).ListCommits), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListCommits", reflect.TypeOf((*MockCommitsCommandAPI)(nil).ListCommits)) } diff --git a/cmd/juju/model/mocks/showcommit_mock.go b/cmd/juju/model/mocks/showcommit_mock.go index 293b5b31666..7a8287bcc6b 100644 --- a/cmd/juju/model/mocks/showcommit_mock.go +++ b/cmd/juju/model/mocks/showcommit_mock.go @@ -8,7 +8,6 @@ import ( gomock "github.com/golang/mock/gomock" model "github.com/juju/juju/core/model" reflect "reflect" - time "time" ) // MockShowCommitCommandAPI is a mock of ShowCommitCommandAPI interface @@ -49,16 +48,16 @@ func (mr *MockShowCommitCommandAPIMockRecorder) Close() *gomock.Call { } // ShowCommit mocks base method -func (m *MockShowCommitCommandAPI) ShowCommit(arg0 func(time.Time) string, arg1 int) (model.GenerationCommit, error) { +func (m *MockShowCommitCommandAPI) ShowCommit(arg0 int) (model.GenerationCommit, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ShowCommit", arg0, arg1) + ret := m.ctrl.Call(m, "ShowCommit", arg0) ret0, _ := ret[0].(model.GenerationCommit) ret1, _ := ret[1].(error) return ret0, ret1 } // ShowCommit indicates an expected call of ShowCommit -func (mr *MockShowCommitCommandAPIMockRecorder) ShowCommit(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockShowCommitCommandAPIMockRecorder) ShowCommit(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShowCommit", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).ShowCommit), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShowCommit", reflect.TypeOf((*MockShowCommitCommandAPI)(nil).ShowCommit), arg0) } diff --git a/cmd/juju/model/package_test.go b/cmd/juju/model/package_test.go index 8c2489e5a4e..02372b510b1 100644 --- a/cmd/juju/model/package_test.go +++ b/cmd/juju/model/package_test.go @@ -31,7 +31,7 @@ type generationBaseSuite struct { func (s *generationBaseSuite) SetUpTest(c *gc.C) { s.FakeJujuXDGDataHomeSuite.SetUpTest(c) - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) s.store = jujuclient.NewMemStore() s.store.CurrentControllerName = "testing" s.store.Controllers["testing"] = jujuclient.ControllerDetails{} diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index 128c8486991..e2ddb1369da 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -4,13 +4,11 @@ package model import ( - "os" - "strconv" - "time" - "github.com/juju/cmd" "github.com/juju/errors" "github.com/juju/gnuflag" + "os" + "strconv" "github.com/juju/juju/api/modelgeneration" jujucmd "github.com/juju/juju/cmd" @@ -64,7 +62,7 @@ type ShowCommitCommandAPI interface { Close() error // ShowCommit shows the branches which were committed - ShowCommit(func(time.Time) string, int) (model.GenerationCommit, error) + ShowCommit(int) (model.GenerationCommit, error) } // NewShowCommitCommand wraps NewShowCommitCommand with sane model settings. @@ -136,10 +134,7 @@ func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { } defer func() { _ = client.Close() }() - formatTime := func(t time.Time) string { - return common.FormatTime(&t, c.isoTime) - } - cmt, err := client.ShowCommit(formatTime, c.generationId) + cmt, err := client.ShowCommit(c.generationId) if err != nil { return err } @@ -149,11 +144,12 @@ func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { // Run implements the meaty part of the cmd.Command interface. func (c *ShowCommitCommand) getFormattedOutput(gcm model.GenerationCommit) formattedShowCommit { applications := map[string]formattedShowCommitApplications{gcm.BranchName: {gcm.Applications}} + commit := formattedShowCommit{ Branch: applications, - CommittedAt: gcm.Completed, + CommittedAt: common.FormatTime(&gcm.Completed, c.isoTime), CommittedBy: gcm.CompletedBy, - Created: gcm.Created, + Created: common.FormatTime(&gcm.Created, c.isoTime), CreatedBy: gcm.CreatedBy, } return commit diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go index 221f1f234f8..9eab7d8a9cd 100644 --- a/cmd/juju/model/showcommit_test.go +++ b/cmd/juju/model/showcommit_test.go @@ -7,7 +7,7 @@ import ( "github.com/golang/mock/gomock" "github.com/pkg/errors" gc "gopkg.in/check.v1" - "regexp" + "time" "github.com/juju/cmd" "github.com/juju/cmd/cmdtesting" @@ -48,9 +48,9 @@ func (s *showCommitsSuite) TestInitMoreArgs(c *gc.C) { } func (s *showCommitsSuite) getGenerationCommitValue() coremodel.GenerationCommit { values := coremodel.GenerationCommit{ - Completed: "0001-01-01", + Completed: time.Unix(12345, 0), CompletedBy: "test-user", - Created: "0001-01-00", + Created: time.Unix(12345, 0), CreatedBy: "test-user", GenerationId: 1, BranchName: "bla", @@ -63,42 +63,7 @@ func (s *showCommitsSuite) getGenerationCommitValue() coremodel.GenerationCommit return values } -func (s *showCommitsSuite) TestRunCommandJsonOutput(c *gc.C) { - defer s.setup(c).Finish() - result := s.getGenerationCommitValue() - unwrap := regexp.MustCompile(`[\s+\n]`) - expected := unwrap.ReplaceAllLiteralString(` -{ - "branch": { - "bla": { - "applications": [ - { - "ApplicationName": "redis", - "UnitsTracking": [ - "redis/0" - ], - "ConfigChanges": { - "databases": 8 - } - } - ] - } - }, - "committed-at": "0001-01-01", - "committed-by": "test-user", - "created": "0001-01-00", - "created-by": "test-user" -} -`, "") - expected = expected + "\n" - s.api.EXPECT().ShowCommit(gomock.Any(), 1).Return(result, nil) - ctx, err := s.runCommand(c, "1", "--format=json") - c.Assert(err, jc.ErrorIsNil) - output := cmdtesting.Stdout(ctx) - c.Assert(output, gc.Equals, expected) -} - -func (s *showCommitsSuite) TestRunCommandYamlOutput(c *gc.C) { +func (s *showCommitsSuite) TestYamlOutput(c *gc.C) { defer s.setup(c).Finish() result := s.getGenerationCommitValue() expected := ` @@ -110,22 +75,21 @@ branch: - redis/0 config: databases: 8 -committed-at: "0001-01-01" +committed-at: 01 Jan 1970 04:25:45+01:00 committed-by: test-user -created: 0001-01-00 +created: 01 Jan 1970 04:25:45+01:00 created-by: test-user `[1:] - s.api.EXPECT().ShowCommit(gomock.Any(), 1).Return(result, nil) + s.api.EXPECT().ShowCommit(1).Return(result, nil) ctx, err := s.runCommand(c, "1", "--format=yaml") c.Assert(err, jc.ErrorIsNil) - output := cmdtesting.Stdout(ctx) - c.Assert(output, gc.Matches, expected) + c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) } func (s *showCommitsSuite) TestRunCommandAPIError(c *gc.C) { defer s.setup(c).Finish() - s.api.EXPECT().ShowCommit(gomock.Any(), gomock.Any()).Return(coremodel.GenerationCommit{}, errors.New("boom")) + s.api.EXPECT().ShowCommit(gomock.Any()).Return(coremodel.GenerationCommit{}, errors.New("boom")) _, err := s.runCommand(c, "1") c.Assert(err, gc.ErrorMatches, "boom") diff --git a/cmd/juju/status/status_internal_test.go b/cmd/juju/status/status_internal_test.go index e7cb49cf1d7..0e452aca78e 100644 --- a/cmd/juju/status/status_internal_test.go +++ b/cmd/juju/status/status_internal_test.go @@ -165,7 +165,7 @@ func (ctx *context) setAgentPresence(c *gc.C, p presence.Agent) *presence.Pinger func (s *StatusSuite) newContext(c *gc.C) *context { st := s.Environ.(testing.GetStater).GetStateInAPIServer() err := st.UpdateControllerConfig(map[string]interface{}{ - "features": []interface{}{feature.Generations}, + "features": []interface{}{feature.Branches}, }, nil) c.Assert(err, jc.ErrorIsNil) diff --git a/core/model/generation.go b/core/model/generation.go index daf1bff2c71..b9119910c4f 100644 --- a/core/model/generation.go +++ b/core/model/generation.go @@ -5,6 +5,7 @@ package model import ( "github.com/juju/errors" + "time" ) // TODO (manadart 2019-04-21) Change the nomenclature here to indicate "branch" @@ -89,13 +90,13 @@ type GenerationCommit struct { BranchName string `yaml:"branch"` // Created is the Unix timestamp at generation creation. - Completed string `yaml:"completed"` + Completed time.Time `yaml:"completed"` // Created is the user who created the generation. CompletedBy string `yaml:"completed-by"` // Created is the Unix timestamp at generation creation. - Created string `yaml:"created,omitempty"` + Created time.Time `yaml:"created,omitempty"` // Created is the user who created the generation. CreatedBy string `yaml:"created-by,omitempty"` diff --git a/environs/bootstrap/prepare.go b/environs/bootstrap/prepare.go index d09a8030520..3116d9770f9 100644 --- a/environs/bootstrap/prepare.go +++ b/environs/bootstrap/prepare.go @@ -225,7 +225,7 @@ func prepare( details.Password = args.AdminSecret details.LastKnownAccess = string(permission.SuperuserAccess) details.ModelUUID = cfg.UUID() - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { details.ActiveBranch = model.GenerationMaster } details.ControllerDetails.Cloud = args.Cloud.Name diff --git a/feature/flags.go b/feature/flags.go index 3c1f7988168..a08a3e3a736 100644 --- a/feature/flags.go +++ b/feature/flags.go @@ -47,8 +47,9 @@ const OldPresence = "old-presence" // Mongo-based lease store, rather than by the Raft FSM. const LegacyLeases = "legacy-leases" -// Generations will allow for model generation functionality to be used. -const Generations = "generations" +// Branches will allow for model generation functionality to be used. +// Historically we called this feature generations. There can be left overs where it is still called that. +const Branches = "branches" // MongoDbSnap tells Juju to install MongoDB as a snap, rather than installing // it from APT. diff --git a/jujuclient/models.go b/jujuclient/models.go index 04f067c96c4..033d2b33604 100644 --- a/jujuclient/models.go +++ b/jujuclient/models.go @@ -47,7 +47,7 @@ func ReadModelsFile(file string) (map[string]*ControllerModels, error) { if err := addModelType(models); err != nil { return nil, err } - if featureflag.Enabled(feature.Generations) { + if featureflag.Enabled(feature.Branches) { if err := addGeneration(models); err != nil { return nil, err } diff --git a/jujuclient/modelsfile_test.go b/jujuclient/modelsfile_test.go index 120b9ebb64e..c25da02b11e 100644 --- a/jujuclient/modelsfile_test.go +++ b/jujuclient/modelsfile_test.go @@ -111,7 +111,7 @@ func (s *ModelsFileSuite) TestReadEmptyFile(c *gc.C) { } func (s *ModelsFileSuite) TestMigrateLegacyLocal(c *gc.C) { - s.SetFeatureFlags(feature.Generations) + s.SetFeatureFlags(feature.Branches) err := ioutil.WriteFile(jujuclient.JujuModelsPath(), []byte(testLegacyModelsYAML), 0644) c.Assert(err, jc.ErrorIsNil) From b62da3db5e3462b839251cb8d73ad61bb960c5b6 Mon Sep 17 00:00:00 2001 From: nam Date: Tue, 12 Nov 2019 13:55:47 +0100 Subject: [PATCH 13/13] refactoring: incorporate renaming and reuse feedback --- api/modelgeneration/modelgeneration.go | 30 +++-- api/modelgeneration/modelgeneration_test.go | 2 +- .../client/modelgeneration/interface.go | 4 +- .../modelgeneration/mocks/package_mock.go | 20 ++-- .../client/modelgeneration/modelgeneration.go | 54 ++++----- .../facades/client/modelgeneration/shim.go | 12 +- apiserver/facades/schema.json | 113 ++++++------------ apiserver/params/params.go | 46 +------ cmd/juju/model/listcommits_test.go | 1 - cmd/juju/model/showcommit.go | 23 ++-- cmd/juju/model/showcommit_test.go | 31 +++-- core/model/generation.go | 19 +-- state/modelgeneration.go | 12 +- 13 files changed, 142 insertions(+), 225 deletions(-) diff --git a/api/modelgeneration/modelgeneration.go b/api/modelgeneration/modelgeneration.go index cf5305de96c..c4f3318633a 100644 --- a/api/modelgeneration/modelgeneration.go +++ b/api/modelgeneration/modelgeneration.go @@ -69,9 +69,9 @@ func (c *Client) CommitBranch(branchName string) (int, error) { return result.Result, nil } -// ListCommits lists the committed branches commits, +// ListCommits returns the details of all committed model branches. func (c *Client) ListCommits() (model.GenerationCommits, error) { - var result params.GenerationCommitResults + var result params.BranchResults err := c.facade.FacadeCall("ListCommits", nil, &result) if err != nil { return nil, errors.Trace(err) @@ -82,11 +82,9 @@ func (c *Client) ListCommits() (model.GenerationCommits, error) { return generationCommitsFromResults(result), nil } -// CommitBranch commits the branch with the input name to the model, -// effectively completing it and applying all branch changes across the model. -// The new generation ID of the model is returned. +// ShowCommit details of the branch with the input generation ID. func (c *Client) ShowCommit(generationId int) (model.GenerationCommit, error) { - var result params.GenerationCommitResult + var result params.GenerationResult arg := params.GenerationId{GenerationId: generationId} err := c.facade.FacadeCall("ShowCommit", arg, &result) if err != nil { @@ -158,7 +156,7 @@ func (c *Client) BranchInfo( arg.BranchNames = []string{branchName} } - var result params.GenerationResults + var result params.BranchResults err := c.facade.FacadeCall("BranchInfo", arg, &result) if err != nil { return nil, errors.Trace(err) @@ -176,7 +174,7 @@ func argForBranch(branchName string) params.BranchArg { } func generationInfoFromResult( - results params.GenerationResults, detailed bool, formatTime func(time.Time) string, + results params.BranchResults, detailed bool, formatTime func(time.Time) string, ) model.GenerationSummaries { summaries := make(model.GenerationSummaries) for _, res := range results.Generations { @@ -204,9 +202,9 @@ func generationInfoFromResult( return summaries } -func generationCommitsFromResults(results params.GenerationCommitResults) model.GenerationCommits { - commits := make(model.GenerationCommits, len(results.GenerationCommits)) - for i, gen := range results.GenerationCommits { +func generationCommitsFromResults(results params.BranchResults) model.GenerationCommits { + commits := make(model.GenerationCommits, len(results.Generations)) + for i, gen := range results.Generations { commits[i] = model.GenerationCommit{ GenerationId: gen.GenerationId, Completed: time.Unix(gen.Completed, 0), @@ -217,14 +215,14 @@ func generationCommitsFromResults(results params.GenerationCommitResults) model. return commits } -func generationCommitFromResult(result params.GenerationCommitResult) model.GenerationCommit { - genCommit := result.GenerationCommit - appChanges := make([]model.GenerationCommitApplication, len(genCommit.Applications)) +func generationCommitFromResult(result params.GenerationResult) model.GenerationCommit { + genCommit := result.Generation + appChanges := make([]model.GenerationApplication, len(genCommit.Applications)) for i, a := range genCommit.Applications { - app := model.GenerationCommitApplication{ + app := model.GenerationApplication{ ApplicationName: a.ApplicationName, ConfigChanges: a.ConfigChanges, - UnitsTracking: a.UnitsTracking, + UnitDetail: &model.GenerationUnits{UnitsTracking: a.UnitsTracking}, } appChanges[i] = app } diff --git a/api/modelgeneration/modelgeneration_test.go b/api/modelgeneration/modelgeneration_test.go index a6057c2082f..0cb27868fcd 100644 --- a/api/modelgeneration/modelgeneration_test.go +++ b/api/modelgeneration/modelgeneration_test.go @@ -127,7 +127,7 @@ func (s *modelGenerationSuite) TestHasActiveBranch(c *gc.C) { func (s *modelGenerationSuite) TestBranchInfo(c *gc.C) { defer s.setUpMocks(c).Finish() - resultSource := params.GenerationResults{Generations: []params.Generation{{ + resultSource := params.BranchResults{Generations: []params.Generation{{ BranchName: "new-branch", Created: time.Time{}.Unix(), CreatedBy: "test-user", diff --git a/apiserver/facades/client/modelgeneration/interface.go b/apiserver/facades/client/modelgeneration/interface.go index 2bda20d6942..a7320d09b1f 100644 --- a/apiserver/facades/client/modelgeneration/interface.go +++ b/apiserver/facades/client/modelgeneration/interface.go @@ -26,8 +26,8 @@ type Model interface { AddBranch(string, string) error Branch(string) (Generation, error) Branches() ([]Generation, error) - CommittedBranch(int) (Generation, error) - CommittedBranches() ([]Generation, error) + Generation(int) (Generation, error) + Generations() ([]Generation, error) } // ModelCache describes a cached model used by the model generation API. diff --git a/apiserver/facades/client/modelgeneration/mocks/package_mock.go b/apiserver/facades/client/modelgeneration/mocks/package_mock.go index 83f285248bf..591b1813686 100644 --- a/apiserver/facades/client/modelgeneration/mocks/package_mock.go +++ b/apiserver/facades/client/modelgeneration/mocks/package_mock.go @@ -148,34 +148,34 @@ func (mr *MockModelMockRecorder) Branches() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Branches", reflect.TypeOf((*MockModel)(nil).Branches)) } -// CommittedBranch mocks base method -func (m *MockModel) CommittedBranch(arg0 int) (modelgeneration.Generation, error) { +// Generation mocks base method +func (m *MockModel) Generation(arg0 int) (modelgeneration.Generation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CommittedBranch", arg0) + ret := m.ctrl.Call(m, "Generation", arg0) ret0, _ := ret[0].(modelgeneration.Generation) ret1, _ := ret[1].(error) return ret0, ret1 } -// CommittedBranch indicates an expected call of CommittedBranch +// Generation indicates an expected call of Generation func (mr *MockModelMockRecorder) CommittedBranch(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CommittedBranch", reflect.TypeOf((*MockModel)(nil).CommittedBranch), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Generation", reflect.TypeOf((*MockModel)(nil).Generation), arg0) } -// CommittedBranches mocks base method -func (m *MockModel) CommittedBranches() ([]modelgeneration.Generation, error) { +// Generations mocks base method +func (m *MockModel) Generations() ([]modelgeneration.Generation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CommittedBranches") + ret := m.ctrl.Call(m, "Generations") ret0, _ := ret[0].([]modelgeneration.Generation) ret1, _ := ret[1].(error) return ret0, ret1 } -// CommittedBranches indicates an expected call of CommittedBranches +// Generations indicates an expected call of Generations func (mr *MockModelMockRecorder) CommittedBranches() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CommittedBranches", reflect.TypeOf((*MockModel)(nil).CommittedBranches)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Generations", reflect.TypeOf((*MockModel)(nil).Generations)) } // ModelTag mocks base method diff --git a/apiserver/facades/client/modelgeneration/modelgeneration.go b/apiserver/facades/client/modelgeneration/modelgeneration.go index 26176708c5a..bb33c9174cd 100644 --- a/apiserver/facades/client/modelgeneration/modelgeneration.go +++ b/apiserver/facades/client/modelgeneration/modelgeneration.go @@ -251,8 +251,8 @@ func (api *API) AbortBranch(arg params.BranchArg) (params.ErrorResult, error) { // master generation. // An error is returned if no in-flight branch matching in input is found. func (api *API) BranchInfo( - args params.BranchInfoArgs) (params.GenerationResults, error) { - result := params.GenerationResults{} + args params.BranchInfoArgs) (params.BranchResults, error) { + result := params.BranchResults{} isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -270,19 +270,19 @@ func (api *API) BranchInfo( branches = make([]Generation, len(args.BranchNames)) for i, name := range args.BranchNames { if branches[i], err = api.model.Branch(name); err != nil { - return generationResultsError(err) + return branchResultsError(err) } } } else { if branches, err = api.model.Branches(); err != nil { - return generationResultsError(err) + return branchResultsError(err) } } results := make([]params.Generation, len(branches)) for i, b := range branches { if results[i], err = api.oneBranchInfo(b, args.Detailed); err != nil { - return generationResultsError(err) + return branchResultsError(err) } } result.Generations = results @@ -292,8 +292,8 @@ func (api *API) BranchInfo( // ShowCommit will return details a commit given by its generationId // An error is returned if either no branch can be found corresponding to the generation id. // Or the generation id given is below 1. -func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) { - result := params.GenerationCommitResult{} +func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationResult, error) { + result := params.GenerationResult{} isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -304,10 +304,10 @@ func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResu } if arg.GenerationId < 1 { err := errors.Errorf("supplied generation id has to be higher than 0") - return generationCommitResultError(err) + return generationResultError(err) } - branch, err := api.model.CommittedBranch(arg.GenerationId) + branch, err := api.model.Generation(arg.GenerationId) if err != nil { result.Error = common.ServerError(err) return result, nil @@ -315,17 +315,17 @@ func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResu generationCommit, err := api.getGenerationCommit(branch) if err != nil { - return generationCommitResultError(err) + return generationResultError(err) } - result.GenerationCommit = generationCommit + result.Generation = generationCommit return result, nil } // ListCommits will return the commits, hence only branches with generation_id higher than 0 -func (api *API) ListCommits() (params.GenerationCommitResults, error) { - var result params.GenerationCommitResults +func (api *API) ListCommits() (params.BranchResults, error) { + var result params.BranchResults isModelAdmin, err := api.hasAdminAccess() if err != nil { @@ -336,13 +336,13 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) { } var branches []Generation - if branches, err = api.model.CommittedBranches(); err != nil { - return generationCommitResultsError(err) + if branches, err = api.model.Generations(); err != nil { + return branchResultsError(err) } - results := make([]params.GenerationCommit, len(branches)) + results := make([]params.Generation, len(branches)) for i, b := range branches { - gen := params.GenerationCommit{ + gen := params.Generation{ BranchName: b.BranchName(), Completed: b.Completed(), CompletedBy: b.CompletedBy(), @@ -351,7 +351,7 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) { results[i] = gen } - result.GenerationCommits = results + result.Generations = results return result, nil } @@ -403,12 +403,12 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati }, nil } -func (api *API) getGenerationCommit(branch Generation) (params.GenerationCommit, error) { +func (api *API) getGenerationCommit(branch Generation) (params.Generation, error) { generation, err := api.oneBranchInfo(branch, true) if err != nil { - return params.GenerationCommit{}, errors.Trace(err) + return params.Generation{}, errors.Trace(err) } - return params.GenerationCommit{ + return params.Generation{ BranchName: branch.BranchName(), Completed: branch.Completed(), CompletedBy: branch.CompletedBy(), @@ -443,16 +443,12 @@ func (api *API) HasActiveBranch(arg params.BranchArg) (params.BoolResult, error) return result, nil } -func generationResultsError(err error) (params.GenerationResults, error) { - return params.GenerationResults{Error: common.ServerError(err)}, nil -} - -func generationCommitResultsError(err error) (params.GenerationCommitResults, error) { - return params.GenerationCommitResults{Error: common.ServerError(err)}, nil +func branchResultsError(err error) (params.BranchResults, error) { + return params.BranchResults{Error: common.ServerError(err)}, nil } -func generationCommitResultError(err error) (params.GenerationCommitResult, error) { - return params.GenerationCommitResult{Error: common.ServerError(err)}, nil +func generationResultError(err error) (params.GenerationResult, error) { + return params.GenerationResult{Error: common.ServerError(err)}, nil } func intResultsError(err error) (params.IntResult, error) { diff --git a/apiserver/facades/client/modelgeneration/shim.go b/apiserver/facades/client/modelgeneration/shim.go index 83e48caebbd..6b8b81262da 100644 --- a/apiserver/facades/client/modelgeneration/shim.go +++ b/apiserver/facades/client/modelgeneration/shim.go @@ -37,17 +37,17 @@ func (g *modelShim) Branches() ([]Generation, error) { return res, nil } -// CommittedBranch wraps the state model CommittedBranch method, +// Generation wraps the state model Generation method, // returning the locally defined Generation interface. -func (g *modelShim) CommittedBranch(id int) (Generation, error) { - m, err := g.Model.CommittedBranch(id) +func (g *modelShim) Generation(id int) (Generation, error) { + m, err := g.Model.Generation(id) return m, errors.Trace(err) } -// Branches wraps the state model CommittedBranches method, +// Branches wraps the state model Generations method, // returning a collection of the Generation interface. -func (g *modelShim) CommittedBranches() ([]Generation, error) { - branches, err := g.Model.CommittedBranches() +func (g *modelShim) Generations() ([]Generation, error) { + branches, err := g.Model.Generations() if err != nil { return nil, errors.Trace(err) } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index db36a033dbd..2ed327711f0 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -23070,7 +23070,7 @@ "$ref": "#/definitions/BranchInfoArgs" }, "Result": { - "$ref": "#/definitions/GenerationResults" + "$ref": "#/definitions/BranchResults" } } }, @@ -23100,7 +23100,7 @@ "type": "object", "properties": { "Result": { - "$ref": "#/definitions/GenerationCommitResults" + "$ref": "#/definitions/BranchResults" } } }, @@ -23111,7 +23111,7 @@ "$ref": "#/definitions/GenerationId" }, "Result": { - "$ref": "#/definitions/GenerationCommitResult" + "$ref": "#/definitions/GenerationResult" } } }, @@ -23174,6 +23174,24 @@ "detailed" ] }, + "BranchResults": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/Error" + }, + "generations": { + "type": "array", + "items": { + "$ref": "#/definitions/Generation" + } + } + }, + "additionalProperties": false, + "required": [ + "generations" + ] + }, "BranchTrackArg": { "type": "object", "properties": { @@ -23269,11 +23287,20 @@ "branch": { "type": "string" }, + "completed": { + "type": "integer" + }, + "completed-by": { + "type": "string" + }, "created": { "type": "integer" }, "created-by": { "type": "string" + }, + "generation-id": { + "type": "integer" } }, "additionalProperties": false, @@ -23322,75 +23349,6 @@ "config" ] }, - "GenerationCommit": { - "type": "object", - "properties": { - "applications": { - "type": "array", - "items": { - "$ref": "#/definitions/GenerationApplication" - } - }, - "branch": { - "type": "string" - }, - "completed": { - "type": "integer" - }, - "completed-by": { - "type": "string" - }, - "created": { - "type": "integer" - }, - "created-by": { - "type": "string" - }, - "generation-id": { - "type": "integer" - } - }, - "additionalProperties": false, - "required": [ - "branch", - "completed", - "completed-by", - "generation-id" - ] - }, - "GenerationCommitResult": { - "type": "object", - "properties": { - "error": { - "$ref": "#/definitions/Error" - }, - "generation-commit": { - "$ref": "#/definitions/GenerationCommit" - } - }, - "additionalProperties": false, - "required": [ - "generation-commit" - ] - }, - "GenerationCommitResults": { - "type": "object", - "properties": { - "error": { - "$ref": "#/definitions/Error" - }, - "generation-commit": { - "type": "array", - "items": { - "$ref": "#/definitions/GenerationCommit" - } - } - }, - "additionalProperties": false, - "required": [ - "generation-commit" - ] - }, "GenerationId": { "type": "object", "properties": { @@ -23403,22 +23361,19 @@ "generation-id" ] }, - "GenerationResults": { + "GenerationResult": { "type": "object", "properties": { "error": { "$ref": "#/definitions/Error" }, - "generations": { - "type": "array", - "items": { - "$ref": "#/definitions/Generation" - } + "generation": { + "$ref": "#/definitions/Generation" } }, "additionalProperties": false, "required": [ - "generations" + "generation" ] }, "IntResult": { diff --git a/apiserver/params/params.go b/apiserver/params/params.go index 517d9256a07..572d7129dd6 100644 --- a/apiserver/params/params.go +++ b/apiserver/params/params.go @@ -1238,38 +1238,22 @@ type Generation struct { // Created is the user who created the generation. CreatedBy string `json:"created-by"` - // Applications holds the collection of application changes - // made under this generation. - Applications []GenerationApplication `json:"applications"` -} - -// GenerationCommits represents a model generation's commit details. -type GenerationCommit struct { - // BranchName uniquely identifies a branch *amongst in-flight branches*. - BranchName string `json:"branch"` - // Completed is the Unix timestamp at generation completion/commit. - Completed int64 `json:"completed"` + Completed int64 `json:"completed,omitempty"` // CompletedBy is the user who committed/completed the generation. - CompletedBy string `json:"completed-by"` + CompletedBy string `json:"completed-by,omitempty"` // GenerationId is the id . - GenerationId int `json:"generation-id"` - - // Created is the Unix timestamp at generation creation. - Created int64 `json:"created,omitempty"` - - // CreatedBy is the user who created the generation. - CreatedBy string `json:"created-by,omitempty"` + GenerationId int `json:"generation-id,omitempty"` // Applications holds the collection of application changes // made under this generation. - Applications []GenerationApplication `json:"applications,omitempty"` + Applications []GenerationApplication `json:"applications"` } -// GenerationResults transports a collection of generation details. -type GenerationResults struct { +// BranchResults transports a collection of generation details. +type BranchResults struct { // Generations holds the details of the requested generations. Generations []Generation `json:"generations"` @@ -1286,24 +1270,6 @@ type GenerationResult struct { Error *Error `json:"error,omitempty"` } -// GenerationCommitResults transports a collection of GenerationCommit detail. -type GenerationCommitResults struct { - // Generation holds the details of the requested generation. - GenerationCommits []GenerationCommit `json:"generation-commit"` - - // Error holds the value of any error that occurred processing the request. - Error *Error `json:"error,omitempty"` -} - -// GenerationCommitResult transports a GenerationCommit detail. -type GenerationCommitResult struct { - // Generation holds the details of the requested generation. - GenerationCommit GenerationCommit `json:"generation-commit"` - - // Error holds the value of any error that occurred processing the request. - Error *Error `json:"error,omitempty"` -} - // CharmProfilingInfoResult contains the result based on ProfileInfoArg values // to update profiles on a machine. type CharmProfilingInfoResult struct { diff --git a/cmd/juju/model/listcommits_test.go b/cmd/juju/model/listcommits_test.go index 83bd14c1dcb..9c021d3f7b0 100644 --- a/cmd/juju/model/listcommits_test.go +++ b/cmd/juju/model/listcommits_test.go @@ -4,7 +4,6 @@ package model_test import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pkg/errors" gc "gopkg.in/check.v1" diff --git a/cmd/juju/model/showcommit.go b/cmd/juju/model/showcommit.go index e2ddb1369da..290a388e9a8 100644 --- a/cmd/juju/model/showcommit.go +++ b/cmd/juju/model/showcommit.go @@ -141,28 +141,27 @@ func (c *ShowCommitCommand) Run(ctx *cmd.Context) error { return errors.Trace(c.out.Write(ctx, c.getFormattedOutput(cmt))) } -// Run implements the meaty part of the cmd.Command interface. func (c *ShowCommitCommand) getFormattedOutput(gcm model.GenerationCommit) formattedShowCommit { applications := map[string]formattedShowCommitApplications{gcm.BranchName: {gcm.Applications}} commit := formattedShowCommit{ - Branch: applications, - CommittedAt: common.FormatTime(&gcm.Completed, c.isoTime), - CommittedBy: gcm.CompletedBy, - Created: common.FormatTime(&gcm.Created, c.isoTime), - CreatedBy: gcm.CreatedBy, + BranchApplication: applications, + CommittedAt: common.FormatTime(&gcm.Completed, c.isoTime), + CommittedBy: gcm.CompletedBy, + Created: common.FormatTime(&gcm.Created, c.isoTime), + CreatedBy: gcm.CreatedBy, } return commit } type formattedShowCommit struct { - Branch map[string]formattedShowCommitApplications `json:"branch" yaml:"branch"` - CommittedAt string `json:"committed-at" yaml:"committed-at"` - CommittedBy string `json:"committed-by" yaml:"committed-by"` - Created string `json:"created" yaml:"created"` - CreatedBy string `json:"created-by" yaml:"created-by"` + BranchApplication map[string]formattedShowCommitApplications `json:"branch" yaml:"branch"` + CommittedAt string `json:"committed-at" yaml:"committed-at"` + CommittedBy string `json:"committed-by" yaml:"committed-by"` + Created string `json:"created" yaml:"created"` + CreatedBy string `json:"created-by" yaml:"created-by"` } type formattedShowCommitApplications struct { - Applications []model.GenerationCommitApplication `json:"applications" yaml:"applications"` + Applications []model.GenerationApplication `json:"applications" yaml:"applications"` } diff --git a/cmd/juju/model/showcommit_test.go b/cmd/juju/model/showcommit_test.go index 9eab7d8a9cd..5860c771332 100644 --- a/cmd/juju/model/showcommit_test.go +++ b/cmd/juju/model/showcommit_test.go @@ -54,10 +54,16 @@ func (s *showCommitsSuite) getGenerationCommitValue() coremodel.GenerationCommit CreatedBy: "test-user", GenerationId: 1, BranchName: "bla", - Applications: []coremodel.GenerationCommitApplication{{ + Applications: []coremodel.GenerationApplication{{ ApplicationName: "redis", - UnitsTracking: []string{"redis/0"}, - ConfigChanges: map[string]interface{}{"databases": 8}, + UnitDetail: &coremodel.GenerationUnits{ + UnitsTracking: []string{"redis/0", "redis/1", "redis/2"}}, + ConfigChanges: map[string]interface{}{"databases": 8}, + }, { + ApplicationName: "mysql", + UnitDetail: &coremodel.GenerationUnits{ + UnitsTracking: []string{"mysql/0", "mysql/1", "mysql/2"}}, + ConfigChanges: map[string]interface{}{"connection": "localhost"}, }}, } return values @@ -72,16 +78,27 @@ branch: applications: - application: redis units: - - redis/0 + tracking: + - redis/0 + - redis/1 + - redis/2 config: databases: 8 -committed-at: 01 Jan 1970 04:25:45+01:00 + - application: mysql + units: + tracking: + - mysql/0 + - mysql/1 + - mysql/2 + config: + connection: localhost +committed-at: 1970-01-01 03:25:45Z committed-by: test-user -created: 01 Jan 1970 04:25:45+01:00 +created: 1970-01-01 03:25:45Z created-by: test-user `[1:] s.api.EXPECT().ShowCommit(1).Return(result, nil) - ctx, err := s.runCommand(c, "1", "--format=yaml") + ctx, err := s.runCommand(c, "1", "--format=yaml", "--utc") c.Assert(err, jc.ErrorIsNil) c.Assert(cmdtesting.Stdout(ctx), gc.Equals, expected) } diff --git a/core/model/generation.go b/core/model/generation.go index b9119910c4f..cbc0902ecf9 100644 --- a/core/model/generation.go +++ b/core/model/generation.go @@ -46,7 +46,7 @@ type GenerationApplication struct { ApplicationName string `yaml:"application"` // UnitProgress is summary information about units tracking the branch. - UnitProgress string `yaml:"progress"` + UnitProgress string `yaml:"progress,omitempty"` // UnitDetail specifies which units are and are not tracking the branch. UnitDetail *GenerationUnits `yaml:"units,omitempty"` @@ -58,19 +58,6 @@ type GenerationApplication struct { ConfigChanges map[string]interface{} `yaml:"config"` } -// GenerationCommitApplication represents committed changes to an application -// made under a generation. -type GenerationCommitApplication struct { - // ApplicationsName is the name of the application. - ApplicationName string `yaml:"application"` - - // UnitDetail specifies which units are and are not tracking the branch. - UnitsTracking []string `yaml:"units,omitempty"` - - // Config changes are the committed changes from the generation - ConfigChanges map[string]interface{} `yaml:"config"` -} - // Generation represents detail of a model generation including config changes. type Generation struct { // Created is the formatted time at generation creation. @@ -84,7 +71,7 @@ type Generation struct { Applications []GenerationApplication `yaml:"applications"` } -// GenerationCommits represents a model generation's commit details. +// GenerationCommit represents a model generation's commit details. type GenerationCommit struct { // BranchName uniquely identifies a branch *amongst in-flight branches*. BranchName string `yaml:"branch"` @@ -106,7 +93,7 @@ type GenerationCommit struct { // Applications holds the collection of application changes // made under this generation. - Applications []GenerationCommitApplication `yaml:"applications,omitempty"` + Applications []GenerationApplication `yaml:"applications,omitempty"` } // GenerationSummaries is a type alias for a representation diff --git a/state/modelgeneration.go b/state/modelgeneration.go index b2ff0318d30..8927651f292 100644 --- a/state/modelgeneration.go +++ b/state/modelgeneration.go @@ -692,8 +692,8 @@ func insertGenerationTxnOps(id, branchName, userName string, now *time.Time) []t } } -// CommittedBranches returns all committed branches. -func (m *Model) CommittedBranches() ([]*Generation, error) { +// Generations returns all committed branches. +func (m *Model) Generations() ([]*Generation, error) { b, err := m.st.CommittedBranches() return b, errors.Trace(err) } @@ -721,7 +721,7 @@ func (st *State) Branches() ([]*Generation, error) { return branches, nil } -// CommittedBranches returns all committed branches. +// Generations returns all committed branches. func (st *State) CommittedBranches() ([]*Generation, error) { col, closer := st.db().GetCollection(generationsC) defer closer() @@ -746,9 +746,9 @@ func (m *Model) Branch(name string) (*Generation, error) { return gen, errors.Trace(err) } -// CommittedBranch retrieves the generation with the the input generation_id from the +// Generation retrieves the generation with the the input generation_id from the // collection of completed generations. -func (m *Model) CommittedBranch(id int) (*Generation, error) { +func (m *Model) Generation(id int) (*Generation, error) { gen, err := m.st.CommittedBranch(id) return gen, errors.Trace(err) } @@ -781,7 +781,7 @@ func (st *State) Branch(name string) (*Generation, error) { return newGeneration(st, doc), nil } -// CommittedBranch retrieves the generation with the the input id from the +// Generation retrieves the generation with the the input id from the // collection of completed generations. func (st *State) CommittedBranch(id int) (*Generation, error) { doc, err := st.getCommittedBranchDoc(id)