Skip to content

Commit

Permalink
Merge branch '1.25' into null-merge-1.25
Browse files Browse the repository at this point in the history
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently committed Nov 4, 2015
2 parents 4044f05 + dac4b8e commit 5e37435
Show file tree
Hide file tree
Showing 21 changed files with 62 additions and 46 deletions.
4 changes: 2 additions & 2 deletions component/all/payload.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
"github.com/juju/juju/cmd/juju/commands"
"github.com/juju/juju/payload"
"github.com/juju/juju/payload/api/client"
internalclient "github.com/juju/juju/payload/api/internal/client"
internalserver "github.com/juju/juju/payload/api/internal/server"
internalclient "github.com/juju/juju/payload/api/private/client"
internalserver "github.com/juju/juju/payload/api/private/server"
"github.com/juju/juju/payload/api/server"
"github.com/juju/juju/payload/context"
"github.com/juju/juju/payload/persistence"
Expand Down
11 changes: 0 additions & 11 deletions payload/api/internal/client/package_test.go
View file

This file was deleted.

2 changes: 1 addition & 1 deletion payload/api/internal/client/client.go → payload/api/private/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ import (
"github.com/juju/loggo"
)

var logger = loggo.GetLogger("juju.payload.api.internal.client")
var logger = loggo.GetLogger("juju.payload.api.private.client")
2 changes: 1 addition & 1 deletion payload/api/internal/package_test.go → payload/api/private/client/package_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package internal_test
package client_test

import (
"testing"
Expand Down
4 changes: 2 additions & 2 deletions payload/api/internal/client/unitfacade.go → payload/api/private/client/unitfacade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/juju/errors"

"github.com/juju/juju/payload"
"github.com/juju/juju/payload/api/internal"
internal "github.com/juju/juju/payload/api/private"
)

