Skip to content

Commit

Permalink
fix: Environment cloning for app commands shouldn't leave strange sta…
Browse files Browse the repository at this point in the history
…te in JX_HOME

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 changes `jx add app` and `jx delete app` to perform more standard
PR creation via `ForkAndPullRepo`, using a temporary directory for the
clone, and changes `jx get apps` to just clone to a temporary
directory. This all just applies when gitops is enabled.

Also adds `--auto-merge` to `jx delete app`

fixes jenkins-x#6350

Signed-off-by: Andrew Bayer <[email protected]>
  • Loading branch information
abayer committed Jan 9, 2020
1 parent eb7a0c4 commit a2dc804
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 27 deletions.
29 changes: 24 additions & 5 deletions pkg/apps/gitops.go
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, dir, &details, nil, "", autoMerge)
if err != nil {
return errors.Wrapf(err, "creating pr for %s", app)
}
Expand Down Expand Up @@ -104,7 +104,12 @@ 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)
dir, err := ioutil.TempDir("", "create-pr")
if err != nil {
return err
}
defer os.RemoveAll(dir)
_, err = options.Create(o.DevEnv, dir, &details, nil, app, autoMerge)
if err != nil {
return err
}
Expand Down Expand Up @@ -156,7 +161,12 @@ 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)
dir, err := ioutil.TempDir("", "create-pr")
if err != nil {
return err
}
defer os.RemoveAll(dir)
info, err := options.Create(o.DevEnv, dir, &details, nil, "", autoMerge)
if err != nil {
return err
}
Expand All @@ -166,9 +176,18 @@ 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, err := ioutil.TempDir("", "create-pr")
if err != nil {
return nil, err
}
defer os.RemoveAll(dir)
err = o.Gitter.Clone(o.DevEnv.Spec.Source.URL, dir)
if err != nil {
return nil, errors.Wrapf(err, "failed to clone %s to dir %s", o.DevEnv.Spec.Source.URL, 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
7 changes: 6 additions & 1 deletion pkg/apps/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ 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)
dir, err := ioutil.TempDir("", "create-pr")
if err != nil {
return err
}
defer os.RemoveAll(dir)
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
5 changes: 0 additions & 5 deletions pkg/cmd/add/add_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,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
8 changes: 3 additions & 5 deletions pkg/cmd/deletecmd/delete_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type DeleteAppOptions struct {
Namespace string
Purge bool
Alias string
AutoMerge bool
}

// NewCmdDeleteApp creates a command object for this command
Expand Down Expand Up @@ -73,6 +74,7 @@ func NewCmdDeleteApp(commonOpts *opts.CommonOptions) *cobra.Command {
cmd.Flags().StringVarP(&o.Namespace, opts.OptionNamespace, "n", defaultNamespace, "The Namespace to install into (available when NOT using GitOps for your dev environment)")
cmd.Flags().StringVarP(&o.Alias, opts.OptionAlias, "", "",
"An alias to use for the app (available when using GitOps for your dev environment)")
cmd.Flags().BoolVarP(&o.AutoMerge, "auto-merge", "", false, "Automatically merge GitOps pull requests that pass CI")

return cmd
}
Expand All @@ -88,6 +90,7 @@ func (o *DeleteAppOptions) Run() error {
GitOps: o.GitOps,
BatchMode: o.BatchMode,
Helmer: o.Helm(),
AutoMerge: o.AutoMerge,
}

if o.GitOps {
Expand All @@ -102,13 +105,8 @@ func (o *DeleteAppOptions) Run() error {
if err != nil {
return errors.Wrapf(err, "creating git provider for %s", o.DevEnv.Spec.Source.URL)
}
environmentsDir, err := o.EnvironmentsDir()
if err != nil {
return errors.Wrapf(err, "getting environments dir")
}
installOptions.GitProvider = gitProvider
installOptions.Gitter = o.Git()
installOptions.EnvironmentsDir = environmentsDir
}
if !o.GitOps {
err := o.EnsureHelm()
Expand Down
2 changes: 0 additions & 2 deletions pkg/cmd/get/get_apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,8 @@ func (o *GetAppsOptions) Run() error {
if err != nil {
return errors.Wrapf(err, "creating git provider for %s", o.DevEnv.Spec.Source.URL)
}
environmentsDir := envsDir
installOptions.GitProvider = gitProvider
installOptions.Gitter = o.Git()
installOptions.EnvironmentsDir = environmentsDir
}

apps, err := installOptions.GetApps(o.Args)
Expand Down
5 changes: 0 additions & 5 deletions pkg/cmd/upgrade/upgrade_apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,6 @@ func (o *UpgradeAppsOptions) Run() error {
if !o.HelmUpdate {
return util.InvalidOptionf(optionHelmUpdate, o.HelmUpdate, msg, optionHelmUpdate)
}
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
7 changes: 3 additions & 4 deletions pkg/environments/gitops.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,13 @@ type EnvironmentPullRequestOptions struct {
// the message as the body for both the commit and the pull request,
// and the pullRequestInfo for any existing PR that exists to modify the environment that we want to merge these
// changes into.
func (o *EnvironmentPullRequestOptions) Create(env *jenkinsv1.Environment, environmentsDir string,
func (o *EnvironmentPullRequestOptions) Create(env *jenkinsv1.Environment, prDir string,
pullRequestDetails *gits.PullRequestDetails, filter *gits.PullRequestFilter, chartName string, autoMerge bool) (*gits.PullRequestInfo, error) {
dir := filepath.Join(environmentsDir, env.Name)
dir, base, upstreamRepo, forkURL, err := gits.ForkAndPullRepo(env.Spec.Source.URL, dir, env.Spec.Source.Ref, pullRequestDetails.BranchName, o.GitProvider, o.Gitter, "")
dir, base, upstreamRepo, forkURL, err := gits.ForkAndPullRepo(env.Spec.Source.URL, prDir, env.Spec.Source.Ref, pullRequestDetails.BranchName, o.GitProvider, o.Gitter, "")

if err != nil {
return nil, errors.Wrapf(err, "pulling environment repo %s into %s", env.Spec.Source.URL,
environmentsDir)
prDir)
}

err = ModifyChartFiles(dir, pullRequestDetails, o.ModifyChartFn, chartName)
Expand Down

0 comments on commit a2dc804

Please sign in to comment.