Skip to content

client: configuration rework#577

Merged
bobheadxi merged 37 commits into
masterfrom
client/#101-configuration
Feb 23, 2019
Merged

client: configuration rework#577
bobheadxi merged 37 commits into
masterfrom
client/#101-configuration

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Feb 21, 2019

Copy link
Copy Markdown
Member

🎟️ Ticket(s): Closes #101


👷 Changes

Many, many changes to configuration, which then caused many changes in client, which means a lot had to change in cmd, which means local must be updated, and I cleaned up a lot of misc stuff... therefore, huge mess of a PR.

Moral of the story: fix yo shit before building too much on top of your shit

Rough tl;dr

  • Inertia now configures projects and remotes separately
  • "projects" are per-repo
    • stored in /path/to/project/inertia.toml
    • "profiles" provide build and deployment configuration, and can (or will be able to be) applied to a remote
    • new command chain inertia project to manage configuration (replaces inertia config)
  • "remotes" are remote VPS instances, and are globally scoped
    • stored in ~/.inertia/inertia.global
    • a remote has a set of "profiles", which maps projects to their applied profiles
    • differentiation between SSH-specific configuration and the rest of the stuff is improved (so that if you use Inertia's auth to access a daemon, Inertia wont be as annoying in pestering you about SSH stuff anymore)

Misc changes

  • shit tons of cleanup
  • added some aliases to a few wordy commands

Examples (updated)

inertia.toml

name = "test"
url = ""

[[profile]]
  name = "dev"
  branch = ""
  [profile.build]
    type = "dockerfile"
    buildfile = "Dockerfile.dev"

[[profile]]
  name = "staging"
  branch = ""
  [profile.build]
    type = "dockerfile"
    buildfile = "Dockerfile.staging"

inertia.global

[[remote]]
  version = ""
  name = "dev"
  ip = "0.0.0.0"
  [remote.ssh]
    user = "bob"
    pemfile = ""
    ssh-port = ""
  [remote.daemon]
    port = "4043"
    token = ""
    webhook-secret = ""
    verify-ssl = false
  [remote.profiles]
    asdf = "asdf"
    oipo = "oiup"

[[remote]]
  version = ""
  name = "staging"
  ip = "0.0.0.0"
  [remote.ssh]
    user = "bob"
    pemfile = ""
    ssh-port = ""
  [remote.daemon]
    port = "4043"
    token = ""
    webhook-secret = ""
    verify-ssl = false
  [remote.profiles]
    fdsa = "fdsaf"
    wqrte = "erterh"

Questions

  • each remote definitely needs version, to track daemon version. do projects need it?
  • is this scalable, potentially for things like Dockerhub deployment Pull, deploy, update Docker-image-based deployments #201 ?
  • does it make sense?
  • given the new structure, is toml still the right choice? would yml be better? toml

🔦 Testing Instructions

nothing yet (tests are all busted)

@bobheadxi bobheadxi added the pr: wip in progress but seeking feedback label Feb 21, 2019
@bobheadxi

Copy link
Copy Markdown
Member Author

paging @iKevinY , @rwblickhan , @brian-nguyen for thoughts 🙏

@rwblickhan

Copy link
Copy Markdown
Contributor

Does this mean we can deploy multiple projects to a single remote?

@bobheadxi

bobheadxi commented Feb 21, 2019

Copy link
Copy Markdown
Member Author

@rwblickhan no, but it does mean that you can more easily deploy a different project to the same remote to "recycle" them so to speak. For example:

inertia remote add local
inertia init
inertia project apply [profile-name] local # not implemented yet

then in another project:

inertia project apply [profile-name] local

then, when you run inertia local up in the two different projects, Inertia will apply your configured "profile" (but it'll still be one project per remote)

that said, if multi-project remotes is something people want... this would help that too 🤔

@mRabitsky mRabitsky left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let's not switch to YAML. YAML is increasingly becoming more and more heinous in my view, and it's only major advantage right now is that there are more [mature] libraries available for working with it. TOML is definitely the better choice, although I definitely think the current structure needs some work: there's a lot of nesting — an example of what good TOML looks like would be Rust's Cargo files.
With some consideration for nesting and similar design choices, TOML should definitely remain the choice of configuration markup in my opinion. As for the question of projects needing versions, perhaps that's something that Git's hashes can handle? (I think that's how it works in other similar systems).

@yaoharry

Copy link
Copy Markdown
Member

I am for the shift to YAML because of the uniformity with cloud providers configuration files e.g gcloud and aws.

@bobheadxi

bobheadxi commented Feb 23, 2019

Copy link
Copy Markdown
Member Author

@yaoharry the thing is, yml was more designed to be handwritten, whereas toml is more consistent and generally generated - hence why I feel like it's been best for Inertia in the past. I think we'll probably stick with it for now, and hopefully we never get ot a point where you have to do a lot of hand-writing inertia configuration 😋 thanks for the feedback though!

@mRabitsky note taken about the nested properties - I've update the configuration to use lists instead of maps. We lose the uniqueness guarantees, but we have a much nicer-looking configuration now (let me know if this still isn't what you had in mind:

inertia.toml

name = "test"
url = ""

[[profile]]
  name = "dev"
  branch = ""
  [profile.build]
    type = "dockerfile"
    buildfile = "Dockerfile.dev"

[[profile]]
  name = "staging"
  branch = ""
  [profile.build]
    type = "dockerfile"
    buildfile = "Dockerfile.staging"

inertia.global

[[remote]]
  version = ""
  name = "dev"
  ip = "0.0.0.0"
  [remote.ssh]
    user = "bob"
    pemfile = ""
    ssh-port = ""
  [remote.daemon]
    port = "4043"
    token = ""
    webhook-secret = ""
    verify-ssl = false
  [remote.profiles]
    asdf = "asdf"
    oipo = "oiup"

[[remote]]
  version = ""
  name = "staging"
  ip = "0.0.0.0"
  [remote.ssh]
    user = "bob"
    pemfile = ""
    ssh-port = ""
  [remote.daemon]
    port = "4043"
    token = ""
    webhook-secret = ""
    verify-ssl = false
  [remote.profiles]
    fdsa = "fdsaf"
    wqrte = "erterh"

@bobheadxi

Copy link
Copy Markdown
Member Author

@mRabitsky re:

As for the question of projects needing versions, perhaps that's something that Git's hashes can handle? (I think that's how it works in other similar systems).

By version I was referring to Inertia version. In remote, this is used to determine which daemon build to deploy - hence why I'm unsure if the version needs to be tracked elsewhere. Right now, if the format changes Inertia will just silently break where the format no longer matches

@bobheadxi bobheadxi added pr: finalized needs review and final approval and removed pr: wip in progress but seeking feedback labels Feb 23, 2019
@bobheadxi bobheadxi marked this pull request as ready for review February 23, 2019 08:43
@bobheadxi bobheadxi requested review from a team, mRabitsky and yaoharry February 23, 2019 08:43
@bobheadxi bobheadxi force-pushed the client/#101-configuration branch from 7325533 to f56b3ca Compare February 23, 2019 09:36
@codecov

codecov Bot commented Feb 23, 2019

Copy link
Copy Markdown

Codecov Report

Merging #577 into master will decrease coverage by 1.14%.
The diff coverage is 62.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
- Coverage    56.5%   55.36%   -1.13%     
==========================================
  Files          62       67       +5     
  Lines        3020     3004      -16     
==========================================
- Hits         1706     1663      -43     
- Misses       1105     1145      +40     
+ Partials      209      196      -13
Impacted Files Coverage Δ
cmd/core/utils/output/print.go 0% <ø> (ø)
local/env.go 20% <ø> (ø) ⬆️
local/init.go 0% <0%> (ø)
provision/ec2.go 8% <0%> (-0.12%) ⬇️
main.go 25% <0%> (ø) ⬆️
common/git.go 81.82% <100%> (+1.82%) ⬆️
client/util.go 100% <100%> (ø)
local/git/git.go 45% <100%> (ø)
cfg/identity.go 100% <100%> (ø)
cfg/global.go 100% <100%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afc9e39...4133120. Read the comment docs.

@iKevinY iKevinY left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't dove into reading any code changes, but the configuration file split looks good to me. Sticking with TOML seems fine; it's human-editable enough.

One minor nitpick: I think pemfile should be changed to something like identityfile (this is the terminology that SSH uses within ~/.ssh/config to refer to a private key file); while AWS issues PEM files, chances are that other cloud providers will just have typical public-private RSA keys.

@bobheadxi

Copy link
Copy Markdown
Member Author

thanks @iKevinY 😋 i've updated the references

you don't have to read the code, sorry for the sinful PR 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: finalized needs review and final approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config: improve format of inertia configuration

6 participants