From 84557e1b487f6e628dfb1ca993634ba0e5db7600 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Wed, 17 Jul 2024 17:30:07 +0200 Subject: [PATCH 1/4] fix(machine): clean up controller units when removing controller machines This patch fixes an issue that appears when trying to remove a controller machine from an HA cluster. The fix is basically to reintroduce the cleanup that was used before https://github.com/juju/juju/pull/17653. --- state/cleanup.go | 59 +++++++++++++++++++++++++++++++++++++ state/cleanup_test.go | 66 +++++++++++++++++++++++++++++++++++------- state/enableha_test.go | 5 ++-- state/machine.go | 40 +++++++++++-------------- state/machine_test.go | 4 ++- 5 files changed, 137 insertions(+), 37 deletions(-) diff --git a/state/cleanup.go b/state/cleanup.go index 6a3459844a9..a3f48614d93 100644 --- a/state/cleanup.go +++ b/state/cleanup.go @@ -46,6 +46,7 @@ const ( cleanupDyingMachine cleanupKind = "dyingMachine" cleanupForceDestroyedMachine cleanupKind = "machine" cleanupForceRemoveMachine cleanupKind = "forceRemoveMachine" + cleanupEvacuateMachine cleanupKind = "evacuateMachine" cleanupAttachmentsForDyingStorage cleanupKind = "storageAttachments" cleanupAttachmentsForDyingVolume cleanupKind = "volumeAttachments" cleanupAttachmentsForDyingFilesystem cleanupKind = "filesystemAttachments" @@ -221,6 +222,8 @@ func (st *State) Cleanup() (err error) { err = st.cleanupForceRemoveMachine(doc.Prefix, args) case cleanupAttachmentsForDyingStorage: err = st.cleanupAttachmentsForDyingStorage(doc.Prefix, args) + case cleanupEvacuateMachine: + err = st.cleanupEvacuateMachine(doc.Prefix, args) case cleanupAttachmentsForDyingVolume: err = st.cleanupAttachmentsForDyingVolume(doc.Prefix) case cleanupAttachmentsForDyingFilesystem: @@ -1436,6 +1439,62 @@ func (st *State) cleanupForceRemoveMachine(machineId string, cleanupArgs []bson. return machine.Remove() } +// cleanupEvacuateMachine is initiated by machine.Destroy() to gracefully remove units +// from the machine before then kicking off machine destroy. +func (st *State) cleanupEvacuateMachine(machineId string, cleanupArgs []bson.Raw) error { + if len(cleanupArgs) > 0 { + return errors.Errorf("expected no arguments, got %d", len(cleanupArgs)) + } + + machine, err := st.Machine(machineId) + if errors.IsNotFound(err) { + return nil + } else if err != nil { + return errors.Trace(err) + } + if machine.Life() != Alive { + return nil + } + + units, err := machine.Units() + if err != nil { + return errors.Trace(err) + } + + if len(units) == 0 { + if err := machine.advanceLifecycle(Dying, false, false, 0); err != nil { + return errors.Trace(err) + } + return nil + } + + buildTxn := func(attempt int) ([]txn.Op, error) { + if attempt > 0 { + units, err = machine.Units() + if err != nil { + return nil, errors.Trace(err) + } + } + var ops []txn.Op + for _, unit := range units { + destroyOp := unit.DestroyOperation() + op, err := destroyOp.Build(attempt) + if err != nil && !errors.Is(err, jujutxn.ErrNoOperations) { + return nil, errors.Trace(err) + } + ops = append(ops, op...) + } + return ops, nil + } + + err = st.db().Run(buildTxn) + if err != nil { + return errors.Trace(err) + } + + return errors.Errorf("waiting for units to be removed from %s", machineId) +} + // cleanupContainers recursively calls cleanupForceDestroyedMachine on the supplied // machine's containers, and removes them from state entirely. func (st *State) cleanupContainers(machine *Machine, maxWait time.Duration) error { diff --git a/state/cleanup_test.go b/state/cleanup_test.go index f2c085a679e..9a3094358a0 100644 --- a/state/cleanup_test.go +++ b/state/cleanup_test.go @@ -6,6 +6,7 @@ package state_test import ( "bytes" "sort" + "strconv" "time" "github.com/juju/charm/v12" @@ -473,6 +474,56 @@ func (s *CleanupSuite) TestDestroyControllerMachineErrors(c *gc.C) { assertLife(c, manager, state.Alive) } +func (s *CleanupSuite) TestDestroyControllerMachineHAWithControllerCharm(c *gc.C) { + cons := constraints.Value{ + Mem: newUint64(100), + } + changes, err := s.State.EnableHA(3, cons, state.UbuntuBase("12.04"), nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(changes.Added, gc.HasLen, 3) + + ch := s.AddTestingCharmWithSeries(c, "juju-controller", "") + app := s.AddTestingApplicationForBase(c, state.UbuntuBase("12.04"), "controller", ch) + + var machines []*state.Machine + var units []*state.Unit + for i := 0; i < 3; i++ { + m, err := s.State.Machine(strconv.Itoa(i)) + c.Assert(err, jc.ErrorIsNil) + c.Assert(m.Jobs(), gc.DeepEquals, []state.MachineJob{ + state.JobHostUnits, + state.JobManageModel, + }) + + if i == 0 { + node, err := s.State.ControllerNode(m.Id()) + c.Assert(err, jc.ErrorIsNil) + node.SetHasVote(true) + } + + u, err := app.AddUnit(state.AddUnitParams{}) + c.Assert(err, jc.ErrorIsNil) + err = u.SetCharmURL(ch.URL()) + c.Assert(err, jc.ErrorIsNil) + err = u.AssignToMachine(m) + c.Assert(err, jc.ErrorIsNil) + + machines = append(machines, m) + units = append(units, u) + } + + for _, m := range machines { + assertLife(c, m, state.Alive) + } + for _, u := range units { + assertLife(c, u, state.Alive) + } + + s.assertDoesNotNeedCleanup(c) + err = machines[2].Destroy() + c.Assert(err, gc.ErrorMatches, ".* has unit .* assigned") +} + const dontWait = time.Duration(0) func (s *CleanupSuite) TestCleanupForceDestroyedMachineUnit(c *gc.C) { @@ -534,7 +585,6 @@ func (s *CleanupSuite) TestCleanupForceDestroyedControllerMachine(c *gc.C) { // The machine should no longer want the vote, should be forced to not have the vote, and forced to not be a // controller member anymore c.Assert(machine.Refresh(), jc.ErrorIsNil) - c.Check(machine.Life(), gc.Equals, state.Dying) node, err = s.State.ControllerNode(machine.Id()) c.Assert(err, jc.ErrorIsNil) c.Check(node.WantsVote(), jc.IsFalse) @@ -548,21 +598,15 @@ func (s *CleanupSuite) TestCleanupForceDestroyedControllerMachine(c *gc.C) { s.assertCleanupRuns(c) c.Assert(node.SetHasVote(false), jc.ErrorIsNil) // However, if we remove the vote, it can be cleaned up. - // ForceDestroy sets up a cleanupForceDestroyedMachine, which - // calls advanceLifecycle(Dead) which sets up a - // cleanupDyingMachine, which in turn creates a delayed - // cleanupForceRemoveMachine. - // Run the first two. - s.assertCleanupCountDirty(c, 2) - // After we've run the cleanup for the controller machine, the machine should be dead, and it should not be + // ForceDestroy sets up a cleanupEvacuateMachine, which will not + // add any other cleanup ops. + // After we've run the cleanup for the controller machine, the machine should be dying, and it should not be // present in the other documents. - assertLife(c, machine, state.Dead) + assertLife(c, machine, state.Dying) controllerIds, err = s.State.ControllerIds() c.Assert(err, jc.ErrorIsNil) sort.Strings(controllerIds) sort.Strings(changes.Added) - // Only the machines that were added should still be part of the controller - c.Check(controllerIds, gc.DeepEquals, changes.Added) } func (s *CleanupSuite) TestCleanupForceDestroyMachineCleansStorageAttachments(c *gc.C) { diff --git a/state/enableha_test.go b/state/enableha_test.go index 49ba19bcb1d..1413f6c4dd2 100644 --- a/state/enableha_test.go +++ b/state/enableha_test.go @@ -525,8 +525,9 @@ func (s *EnableHASuite) TestForceDestroyFromHA(c *gc.C) { err = m0.ForceDestroy(dontWait) c.Assert(err, jc.ErrorIsNil) c.Assert(m0.Refresh(), jc.ErrorIsNil) - // Could this actually get all the way to Dead? - c.Check(m0.Life(), gc.Equals, state.Dying) + // Force remove of controller machines will first clean up units and + // after that it will pass the machine life to Dying. + c.Check(m0.Life(), gc.Equals, state.Alive) c.Assert(node.Refresh(), jc.ErrorIsNil) c.Assert(node.WantsVote(), jc.IsFalse) } diff --git a/state/machine.go b/state/machine.go index a9e2d367009..7b0910181d8 100644 --- a/state/machine.go +++ b/state/machine.go @@ -639,31 +639,25 @@ func (m *Machine) forceDestroyOps(maxWait time.Duration) ([]txn.Op, error) { if len(controllerIds) <= 1 { return nil, errors.Errorf("controller %s is the only controller", m.Id()) } - // We set the machine to Dying if it isn't already dead. - var machineOp txn.Op - if m.Life() < Dead { - // Make sure we don't want the vote, and we are queued to be Dying. - // Since we are force deleting, life assert should be current machine's life. - machineOp = txn.Op{ - C: machinesC, - Id: m.doc.DocID, - Assert: bson.D{{"life", bson.D{{"$in", []Life{Alive, Dying}}}}}, - Update: bson.D{{"$set", bson.D{{"life", Dying}}}}, - } - } - controllerOp := txn.Op{ - C: controllersC, - Id: modelGlobalKey, - Assert: bson.D{{"controller-ids", controllerIds}}, - } - // Note that ForceDestroy does *not* cleanup the replicaset, so it might cause problems. - // However, we're letting the user handle times when the machine agent isn't running, etc. - // We may need to update the peergrouper for this. return []txn.Op{ - machineOp, - controllerOp, + { + C: machinesC, + Id: m.doc.DocID, + Assert: bson.D{ + {"life", Alive}, + advanceLifecycleUnitAsserts(m.doc.Principals), + }, + // To prevent race conditions, we remove the ability for new + // units to be deployed to the machine. + Update: bson.D{{"$pull", bson.D{{"jobs", JobHostUnits}}}}, + }, + { + C: controllersC, + Id: modelGlobalKey, + Assert: bson.D{{"controller-ids", controllerIds}}, + }, setControllerWantsVoteOp(m.st, m.Id(), false), - newCleanupOp(cleanupForceDestroyedMachine, m.doc.Id, maxWait), + newCleanupOp(cleanupEvacuateMachine, m.doc.Id), }, nil } else { // Make sure the machine doesn't become a manager while we're destroying it diff --git a/state/machine_test.go b/state/machine_test.go index 2a544a8ad04..e19ffdb2c4b 100644 --- a/state/machine_test.go +++ b/state/machine_test.go @@ -374,7 +374,9 @@ func (s *MachineSuite) TestLifeJobManageModelWithControllerCharm(c *gc.C) { err = m2.Refresh() c.Assert(err, jc.ErrorIsNil) - c.Assert(m2.Life(), gc.Equals, state.Dying) + // Force remove of controller machines will first clean up units and + // after that it will pass the machine life to Dying. + c.Assert(m2.Life(), gc.Equals, state.Alive) cn2, err := s.State.ControllerNode(m2.Id()) c.Assert(err, jc.ErrorIsNil) From 4ed2e921defab0a2628a363e98cc35ba25075d9a Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Wed, 10 Jul 2024 09:40:29 +0100 Subject: [PATCH 2/4] docs: add conventional commits guidelines to contributing.md Conventional commtis were mentioned in the TLDR of the document but not in the body. The section is added along with a link to the office juju spec on the matter. --- CONTRIBUTING.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a0dd28582f..edcd74ce114 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -301,6 +301,39 @@ with an environment variable: JUJU_NOTEST_MONGOJS=1 go test github.com/juju/juju/... ``` +Conventional commits +-------------------- + +Once you have written some code and have tested the changes, the next step is to +`git commit` it. For commit messages Juju follows the conventional commits guidelines specified [in this +document](https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/). +In short the commits should be of the following form: +``` +(optional ): + +[optional body] + +[optional footer(s)] +``` +- **Type:** The type describes the kind of change (e.g., feat, fix, docs, style, + refactor, test, chore). +- **Scope:** The scope indicates the part of the codebase affected (e.g., model, + api, cli). +- **Description:** The description briefly describes the change. It should not + end in any punctuation. +- **Body:** The body should be a detailed explanation of the change. It should + specify what has been changed and why and include descriptions of the + behaviour before and after. +- **Footer:** The footer includes information about breaking changes, issues + closed, etc. + +The type, scope and description should all begin with a lower case letter. None +of the lines can exceed 100 characters in length. + +There is a CI action on the Juju GitHub repository that will flag any syntactic +issues with your commit messages. The action is required to pass before code can +be merged so make sure to take a look once your PR is up. + Pushing ------- @@ -318,6 +351,9 @@ Go to the web page (https://github.com/$YOUR_GITHUB_USERNAME/juju) and hit the This creates a numbered pull request on the github site, where members of the Juju project can see and comment on the changes. +The title of the PR should match the form of the title of a conventional commit. +This can be the title of the most significant commit in the PR. + Make sure to add a clear description of why and what has been changed, and include the Launchpad bug number if one exists. From b8c291c8543fa9dbfca1123ed10cab660c43bad6 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 18 Jul 2024 17:04:25 +0100 Subject: [PATCH 3/4] docs: add conventional commits spec in markdown format This is a copy of the private to canonical doc found here: https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/edit --- CONTRIBUTING.md | 3 +- doc/conventional-commits.md | 153 ++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 doc/conventional-commits.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index edcd74ce114..9e872595e79 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,8 +305,7 @@ Conventional commits -------------------- Once you have written some code and have tested the changes, the next step is to -`git commit` it. For commit messages Juju follows the conventional commits guidelines specified [in this -document](https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/). +`git commit` it. For commit messages Juju follows [conventional commits guidelines](doc/conventional-commits.md). In short the commits should be of the following form: ``` (optional ): diff --git a/doc/conventional-commits.md b/doc/conventional-commits.md new file mode 100644 index 00000000000..3bc01b2ac1b --- /dev/null +++ b/doc/conventional-commits.md @@ -0,0 +1,153 @@ +The [Conventional Commits standard](https://www.conventionalcommits.org/en/v1.0.0/) is adopted for the Juju project with the +following structure: + +``` +(optional ): + +[optional body] + +[optional footer(s)] +``` + +- **type**: Describes the kind of change (e.g., feat, fix, docs, style, refactor, test, chore). + +|Type|Kind of change|Description| +|---|---|---| +|feat|Features|A new feature| +|fix|Bug fixes|A bug fix| +|docs|Documentation|Documentation only changes| +|style|Styles|Changes that do not affect the meaning of the code| +|refactor|Code refactoring|A code change that neither fixes a bug nor adds a feature| +|perf|Performance improvements|A code change that improves performance| +|test|Tests|Adding missing tests or correcting existing tests| +|build|Builds|Changes that affect the build system or external dependencies| +|ci|Continuous integration|Changes to our CI configuration files and scripts| +|chore|Chores|Necessary yet mundane/trivial changes (updating dependencies, merging through …)| +|revert|Reverts|Reverts a previous commit| + +- **scope**: A scope indicating the part of the codebase affected (e.g., model, api, cli). + - Can be Optional is it’s hard to define the scope +- **description**: A brief summary of the change. + - Must be provided after the BREAKING CHANGE:, describing what has changed about the API, e.g., BREAKING CHANGE: environment variables now take precedence over config files. + - Should use lowercase. + - Should not end in any punctuation. +- **body**: Detailed explanation of the change. + - Can be Optional for small/trivial `fix`, but NOT for other types. + - Could consist of several paragraphs. + - Explanation of change should details what was before, and what is after, and avoid contextual terms like 'now' + - Good body should be on the form: + * Before this commit `` + * After this commit `` +- **footer**: (Optional) Information about breaking changes, issues closed, etc. + - A commit can contain a footer which must consist of one or more [Git trailers](https://git-scm.com/docs/git-interpret-trailers). + - The footer must start at the first occurrence of a blank line, followed by a Git trailer. + - Each trailer must start on its own line, using the format ``. + - The trailer `` must be either BREAKING CHANGE or be one or more words, grouped by hyphens (e.g. Co-Authored-By, fixes, etc.). + - The trailer `` must be one of `:` or `#`, supporting both `Co-Authored-By: Thomas Miller ` and `Fixes #999`. + - The trailer `` must be present and can contain any characters, either on a single line, split over multiple lines, or split over multiple paragraphs. + + +Here is the reference example of a conventional commit message: + +``` +feat(api): add user authentication feature + +# scope: Indicates the part of the codebase affected +# Here, 'api' is specified as the scope + +# description: A brief summary of the change in lower-case +# "add user authentication feature" summarizes the change concisely + +This commit adds user authentication to the API. Users can now sign up, +log in, and log out. Passwords are hashed using bcrypt. Token-based  +authentication is implemented using JWT. + +# body: Detailed explanation of the change +# A more detailed explanation of what has been changed and why + +BREAKING CHANGE: The user authentication changes the login endpoint +from `/api/login` to `/api/v1/login`. All previous tokens are now invalid, +and users will need to reauthenticate. + +# footer: Information about breaking changes +# Here, the footer indicates a breaking change, describing what has changed +# about the API and how it impacts users + +Authored-By: Thomas Miller <[thomas.miller@canonical.com](mailto:thomas.miller@canonical.com)> + +Fixes #123 + +# footer: Issues closed or other information +# This footer references to and author and an issue that this commit closes. +``` + +The PR title, where a conventional commit is introduced, should be the same format as a first line of a commit description: `(optional ): `. In case this commit is core/base/most significant for this PR. + +### Footer and Multi-paragraph Body +### Juju-Specific Scopes + +Define specific scopes relevant to the Juju project to enhance clarity. Examples include: + +- model: Logical grouping of applications, representing an environment. +- controller: Manages and oversees multiple models, providing centralized management. +- charm: Encapsulated operational code for applications, defining how they are deployed and managed. +- application: Deployed instance of a charm, representing a running service. +- unit: Individual instance of an application, running on a machine. +- relation: Connection between different applications, enabling them to communicate and share data. +- bundle: Collection of applications and relations, defining a complex deployment setup. +- action: Scripted operation on an application or unit, allowing for specific tasks to be performed. +- machine: Physical or virtual host running units, part of the infrastructure. +- storage: Persistent data storage for units, ensuring data remains available across restarts. +- offer: Exposed application endpoint to other models, allowing cross-model relations. +- endpoint: Specific interaction point for a relation within an application. +- constraint: Resource specifications for machines and applications, such as CPU and memory requirements. +- user: Individual with access to the Juju environment, assigned roles and permissions. +- space: Logical network segmentation within models, aiding in network isolation and security. +- subnet: IP address range within a space, used for assigning IPs to machines and units. +- backup: Process of saving the state of a model or controller for recovery purposes. +- migration: Moving a model or application from one controller to another. +- resource: Files or data required by charms during deployment and operation. +- upgrade: Process of updating charms, applications, or Juju itself to newer versions. +- cloud: Infrastructure provider where models and applications are deployed, such as AWS, Azure, or OpenStack. +- credential: Authentication details for accessing cloud providers and resources. + +This list just illustrates the idea and suggestion of which single words should be used for scope definition. It is NOT complete and is NOT limited by these scopes. + +### Other Examples +``` +feat(networking): add juju networking support for all provider + + +``` +--- +``` +feat!: remove ticket list endpoint + +refers to JIRA-666 + +BREAKING CHANGES: ticket enpoints no longer supports list all entites. +``` +--- +``` +fix(migration): fix the universal migration to any major/minor version in future + + +``` +--- +``` +docs: add support for multi-version + tips windows + + +``` +--- +``` +refactor(cmr): remove all uclear and understandle code from CMR implementation + + +``` +--- +``` +fix(backup): fix backup db state issue + + +``` \ No newline at end of file From c2583f3b035f896c60a6654d626a14d500127bd7 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Fri, 19 Jul 2024 12:19:18 +0100 Subject: [PATCH 4/4] fix: auth for external users Auth for external user was broken because if the user is not local, we check their username against the logged in user name from state, and the local username we were checking it against was empty. This was because of a check against local users that would set their account detials to empty if they had no macaroons. The macaroons are not stored in the AccountDetails of the external user in this case, and so the username it was compared against was empty. This fix sets the account details to the username and macaroons (which may be empty). --- cmd/modelcmd/base.go | 16 +- cmd/modelcmd/base_test.go | 42 +++- cmd/modelcmd/mocks/api_mock.go | 391 +++++++++++++++++++++++++++++++++ cmd/modelcmd/package_test.go | 1 + 4 files changed, 439 insertions(+), 11 deletions(-) create mode 100644 cmd/modelcmd/mocks/api_mock.go diff --git a/cmd/modelcmd/base.go b/cmd/modelcmd/base.go index 9f982d0f177..e33a7f58403 100644 --- a/cmd/modelcmd/base.go +++ b/cmd/modelcmd/base.go @@ -226,16 +226,12 @@ func processAccountDetails(accountDetails *jujuclient.AccountDetails) *jujuclien } else { u := names.NewUserTag(accountDetails.User) if !u.IsLocal() { - if len(accountDetails.Macaroons) == 0 { - accountDetails = &jujuclient.AccountDetails{} - } else { - // If the account has macaroon set, use those to login - // to avoid an unnecessary auth round trip. - // Used for embedded commands. - accountDetails = &jujuclient.AccountDetails{ - User: u.Id(), - Macaroons: accountDetails.Macaroons, - } + // If the account has macaroon set, use those to login + // to avoid an unnecessary auth round trip. + // Used for embedded commands. + accountDetails = &jujuclient.AccountDetails{ + User: u.Id(), + Macaroons: accountDetails.Macaroons, } } } diff --git a/cmd/modelcmd/base_test.go b/cmd/modelcmd/base_test.go index 41ae0c80056..ec5e3214ac0 100644 --- a/cmd/modelcmd/base_test.go +++ b/cmd/modelcmd/base_test.go @@ -17,6 +17,7 @@ import ( cookiejar "github.com/juju/persistent-cookiejar" "github.com/juju/testing" jc "github.com/juju/testing/checkers" + "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" "gopkg.in/macaroon-bakery.v2/httpbakery" "gopkg.in/macaroon.v2" @@ -25,6 +26,7 @@ import ( apitesting "github.com/juju/juju/api/testing" "github.com/juju/juju/cloud" "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/cmd/modelcmd/mocks" "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" @@ -190,6 +192,42 @@ To access it run 'juju switch bar:admin/badmodel'.`, } } +func (s *BaseCommandSuite) setupMocks(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + return ctrl +} + +func (s *BaseCommandSuite) TestNewAPIRootExternalUser(c *gc.C) { + ctrl := s.setupMocks(c) + conn := mocks.NewMockConnection(ctrl) + apiOpen := func(info *api.Info, opts api.DialOpts) (api.Connection, error) { + return conn, nil + } + externalName := "alastair@external" + conn.EXPECT().AuthTag().Return(names.NewUserTag(externalName)).MinTimes(1) + conn.EXPECT().APIHostPorts() + conn.EXPECT().ServerVersion() + conn.EXPECT().Addr() + conn.EXPECT().IPAddr() + conn.EXPECT().PublicDNSName() + conn.EXPECT().ControllerAccess().MinTimes(1) + + s.store.Accounts["foo"] = jujuclient.AccountDetails{ + User: externalName, + } + + baseCmd := new(modelcmd.ModelCommandBase) + baseCmd.SetClientStore(s.store) + baseCmd.SetAPIOpen(apiOpen) + modelcmd.InitContexts(&cmd.Context{Stderr: io.Discard}, baseCmd) + modelcmd.SetRunStarted(baseCmd) + + c.Assert(baseCmd.SetModelIdentifier("foo:admin/badmodel", false), jc.ErrorIsNil) + + _, err := baseCmd.NewAPIRoot() + c.Assert(err, jc.ErrorIsNil) +} + type NewGetBootstrapConfigParamsFuncSuite struct { testing.IsolationSuite } @@ -462,7 +500,9 @@ func (s *BaseCommandSuite) TestProcessAccountDetails(c *gc.C) { input: jujuclient.AccountDetails{ User: names.NewUserTag("alice@wonderland.canonical.com").String(), }, - expectedOutput: jujuclient.AccountDetails{}, + expectedOutput: jujuclient.AccountDetails{ + User: names.NewUserTag("alice@wonderland.canonical.com").String(), + }, }} for i, test := range tests { c.Logf("running test case %d", i) diff --git a/cmd/modelcmd/mocks/api_mock.go b/cmd/modelcmd/mocks/api_mock.go new file mode 100644 index 00000000000..4d8fe92c87c --- /dev/null +++ b/cmd/modelcmd/mocks/api_mock.go @@ -0,0 +1,391 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/api (interfaces: Connection) +// +// Generated by this command: +// +// mockgen -package mocks -destination mocks/api_mock.go github.com/juju/juju/api Connection +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + http "net/http" + url "net/url" + reflect "reflect" + + base "github.com/juju/juju/api/base" + network "github.com/juju/juju/core/network" + proxy "github.com/juju/juju/proxy" + names "github.com/juju/names/v5" + version "github.com/juju/version/v2" + gomock "go.uber.org/mock/gomock" + httprequest "gopkg.in/httprequest.v1" + macaroon "gopkg.in/macaroon.v2" +) + +// MockConnection is a mock of Connection interface. +type MockConnection struct { + ctrl *gomock.Controller + recorder *MockConnectionMockRecorder +} + +// MockConnectionMockRecorder is the mock recorder for MockConnection. +type MockConnectionMockRecorder struct { + mock *MockConnection +} + +// NewMockConnection creates a new mock instance. +func NewMockConnection(ctrl *gomock.Controller) *MockConnection { + mock := &MockConnection{ctrl: ctrl} + mock.recorder = &MockConnectionMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockConnection) EXPECT() *MockConnectionMockRecorder { + return m.recorder +} + +// APICall mocks base method. +func (m *MockConnection) APICall(arg0 string, arg1 int, arg2, arg3 string, arg4, arg5 any) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "APICall", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(error) + return ret0 +} + +// APICall indicates an expected call of APICall. +func (mr *MockConnectionMockRecorder) APICall(arg0, arg1, arg2, arg3, arg4, arg5 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APICall", reflect.TypeOf((*MockConnection)(nil).APICall), arg0, arg1, arg2, arg3, arg4, arg5) +} + +// APIHostPorts mocks base method. +func (m *MockConnection) APIHostPorts() []network.MachineHostPorts { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "APIHostPorts") + ret0, _ := ret[0].([]network.MachineHostPorts) + return ret0 +} + +// APIHostPorts indicates an expected call of APIHostPorts. +func (mr *MockConnectionMockRecorder) APIHostPorts() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIHostPorts", reflect.TypeOf((*MockConnection)(nil).APIHostPorts)) +} + +// Addr mocks base method. +func (m *MockConnection) Addr() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Addr") + ret0, _ := ret[0].(string) + return ret0 +} + +// Addr indicates an expected call of Addr. +func (mr *MockConnectionMockRecorder) Addr() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Addr", reflect.TypeOf((*MockConnection)(nil).Addr)) +} + +// AuthTag mocks base method. +func (m *MockConnection) AuthTag() names.Tag { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthTag") + ret0, _ := ret[0].(names.Tag) + return ret0 +} + +// AuthTag indicates an expected call of AuthTag. +func (mr *MockConnectionMockRecorder) AuthTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthTag", reflect.TypeOf((*MockConnection)(nil).AuthTag)) +} + +// BakeryClient mocks base method. +func (m *MockConnection) BakeryClient() base.MacaroonDischarger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BakeryClient") + ret0, _ := ret[0].(base.MacaroonDischarger) + return ret0 +} + +// BakeryClient indicates an expected call of BakeryClient. +func (mr *MockConnectionMockRecorder) BakeryClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BakeryClient", reflect.TypeOf((*MockConnection)(nil).BakeryClient)) +} + +// BestFacadeVersion mocks base method. +func (m *MockConnection) BestFacadeVersion(arg0 string) int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BestFacadeVersion", arg0) + ret0, _ := ret[0].(int) + return ret0 +} + +// BestFacadeVersion indicates an expected call of BestFacadeVersion. +func (mr *MockConnectionMockRecorder) BestFacadeVersion(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BestFacadeVersion", reflect.TypeOf((*MockConnection)(nil).BestFacadeVersion), arg0) +} + +// Broken mocks base method. +func (m *MockConnection) Broken() <-chan struct{} { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Broken") + ret0, _ := ret[0].(<-chan struct{}) + return ret0 +} + +// Broken indicates an expected call of Broken. +func (mr *MockConnectionMockRecorder) Broken() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Broken", reflect.TypeOf((*MockConnection)(nil).Broken)) +} + +// Close mocks base method. +func (m *MockConnection) 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 *MockConnectionMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockConnection)(nil).Close)) +} + +// ConnectControllerStream mocks base method. +func (m *MockConnection) ConnectControllerStream(arg0 string, arg1 url.Values, arg2 http.Header) (base.Stream, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectControllerStream", arg0, arg1, arg2) + ret0, _ := ret[0].(base.Stream) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConnectControllerStream indicates an expected call of ConnectControllerStream. +func (mr *MockConnectionMockRecorder) ConnectControllerStream(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectControllerStream", reflect.TypeOf((*MockConnection)(nil).ConnectControllerStream), arg0, arg1, arg2) +} + +// ConnectStream mocks base method. +func (m *MockConnection) ConnectStream(arg0 string, arg1 url.Values) (base.Stream, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectStream", arg0, arg1) + ret0, _ := ret[0].(base.Stream) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConnectStream indicates an expected call of ConnectStream. +func (mr *MockConnectionMockRecorder) ConnectStream(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectStream", reflect.TypeOf((*MockConnection)(nil).ConnectStream), arg0, arg1) +} + +// Context mocks base method. +func (m *MockConnection) Context() context.Context { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Context") + ret0, _ := ret[0].(context.Context) + return ret0 +} + +// Context indicates an expected call of Context. +func (mr *MockConnectionMockRecorder) Context() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Context", reflect.TypeOf((*MockConnection)(nil).Context)) +} + +// ControllerAccess mocks base method. +func (m *MockConnection) ControllerAccess() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerAccess") + ret0, _ := ret[0].(string) + return ret0 +} + +// ControllerAccess indicates an expected call of ControllerAccess. +func (mr *MockConnectionMockRecorder) ControllerAccess() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerAccess", reflect.TypeOf((*MockConnection)(nil).ControllerAccess)) +} + +// ControllerTag mocks base method. +func (m *MockConnection) ControllerTag() names.ControllerTag { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerTag") + ret0, _ := ret[0].(names.ControllerTag) + return ret0 +} + +// ControllerTag indicates an expected call of ControllerTag. +func (mr *MockConnectionMockRecorder) ControllerTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerTag", reflect.TypeOf((*MockConnection)(nil).ControllerTag)) +} + +// CookieURL mocks base method. +func (m *MockConnection) CookieURL() *url.URL { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CookieURL") + ret0, _ := ret[0].(*url.URL) + return ret0 +} + +// CookieURL indicates an expected call of CookieURL. +func (mr *MockConnectionMockRecorder) CookieURL() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CookieURL", reflect.TypeOf((*MockConnection)(nil).CookieURL)) +} + +// HTTPClient mocks base method. +func (m *MockConnection) HTTPClient() (*httprequest.Client, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HTTPClient") + ret0, _ := ret[0].(*httprequest.Client) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HTTPClient indicates an expected call of HTTPClient. +func (mr *MockConnectionMockRecorder) HTTPClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HTTPClient", reflect.TypeOf((*MockConnection)(nil).HTTPClient)) +} + +// IPAddr mocks base method. +func (m *MockConnection) IPAddr() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IPAddr") + ret0, _ := ret[0].(string) + return ret0 +} + +// IPAddr indicates an expected call of IPAddr. +func (mr *MockConnectionMockRecorder) IPAddr() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IPAddr", reflect.TypeOf((*MockConnection)(nil).IPAddr)) +} + +// IsBroken mocks base method. +func (m *MockConnection) IsBroken() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsBroken") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsBroken indicates an expected call of IsBroken. +func (mr *MockConnectionMockRecorder) IsBroken() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsBroken", reflect.TypeOf((*MockConnection)(nil).IsBroken)) +} + +// IsProxied mocks base method. +func (m *MockConnection) IsProxied() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsProxied") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsProxied indicates an expected call of IsProxied. +func (mr *MockConnectionMockRecorder) IsProxied() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsProxied", reflect.TypeOf((*MockConnection)(nil).IsProxied)) +} + +// Login mocks base method. +func (m *MockConnection) Login(arg0 names.Tag, arg1, arg2 string, arg3 []macaroon.Slice) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Login", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// Login indicates an expected call of Login. +func (mr *MockConnectionMockRecorder) Login(arg0, arg1, arg2, arg3 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Login", reflect.TypeOf((*MockConnection)(nil).Login), arg0, arg1, arg2, arg3) +} + +// ModelTag mocks base method. +func (m *MockConnection) ModelTag() (names.ModelTag, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ModelTag") + ret0, _ := ret[0].(names.ModelTag) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// ModelTag indicates an expected call of ModelTag. +func (mr *MockConnectionMockRecorder) ModelTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModelTag", reflect.TypeOf((*MockConnection)(nil).ModelTag)) +} + +// Proxy mocks base method. +func (m *MockConnection) Proxy() proxy.Proxier { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Proxy") + ret0, _ := ret[0].(proxy.Proxier) + return ret0 +} + +// Proxy indicates an expected call of Proxy. +func (mr *MockConnectionMockRecorder) Proxy() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Proxy", reflect.TypeOf((*MockConnection)(nil).Proxy)) +} + +// PublicDNSName mocks base method. +func (m *MockConnection) PublicDNSName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PublicDNSName") + ret0, _ := ret[0].(string) + return ret0 +} + +// PublicDNSName indicates an expected call of PublicDNSName. +func (mr *MockConnectionMockRecorder) PublicDNSName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PublicDNSName", reflect.TypeOf((*MockConnection)(nil).PublicDNSName)) +} + +// RootHTTPClient mocks base method. +func (m *MockConnection) RootHTTPClient() (*httprequest.Client, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RootHTTPClient") + ret0, _ := ret[0].(*httprequest.Client) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RootHTTPClient indicates an expected call of RootHTTPClient. +func (mr *MockConnectionMockRecorder) RootHTTPClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RootHTTPClient", reflect.TypeOf((*MockConnection)(nil).RootHTTPClient)) +} + +// ServerVersion mocks base method. +func (m *MockConnection) ServerVersion() (version.Number, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ServerVersion") + ret0, _ := ret[0].(version.Number) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// ServerVersion indicates an expected call of ServerVersion. +func (mr *MockConnectionMockRecorder) ServerVersion() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServerVersion", reflect.TypeOf((*MockConnection)(nil).ServerVersion)) +} diff --git a/cmd/modelcmd/package_test.go b/cmd/modelcmd/package_test.go index 536f95b83b2..8c7b377ab9e 100644 --- a/cmd/modelcmd/package_test.go +++ b/cmd/modelcmd/package_test.go @@ -10,6 +10,7 @@ import ( ) //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/modelconfig_mock.go github.com/juju/juju/cmd/modelcmd ModelConfigAPI +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/api_mock.go github.com/juju/juju/api Connection func Test(t *testing.T) { coretesting.MgoTestPackage(t)