juju-core

Merge lp:~axwalk/juju-core/lp1313960-bootstrap-to into lp:~go-bot/juju-core/trunk

  1. lp1313960-bootstrap-to
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2687
Proposed branch: lp:~axwalk/juju-core/lp1313960-bootstrap-to
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 69 lines (+13/-19)
2 files modified
cmd/juju/bootstrap.go (+9/-15)
cmd/juju/bootstrap_test.go (+4/-4)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1313960-bootstrap-to
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+217837@code.launchpad.net

Commit message

cmd/juju: bootstrap --to <placement>

Change bootstrap to use "--to <placement>",
to make way for repurposing positional args.

Fixes lp:1313960

https://codereview.appspot.com/90970043/

Description of the change

cmd/juju: bootstrap --to <placement>

Change bootstrap to use "--to <placement>",
to make way for repurposing positional args.

Fixes lp:1313960

https://codereview.appspot.com/90970043/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (3.3 KiB)

Reviewers: mp+217837_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: bootstrap --to <placement>

Change bootstrap to use "--to <placement>",
to make way for repurposing positional args.

Fixes lp:1313960

https://code.launchpad.net/~axwalk/juju-core/lp1313960-bootstrap-to/+merge/217837

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/90970043/

Affected files (+15, -19 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140430030329-cl8i2buuaypidtg4
+New revision: <email address hidden>

Index: cmd/juju/bootstrap.go
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2014-04-24 12:33:19 +0000
+++ cmd/juju/bootstrap.go 2014-04-30 22:05:31 +0000
@@ -72,7 +72,6 @@
  func (c *BootstrapCommand) Info() *cmd.Info {
   return &cmd.Info{
    Name: "bootstrap",
- Args: "[placement]",
    Purpose: "start up an environment from scratch",
    Doc: bootstrapDoc,
   }
@@ -84,6 +83,7 @@
   f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of
tools before bootstrapping")
   f.Var(seriesVar{&c.Series}, "series", "upload tools for supplied
comma-separated series list")
   f.StringVar(&c.MetadataSource, "metadata-source", "", "local path to use
as tools and/or metadata source")
+ f.StringVar(&c.Placement, "to", "", "a placement directive indicating an
instance to bootstrap")
  }

  func (c *BootstrapCommand) Init(args []string) (err error) {
@@ -96,20 +96,14 @@
   }
   // Parse the placement directive. Bootstrap currently only
   // supports provider-specific placement directives.
- placement, err := cmd.ZeroOrOneArgs(args)
- if err != nil {
- return err
- }
- if placement == "" {
- return nil
- }
- _, err = instance.ParsePlacement(placement)
- if err != instance.ErrPlacementScopeMissing {
- // We only support unscoped placement directives for bootstrap.
- return fmt.Errorf("unsupported bootstrap placement directive %q",
placement)
- }
- c.Placement = placement
- return nil
+ if c.Placement != "" {
+ _, err = instance.ParsePlacement(c.Placement)
+ if err != instance.ErrPlacementScopeMissing {
+ // We only support unscoped placement directives for bootstrap.
+ return fmt.Errorf("unsupported bootstrap placement directive %q",
c.Placement)
+ }
+ }
+ return cmd.CheckEmpty(args)
  }

  // Run connects to the environment specified on the command line and
bootstraps

Index: cmd/juju/bootstrap_test.go
=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go 2014-04-25 04:23:08 +0000
+++ cmd/juju/bootstrap_test.go 2014-04-30 22:05:31 +0000
@@ -376,12 +376,12 @@
   },
  }, {
   info: "placement",
- args: []string{"something"},
+ args: []string{"--to", "something"},
   placement: "something",
  }, {
- info: "invalid placement: ssh",
- args: []string{"ssh:someplace"},
- err: `unsupported bootstrap placement dire...

Read more...

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM

https://codereview.appspot.com/90970043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2014-04-24 12:33:19 +0000
+++ cmd/juju/bootstrap.go 2014-04-30 22:07:50 +0000
@@ -72,7 +72,6 @@
72func (c *BootstrapCommand) Info() *cmd.Info {72func (c *BootstrapCommand) Info() *cmd.Info {
73 return &cmd.Info{73 return &cmd.Info{
74 Name: "bootstrap",74 Name: "bootstrap",
75 Args: "[placement]",
76 Purpose: "start up an environment from scratch",75 Purpose: "start up an environment from scratch",
77 Doc: bootstrapDoc,76 Doc: bootstrapDoc,
78 }77 }
@@ -84,6 +83,7 @@
84 f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of tools before bootstrapping")83 f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of tools before bootstrapping")
85 f.Var(seriesVar{&c.Series}, "series", "upload tools for supplied comma-separated series list")84 f.Var(seriesVar{&c.Series}, "series", "upload tools for supplied comma-separated series list")
86 f.StringVar(&c.MetadataSource, "metadata-source", "", "local path to use as tools and/or metadata source")85 f.StringVar(&c.MetadataSource, "metadata-source", "", "local path to use as tools and/or metadata source")
86 f.StringVar(&c.Placement, "to", "", "a placement directive indicating an instance to bootstrap")
87}87}
8888
89func (c *BootstrapCommand) Init(args []string) (err error) {89func (c *BootstrapCommand) Init(args []string) (err error) {
@@ -96,20 +96,14 @@
96 }96 }
97 // Parse the placement directive. Bootstrap currently only97 // Parse the placement directive. Bootstrap currently only
98 // supports provider-specific placement directives.98 // supports provider-specific placement directives.
99 placement, err := cmd.ZeroOrOneArgs(args)99 if c.Placement != "" {
100 if err != nil {100 _, err = instance.ParsePlacement(c.Placement)
101 return err101 if err != instance.ErrPlacementScopeMissing {
102 }102 // We only support unscoped placement directives for bootstrap.
103 if placement == "" {103 return fmt.Errorf("unsupported bootstrap placement directive %q", c.Placement)
104 return nil104 }
105 }105 }
106 _, err = instance.ParsePlacement(placement)106 return cmd.CheckEmpty(args)
107 if err != instance.ErrPlacementScopeMissing {
108 // We only support unscoped placement directives for bootstrap.
109 return fmt.Errorf("unsupported bootstrap placement directive %q", placement)
110 }
111 c.Placement = placement
112 return nil
113}107}
114108
115// Run connects to the environment specified on the command line and bootstraps109// Run connects to the environment specified on the command line and bootstraps
116110
=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go 2014-04-25 04:23:08 +0000
+++ cmd/juju/bootstrap_test.go 2014-04-30 22:07:50 +0000
@@ -376,12 +376,12 @@
376 },376 },
377}, {377}, {
378 info: "placement",378 info: "placement",
379 args: []string{"something"},379 args: []string{"--to", "something"},
380 placement: "something",380 placement: "something",
381}, {381}, {
382 info: "invalid placement: ssh",382 info: "additional args",
383 args: []string{"ssh:someplace"},383 args: []string{"anything", "else"},
384 err: `unsupported bootstrap placement directive "ssh:someplace"`,384 err: `unrecognized args: \["anything" "else"\]`,
385}}385}}
386386
387func (s *BootstrapSuite) TestBootstrapTwice(c *gc.C) {387func (s *BootstrapSuite) TestBootstrapTwice(c *gc.C) {

Subscribers

People subscribed via source and target branches

to status/vote changes: