Skip to content

Commit

Permalink
fix: Cloning for environments shouldn't leave strange state in JX_HOME
Browse files Browse the repository at this point in the history 
Until this, `jx delete app`, `jx add app`, and `jx get apps` all did
weird things with/under `~/.jx/environments`. All of them used
`ForkAndPullRepo` to clone under there - `jx get apps` directly into
`~/.jx/environments`, and `jx add app` and `jx delete app` into
`~/.jx/environments/dev`. Literally none of this made sense. =)

This reworks not just the app commands, but all commands that create
PRs (i.e., gitops) against the dev/prod/staging environments to use temporary
directories for the fork/clone/push, with the option, just used in
tests, of specifying an existing directory to use instead, while `jx
get apps` just clones to a temporary directory as well.

Switching all this to temporary directories also really helps users
who manage/work with multiple JX clusters from a single
laptop/desktop/host, since namespacing `~/.jx/environments` by cluster
is basically impossible so far as I can tell.

Also adds `--auto-merge` to `jx delete app` because I was here.

fixes jenkins-x#6350

Signed-off-by: Andrew Bayer <[email protected]>
  • Loading branch information
@abayer
abayer committed Jan 10, 2020
1 parent eb7a0c4 commit 833a165
Show file tree
Hide file tree
Showing 19 changed files with 262 additions and 165 deletions.
35 changes: 30 additions & 5 deletions pkg/apps/gitops.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (o *GitOpsOptions) AddApp(app string, dir string, version string, repositor
GitProvider: o.GitProvider,
}

info, err := options.Create(o.DevEnv, o.EnvironmentsDir, &details, nil, "", autoMerge)
info, err := options.Create(o.DevEnv, o.EnvironmentCloneDir, &details, nil, "", autoMerge)
if err != nil {
return errors.Wrapf(err, "creating pr for %s", app)
}
Expand Down Expand Up @@ -104,7 +104,8 @@ func (o *GitOpsOptions) UpgradeApp(app string, version string, repository string
o.Helmer, inspectChartFunc, o.Verbose, o.valuesFiles),
GitProvider: o.GitProvider,
}
_, err = options.Create(o.DevEnv, o.EnvironmentsDir, &details, nil, app, autoMerge)

_, err = options.Create(o.DevEnv, o.EnvironmentCloneDir, &details, nil, app, autoMerge)
if err != nil {
return err
}
Expand Down Expand Up @@ -156,7 +157,7 @@ func (o *GitOpsOptions) DeleteApp(app string, alias string, autoMerge bool) erro
GitProvider: o.GitProvider,
}

info, err := options.Create(o.DevEnv, o.EnvironmentsDir, &details, nil, "", autoMerge)
info, err := options.Create(o.DevEnv, o.EnvironmentCloneDir, &details, nil, "", autoMerge)
if err != nil {
return err
}
Expand All @@ -166,9 +167,33 @@ func (o *GitOpsOptions) DeleteApp(app string, alias string, autoMerge bool) erro

