ks upgrade relocates vendored packages to versioned paths

Example:
vendor/<registry>/<pkg> -> vendor/<registry>/<pkg>@<version>

Closes #663
Signed-off-by: Oren Shomron <shomron@gmail.com>

pkg/actions/upgrade.go

@@ -15,7 +15,14 @@
 
 package actions
 
-import "github.com/ksonnet/ksonnet/pkg/app"
+import (
+	"io"
+	"os"
+
+	"github.com/ksonnet/ksonnet/pkg/app"
+	"github.com/ksonnet/ksonnet/pkg/registry"
+	"github.com/ksonnet/ksonnet/pkg/upgrade"
+)
 
 // RunUpgrade runs `upgrade`.
 func RunUpgrade(m map[string]interface{}) error {
@@ -29,26 +36,36 @@ func RunUpgrade(m map[string]interface{}) error {
 
 // Upgrade upgrades an application.
 type Upgrade struct {
-	app    app.App
-	dryRun bool
+	app       app.App
+	pm        registry.PackageManager
+	upgradeFn func(a app.App, out io.Writer, pl upgrade.PackageLister, dryRun bool) error
+	dryRun    bool
 }
 
 func newUpgrade(m map[string]interface{}) (*Upgrade, error) {
 	ol := newOptionLoader(m)
 
-	a := &Upgrade{
-		app:    ol.LoadApp(),
-		dryRun: ol.LoadBool(OptionDryRun),
+	a := ol.LoadApp()
+	if ol.err != nil {
+		return nil, ol.err
+	}
+	pm := registry.NewPackageManager(a)
+
+	u := &Upgrade{
+		app:       a,
+		pm:        pm,
+		upgradeFn: upgrade.Upgrade,
+		dryRun:    ol.LoadBool(OptionDryRun),
 	}
 
 	if ol.err != nil {
 		return nil, ol.err
 	}
 
-	return a, nil
+	return u, nil
 }
 
 // Upgrade upgrades a ksonnet application.
 func (u *Upgrade) run() error {
-	return u.app.Upgrade(u.dryRun)
+	return u.upgradeFn(u.app, os.Stdout, u.pm, u.dryRun)
 }

pkg/actions/upgrade_test.go

@@ -16,26 +16,34 @@
 package actions
 
 import (
+	"io"
 	"testing"
 
+	"github.com/ksonnet/ksonnet/pkg/app"
 	amocks "github.com/ksonnet/ksonnet/pkg/app/mocks"
+	"github.com/ksonnet/ksonnet/pkg/upgrade"
 	"github.com/stretchr/testify/require"
 )
 
 func TestUpgrade(t *testing.T) {
 	withApp(t, func(appMock *amocks.App) {
-		appMock.On("Upgrade", true).Return(nil)
-
 		in := map[string]interface{}{
 			OptionApp:    appMock,
 			OptionDryRun: true,
 		}
 
-		a, err := newUpgrade(in)
+		var called bool
+		u, err := newUpgrade(in)
+		u.upgradeFn = func(a app.App, out io.Writer, pl upgrade.PackageLister, dryRun bool) error {
+			called = true
+			return nil
+		}
+
 		require.NoError(t, err)
 
-		err = a.run()
+		err = u.run()
 		require.NoError(t, err)
+		require.True(t, called)
 	})
 }
 

pkg/app/app.go

@@ -70,8 +70,8 @@ type App interface {
 	EnvironmentParams(name string) (string, error)
 	// Fs is the app's afero Fs.
 	Fs() afero.Fs
-	// Init inits an environment.
-	Init() error
+	// CheckUpgrade checks whether an app should be upgraded.
+	CheckUpgrade() (bool, error)
 	// LibPath returns the path of the lib for an environment.
 	LibPath(envName string) (string, error)
 	// Libraries returns all environments.
@@ -111,8 +111,6 @@ func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
 		}
 	}
 
-	log.Debugf("called")
-
 	spec, err := read(fs, appRoot)
 	if os.IsNotExist(err) {
 		// During `ks init`, app.yaml will not yet exist - generate a new one.
@@ -133,7 +131,7 @@ func Load(fs afero.Fs, cwd string, skipFindRoot bool) (App, error) {
 		// 0.2.0, but will be persisted back as 0.2.0. This behavior will be
 		// subsequently changed with new upgrade framework.
 		a := NewApp010(fs, appRoot)
-		log.Debugf("Upgrading app [%p] version to latest (0.2.0)", a.baseApp)
+		log.Debugf("Interpreting app version as latest (0.2.0)", a.baseApp)
 		a.config.APIVersion = "0.2.0"
 		a.baseApp.config.APIVersion = "0.2.0"
 		return a, nil

pkg/app/app001.go

@@ -120,13 +120,13 @@ func (a *App001) Environments() (EnvironmentConfigs, error) {
 	return specs, nil
 }
 
-// Init initializes the App.
-func (a *App001) Init() error {
+// CheckUpgrade initializes the App.
+func (a *App001) CheckUpgrade() (bool, error) {
 	msg := "Your application's apiVersion is below 0.1.0. In order to use all ks features, you " +
 		"can upgrade your application using `ks upgrade`."
 	log.Warn(msg)
 
-	return nil
+	return true, nil
 }
 
 // LibPath returns the lib path for an env environment.

pkg/app/app001_test.go

@@ -237,10 +237,11 @@ func TestApp001_RemoveEnvironment(t *testing.T) {
 	})
 }
 
-func TestApp001_Init(t *testing.T) {
+func TestApp001_CheckUpgrade(t *testing.T) {
 	withApp001Fs(t, "app001_app.yaml", func(app *App001) {
-		err := app.Init()
+		needUpgrade, err := app.CheckUpgrade()
 		require.NoError(t, err)
+		assert.True(t, needUpgrade)
 	})
 }
 

pkg/app/app010.go

@@ -20,7 +20,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/ksonnet/ksonnet/pkg/lib"
@@ -125,24 +124,44 @@ func (a *App010) Environments() (EnvironmentConfigs, error) {
 	return environments, nil
 }
 
-// Init initializes the App.
-func (a *App010) Init() error {
-	// check to see if there are spec.json files.
+// CheckUpgrade initializes the App.
+func (a *App010) CheckUpgrade() (bool, error) {
+	if a == nil {
+		return false, errors.Errorf("nil reciever")
+	}
+
+	var needUpgrade bool
 
+	legacyLibs, err := a.checkUpgradeVendoredPackages()
+	if err != nil {
+		return false, err
+	}
+
+	if len(legacyLibs) > 0 {
+		logrus.Warnf("Versioned packages stored in unversioned paths - please run `ks upgrade` to correct.")
+		needUpgrade = true
+	}
+
+	// check to see if there are spec.json files.
 	legacyEnvs, err := a.findLegacySpec()
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	if len(legacyEnvs) == 0 {
-		return nil
+		return needUpgrade, nil
 	}
 
-	msg := "Your application's apiVersion is 0.1.0, but legacy environment declarations " +
-		"were found in environments: %s. In order to proceed, you will have to run `ks upgrade` to " +
-		"upgrade your application. <see url>"
+	needUpgrade = true
+	apiVersion := "0.1.0"
+	if a.config != nil {
+		apiVersion = a.config.APIVersion
+	}
+	logrus.Warnf("Your application's apiVersion is %s, but legacy environment declarations "+
+		"were found in environments: %s. In order to proceed, you will have to run `ks upgrade` to "+
+		"upgrade your application. <see url>", apiVersion, strings.Join(legacyEnvs, ", "))
 
-	return errors.Errorf(msg, strings.Join(legacyEnvs, ", "))
+	return needUpgrade, nil
 }
 
 // LibPath returns the lib path for an env environment.
@@ -272,56 +291,14 @@ func (a *App010) UpdateTargets(envName string, targets []string) error {
 
 // Upgrade upgrades the app to the latest apiVersion.
 func (a *App010) Upgrade(dryRun bool) error {
-	if err := a.checkForOldKSLibLocation(dryRun); err != nil {
-		return err
+	if a == nil {
+		return errors.Errorf("nil receiver")
 	}
-
-	return nil
-}
-
-var (
-	// reKSLibName matches a ksonnet library directory e.g. v1.10.3.
-	reKSLibName = regexp.MustCompile(`^v\d+\.\d+\.\d+$`)
-)
-
-func (a *App010) checkForOldKSLibLocation(dryRun bool) error {
-	libRoot := filepath.Join(a.Root(), LibDirName)
-	fis, err := afero.ReadDir(a.Fs(), libRoot)
-	if err != nil {
-		return err
-	}
-
-	if dryRun {
-		fmt.Fprintf(a.out, "[dry run] Updating ksonnet-lib paths\n")
-	}
-
-	if err = a.fs.MkdirAll(filepath.Join(libRoot, lib.KsonnetLibHome), DefaultFolderPermissions); err != nil {
-		return err
-	}
-
-	for _, fi := range fis {
-		if !fi.IsDir() {
-			continue
-		}
-
-		if reKSLibName.MatchString(fi.Name()) {
-			p := filepath.Join(libRoot, fi.Name())
-			new := filepath.Join(libRoot, lib.KsonnetLibHome, fi.Name())
-
-			if dryRun {
-				fmt.Fprintf(a.out, "[dry run] Moving %q from %s to %s\n", fi.Name(), p, new)
-				continue
-			}
-
-			fmt.Fprintf(a.out, "Moving %q from %s to %s\n", fi.Name(), p, new)
-			err = a.fs.Rename(p, new)
-			if err != nil {
-				return errors.Wrapf(err, "renaming %s to %s", p, new)
-			}
-		}
+	if a.config == nil {
+		return errors.Errorf("invalid app - config is nil")
 	}
-
-	return nil
+	a.config.APIVersion = "0.2.0"
+	return a.save()
 }
 
 func (a *App010) findLegacySpec() ([]string, error) {
@@ -351,3 +328,68 @@ func (a *App010) findLegacySpec() ([]string, error) {
 
 	return found, nil
 }
+
+// Returns library configurations for the app, whether they are global or environment-scoped.
+func (a *App010) allLibraries() (LibraryConfigs, error) {
+	if a == nil {
+		return nil, errors.Errorf("nil receiver")
+	}
+
+	combined := LibraryConfigs{}
+
+	libs, err := a.Libraries()
+	if err != nil {
+		return nil, errors.Wrapf(err, "checking libraries")
+	}
+	for _, lib := range libs {
+		combined[lib.Name] = lib
+	}
+
+	envs, err := a.Environments()
+	if err != nil {
+		return nil, errors.Wrapf(err, "checking environments")
+	}
+	for _, env := range envs {
+		for _, lib := range env.Libraries {
+			// NOTE We do not check for collisions at this time
+			combined[lib.Name] = lib
+		}
+	}
+
+	return combined, nil
+}
+
+// checkUpgradeVendoredPackages checks whether vendored packages need to be upgraded.
+// Upgrades are necessary if a versioned package is stored in pre-ksonnet 0.12.0, unversioned directory.
+func (a *App010) checkUpgradeVendoredPackages() ([]*LibraryConfig, error) {
+	if a == nil {
+		return nil, errors.Errorf("nil receiver")
+	}
+	fs := a.Fs()
+	if fs == nil {
+		return nil, errors.Errorf("nil filesystem interface")
+	}
+
+	combined, err := a.allLibraries()
+	if err != nil {
+		return nil, errors.Wrapf(err, "retrieving libraries")
+	}
+
+	results := make([]*LibraryConfig, 0)
+	for _, l := range combined {
+		if l.Version == "" {
+			continue
+		}
+
+		path := filepath.Join(a.VendorPath(), l.Registry, l.Name)
+		ok, err := afero.DirExists(fs, path)
+		if err != nil {
+			return nil, err
+		}
+		if ok {
+			results = append(results, l)
+		}
+	}
+
+	return results, nil
+}

pkg/app/app010_test.go

@@ -160,19 +160,21 @@ func TestApp010_Environment(t *testing.T) {
 	}
 }
 
-func TestApp010_Init_no_legacy_environments(t *testing.T) {
+func TestApp010_CheckUpgrade_no_legacy_environments(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(app *App010) {
-		err := app.Init()
+		needUpgrade, err := app.CheckUpgrade()
 		require.NoError(t, err)
+		assert.False(t, needUpgrade)
 	})
 }
 
-func TestApp010_Init_legacy_environments(t *testing.T) {
+func TestApp010_CheckUpgrade_legacy_environments(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(app *App010) {
 		stageFile(t, app.Fs(), "spec.json", "/environments/default/spec.json")
 
-		err := app.Init()
-		require.Error(t, err)
+		needUpgrade, err := app.CheckUpgrade()
+		require.NoError(t, err)
+		assert.True(t, needUpgrade)
 	})
 }
 
@@ -269,84 +271,6 @@ func TestApp010_RenameEnvironment(t *testing.T) {
 	}
 }
 
-func TestApp010_Upgrade(t *testing.T) {
-	cases := []struct {
-		name         string
-		init         func(t *testing.T, app *App010)
-		checkUpgrade func(t *testing.T, app *App010)
-		dryRun       bool
-		isErr        bool
-	}{
-		{
-			name: "ksonnet lib doesn't need to be upgraded",
-			init: func(t *testing.T, app *App010) {
-				err := app.Fs().MkdirAll("/lib", DefaultFolderPermissions)
-				require.NoError(t, err)
-
-				p := filepath.Join(app.Root(), "lib", "ksonnet-lib", "v1.10.3")
-				err = app.Fs().MkdirAll(p, DefaultFolderPermissions)
-				require.NoError(t, err)
-			},
-			dryRun: false,
-		},
-		{
-			name: "ksonnet lib needs to be upgraded",
-			init: func(t *testing.T, app *App010) {
-				err := app.Fs().MkdirAll("/lib", DefaultFolderPermissions)
-				require.NoError(t, err)
-
-				p := filepath.Join(app.Root(), "lib", "v1.10.3")
-				err = app.Fs().MkdirAll(p, DefaultFolderPermissions)
-				require.NoError(t, err)
-			},
-			dryRun: false,
-		},
-		{
-			name: "ksonnet lib needs to be upgraded - dry run",
-			init: func(t *testing.T, app *App010) {
-				err := app.Fs().MkdirAll("/lib", DefaultFolderPermissions)
-				require.NoError(t, err)
-
-				p := filepath.Join(app.Root(), "lib", "v1.10.3")
-				err = app.Fs().MkdirAll(p, DefaultFolderPermissions)
-				require.NoError(t, err)
-			},
-			checkUpgrade: func(t *testing.T, app *App010) {
-				isDir, err := afero.IsDir(app.Fs(), filepath.Join("/lib", "v1.10.3"))
-				require.NoError(t, err)
-				require.True(t, isDir)
-			},
-			dryRun: true,
-		},
-		{
-			name:  "lib doesn't exist",
-			isErr: true,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			withApp010Fs(t, "app010_app.yaml", func(app *App010) {
-				if tc.init != nil {
-					tc.init(t, app)
-				}
-
-				err := app.Upgrade(tc.dryRun)
-				if tc.isErr {
-					require.Error(t, err)
-					return
-				}
-
-				require.NoError(t, err)
-
-				if tc.checkUpgrade != nil {
-					tc.checkUpgrade(t, app)
-				}
-			})
-		})
-	}
-}
-
 func TestApp0101_UpdateTargets(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(app *App010) {
 		err := app.UpdateTargets("default", []string{"foo"})

pkg/app/mocks/App.go

@@ -53,6 +53,27 @@ func (_m *App) AddRegistry(spec *app.RegistryConfig, isOverride bool) error {
 	return r0
 }
 
+// CheckUpgrade provides a mock function with given fields:
+func (_m *App) CheckUpgrade() (bool, error) {
+	ret := _m.Called()
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func() bool); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // CurrentEnvironment provides a mock function with given fields:
 func (_m *App) CurrentEnvironment() string {
 	ret := _m.Called()
@@ -150,20 +171,6 @@ func (_m *App) Fs() afero.Fs {
 	return r0
 }
 
-// Init provides a mock function with given fields:
-func (_m *App) Init() error {
-	ret := _m.Called()
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func() error); ok {
-		r0 = rf()
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
-}
-
 // LibPath provides a mock function with given fields: envName
 func (_m *App) LibPath(envName string) (string, error) {
 	ret := _m.Called(envName)

pkg/clicmd/root.go

@@ -23,6 +23,7 @@ import (
 	"github.com/ksonnet/ksonnet/pkg/app"
 	"github.com/ksonnet/ksonnet/pkg/log"
 	"github.com/ksonnet/ksonnet/pkg/plugin"
+	"github.com/pkg/errors"
 	"github.com/shomron/pflag"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
@@ -93,6 +94,26 @@ func parseCommand(args []string) (string, error) {
 	return fset.Args()[0], nil
 }
 
+// checkUpgrade runs upgrade validations unless the user is running an excluded command.
+// If upgrades are found to be necessary, they will be reported to the user.
+func checkUpgrade(a app.App, cmd string) error {
+	skip := map[string]struct{}{
+		"init":    struct{}{},
+		"upgrade": struct{}{},
+		"help":    struct{}{},
+		"":        struct{}{},
+	}
+	if _, ok := skip[cmd]; ok {
+		return nil
+	}
+
+	if a == nil {
+		return errors.Errorf("nil receiver")
+	}
+	_, _ = a.CheckUpgrade() // NOTE we're surpressing any validation errors here
+	return nil
+}
+
 func NewRoot(appFs afero.Fs, wd string, args []string) (*cobra.Command, error) {
 	if appFs == nil {
 		appFs = afero.NewOsFs()
@@ -111,6 +132,11 @@ func NewRoot(appFs afero.Fs, wd string, args []string) (*cobra.Command, error) {
 		if err != nil {
 			return nil, err
 		}
+
+	}
+
+	if err := checkUpgrade(a, cmdName); err != nil {
+		return nil, errors.Wrap(err, "checking if app needs upgrade")
 	}
 
 	rootCmd := &cobra.Command{

pkg/pkg/local.go

@@ -64,8 +64,7 @@ func NewLocal(a app.App, name, registryName string, version string, installCheck
 		// Fallback succeeded - clear out original error to allow processing to continue
 		err = nil
 
-		// Alert the user that the application should be upgraded
-		log.Warnf("Versioned package %s/%s@%s stored in unversioned path - please run `ks upgrade` to correct.", registryName, name, version)
+		log.Debugf("Using legacy path for versioned package %s/%s@%s", registryName, name, version)
 	}
 	if err != nil {
 		return nil, errors.Wrapf(err, "reading package configuration from path: %v", partsPath)

pkg/upgrade/testdata/app010_app.yaml

+apiVersion: 0.1.0
+environments:
+  default:
+    destination:
+      namespace: some-namespace
+      server: http://example.com
+    k8sVersion: v1.7.0
+    path: default
+  us-east/test:
+    destination:
+      namespace: some-namespace
+      server: http://example.com
+    k8sVersion: v1.7.0
+    path: us-east/test
+  us-west/prod:
+    destination:
+      namespace: some-namespace
+      server: http://example.com
+    k8sVersion: v1.7.0
+    path: us-west/prod
+  us-west/test:
+    destination:
+      namespace: some-namespace
+      server: http://example.com
+    k8sVersion: v1.7.0
+    path: us-west/test
+kind: ksonnet.io/app
+name: test-get-envs
+registries:
+  incubator:
+    protocol: ""
+    uri: ""
+version: 0.0.1

pkg/upgrade/testdata/main.jsonnet

+local base = import "../base.libsonnet";
+local k = import "k.libsonnet";
+
+base + {
+  // Insert user-specified overrides here. For example if a component is named "nginx-deployment", you might have something like:
+  //   "nginx-deployment"+: k.deployment.mixin.metadata.labels({foo: "bar"})