Support multiple environment folders and add subcommands #112

JustinKuli · 2020-06-15T20:16:11Z

Main discussion in #92.

Added functional --from-env and --to-env flags on the usual promote command, and added new subcommands to simplify the experience when promoting along the "usual" paths:

services promote env <--from> <--to> <--repo> <--service>
services promote branch <--from> <--to> <--repo> <--service>
services promote repo <--from> <--to> <--service>

There was a bit of refactoring to try to prevent repetition in the new subcommands and keep things organized.

I found that I needed to re-bind the flags at the beginning of each "Action" function, otherwise only some subcommands would have the flags set correctly. I think it is something to do with the order that the init functions are executed in. The solution here was based on this comment: spf13/cobra#299 (comment)

To test this, you'll need multiple repositories set up for multiple branches and environment folders. I have examples you can clone as needed:

multiple branches: https://github.com/JustinKuli/gitops-example-dev
multiple environment folders: https://github.com/JustinKuli/gitops-example-multienv

These examples should demonstrate the usage (in addition to what's in the README):

services promote env --repo "https://github.com/JustinKuli/gitops-example-multienv.git" --from "dev" --to "staging" --service "promote-demo" --commit-message "Test multienv subcommand"

services promote branch --repo "https://github.com/JustinKuli/gitops-example-dev.git" --from "dev-a" --to "dev-b" --service "promote-demo" --commit-message "Test branch subcommand"

services promote repo --from "https://github.com/JustinKuli/gitops-example-dev.git" --to "https://github.com/JustinKuli/gitops-example-staging.git" --service "promote-demo" --commit-message "Test repo subcommand"

services promote --from "https://github.com/JustinKuli/gitops-example-multienv.git" --from-env-folder staging --to "https://github.com/JustinKuli/gitops-example-dev.git" --to-branch dev-b --service "promote-demo" --commit-message "Test complex promote"

a-roberts · 2020-06-16T13:21:22Z

I'll take a look and give this one a good try.

a-roberts · 2020-06-16T15:54:46Z

My findings with scenario two (services promote branch) are odd.

(base) Adams-MBP:services aroberts$ services promote branch --repo "https://github.com/a-roberts/gitops-example-dev.git" --from "dev-a" --to "dev-b" --service "promote-demo" --commit-message "Test branch subcommand"
2020/06/16 15:52:48 created PR 1

gave me a PR where dev-a is not the same as that branch it's PRing from (I figured it would be, I'm PRing from dev-a to dev-b right?).

If we compare those two branches (dev-a and the generated one), they're not the same and there are conflicts.

Justin then mentioned:

It’s doing something with the master branch: https://github.com/JustinKuli/gitops-example-dev/pull/18/files (there is a file only present on the master branch showing up in the diff)

Over to @JustinKuli I think (he's mentioned spotting something).

JustinKuli · 2020-06-16T16:28:30Z

Good catch @a-roberts. I think all "new" branches created by the ServiceManager were based off the default branch in the repository. So although it was copying the files from "dev-a", and creating the PR onto "dev-b", the temporary branch was actually created off of "master".

Bad PR: https://github.com/JustinKuli/gitops-example-dev/pull/18

Good PR (with the most recent commit): https://github.com/JustinKuli/gitops-example-dev/pull/19

a-roberts · 2020-06-17T09:28:40Z

Pulled in the latest code to try things this morning, confirmed the first example (promote between envs) is still good.

Scenario two (promote between envs, same repo)

2020/06/17 10:21:47 failed to commit: exit status 1

unless I refork/pull in the latest change from Justin's repo (our error reporting isn't great and that's not specific to this PR). After doing that and promoting again I get https://github.com/a-roberts/gitops-example-dev/pull/1.

The commit message isn't great (promotion from gitops-example-dev to gitops-example-dev #1) - I'd like this to tell me the branch names (without me going into it to inspect what happened).
The files from dev-a are being used in the PR for dev-b correctly now though, which is great.
But I couldn't tell that it was really going from dev-a, apart from looking at the files changed. I think augmenting the commit message would be sufficient.

I'll update this comment once I've tried scenarios three and four.

Update: scenario three's PR looks correct to start with. https://github.com/a-roberts/gitops-example-staging/pull/3/files there are three files here: what's in master for dev, the example.yaml (in dev), and a test.yaml.

Command used:

services promote repo --from "https://github.com/a-roberts/gitops-example-dev.git" --to "https://github.com/a-roberts/gitops-example-staging.git" --service "promote-demo" --commit-message "Test repo subcommand"

But, I then made a change in dev (https://github.com/a-roberts/gitops-example-dev/blob/master/environments/dev/services/service-a/base/config/310-service.yaml) and did the promote command again and this file wasn't used in the new PR https://github.com/a-roberts/gitops-example-staging/pull/4/files and I expected it to be (because that's now in dev).

@JustinKuli so here's another one I think should be looked at.

Update: ah Justin pointed out that my change was to service-a, not promote-demo.

Scenario four (using --from-env-folder)

Adams-MBP:services aroberts$ ./services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder staging --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-b --service "promote-demo" --commit-message "Test complex promote"
2020/06/17 10:45:43 created PR 2
Adams-MBP:services aroberts$ ./services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder dev --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-b --service "promote-demo" --commit-message "Test complex promote"
2020/06/17 10:46:38 created PR 3

so in the first case I'm using staging and the second I'm using dev, but the same env folder (dev) was used for both, see https://github.com/a-roberts/gitops-example-dev/pull/2 which should be staging and https://github.com/a-roberts/gitops-example-dev/pull/3.

mnuttall · 2020-06-17T10:12:32Z

Adam, Justin, this is good work so far, thank you both. I asked Adam whether he thought some extra code coverage would be useful here. He wrote,

yeah

so, one test would add files, then promote, just to make sure everything gets picked up (or at least, look in all the right places for sure, as part of the test)
^^ for the case where I promoted, things looked fine. added to dev, promoted again, wasn't picked up.

and for the wrong env think - likely an easy fix with an easy test modification
^^ for the case where --from-env-folder did not pick up the right option

So do please expand the unit tests to cover all the errors found so far.

@bigkevmcd and I have been talking about mixed environments - e.g. option-1 for dev and staging, option-3 for prod. In that case, you’d have to reference ‘prod’ from the pipelines.yaml in the ‘main’ gitops repo, which is think is a fair price to pay. This introduces ‘promotion between repo types’ scenarios I don't think we've considered up to now. We need to be able to promote from an environment in an option 1 repo, to an option-3, for example. Please ensure we have a test for this!

a-roberts · 2020-06-17T12:20:45Z

My assessment of scenario three was wrong actually - becuse my change was done to service-a and not promote-demo. Justin pointed this out over Slack.

So, I made a new promote-demo file https://github.com/a-roberts/gitops-example-dev/tree/master/environments/dev/services/promote-demo/base/config

Promoted again (same command), new file picked up perfectly (https://github.com/a-roberts/gitops-example-staging/pull/6).

Looks like it's just four that's a problem.

JustinKuli · 2020-06-17T12:51:55Z

Improving the commit messages would be really helpful. In the general case I feel like the commit message would need to specify everything that was passed in for the "from" and "to" Locations (URL, branch, env folder).

That feels too verbose in simpler cases though, what if the subcommands can populate the commit message more specifically with just the piece that they are working with? So that services promote branch just makes a commit message saying "From branch ... to branch ..." instead of specifying everything?

a-roberts · 2020-06-17T12:53:17Z

Improving the commit messages would be really helpful. In the general case I feel like the commit message would need to specify everything that was passed in for the "from" and "to" Locations (URL, branch, env folder).

That feels too verbose in simpler cases though, what if the subcommands can populate the commit message more specifically with just the piece that they are working with? So that services promote branch just makes a commit message saying "From branch ... to branch ..." instead of specifying everything?

Yeah, I think that'd be fine, the repo name is redundant if it's within the same repo really

JustinKuli · 2020-06-18T20:38:03Z

Ok, two updates here @a-roberts @mnuttall.

Commit messages first. Promote now has more helpful default PR and commit messages, including the branch, repository, and environment folder (when an environment folder is given). The subcommands will use their own default messages, which have more limited information, so repo doesn't provide any information on branches. Examples:

services promote repo : https://github.com/JustinKuli/gitops-example-staging/pull/17
services promote branch : https://github.com/JustinKuli/gitops-example-dev/pull/24
services promote env : https://github.com/JustinKuli/gitops-example-multienv/pull/6
services promote : https://github.com/JustinKuli/gitops-example-dev/pull/25

One important thing to note with the change: the commit message, PR title, and PR body are now all the same. Previously the PR title was slightly different than the PR body / commit.

Other change: tests. After struggling with mocking git repositories with multiple branches and environment folders, I gave up and switched to using real example repositories. I prevent PRs and commits from actually being made, so all changes still just exist locally. My example gitops-alternative-layouts repository is used directly in order to have the pre-built environments so the tests just add a temporary file as a thing to promote and check for.

Linking to my own GitHub doesn't feel right, so we might consider adding these branches to an existing example repository here. Needing to pull these repositories for real (as opposed to mocking them) adds a bit of time to the tests: the promotion tests were taking around 10-12 seconds for me. But since the tests are using real git folders, I feel a bit more confident that everything is working.

a-roberts

Code changes look great, there are only nits that I've found, big +1 on the new commit message format. Plan to try this one out as soon as I can and go from there.

pkg/promotion/promote_test.go

a-roberts · 2020-06-22T08:57:55Z

Tested again this morning and I'm a big fan of the new commit message style.

So I did each of the options both with commit message overrides and without and observed the staged files in the PRs.

1: perfect. Promote environment dev to staging, correct file staged.
2: ditto
3: ditto, master file included which I expected as it's the whole repo being promoted to another and no specific branches were specified
4: I did:

services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder staging --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-b --service "promote-demo" --commit-message "Test complex promote"

services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder staging --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-b --service "promote-demo"

services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder dev --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-a --service "promote-demo" --commit-message "Test complex promote"

services promote --from "https://github.com/a-roberts/gitops-example-multienv.git" --from-env-folder dev --to "https://github.com/a-roberts/gitops-example-dev.git" --to-branch dev-a --service "promote-demo"

You can see the results at https://github.com/a-roberts/gitops-example-dev/pulls and all look great to me.

I'll sift through the code (impl & tests) next to see if there's anything odd, otherwise I'm really happy with the improvements and looking forward to cutting a release. Great work @JustinKuli 😄

a-roberts

Only minor comments/questions from me

pkg/cmd/root.go

pkg/promotion/env_location.go

a-roberts

Lgtm, let's hand this to @mnuttall for the final nod or nay 😄

mnuttall · 2020-06-22T14:56:47Z

pkg/promotion/promote_test.go

+	teams_branch_a_env = EnvLocation{exampleAlternativeRepo, "team-envs", "team-a"}
+)
+
+func TestRepoPromotionPath(t *testing.T) {


This is exactly what I was asking for, thank you :)

mnuttall

LGTM, great piece of work @JustinKuli thank you!

Users will have the ability to specify every facet of the source and destination environments: repo URL, branch name, and environment folder. New tests check for various combinations of those options. Some minor changes were made to some repository implementation details, so that it can checkout specific branches (rather than just the default) and so that calling Clone() multiple times does not result in an error. Implements #92

These new sub-commands simplify the interface when promoting between branches in a repository, between environments in a repository, or between two "simple" repositories. The previous behavior is preserved if no sub-command is given: the user can specify any combination of repository, branch and environment folder (even a local repo). For #92

a-roberts self-requested a review June 16, 2020 13:21

a-roberts requested a review from mnuttall June 16, 2020 13:46

a-roberts requested changes Jun 19, 2020

View reviewed changes

pkg/promotion/promote_test.go Show resolved Hide resolved

pkg/promotion/promote_test.go Outdated Show resolved Hide resolved

pkg/promotion/promote_test.go Show resolved Hide resolved

a-roberts requested changes Jun 22, 2020

View reviewed changes

pkg/cmd/root.go Outdated Show resolved Hide resolved

pkg/promotion/env_location.go Outdated Show resolved Hide resolved

pkg/promotion/env_location.go Outdated Show resolved Hide resolved

a-roberts reviewed Jun 22, 2020

View reviewed changes

pkg/promotion/env_location.go Show resolved Hide resolved

a-roberts self-requested a review June 22, 2020 13:13

a-roberts approved these changes Jun 22, 2020

View reviewed changes

mnuttall reviewed Jun 22, 2020

View reviewed changes

mnuttall approved these changes Jun 22, 2020

View reviewed changes

Justin Kulikauskas added 2 commits June 22, 2020 11:28

mnuttall merged commit 138702f into rhd-gitops-example:master Jun 22, 2020

mnuttall mentioned this pull request Jun 22, 2020

Promote within a repository #92

Closed

JustinKuli mentioned this pull request Jun 22, 2020

Functional tests for local and remote PR creation. --dry-run? #56

Closed

JustinKuli deleted the promote-env branch June 22, 2020 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multiple environment folders and add subcommands #112

Support multiple environment folders and add subcommands #112

JustinKuli commented Jun 15, 2020

a-roberts commented Jun 16, 2020

a-roberts commented Jun 16, 2020 •

edited

Loading

JustinKuli commented Jun 16, 2020

a-roberts commented Jun 17, 2020 •

edited

Loading

mnuttall commented Jun 17, 2020 •

edited

Loading

a-roberts commented Jun 17, 2020 •

edited

Loading

JustinKuli commented Jun 17, 2020

a-roberts commented Jun 17, 2020 •

edited

Loading

JustinKuli commented Jun 18, 2020

a-roberts left a comment

a-roberts commented Jun 22, 2020 •

edited

Loading

a-roberts left a comment

a-roberts left a comment

mnuttall Jun 22, 2020

mnuttall left a comment

Support multiple environment folders and add subcommands #112

Support multiple environment folders and add subcommands #112

Conversation

JustinKuli commented Jun 15, 2020

a-roberts commented Jun 16, 2020

a-roberts commented Jun 16, 2020 • edited Loading

JustinKuli commented Jun 16, 2020

a-roberts commented Jun 17, 2020 • edited Loading

mnuttall commented Jun 17, 2020 • edited Loading

a-roberts commented Jun 17, 2020 • edited Loading

JustinKuli commented Jun 17, 2020

a-roberts commented Jun 17, 2020 • edited Loading

JustinKuli commented Jun 18, 2020

a-roberts left a comment

Choose a reason for hiding this comment

a-roberts commented Jun 22, 2020 • edited Loading

a-roberts left a comment

Choose a reason for hiding this comment

a-roberts left a comment

Choose a reason for hiding this comment

mnuttall Jun 22, 2020

Choose a reason for hiding this comment

mnuttall left a comment

Choose a reason for hiding this comment

a-roberts commented Jun 16, 2020 •

edited

Loading

a-roberts commented Jun 17, 2020 •

edited

Loading

mnuttall commented Jun 17, 2020 •

edited

Loading

a-roberts commented Jun 17, 2020 •

edited

Loading

a-roberts commented Jun 17, 2020 •

edited

Loading

a-roberts commented Jun 22, 2020 •

edited

Loading