// GetApps retrieves all the apps information for the given appNames from the repository and / or the CRD API
func (o *GitOpsOptions) GetApps(appNames map[string]bool, expandFn func([]string) (*v1.AppList, error)) (*v1.AppList, error) {
dir, _, _, _, err := gits.ForkAndPullRepo(o.DevEnv.Spec.Source.URL, o.EnvironmentsDir, o.DevEnv.Spec.Source.Ref, "master", o.GitProvider, o.Gitter, "")
dir := o.EnvironmentCloneDir
if dir == "" {
tmpDir, err := ioutil.TempDir("", "get-apps-")
if err != nil {
return nil, err
}
defer os.RemoveAll(tmpDir)
dir = tmpDir
}

gitInfo, err := gits.ParseGitURL(o.DevEnv.Spec.Source.URL)
if err != nil {
return nil, errors.Wrapf(err, "parsing dev env repo URL %s", o.DevEnv.Spec.Source.URL)
}

providerInfo, err := o.GitProvider.GetRepository(gitInfo.Organisation, gitInfo.Name)
if err != nil {
return nil, errors.Wrapf(err, "determining git provider information for %s", o.DevEnv.Spec.Source.URL)
}
cloneUrl := providerInfo.CloneURL
err = o.Gitter.Clone(cloneUrl, dir)
if err != nil {
return nil, errors.Wrapf(err, "failed to clone %s to dir %s", cloneUrl, dir)
}
err = o.Gitter.Checkout(dir, o.DevEnv.Spec.Source.Ref)
if err != nil {
return nil, errors.Wrapf(err, "couldn't pull the environment repository from %s", o.DevEnv.Name)
return nil, errors.Wrapf(err, "failed to checkout %s to dir %s", o.DevEnv.Spec.Source.Ref, dir)
}

envDir := filepath.Join(dir, helm.DefaultEnvironmentChartDir)
Expand Down
47 changes: 28 additions & 19 deletions pkg/apps/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,24 @@ import (

// InstallOptions are shared options for installing, removing or upgrading apps for either GitOps or HelmOps
type InstallOptions struct {
Helmer helm.Helmer
KubeClient kubernetes.Interface
InstallTimeout string
JxClient versioned.Interface
Namespace string
EnvironmentsDir string
GitProvider gits.GitProvider
Gitter gits.Gitter
Verbose bool
DevEnv *jenkinsv1.Environment
BatchMode bool
IOFileHandles util.IOFileHandles
GitOps bool
TeamName string
BasePath string
VaultClient vault.Client
AutoMerge bool
SecretsScheme string
Helmer helm.Helmer
KubeClient kubernetes.Interface
InstallTimeout string
JxClient versioned.Interface
Namespace string
EnvironmentCloneDir string
GitProvider gits.GitProvider
Gitter gits.Gitter
Verbose bool
DevEnv *jenkinsv1.Environment
BatchMode bool
IOFileHandles util.IOFileHandles
GitOps bool
TeamName string
BasePath string
VaultClient vault.Client
AutoMerge bool
SecretsScheme string

valuesFiles *environments.ValuesFiles // internal variable used to track, most be passed in
}
Expand Down Expand Up @@ -104,7 +104,7 @@ func (o *InstallOptions) AddApp(app string, version string, repository string, u
opts := GitOpsOptions{
InstallOptions: o,
}
err := opts.AddApp(chartDetails.Name, dir, chartDetails.Version, repository, alias, o.AutoMerge)
err = opts.AddApp(chartDetails.Name, dir, chartDetails.Version, repository, alias, o.AutoMerge)
if err != nil {
return errors.Wrapf(err, "adding app %s version %s with alias %s using gitops", chartName, version, alias)
}
Expand Down Expand Up @@ -175,6 +175,12 @@ func (o *InstallOptions) DeleteApp(app string, alias string, releaseName string,
Items: make([]string, 0),
}

// Make sure that we use a temporary directory for GetApps even if we're not using a temporary directory for DeleteApps
var originalEnvDir string
if o.GitOps {
originalEnvDir = o.EnvironmentCloneDir
o.EnvironmentCloneDir = ""
}
apps, err := o.GetApps([]string{app})
if err != nil {
return errors.WithStack(err)
Expand All @@ -185,6 +191,9 @@ func (o *InstallOptions) DeleteApp(app string, alias string, releaseName string,
chartName := apps.Items[0].Labels[helm.LabelAppName]

if o.GitOps {
if originalEnvDir != "" {
o.EnvironmentCloneDir = originalEnvDir
}
opts := GitOpsOptions{
InstallOptions: o,
}
Expand Down
19 changes: 9 additions & 10 deletions pkg/cmd/add/add_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ type AddAppOptions struct {
ValuesFiles []string
HelmUpdate bool
AutoMerge bool

// Used for testing
CloneDir string
}

const (
Expand Down Expand Up @@ -141,11 +144,12 @@ func (o *AddAppOptions) Run() error {
AutoMerge: o.AutoMerge,
SecretsScheme: "vault",

Helmer: o.Helm(),
Namespace: o.Namespace,
KubeClient: kubeClient,
JxClient: jxClient,
InstallTimeout: opts.DefaultInstallTimeout,
Helmer: o.Helm(),
Namespace: o.Namespace,
KubeClient: kubeClient,
JxClient: jxClient,
InstallTimeout: opts.DefaultInstallTimeout,
EnvironmentCloneDir: o.CloneDir,
}

if o.GitOps {
Expand All @@ -163,11 +167,6 @@ func (o *AddAppOptions) Run() error {
return util.InvalidOptionf(optionValues, o.SetValues,
"no more than one --%s can be specified when using GitOps for your dev environment", optionValues)
}
environmentsDir, err := o.EnvironmentsDir()
if err != nil {
return errors.Wrapf(err, "getting environments dir")
}
installOpts.EnvironmentsDir = environmentsDir

gitProvider, _, err := o.CreateGitProviderForURLWithoutKind(o.DevEnv.Spec.Source.URL)
if err != nil {
Expand Down
75 changes: 45 additions & 30 deletions pkg/cmd/add/add_app_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ func TestAddAppForGitOps(t *testing.T) {
DevEnv: testOptions.DevEnv,
HelmUpdate: true, // Flag default when run on CLI
}
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Name: name,
Expand All @@ -96,9 +101,6 @@ func TestAddAppForGitOps(t *testing.T) {
assert.Equal(r, fmt.Sprintf("Add %s %s", name, version), pr.Title)
assert.Equal(r, fmt.Sprintf("Add app %s %s", name, version), pr.Body)
// Validate the branch name
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
branchName, err := o.Git().Branch(devEnvDir)
assert.NoError(r, err)
assert.Equal(r, fmt.Sprintf("add-app-%s-%s", name, version), branchName)
Expand Down Expand Up @@ -153,6 +155,11 @@ func TestAddAppForGitOpsWithShortName(t *testing.T) {
DevEnv: testOptions.DevEnv,
HelmUpdate: true, // Flag default when run on CLI
}
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

pegomock.When(testOptions.MockHelmer.ListRepos()).ThenReturn(
map[string]string{
"repo1": kube.DefaultChartMuseumURL,
Expand Down Expand Up @@ -181,9 +188,6 @@ func TestAddAppForGitOpsWithShortName(t *testing.T) {
assert.Equal(r, fmt.Sprintf("Add %s %s", name, version), pr.Title)
assert.Equal(r, fmt.Sprintf("Add app %s %s", name, version), pr.Body)
// Validate the branch name
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
branchName, err := o.Git().Branch(devEnvDir)
assert.NoError(r, err)
assert.Equal(r, fmt.Sprintf("add-app-%s-%s", name, version), branchName)
Expand Down Expand Up @@ -608,6 +612,10 @@ func TestAddAppForGitOpsWithSecrets(t *testing.T) {
}
o.Args = []string{name}
o.BatchMode = false
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Expand Down Expand Up @@ -651,9 +659,7 @@ func TestAddAppForGitOpsWithSecrets(t *testing.T) {
r.Logf(expect.StripTrailingEmptyLines(console.CurrentState()))

// Validate that the secret reference is generated
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
valuesFromPrPath := filepath.Join(testOptions.GetFullDevEnvDir(envDir), name, helm.ValuesFileName)
valuesFromPrPath := filepath.Join(devEnvDir, name, helm.ValuesFileName)
_, err = os.Stat(valuesFromPrPath)
assert.NoError(r, err)
data, err := ioutil.ReadFile(valuesFromPrPath)
Expand Down Expand Up @@ -961,12 +967,13 @@ func TestAddAppWithValuesFileForGitOps(t *testing.T) {
ValuesFiles: []string{file.Name()},
}
o.Args = []string{name}
err = o.Run()
assert.NoError(t, err)
// Validate that the values.yaml file is in the right place
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir
err = o.Run()
assert.NoError(t, err)
// Validate that the values.yaml file is in the right place
valuesFromPrPath := filepath.Join(devEnvDir, name, helm.ValuesFileName)
_, err = os.Stat(valuesFromPrPath)
assert.NoError(t, err)
Expand Down Expand Up @@ -1017,6 +1024,10 @@ func TestAddAppWithReadmeForGitOps(t *testing.T) {
HelmUpdate: true, // Flag default when run on CLI
}
o.Args = []string{name}
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir
helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Name: name,
Expand All @@ -1033,9 +1044,6 @@ func TestAddAppWithReadmeForGitOps(t *testing.T) {
err = o.Run()
assert.NoError(t, err)
// Validate that the README.md file is in the right place
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
readmeFromPrPath := filepath.Join(devEnvDir, name, "README.MD")
_, err = os.Stat(readmeFromPrPath)
assert.NoError(t, err)
Expand Down Expand Up @@ -1090,6 +1098,10 @@ func TestAddAppWithCustomReadmeForGitOps(t *testing.T) {
}
o.Verbose = true
o.Args = []string{name}
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir
readmeFileName := "README.MD"
readme := "Tasty Cheese!\n"
helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Expand All @@ -1107,9 +1119,6 @@ func TestAddAppWithCustomReadmeForGitOps(t *testing.T) {
err = o.Run()
assert.NoError(t, err)
// Validate that the README.md file is in the right place
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
readmeFromPrPath := filepath.Join(devEnvDir, name, readmeFileName)
_, err = os.Stat(readmeFromPrPath)
assert.NoError(t, err)
Expand Down Expand Up @@ -1154,6 +1163,10 @@ func TestAddLatestAppForGitOps(t *testing.T) {
}
o.Args = []string{name}
o.Verbose = true
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Expand All @@ -1170,9 +1183,6 @@ func TestAddLatestAppForGitOps(t *testing.T) {
assert.Equal(t, fmt.Sprintf("Add %s %s", name, version), pr.Title)
assert.Equal(t, fmt.Sprintf("Add app %s %s", name, version), pr.Body)
// Validate the branch name
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
branchName, err := o.Git().Branch(devEnvDir)
assert.NoError(t, err)
assert.Equal(t, fmt.Sprintf("add-app-%s-%s", name, version), branchName)
Expand Down Expand Up @@ -1224,6 +1234,10 @@ func TestAddAppIncludingConditionalQuestionsForGitOps(t *testing.T) {
}
o.Args = []string{name}
o.BatchMode = false
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Expand Down Expand Up @@ -1275,9 +1289,7 @@ func TestAddAppIncludingConditionalQuestionsForGitOps(t *testing.T) {
<-donec
r.Logf(expect.StripTrailingEmptyLines(console.CurrentState()))

envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
valuesFromPrPath := filepath.Join(testOptions.GetFullDevEnvDir(envDir), name, helm.ValuesFileName)
valuesFromPrPath := filepath.Join(devEnvDir, name, helm.ValuesFileName)
_, err = os.Stat(valuesFromPrPath)
assert.NoError(r, err)
data, err := ioutil.ReadFile(valuesFromPrPath)
Expand Down Expand Up @@ -1333,6 +1345,10 @@ func TestAddAppExcludingConditionalQuestionsForGitOps(t *testing.T) {
}
o.Args = []string{name}
o.BatchMode = false
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(t, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir

helm_test.StubFetchChart(name, "", kube.DefaultChartMuseumURL, &chart.Chart{
Metadata: &chart.Metadata{
Expand Down Expand Up @@ -1375,9 +1391,7 @@ func TestAddAppExcludingConditionalQuestionsForGitOps(t *testing.T) {
<-donec
r.Logf(expect.StripTrailingEmptyLines(console.CurrentState()))

envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
valuesFromPrPath := filepath.Join(testOptions.GetFullDevEnvDir(envDir), name, helm.ValuesFileName)
valuesFromPrPath := filepath.Join(devEnvDir, name, helm.ValuesFileName)
_, err = os.Stat(valuesFromPrPath)
assert.NoError(r, err)
data, err := ioutil.ReadFile(valuesFromPrPath)
Expand Down Expand Up @@ -1415,6 +1429,10 @@ func TestAddAppForGitOpsWithSNAPSHOTVersion(t *testing.T) {
DevEnv: testOptions.DevEnv,
HelmUpdate: true, // Flag default when run on CLI
}
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
o.CloneDir = devEnvDir
pegomock.When(testOptions.MockHelmer.ListRepos()).ThenReturn(
map[string]string{
"repo1": kube.DefaultChartMuseumURL,
Expand Down Expand Up @@ -1443,9 +1461,6 @@ func TestAddAppForGitOpsWithSNAPSHOTVersion(t *testing.T) {
assert.Equal(r, fmt.Sprintf("Add %s %s", name, version), pr.Title)
assert.Equal(r, fmt.Sprintf("Add app %s %s", name, version), pr.Body)
// Validate the branch name
envDir, err := o.CommonOptions.EnvironmentsDir()
assert.NoError(r, err)
devEnvDir := testOptions.GetFullDevEnvDir(envDir)
branchName, err := o.Git().Branch(devEnvDir)
assert.NoError(r, err)
assert.Equal(r, fmt.Sprintf("add-app-%s-%s", name, version), branchName)
Expand Down
Loading

0 comments on commit 833a165

Please sign in to comment.