feat(brig): Interactive prompt for creating projects #534

technosophos · 2018-07-06T19:29:40Z

This adds brig project create to create projects from interactive
prompts or from an existing secret.

bacongobbler · 2018-07-06T19:32:07Z

brig/cmd/brig/commands/project_create.go

+	"os"
+	"strings"
+
+	"github.com/AlecAivazis/survey"


@technosophos - what revision of this package did you use?
It's not in Gopkg.toml, and building off its latest revision currently fails

# github.com/Azure/brigade/vendor/github.com/AlecAivazis/survey vendor/github.com/AlecAivazis/survey/survey.go:57:8: undefined: terminal.Stdio

bacongobbler · 2018-07-06T19:32:34Z

brig/cmd/brig/commands/project_create.go

+The user will be prompted to answer questions to build the configuration. If -f
+is specified in conjunction with -x/--no-prompts, then the values in the file will
+be used without prompting the user for any changes.
+


delete this

What am I deleting here?

bacongobbler · 2018-07-06T19:34:39Z

brig/cmd/brig/commands/project_create.go

+`
+
+var (
+	projectCreateConfig      = ""


We don't need to set these to their default values since .StringVarP and .BoolVarP already do that after .ParseFlags() is called. Should be just projectCreateConfig string, right?

bacongobbler · 2018-07-06T19:36:17Z

brig/cmd/brig/commands/project_create.go

+
+	if globalVerbose {
+		out.Write(data)
+		fmt.Println()


nit: this could be simplified to

fmt.Fprintf(out, "%v\n", data)

bacongobbler · 2018-07-06T19:38:20Z

brig/cmd/brig/commands/project_create.go

+	}
+
+	// Only prompt for key if clone URL requires it
+	if strings.HasPrefix(p.Repo.CloneURL, "ssh") || strings.HasPrefix(p.Repo.CloneURL, "git+ssh") {


do we accept ssh-style clone URLs, such as git@github.com:Azure/brigade? If so should we add a check for that, or should we punt it and document that? Or, perhaps a yes/no prompt for whether we need to attach an SSH key?

I'm going to invert the selection, and not ask if the URL is http or https, since those are really the two forms that cannot take an SSH key

bacongobbler · 2018-07-06T19:44:12Z

brig/cmd/brig/commands/project_create.go

+}
+
+// loadFileValidator validates that a file exists and can be read.
+func loadFileValidator(val interface{}) error {


I see why we have to do this, but do you have any ideas on if there are any cleaner ways to this approach? it feels a little awkward to accept an interface{} then immediately cast it into a string.

I believe that to satisfy the Validator interface, we have to do func (interface{}) error as the signature.

Yeah, I tried brainstorming if there were any Go hacks we could do to make this cleaner, but this looks as good as it gets. :)

technosophos · 2018-07-09T21:05:54Z

@bacongobbler Okay, this is ready for a re-review

radu-matei · 2018-07-16T23:22:38Z

brig/cmd/brig/commands/project_create.go

+		return err
+	}
+
+	store := kube.New(c, globalNamespace)


Configuring globalNamespace which is default means the namespace configured through the survey is not used - so all projects are created in the default namespace, regardless of the value passed.

radu-matei · 2018-07-16T23:22:57Z

brig/cmd/brig/commands/project_create.go

+				Prompt: &survey.Input{
+					Message: "Kubernetes Namespace",
+					Help:    "The namespace in which this project applies",
+					Default: p.Kubernetes.Namespace,


This is not used, see comment above.

Oh, you're right... the value here should just be set to globalNamespace.

technosophos · 2018-07-17T18:54:22Z

@radu-matei The namespace thing was fixed. Now --namespace is the only way to set all the namespace-related stuff. This appears to be the best way to impose consistency.

radu-matei · 2018-07-18T18:25:34Z

Should the --dry-run flag output the secret the command would create?
As I see, right now it only outputs the project ID

radu-matei

I really like the experience introduced by this command - there are a few comments I left, but this looks really good!

radu-matei · 2018-07-18T18:38:53Z

docs/topics/projects.md

+? Configure Advanced Brigade Settings? No
+```
+
+You can use `--dry-run --debug` to see the answers to the question without creating


Is there a --debug flag?

Oops... it was --verbose. Will fix that.

radu-matei · 2018-07-18T18:42:24Z

docs/topics/projects.md

@@ -184,6 +214,9 @@ You can use SSH keys and a `git+ssh` URL to secure a private repository.
 In this case, your project's `cloneURL` should be of the form `git@github.com:Azure/brigade.git`
 and you will need to add the SSH _private key_ to the `values.yaml` file.

+When doing `brig project create`, if you choose a URL that looks like it supports
+an SSH key, you will be prompted to enter the path to the SSH private key file.


This might be a little confusing - could you explain what looks like it supports an SSH key mean, and how to ensure that I can add an SSH key for my repo?

Rewriting that to make 'use' unambiguous

radu-matei · 2018-07-18T18:45:24Z

brig/cmd/brig/commands/project_create.go

+			{
+				Name: "buildStorageSize",
+				Prompt: &survey.Input{
+					Message: "Build storage size",


Would it help here to get more information about the storage size (unit of measure and the format) without asking for more help?

radu-matei · 2018-07-18T18:46:41Z

brig/cmd/brig/commands/project_create.go

+				Prompt: &survey.Input{
+					Message: "Build Storage Class",
+					Help:    "Kubernetes provides named storage classes. If you want to use a custom storage class, set the class name here.",
+					Default: p.Kubernetes.BuildStorageClass,


It's surely not needed for this PR, but would it be nice to get pre-populated values for the storage classes?

That is a good idea. I will file an issue to add this in a subsequent PR.

radu-matei · 2018-07-18T18:53:35Z

docs/topics/projects.md

+can save a local copy of that secret using `brigade project create --out=myproject.json`.
+
+If you have already created the secret, you can fetch it from Kubernetes by running
+`brig project get my/project` where `my/project` is the project name you assigned.


Not needed for this PR, but if the create command can generate a JSON file, and it can also accept a JSON file as input, would it make sense to have an option on the get command to output as JSON?

I have a branch that does this. I will open it as a PR.

radu-matei · 2018-07-18T19:00:19Z

brig/cmd/brig/commands/project_create.go

+		return nil
+	}
+
+	if !projectCreateReplace {


Would it make sense that instead of a --replace flag to have a brig project edit command / shell out directly to kubectl edit secret ?

I think I would opt for doing that as a separate PR. The --replace will walk you through the questions again, but with the old value s pre-populated. I figured this would be useful for people who needed help text and stuff like that.

This adds `brig project create` to create projects from interactive prompts or from an existing secret. See brigadecore#504

…er command is set Along the way, I also added a convenience setter to the brig project create interface.

adamreese · 2018-07-24T06:13:32Z

brig/cmd/brig/commands/project_create.go

+	projectCreateReplace     bool
+)
+
+var defaultProject = &brigade.Project{


It might be safer to make this a non-pointer. Force a copy of the object so the default stays default.

adamreese · 2018-07-24T06:16:55Z

brig/cmd/brig/commands/project_create.go

+
+	configureWorker := false
+	if err := survey.AskOne(&survey.Confirm{
+		Message: "Configure Brigade worker?",


Should this be part of the Advanced Brigade Settings?

Also, grammar fixes, reorganizing questions into more logical units, and generally doing whatever Adam says to do.

technosophos · 2018-07-24T20:05:38Z

🔥

technosophos added the awaiting review label Jul 6, 2018

technosophos self-assigned this Jul 6, 2018

bacongobbler reviewed Jul 6, 2018

View reviewed changes

technosophos force-pushed the feat/project-create branch from 40197e9 to 966fc1a Compare July 9, 2018 21:05

technosophos mentioned this pull request Jul 9, 2018

feat(brig): List builds by project (brig build list PROJECT) #541

Closed

technosophos added this to the 1.0.0-alpha.1 milestone Jul 10, 2018

technosophos force-pushed the feat/project-create branch from a5dbbbe to 82212a8 Compare July 16, 2018 23:24

radu-matei reviewed Jul 16, 2018

View reviewed changes

technosophos force-pushed the feat/project-create branch from 82212a8 to c81a859 Compare July 17, 2018 18:53

radu-matei reviewed Jul 18, 2018

View reviewed changes

technosophos added 3 commits July 18, 2018 16:43

feat(brig): Interactive prompt for creating projects

26e3732

This adds `brig project create` to create projects from interactive prompts or from an existing secret. See brigadecore#504

fix(brig): add tests for brig project create

4b85d1f

docs(brig): Update the docs for brig project create.

de38794

technosophos force-pushed the feat/project-create branch from c81a859 to 527a2d4 Compare July 18, 2018 22:44

technosophos mentioned this pull request Jul 23, 2018

feat(brig): return JSON from 'brig project get' #560

Closed

fix(controller): Fix bug where if workerCommand is set to "", no work…

78242e8

…er command is set Along the way, I also added a convenience setter to the brig project create interface.

technosophos force-pushed the feat/project-create branch from 527a2d4 to 78242e8 Compare July 23, 2018 23:29

adamreese reviewed Jul 24, 2018

View reviewed changes

technosophos force-pushed the feat/project-create branch from d134c2d to fb19f07 Compare July 24, 2018 16:23

adamreese approved these changes Jul 24, 2018

View reviewed changes

adamreese removed the awaiting review label Jul 24, 2018

fix(brig): clone default project instead of modifying it.

1f193f3

Also, grammar fixes, reorganizing questions into more logical units, and generally doing whatever Adam says to do.

technosophos force-pushed the feat/project-create branch from fb19f07 to 1f193f3 Compare July 24, 2018 19:50

technosophos merged commit c2e31f6 into brigadecore:master Jul 24, 2018

technosophos deleted the feat/project-create branch July 24, 2018 20:05

radu-matei mentioned this pull request Jan 4, 2019

Bring brig project create into parity with the chart #745

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(brig): Interactive prompt for creating projects #534

feat(brig): Interactive prompt for creating projects #534

technosophos commented Jul 6, 2018

bacongobbler Jul 6, 2018

radu-matei Jul 16, 2018 •

edited

Loading

bacongobbler Jul 6, 2018

technosophos Jul 9, 2018

bacongobbler Jul 6, 2018 •

edited

Loading

bacongobbler Jul 6, 2018

bacongobbler Jul 6, 2018

technosophos Jul 9, 2018

bacongobbler Jul 6, 2018

technosophos Jul 9, 2018

bacongobbler Jul 9, 2018

technosophos commented Jul 9, 2018

radu-matei Jul 16, 2018

radu-matei Jul 16, 2018

technosophos Jul 17, 2018

technosophos commented Jul 17, 2018

radu-matei commented Jul 18, 2018

radu-matei left a comment

radu-matei Jul 18, 2018

technosophos Jul 18, 2018

radu-matei Jul 18, 2018

technosophos Jul 18, 2018

radu-matei Jul 18, 2018 •

edited

Loading

technosophos Jul 18, 2018

radu-matei Jul 18, 2018

technosophos Jul 18, 2018

radu-matei Jul 18, 2018

technosophos Jul 18, 2018

technosophos Jul 18, 2018

radu-matei Jul 18, 2018

technosophos Jul 18, 2018

adamreese Jul 24, 2018

adamreese Jul 24, 2018

technosophos commented Jul 24, 2018

feat(brig): Interactive prompt for creating projects #534

feat(brig): Interactive prompt for creating projects #534

Conversation

technosophos commented Jul 6, 2018

Choose a reason for hiding this comment

radu-matei Jul 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bacongobbler Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

technosophos commented Jul 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

technosophos commented Jul 17, 2018

radu-matei commented Jul 18, 2018

radu-matei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radu-matei Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

technosophos commented Jul 24, 2018

radu-matei Jul 16, 2018 •

edited

Loading

bacongobbler Jul 6, 2018 •

edited

Loading

radu-matei Jul 18, 2018 •

edited

Loading