draft create should install draft.toml if chart/ exists

[bacongobbler]

Right now if you have an application that is ready to be deployed by draft but is missing a draft.toml, draft create won't provide one.

><> draft create
--> chart directory already exists. Ready to sail!

This app is not ready to sail because it is missing a draft.toml. Instead, what we should do is

• detect that no pack detection is needed
• drop a draft.toml in the app directory if none already exists

In CLI form, this should look like:

><> draft create
!!! chart directory already exists. Skipping pack detection.
--> Ready to sail

[bnookala]

@bacongobbler I made a quick prototype of this change, and I found an edge case that may need accounting. After re-generating draft.toml through draft create, the chart's Chart.yaml file still references the original generated name, kind-meerkat:

$ cat draft.toml
[environments]
  [environments.development]
    name = "smelly-greyhound"
    namespace = "default"
    wait = false
    watch = false
    watch_delay = 2

$ cat chart/Chart.yaml
apiVersion: v1
description: A Helm chart for Kubernetes
name: kind-meerkat
version: 0.1.0

$ helm list
NAME                REVISION    UPDATED                     STATUS      CHART                   NAMESPACE
smelly-greyhound    1           Thu Jul  6 17:00:19 2017    DEPLOYED    kind-meerkat-0.1.0      default

Code is here, for reference: https://github.com/bnookala/draft/blob/draft-create-toml-fix-219/cmd/draft/create.go#L90

[bacongobbler]

Yeah, that is certainly something we need to account for. Right now we have two different places where the application name is written to the filesystem on draft create:

1. Chart.yaml (as you saw)
2. draft.toml

I have toyed around with making draft.toml a single source of truth, and instead having the chart named as "chart" or by overwriting the chart name in-memory any time draft is invoked. It would allow helm package chart/ to work, but I'm not sure if that's the best approach overall as the chart name should be a representation of what is being deployed.

Any thoughts you might have on how we can best approach this problem?

[bnookala]

@bacongobbler Just from the top of my head, perhaps the name of the chart should be based around the application name? For example, an application with a directory structure like so:

myApp/
   package.json
   index.js

Would receive a chart.yaml with a generated structure:

apiVersion: v1
description: A Helm chart for Kubernetes
name: myapp
version: 0.1.0

And that way, you wouldn't have to worry about chart.yaml's name field being out of date when re-running draft create, as it wouldn't change after creation. I'm not confidant that this won't break anything, but seems like it would be a decent compromise - thoughts?

[bnookala]

@bacongobbler I took some time to try and build out the suggestion from my previous comment: https://github.com/bnookala/draft/blob/draft-create-toml-fix-219/cmd/draft/create.go#L68:L85

What do you think of this approach? I've been able to confirm it works via helm, as well:

$ ../../bin/draft create
!!! chart directory already exists. Skipping pack detection.

$ cat chart/Chart.yaml
apiVersion: v1
description: A Helm chart for Kubernetes
name: nodejs
version: 0.1.0

$ cat draft.toml
[environments]
  [environments.development]
    name = "handy-fly"
    namespace = "default"
    wait = false
    watch = false
    watch_delay = 2

$ helm list
NAME            REVISION    UPDATED                     STATUS      CHART                   NAMESPACE
handy-fly       1           Mon Jul 10 17:32:12 2017    DEPLOYED    nodejs-0.1.0            default

[bacongobbler]

I think that's a step in the right direction. It still won't work with helm package, but it does stop the name field from being irrelevant after a name change. I approve. :+1:

[radu-matei]

Reiterating through @squillace's comments on #422:

1. We could detect port 80 and prefix based on that.
2. We could spawn the location, but then add a "if it's a web app, click http://localhost:xxxx; otherwise, open the endpoint using the appropriate tool."
3. We could add some metadata in the draft.toml file for this, and prompt the user to switch it off/on?
4. ....uh....

I'm inclined to go with 2 or 3... thoughts?

[bacongobbler]

@radu-matei did you mean to comment in a different ticket such as #327? This ticket's about providing a draft.toml when charts/ is present in the current directory.

[radu-matei]

Yes, thanks for catching it...