Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: configuration rework #577

Merged
merged 37 commits into from
Feb 23, 2019
Merged

client: configuration rework #577

merged 37 commits into from
Feb 23, 2019

Conversation

bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented Feb 21, 2019

🎟️ 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
Member Author

paging @iKevinY , @rwblickhan , @brian-nguyen for thoughts πŸ™

@rwblickhan
Copy link
Contributor

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

@bobheadxi
Copy link
Member Author

bobheadxi commented Feb 21, 2019

@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 πŸ€”

Copy link

@mRabitsky mRabitsky left a comment

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
Member

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

@bobheadxi
Copy link
Member Author

bobheadxi commented Feb 23, 2019

@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
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
@codecov
Copy link

codecov bot commented Feb 23, 2019

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.

Copy link
Member

@iKevinY iKevinY left a comment

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
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