Skip to content

Commit

Permalink
Refactored packages (#1047)
Browse files Browse the repository at this point in the history
Summary:
`packages` package (!) has been restructured to better separate the underlying concerns. The new structure looks like following:
```
pkg/kudoctl/packages/
├── reader
│   ├── parser.go
│   ├── read_dir.go
│   ├── reader.go
│   ├── reader_tar.go
│   └── reader_test.go
├── resolver
│   ├── resolver.go
│   ├── resolver_local.go
│   ├── resolver_local_test.go
│   ├── resolver_test.go
│   └── resolver_url.go
├── writer
│   ├── writer_tar.go
│   └── writer_test.go
├── testdata
│   └── ...
├── types.go
└── package.go
```

**tl;dr:**
- methods and files has been split across three concerns: `reader`, `writer` and `resolver`
- `finder` got renamed to `resolver`: it doesn't so much search for packages as it does resolving passed
  package name to an underlying location
- eliminated code duplication e.g. `util/kudo/resources.go` was reimplementing what resolver is doing
- all the types now live in `packages/types.go` and are addressed with package prefix e.g. `packages.Files`.
  This allowed to shorten type names.
- moved other methods to where they were used e.g. new `digest.go` is living in the `repo` now
- `install.go` and `upgrade.go` commands are now using the `resolver` directly
  • Loading branch information
Aleksey Dukhovniy authored Nov 14, 2019
1 parent 2a585ae commit bd59d03
Show file tree
Hide file tree
Showing 30 changed files with 911 additions and 853 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/Microsoft/go-winio v0.4.14 // indirect
github.com/containerd/containerd v1.2.9 // indirect
github.com/davecgh/go-spew v1.1.1
github.com/docker/docker v1.4.2-0.20190916154449-92cc603036dd
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
Expand Down
7 changes: 5 additions & 2 deletions pkg/kudoctl/cmd/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package install
import (
"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
"github.com/kudobuilder/kudo/pkg/kudoctl/env"
pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/repo"

Expand Down Expand Up @@ -65,10 +66,12 @@ func installOperator(operatorArgument string, options *Options, fs afero.Fs, set
}

clog.V(3).Printf("getting package crds")
resources, err := kudo.Resources(operatorArgument, options.PackageVersion, repository)

resolver := pkgresolver.New(repository)
pkg, err := resolver.Resolve(operatorArgument, options.PackageVersion)
if err != nil {
return errors.Wrapf(err, "failed to resolve package CRDs for operator: %s", operatorArgument)
}

return kudo.InstallPackage(kc, resources, options.SkipInstance, options.InstanceName, settings.Namespace, options.Parameters)
return kudo.InstallPackage(kc, pkg.Resources, options.SkipInstance, options.InstanceName, settings.Namespace, options.Parameters)
}
4 changes: 2 additions & 2 deletions pkg/kudoctl/cmd/package_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"io"

"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
"github.com/kudobuilder/kudo/pkg/kudoctl/packages/writer"
"github.com/spf13/afero"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -64,7 +64,7 @@ func validateOperatorArg(args []string) error {

// run returns the errors associated with cmd env
func (pkg *packageCreateCmd) run() error {
tarfile, err := packages.CreateTarball(pkg.fs, pkg.path, pkg.destination, pkg.overwrite)
tarfile, err := writer.WriteTgz(pkg.fs, pkg.path, pkg.destination, pkg.overwrite)
if err == nil {
fmt.Fprintf(pkg.out, "Package created: %v\n", tarfile)
}
Expand Down
19 changes: 10 additions & 9 deletions pkg/kudoctl/cmd/package_params_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
"github.com/kudobuilder/kudo/pkg/kudoctl/env"
"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/repo"

"github.com/gosuri/uitable"
Expand Down Expand Up @@ -77,23 +77,24 @@ func (c *paramsListCmd) run(fs afero.Fs, settings *env.Settings) error {
clog.V(4).Printf("repository used %s", repository)

clog.V(3).Printf("getting package pkg files for %v with version: %v", c.path, c.PackageVersion)
pf, err := kudo.PkgFiles(c.path, c.PackageVersion, repository)
resolver := pkgresolver.New(repository)
pf, err := resolver.Resolve(c.path, c.PackageVersion)
if err != nil {
return errors.Wrapf(err, "failed to resolve package files for operator: %s", c.path)
}

return displayParamsTable(pf, c)
return displayParamsTable(pf.Files, c)
}

func displayParamsTable(pf *packages.PackageFiles, cmd *paramsListCmd) error {
sort.Sort(pf.Params)
func displayParamsTable(pf *packages.Files, cmd *paramsListCmd) error {
sort.Sort(pf.Params.Parameters)
table := uitable.New()
tValue := true
// required
if cmd.requiredOnly {
table.AddRow("Name")
found := false
for _, p := range pf.Params {
for _, p := range pf.Params.Parameters {
if p.Default == nil && p.Required == &tValue {
found = true
table.AddRow(p.Name)
Expand All @@ -109,7 +110,7 @@ func displayParamsTable(pf *packages.PackageFiles, cmd *paramsListCmd) error {
// names only
if cmd.namesOnly {
table.AddRow("Name")
for _, p := range pf.Params {
for _, p := range pf.Params.Parameters {
table.AddRow(p.Name)
}
fmt.Fprintln(cmd.out, table)
Expand All @@ -123,8 +124,8 @@ func displayParamsTable(pf *packages.PackageFiles, cmd *paramsListCmd) error {
} else {
table.AddRow("Name", "Default", "Required")
}
sort.Sort(pf.Params)
for _, p := range pf.Params {
sort.Sort(pf.Params.Parameters)
for _, p := range pf.Params.Parameters {
pDefault := ""
if p.Default != nil {
pDefault = *p.Default
Expand Down
6 changes: 3 additions & 3 deletions pkg/kudoctl/cmd/package_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"io"

"github.com/kudobuilder/kudo/pkg/kudoctl/cmd/verify"
"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
"github.com/kudobuilder/kudo/pkg/kudoctl/packages/reader"

"github.com/gosuri/uitable"
"github.com/spf13/afero"
Expand Down Expand Up @@ -39,11 +39,11 @@ func newPackageVerifyCmd(fs afero.Fs, out io.Writer) *cobra.Command {

func (c *packageVerifyCmd) run(fs afero.Fs, path string) error {

pf, err := packages.FromFolder(fs, path)
pf, err := reader.FromDir(fs, path)
if err != nil {
return err
}
warnings, errors := verify.Parameters(pf.Params)
warnings, errors := verify.Parameters(pf.Params.Parameters)

if warnings != nil {
printWarnings(c.out, warnings)
Expand Down
6 changes: 5 additions & 1 deletion pkg/kudoctl/cmd/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/kudobuilder/kudo/pkg/kudoctl/cmd/install"
"github.com/kudobuilder/kudo/pkg/kudoctl/env"
pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/repo"

Expand Down Expand Up @@ -93,10 +94,13 @@ func runUpgrade(args []string, options *options, fs afero.Fs, settings *env.Sett
if err != nil {
return errors.WithMessage(err, "could not build operator repository")
}
resources, err := kudo.Resources(packageToUpgrade, options.PackageVersion, repository)
resolver := pkgresolver.New(repository)
pkg, err := resolver.Resolve(packageToUpgrade, options.PackageVersion)
if err != nil {
return errors.Wrapf(err, "failed to resolve package CRDs for operator: %s", packageToUpgrade)
}

resources := pkg.Resources

return kudo.UpgradeOperatorVersion(kc, resources.OperatorVersion, options.InstanceName, settings.Namespace, options.Parameters)
}
8 changes: 4 additions & 4 deletions pkg/kudoctl/cmd/verify/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var verifiers = []ParameterVerifier{
}

// Parameters verifies parameters
func Parameters(params packages.Params) (warnings ParamWarnings, errors ParamErrors) {
func Parameters(params packages.Parameter) (warnings ParamWarnings, errors ParamErrors) {
for _, verifier := range verifiers {
w, err := verifier.Verify(params)
warnings = append(warnings, w...)
Expand All @@ -30,7 +30,7 @@ func Parameters(params packages.Params) (warnings ParamWarnings, errors ParamErr

// ParameterVerifier defines the interface for all parameter verifiers
type ParameterVerifier interface {
Verify(params packages.Params) (ParamWarnings, ParamErrors)
Verify(params packages.Parameter) (ParamWarnings, ParamErrors)
}

func CreateParamError(param v1beta1.Parameter, reason string) ParamError {
Expand All @@ -41,7 +41,7 @@ func CreateParamError(param v1beta1.Parameter, reason string) ParamError {
type DuplicateVerifier struct {
}

func (DuplicateVerifier) Verify(params packages.Params) (warnings ParamWarnings, errors ParamErrors) {
func (DuplicateVerifier) Verify(params packages.Parameter) (warnings ParamWarnings, errors ParamErrors) {
names := map[string]bool{}
for _, param := range params {
name := strings.ToLower(param.Name)
Expand All @@ -57,7 +57,7 @@ type InvalidCharVerifier struct {
InvalidChars string
}

func (v InvalidCharVerifier) Verify(params packages.Params) (warnings ParamWarnings, errors ParamErrors) {
func (v InvalidCharVerifier) Verify(params packages.Parameter) (warnings ParamWarnings, errors ParamErrors) {
for _, param := range params {
name := strings.ToLower(param.Name)
for _, char := range name {
Expand Down
116 changes: 0 additions & 116 deletions pkg/kudoctl/packages/finder/package_finder.go

This file was deleted.

46 changes: 0 additions & 46 deletions pkg/kudoctl/packages/finder/package_finder_test.go

This file was deleted.

Loading

0 comments on commit bd59d03

Please sign in to comment.