Unverified Commit 230769d7 authored by Oren Shomron's avatar Oren Shomron Committed by GitHub
Browse files

Merge pull request #792 from shomron/issue-766-canonical-env-targets

ks upgrade canonicalizes environment target separators (/ -> .)
parents 3b034ef9 00268faa
......@@ -267,6 +267,10 @@ func (a *App010) Upgrade(dryRun bool) error {
if a.config == nil {
return errors.Errorf("invalid app - config is nil")
}
if dryRun {
return nil
}
a.config.APIVersion = "0.2.0"
return a.save()
}
......
apiVersion: 0.1.0
environments:
default:
destination:
namespace: some-namespace
server: http://example.com
k8sVersion: v1.7.0
path: default
targets:
- /
us-east/test:
destination:
namespace: some-namespace
server: http://example.com
k8sVersion: v1.7.0
path: us-east/test
targets:
- /
- path/to/module
- path/to/other/module
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
apiVersion: 0.2.0
environments:
default:
destination:
namespace: some-namespace
server: http://example.com
k8sVersion: v1.7.0
path: default
targets:
- /
us-east/test:
destination:
namespace: some-namespace
server: http://example.com
k8sVersion: v1.7.0
path: us-east/test
targets:
- /
- path.to.module
- path.to.other.module
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
......@@ -20,6 +20,7 @@ import (
"io"
"path/filepath"
"regexp"
"strings"
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/ksonnet/ksonnet/pkg/lib"
......@@ -60,6 +61,9 @@ func (u *upgrade010) Upgrade(dryRun bool) error {
if err := u.upgradeOldVendoredPackages(u.packageLister, dryRun); err != nil {
return errors.Wrapf(err, "upgrading vendored packages")
}
if err := u.upgradeEnvTargets(u.app, dryRun); err != nil {
return errors.Wrapf(err, "upgrading environment targets")
}
// Upgrade app.yaml
return u.app.Upgrade(dryRun)
......@@ -181,3 +185,56 @@ func (u *upgrade010) upgradeOldVendoredPackages(pl PackageLister, dryRun bool) e
return nil
}
type envListerUpdater interface {
// Environments returns all environments.
Environments() (app.EnvironmentConfigs, error)
// UpdateTargets sets the targets for an environment.
UpdateTargets(envName string, targets []string) error
}
// In ksonnet 0.12.0 (app version 0.2.0) - environment targets began using the dot (.) separator for modules.
// Example:
// `module_a/module_b/module_c` -> `module_a.module_b.component_a`
func (*upgrade010) upgradeEnvTargets(listerUpdater envListerUpdater, dryRun bool) error {
if listerUpdater == nil {
return errors.New("nil envListerUpdater")
}
targets := make(map[string][]string)
envs, err := listerUpdater.Environments()
if err != nil {
return errors.Wrap(err, "fetching environments")
}
for _, e := range envs {
if e == nil {
continue
}
var dirty bool
upgraded := make([]string, len(e.Targets))
for i, t := range e.Targets {
if t == "/" {
// Root module is an exception - leave as-is
upgraded[i] = t
continue
}
if strings.Contains(t, "/") {
dirty = true
upgraded[i] = strings.Replace(t, "/", ".", -1)
}
}
if !dirty || dryRun {
continue
}
targets[e.Name] = upgraded
}
for name, upgraded := range targets {
if err := listerUpdater.UpdateTargets(name, upgraded); err != nil {
return errors.Wrapf(err, "updating targets for environment: %s", name)
}
}
return nil
}
......@@ -21,12 +21,15 @@ import (
"testing"
"github.com/ksonnet/ksonnet/pkg/app"
amocks "github.com/ksonnet/ksonnet/pkg/app/mocks"
"github.com/ksonnet/ksonnet/pkg/pkg"
pmocks "github.com/ksonnet/ksonnet/pkg/pkg/mocks"
rmocks "github.com/ksonnet/ksonnet/pkg/registry/mocks"
"github.com/ksonnet/ksonnet/pkg/util/test"
"github.com/pkg/errors"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)
......@@ -78,13 +81,15 @@ func TestApp010_Upgrade(t *testing.T) {
cases := []struct {
name string
stageFile string
init func(t *testing.T, a *app.App010) upgrader
checkUpgrade func(t *testing.T, a *app.App010)
dryRun bool
isErr bool
}{
{
name: "ksonnet lib doesn't need to be upgraded",
name: "ksonnet lib doesn't need to be upgraded",
stageFile: "app010_app.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
......@@ -98,7 +103,8 @@ func TestApp010_Upgrade(t *testing.T) {
dryRun: false,
},
{
name: "ksonnet lib needs to be upgraded",
name: "ksonnet lib needs to be upgraded",
stageFile: "app010_app.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
......@@ -112,7 +118,8 @@ func TestApp010_Upgrade(t *testing.T) {
dryRun: false,
},
{
name: "ksonnet lib needs to be upgraded - dry run",
name: "ksonnet lib needs to be upgraded - dry run",
stageFile: "app010_app.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
......@@ -131,14 +138,16 @@ func TestApp010_Upgrade(t *testing.T) {
dryRun: true,
},
{
name: "lib doesn't exist",
isErr: true,
name: "lib doesn't exist",
stageFile: "app010_app.yaml",
isErr: true,
init: func(t *testing.T, a *app.App010) upgrader {
return newUpgrade010(a, &b, &defaultPM)
},
},
{
name: "vendored packages need to be upgraded",
name: "vendored packages need to be upgraded",
stageFile: "app010_app.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
......@@ -163,7 +172,8 @@ func TestApp010_Upgrade(t *testing.T) {
dryRun: false,
},
{
name: "unversion packages are untouched",
name: "unversioned packages are untouched",
stageFile: "app010_app.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
......@@ -185,11 +195,29 @@ func TestApp010_Upgrade(t *testing.T) {
},
dryRun: false,
},
{
name: "environment targets are upgraded",
stageFile: "app010_env_targets.yaml",
init: func(t *testing.T, a *app.App010) upgrader {
err := a.Fs().MkdirAll("/lib", app.DefaultFolderPermissions)
require.NoError(t, err)
p := filepath.Join(a.Root(), "lib", "ksonnet-lib", "v1.10.3")
err = a.Fs().MkdirAll(p, app.DefaultFolderPermissions)
require.NoError(t, err)
return newUpgrade010(a, &b, &defaultPM)
},
checkUpgrade: func(t *testing.T, a *app.App010) {
test.AssertContents(t, a.Fs(), "app020_env_targets.yaml", "/app.yaml")
},
dryRun: false,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
withApp010Fs(t, "app010_app.yaml", func(a *app.App010) {
withApp010Fs(t, tc.stageFile, func(a *app.App010) {
var u upgrader
if tc.init != nil {
u = tc.init(t, a)
......@@ -213,3 +241,99 @@ func TestApp010_Upgrade(t *testing.T) {
})
}
}
func Test_upgrade010_upgradeEnvTargets(t *testing.T) {
makeApp := func(targets map[string][]string) *amocks.App {
envs := make(app.EnvironmentConfigs)
for name, t := range targets {
tCopy := make([]string, len(t))
copy(tCopy, t)
envs[name] = &app.EnvironmentConfig{
Name: name,
Targets: tCopy,
}
}
var a = new(amocks.App)
a.On("Environments").Return(func() app.EnvironmentConfigs {
return envs
}, nil)
a.On("UpdateTargets", mock.Anything, mock.Anything).Return(func(name string, targets []string) error {
tCopy := make([]string, len(targets))
copy(tCopy, targets)
e, ok := envs[name]
if !ok {
return errors.Errorf("unknown environment: %s", name)
}
e.Targets = tCopy
return nil
})
return a
}
tests := []struct {
name string
targets map[string][]string
expected map[string][]string
dryRun bool
expectUpdate bool
wantErr bool
}{
{
name: "nothing to upgrade",
targets: map[string][]string{
"default": []string{"/", "module_a", "module_b"},
},
expected: map[string][]string{
"default": []string{"/", "module_a", "module_b"},
},
expectUpdate: false,
},
{
name: "need to upgrade",
targets: map[string][]string{
"default": []string{"/", "prefix/module_b", "prefix/module/module_b"},
"stage": []string{"prefix/module_b", "/"},
},
expected: map[string][]string{
"default": []string{"/", "prefix.module_b", "prefix.module.module_b"},
"stage": []string{"prefix.module_b", "/"},
},
expectUpdate: true,
},
{
name: "dry run",
targets: map[string][]string{
"default": []string{"/", "module/module_a", "prefix/module/module_b"},
},
expected: map[string][]string{
"default": []string{"/", "module/module_a", "prefix/module/module_b"},
},
dryRun: true,
expectUpdate: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var u = new(upgrade010)
a := makeApp(tt.targets)
if err := u.upgradeEnvTargets(a, tt.dryRun); (err != nil) != tt.wantErr {
t.Errorf("upgrade010.upgradeEnvTargets() error = %v, wantErr %v", err, tt.wantErr)
}
if !tt.expectUpdate {
a.AssertNotCalled(t, "UpdateTargets", mock.Anything, mock.Anything)
return
}
a.AssertExpectations(t)
updated, err := a.Environments()
require.NoError(t, err)
for name, targets := range tt.expected {
assert.Equal(t, targets, updated[name].Targets, "targets for %s", name)
}
})
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment