Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Commit

Permalink
Refactor construction to use Dig for DI
Browse files Browse the repository at this point in the history
What I Did
------------
Refactored a bunch of stuff, should be no customer/vendor visible changes.

How I Did it
------------

- Incorporate [uber-go/dig](https://github.com/uber-go/dig) for dependency injection. This library is not crazy busy, but did have a commit a few days ago. It's pretty small too.
- Change a lot a lot of `XXXFromViper` methods to just `NewXXX`, and they just receive the deps they need.
- Cleaned up a lot of constructor functions, embedding `dig.In` lets us drop some entirely
- Added a test in pkg/ship/dig_test.go to ensure that we can instatiate a `ship.Ship`

Other random stuff I did
-----------------

- Removed some old `Installation` stuff from ConfigCtx, it gets applied separately now via @kevinherro 's `InstallationContext`
- refactored ship to call its own `ExitWithError` during `ExecuteAndMaybeExit`, instead of forcing clients to call it
- replaced some `templates.NewStaticContext` to use the `BuilderBuilder` version

How to verify it
------------

Write new code, enjoy less boilerplate.

Description for the Changelog
------------

None, this is purely a refactor

Picture of a Boat (not required but encouraged)
------------

![](https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcTw-m9sgmGy5ac22drQCz6SHNWQggn5PrM_bMJXjx1RfQVwHvyl)
  • Loading branch information
dexhorthy committed Jun 11, 2018
1 parent 41c54dc commit 6bca9de
Show file tree
Hide file tree
Showing 63 changed files with 6,647 additions and 489 deletions.
8 changes: 7 additions & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ build-deps:
go get -u github.com/golang/lint/golint
go get golang.org/x/tools/cmd/goimports

-dep-deps:
dep-deps:
go get -u github.com/golang/dep/cmd/dep

docker:
Expand Down Expand Up @@ -62,8 +62,9 @@ _mockgen:

mockgen: _mockgen fmt

dep:
dep ensure
deps:
dep ensure -v; dep prune -v


fmt:
goimports -w pkg
Expand Down
15 changes: 3 additions & 12 deletions pkg/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import (
"fmt"
"os"

"context"

"strings"

"github.com/pkg/errors"
"context"

"github.com/replicatedcom/ship/pkg/cli/devtool_releaser"
"github.com/replicatedcom/ship/pkg/e2e"
"github.com/replicatedcom/ship/pkg/ship"
Expand All @@ -33,15 +32,7 @@ application specs to be used in on-prem installations.
SilenceUsage: true,
SilenceErrors: true,
RunE: func(cmd *cobra.Command, args []string) error {
rc, err := ship.FromViper(viper.GetViper())
if err != nil {
return errors.Wrap(err, "initialize")
}
err = rc.Execute(context.Background())
if err != nil {
rc.ExitWithError(err)
}
return nil // let ExitWithError handle it ^
return ship.RunE(context.Background())
},
}
cobra.OnInitialize(initConfig)
Expand Down
27 changes: 12 additions & 15 deletions pkg/e2e/case.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@ package e2e
import (
"testing"

"context"
"encoding/json"
"io/ioutil"
"net/url"

"time"

"context"

"github.com/replicatedcom/ship/pkg/api"
"github.com/replicatedcom/ship/pkg/ship"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
)

type CaseRunner struct {
t *testing.T
assert *require.Assertions
req *require.Assertions
testcase testcase
}

Expand All @@ -32,46 +32,43 @@ type testcase struct {
func (r *CaseRunner) promoteRelease() {

gqlServer, err := url.Parse(r.testcase.config.GQL)
r.assert.NoError(err)
r.req.NoError(err)
client := &GraphQLClient{
GQLServer: gqlServer,
Token: r.testcase.config.Token,
}

spec, err := json.Marshal(r.testcase.Spec)
r.assert.NoError(err)
r.req.NoError(err)

_, err = client.PromoteRelease(
string(spec),
r.testcase.config.ChannelID,
r.testcase.config.Semver,
`Integration test run on `+time.Now().String(),
)
r.assert.NoError(err)
r.req.NoError(err)

}

func (r *CaseRunner) Run() {

r.promoteRelease()
r.runShipForCustomer()
r.validateFiles()

}

func (r *CaseRunner) runShipForCustomer() {
// todo do each testcase in its own tmp directory,
// also maybe fork or docker run or something
s, err := ship.FromViper(viper.GetViper())
r.assert.NoError(err)
s, err := ship.Get()
r.req.NoError(err)

err = s.Execute(context.Background())
r.assert.NoError(err)
r.req.NoError(err)
}
func (r *CaseRunner) validateFiles() {
for path, expected := range r.testcase.Expect {
actual, err := ioutil.ReadFile(path)
r.assert.NoError(err)
r.assert.Equal(expected, string(actual))
r.req.NoError(err)
r.req.Equal(expected, string(actual))
}

}
2 changes: 1 addition & 1 deletion pkg/e2e/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (r *Runner) TestCase(test testcase) func(t *testing.T) {
return func(t *testing.T) {
(&CaseRunner{
t: t,
assert: require.New(t),
req: require.New(t),
testcase: test,
}).Run()
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/lifecycle/message/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (
"github.com/replicatedcom/ship/pkg/lifecycle/render/config"
"github.com/replicatedcom/ship/pkg/templates"
"github.com/spf13/viper"
"go.uber.org/dig"
)

type DaemonMessenger struct {
dig.In
Logger log.Logger
UI cli.Ui
Viper *viper.Viper
Expand Down
5 changes: 4 additions & 1 deletion pkg/lifecycle/message/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ import (
"github.com/replicatedcom/ship/pkg/lifecycle/render/config"
"github.com/replicatedcom/ship/pkg/templates"
"github.com/spf13/viper"
"go.uber.org/dig"
)

var _ Messenger = &CLIMessenger{}

type CLIMessenger struct {
dig.In

Logger log.Logger
UI cli.Ui
Viper *viper.Viper
Expand Down Expand Up @@ -50,7 +53,7 @@ func (e *CLIMessenger) Execute(ctx context.Context, release *api.Release, step *

func (e *CLIMessenger) getBuilder(release *api.Release) templates.Builder {
builder := e.BuilderBuilder.NewBuilder(
templates.NewStaticContext(),
e.BuilderBuilder.NewStaticContext(),
builderContext{
logger: e.Logger,
viper: e.Viper,
Expand Down
22 changes: 7 additions & 15 deletions pkg/lifecycle/message/messenger.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import (

"github.com/replicatedcom/ship/pkg/api"
"github.com/replicatedcom/ship/pkg/lifecycle/render/config"
"github.com/replicatedcom/ship/pkg/logger"
"github.com/replicatedcom/ship/pkg/templates"
"github.com/replicatedcom/ship/pkg/ui"
"github.com/spf13/viper"
)

Expand All @@ -16,22 +14,16 @@ type Messenger interface {
WithDaemon(d config.Daemon) Messenger
}

func FromViper(v *viper.Viper) Messenger {
func NewMessenger(
v *viper.Viper,
cli CLIMessenger,
daemon DaemonMessenger,
) Messenger {
if v.GetBool("headless") {
return &CLIMessenger{
Logger: logger.FromViper(v),
UI: ui.FromViper(v),
Viper: v,
BuilderBuilder: templates.BuilderBuilderFromViper(v),
}
return &cli
}

return &DaemonMessenger{
Logger: logger.FromViper(v),
UI: ui.FromViper(v),
Viper: v,
BuilderBuilder: templates.BuilderBuilderFromViper(v),
}
return &daemon
}

func (m *DaemonMessenger) WithDaemon(d config.Daemon) Messenger {
Expand Down
2 changes: 1 addition & 1 deletion pkg/lifecycle/render/config/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type ShipDaemon struct {
Fs afero.Afero
Viper *viper.Viper
UI cli.Ui
StateManager *state.StateManager
StateManager *state.Manager
ConfigRenderer *APIConfigRenderer

sync.Mutex
Expand Down
2 changes: 1 addition & 1 deletion pkg/lifecycle/render/config/headless_daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

type HeadlessDaemon struct {
StateManager *state.StateManager
StateManager *state.Manager
Logger log.Logger
UI cli.Ui
ConfigRenderer *APIConfigRenderer
Expand Down
2 changes: 1 addition & 1 deletion pkg/lifecycle/render/config/headless_daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ func TestHeadlessDaemon(t *testing.T) {
Viper: v,
}

manager := &state.StateManager{
manager := &state.Manager{
Logger: testLogger,
FS: fakeFS,
}
Expand Down
68 changes: 49 additions & 19 deletions pkg/lifecycle/render/config/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package config
import (
"context"

"github.com/go-kit/kit/log"
"github.com/mitchellh/cli"
"github.com/replicatedcom/ship/pkg/api"
"github.com/replicatedcom/ship/pkg/fs"
"github.com/replicatedcom/ship/pkg/lifecycle/render/state"
"github.com/replicatedcom/ship/pkg/logger"
"github.com/replicatedcom/ship/pkg/templates"
"github.com/replicatedcom/ship/pkg/ui"
"github.com/spf13/afero"
"github.com/spf13/viper"
)

Expand All @@ -18,9 +19,9 @@ type Resolver interface {
WithDaemon(d Daemon) Resolver
}

func ResolverFromViper(v *viper.Viper) Resolver {
func NewResolver(logger log.Logger) Resolver {
return &DaemonResolver{
Logger: logger.FromViper(v),
Logger: logger,
}

}
Expand All @@ -29,31 +30,60 @@ func (r *DaemonResolver) WithDaemon(d Daemon) Resolver {
return r
}

func DaemonFromViper(v *viper.Viper) Daemon {
func NewDaemon(
v *viper.Viper,
headless *HeadlessDaemon,
headed *ShipDaemon,
) Daemon {
if v.GetBool("headless") {
return headless
}
return headed
}

renderer := &APIConfigRenderer{
Logger: logger.FromViper(v),
func NewRenderer(
logger log.Logger,
v *viper.Viper,
builderBuilder *templates.BuilderBuilder,
) *APIConfigRenderer {
return &APIConfigRenderer{
Logger: logger,
Viper: v,
BuilderBuilder: templates.BuilderBuilderFromViper(v),
BuilderBuilder: builderBuilder,
}
}

if v.GetBool("headless") {
return &HeadlessDaemon{
StateManager: state.ManagerFromViper(v),
Logger: logger.FromViper(v),
UI: ui.FromViper(v),
ConfigRenderer: renderer,
}
func NewHeadlessDaemon(
v *viper.Viper,
logger log.Logger,
renderer *APIConfigRenderer,
stateManager *state.Manager,
) *HeadlessDaemon {
return &HeadlessDaemon{
StateManager: stateManager,
Logger: logger,
UI: ui.FromViper(v),
ConfigRenderer: renderer,
}
}

func NewHeadedDaemon(
v *viper.Viper,
renderer *APIConfigRenderer,
stateManager *state.Manager,
logger log.Logger,
ui cli.Ui,
fs afero.Afero,
) *ShipDaemon {
return &ShipDaemon{
Logger: logger.FromViper(v),
Fs: fs.FromViper(v),
UI: ui.FromViper(v),
StateManager: state.ManagerFromViper(v),
Logger: logger,
Fs: fs,
UI: ui,
StateManager: stateManager,
Viper: v,
ConfigSaved: make(chan interface{}),
MessageConfirmed: make(chan string, 1),
ConfigRenderer: renderer,
}

}
6 changes: 2 additions & 4 deletions pkg/lifecycle/render/docker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/replicatedcom/ship/pkg/api"
"github.com/replicatedcom/ship/pkg/logger"
"github.com/spf13/viper"
)

type PullURLResolver interface {
Expand All @@ -25,9 +23,9 @@ type URLResolver struct {
Logger log.Logger
}

func URLResolverFromViper(v *viper.Viper) PullURLResolver {
func URLResolverFromViper(logger log.Logger) PullURLResolver {
return &URLResolver{
Logger: logger.FromViper(v),
Logger: logger,
}
}

Expand Down
Loading

0 comments on commit 6bca9de

Please sign in to comment.