type facadeCaller interface {
Expand Down Expand Up @@ -115,7 +115,7 @@ func (c UnitFacadeClient) lookUp(fullIDs []string) ([]string, error) {

var ids []string
for _, result := range results {
if result.Error != nil && !result.NotFound {
if result.Error != nil {
// TODO(ericsnow) Do not short-circuit?
return nil, errors.Annotate(result.Error, "while looking up IDs")
}
Expand Down
7 changes: 5 additions & 2 deletions ...ad/api/internal/client/unitfacade_test.go → ...oad/api/private/client/unitfacade_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package client_test

import (
Expand All @@ -11,8 +14,8 @@ import (
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/payload"
"github.com/juju/juju/payload/api"
"github.com/juju/juju/payload/api/internal"
"github.com/juju/juju/payload/api/internal/client"
internal "github.com/juju/juju/payload/api/private"
"github.com/juju/juju/payload/api/private/client"
)

type clientSuite struct {
Expand Down
2 changes: 1 addition & 1 deletion payload/api/internal/data.go → payload/api/private/data.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package internal
package private

// TODO(ericsnow) Eliminate the params import if possible.

Expand Down
2 changes: 1 addition & 1 deletion payload/api/internal/helpers.go → payload/api/private/helpers.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package internal
package private

// TODO(ericsnow) Eliminate the apiserver/common import if possible.
// TODO(ericsnow) Eliminate the params import if possible.
Expand Down
2 changes: 1 addition & 1 deletion payload/api/internal/helpers_test.go → payload/api/private/helpers_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package internal
package private

import (
"github.com/juju/errors"
Expand Down
14 changes: 14 additions & 0 deletions payload/api/private/package_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2015 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package private_test

import (
"testing"

gc "gopkg.in/check.v1"
)

func Test(t *testing.T) {
gc.TestingT(t)
}
0 payload/api/internal/server/package_test.go → payload/api/private/server/package_test.go
View file
File renamed without changes.
2 changes: 1 addition & 1 deletion payload/api/internal/server/server.go → payload/api/private/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ import (
"github.com/juju/loggo"
)

var logger = loggo.GetLogger("juju.payload.api.internal.server")
var logger = loggo.GetLogger("juju.payload.api.private.server")
2 changes: 1 addition & 1 deletion payload/api/internal/server/unitfacade.go → payload/api/private/server/unitfacade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/payload"
"github.com/juju/juju/payload/api"
"github.com/juju/juju/payload/api/internal"
internal "github.com/juju/juju/payload/api/private"
)

// UnitPayloads exposes the State functionality for a unit's payloads.
Expand Down
2 changes: 1 addition & 1 deletion ...ad/api/internal/server/unitfacade_test.go → ...oad/api/private/server/unitfacade_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/payload"
"github.com/juju/juju/payload/api"
"github.com/juju/juju/payload/api/internal"
internal "github.com/juju/juju/payload/api/private"
)

var _ = gc.Suite(&suite{})
Expand Down
9 changes: 5 additions & 4 deletions payload/context/register.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ func NewRegisterCmd(ctx HookContext) (*RegisterCmd, error) {
// The component wasn't tracked properly.
return nil, errors.Trace(err)
}
return &RegisterCmd{api: compCtx}, nil
return &RegisterCmd{hctx: compCtx}, nil
}

// RegisterCmd is a command that registers a payload with juju.
type RegisterCmd struct {
cmd.CommandBase

api Component
hctx Component
typ string
class string
id string
Expand Down Expand Up @@ -79,15 +79,16 @@ func (c *RegisterCmd) Run(ctx *cmd.Context) error {
},
ID: c.id,
Status: payload.StateRunning,
Labels: c.labels,
Unit: "a-service/0",
}
if err := c.api.Track(pl); err != nil {
if err := c.hctx.Track(pl); err != nil {
return errors.Trace(err)
}

// We flush to state immedeiately so that status reflects the
// payload correctly.
if err := c.api.Flush(); err != nil {
if err := c.hctx.Flush(); err != nil {
return errors.Trace(err)
}

Expand Down
11 changes: 6 additions & 5 deletions payload/context/register_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (registerSuite) TestInitWithLabels(c *gc.C) {

func (registerSuite) TestRun(c *gc.C) {
f := &stubRegisterContext{}
r := RegisterCmd{api: f}
r := RegisterCmd{hctx: f}
err := r.Init([]string{"type", "class", "id", "tag1", "tag 2"})
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -72,14 +72,15 @@ func (registerSuite) TestRun(c *gc.C) {
},
ID: "id",
Status: payload.StateRunning,
Labels: []string{"tag1", "tag 2"},
Unit: "a-service/0",
})
// TODO (natefinch): we need to do something with the labels
}

func (registerSuite) TestRunUnknownClass(c *gc.C) {
f := &stubRegisterContext{}
r := RegisterCmd{api: f}
r := RegisterCmd{hctx: f}
err := r.Init([]string{"type", "badclass", "id"})
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -90,7 +91,7 @@ func (registerSuite) TestRunUnknownClass(c *gc.C) {

func (registerSuite) TestRunUnknownType(c *gc.C) {
f := &stubRegisterContext{}
r := RegisterCmd{api: f}
r := RegisterCmd{hctx: f}
err := r.Init([]string{"badtype", "class", "id"})
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -101,7 +102,7 @@ func (registerSuite) TestRunUnknownType(c *gc.C) {

func (registerSuite) TestRunTrackErr(c *gc.C) {
f := &stubRegisterContext{trackerr: errors.Errorf("boo")}
r := RegisterCmd{api: f}
r := RegisterCmd{hctx: f}
err := r.Init([]string{"type", "class", "id", "tag1", "tag 2"})
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -112,7 +113,7 @@ func (registerSuite) TestRunTrackErr(c *gc.C) {

func (registerSuite) TestRunFlushErr(c *gc.C) {
f := &stubRegisterContext{flusherr: errors.Errorf("boo")}
r := RegisterCmd{api: f}
r := RegisterCmd{hctx: f}
err := r.Init([]string{"type", "class", "id", "tag1", "tag 2"})
c.Assert(err, jc.ErrorIsNil)

Expand Down
8 changes: 4 additions & 4 deletions payload/context/status-set.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ func NewStatusSetCmd(ctx HookContext) (*StatusSetCmd, error) {
// The component wasn't tracked properly.
return nil, errors.Trace(err)
}
return &StatusSetCmd{api: compCtx}, nil
return &StatusSetCmd{hctx: compCtx}, nil
}

// StatusSetCmd is a command that registers a payload with juju.
type StatusSetCmd struct {
cmd.CommandBase

api Component
hctx Component
class string
id string
status string
Expand Down Expand Up @@ -65,15 +65,15 @@ func (c *StatusSetCmd) Run(ctx *cmd.Context) error {
return errors.Trace(err)
}

if err := c.api.SetStatus(c.class, c.id, c.status); err != nil {
if err := c.hctx.SetStatus(c.class, c.id, c.status); err != nil {
return errors.Trace(err)
}

// TODO(ericsnow) Is the flush really necessary?

// We flush to state immedeiately so that status reflects the
// payload correctly.
if err := c.api.Flush(); err != nil {
if err := c.hctx.Flush(); err != nil {
return errors.Trace(err)
}

Expand Down
2 changes: 1 addition & 1 deletion payload/context/status-set_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *statusSetSuite) TestInvalidStatjs(c *gc.C) {
s.init(c, "docker", "foo", "created")
err := s.cmd.Run(s.ctx)

c.Check(err, gc.ErrorMatches, `state .* not valid`)
c.Check(err, gc.ErrorMatches, `status .* not supported; expected .*`)
}

func (s *statusSetSuite) TestStatusSet(c *gc.C) {
Expand Down
8 changes: 4 additions & 4 deletions payload/context/unregister.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const UnregisterCmdName = "payload-unregister"
type UnregisterCmd struct {
cmd.CommandBase

api Component
hctx Component
class string
id string
}
Expand All @@ -28,7 +28,7 @@ func NewUnregisterCmd(ctx HookContext) (*UnregisterCmd, error) {
return nil, errors.Trace(err)
}
c := &UnregisterCmd{
api: compCtx,
hctx: compCtx,
}
return c, nil
}
Expand Down Expand Up @@ -72,15 +72,15 @@ func (c *UnregisterCmd) Run(ctx *cmd.Context) error {

// TODO(ericsnow) Verify that Untrack gives a meaningful error when
// the ID is not found.
if err := c.api.Untrack(c.class, c.id); err != nil {
if err := c.hctx.Untrack(c.class, c.id); err != nil {
return errors.Trace(err)
}

// TODO(ericsnow) Is the flush really necessary?

// We flush to state immedeiately so that status reflects the
// payload correctly.
if err := c.api.Flush(); err != nil {
if err := c.hctx.Flush(); err != nil {
return errors.Trace(err)
}

Expand Down
4 changes: 2 additions & 2 deletions payload/payload_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ func (s *payloadSuite) TestValidateMissingStatus(c *gc.C) {
payload.Status = ""
err := payload.Validate()

c.Check(err, gc.ErrorMatches, `state .* not valid`)
c.Check(err, gc.ErrorMatches, `status .* not supported; expected one of .*`)
}

func (s *payloadSuite) TestValidateUnknownStatus(c *gc.C) {
payload := s.newPayload("spam", "docker")
payload.Status = "some-unknown-value"
err := payload.Validate()

c.Check(err, gc.ErrorMatches, `state .* not valid`)
c.Check(err, gc.ErrorMatches, `status .* not supported; expected one of .*`)
}

func (s *payloadSuite) TestValidateMissingUnit(c *gc.C) {
Expand Down
10 changes: 9 additions & 1 deletion payload/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
package payload

import (
"fmt"
"sort"
"strings"

"github.com/juju/errors"
"github.com/juju/utils/set"
)
Expand All @@ -29,7 +33,11 @@ var okayStates = set.NewStrings(
// ValidateState verifies the state passed in is a valid okayState.
func ValidateState(state string) error {
if !okayStates.Contains(state) {
return errors.NotValidf("state (%q)", state)
supported := okayStates.Values()
sort.Strings(supported)
states := strings.Join(supported, `", "`)
msg := fmt.Sprintf(`status %q not supported; expected one of ["%s"]`, state, states)
return errors.NewNotValid(nil, msg)
}
return nil
}

0 comments on commit 5e37435

Please sign in to comment.