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

feat: add flag for router IP local run #565

Merged
merged 13 commits into from
Jan 10, 2023
Merged

feat: add flag for router IP local run #565

merged 13 commits into from
Jan 10, 2023

Conversation

joshua-mo-143
Copy link
Member

@joshua-mo-143 joshua-mo-143 commented Jan 4, 2023

Reference: issue #350

Summary:

  • adds local_ip_address crate to be able to grab local router IP
  • add new flag --router_ip to be able to use router IP instead of localhost, which allows other devices on the network to access the local deployment.
  • relevant testing has been added to account for new feature

Notes:
local_ip_address was added for this feature implementation as it looks to be a stable crate that doesn't appear to be going inactive any time soon and appears to work across OSes with no code change required (tested using Ubuntu and Win11). In the event that it does (or there's some breaking issue that renders it unusable), it looks like a method would have to be implemented per OS to get the router IP.

Windows users who are using this flag in Bash/WSL may have problems getting other devices to get this to work if they are not aware of the fact that there are extra steps1 required to be able to connect external devices to a locally deployed run in Bash/WSL due to the fact it uses the WSL virtual ethernet. Not a huge problem as it seems most people who use shuttle are currently running Linux, but it's something to keep in mind should any issues regarding this pop up.

The new test also uses the same example as one of the other tests (the "hello world" Rocket example), which may or may not break the tests. Not entirely sure because trying to run all integration tests is broken on my laptop due to memory issues, but although the test passes in isolation it's probably something to be aware of.

Footnotes

  1. (https://learn.microsoft.com/en-us/windows/wsl/networking)

Copy link
Contributor

@chesedo chesedo left a comment

Choose a reason for hiding this comment

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

Nice, thanks for this @joshua-mo-143!

I'm wondering, won't binding to the address 0.0.0.0 be enough? Then the local_ip crate won't be needed and I would expect users to know how to get their address on the network if they want to test from other devices.

cargo-shuttle/tests/integration/run.rs Outdated Show resolved Hide resolved
cargo-shuttle/src/lib.rs Outdated Show resolved Hide resolved
@joshua-mo-143
Copy link
Member Author

Ah that makes a lot more sense, I didn't realise you could bind it to 0.0.0.0 - that would probably be more optimal since it eliminates the need to add a dependency and would eliminate WSL-specific issues which would be helpful for compatibility. Looks like I still have a lot to learn about using IPs and networking 😅

Forgot to delete a dependency and it looks like I can't squash commits, whoops 🤦
Re-pushing this amendment but without the git diff. Probably should have changed that before submitting initially.
@oddgrd
Copy link
Contributor

oddgrd commented Jan 5, 2023

This looks cool Josh! It looks like you need to do a cargo fmt to make the CI happy, and maybe clippy. And dont worry about squashing commits, we'll do that when we merge. 🙂

Copy link
Contributor

@chesedo chesedo left a comment

Choose a reason for hiding this comment

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

So sorry for another round of changes, but it is small 😄

@@ -143,6 +143,9 @@ pub struct RunArgs {
/// port to start service on
#[clap(long, default_value = "8000")]
pub port: u16,
/// use router ip address instead of localhost
#[clap(long)]
pub router_ip: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sold on the name, but can't think of anything better either 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

The only other things I can really think of are network or local_ip, but yeah to be honest I'm not huge on either of them and router_ip is not really that descriptive

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly tempted to use network or lan as the flag since router_ip or use_router_ip isn't technically accurate anymore, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I was thinking more like 'host' but don't like it. How about external?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok that sounds like a good idea, I'll add that in then

cargo-shuttle/tests/integration/run.rs Outdated Show resolved Hide resolved
cargo-shuttle/tests/integration/run.rs Outdated Show resolved Hide resolved
@joshua-mo-143
Copy link
Member Author

It's ok, I appreciate the checks to make sure everything is optimised before pushing through - I'm personally more concerned about solving the circleCI cargo shuttle test failing since that seems to be a pressing issue. It looks like it's running into the same issue I had in local which is that it failed on trying to wait for start up

@chesedo
Copy link
Contributor

chesedo commented Jan 6, 2023

The "rocket/postgres" has intermittent issues starting up, so don't worry about it too much. If your new test works locally then I'm fine merging this even if the other tests fail on the CI

+ cargo_shuttle_run() now returns a string containing the full base URL instead of port
+ Relevant changes have been made to tests to account for this
Changed as per discussion on the PR.
Copy link
Contributor

@oddgrd oddgrd left a comment

Choose a reason for hiding this comment

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

Thanks a lot Josh, this is a really useful feature! 🚀

@oddgrd oddgrd merged commit 8f71804 into shuttle-hq:main Jan 10, 2023
chesedo added a commit that referenced this pull request Jan 16, 2023
* feat(blog): add missing sqlx migration code to auth blog post (#408)

* feat(blog): add missing sqlx migration code

* fix: nit

* misc: 0.7.0 (#407)

* misc: drop patches

* infra: pin postgres to version 14

* refactor: fix thruster example app name

* infra: db_fqdn fix

* tests: warp hello world

* fix(cargo-shuttle): prevent crash when config owned by root (#409)

* fix: use correct timeout start point (#410)

* fix: use correct timeout start point

* tweak health check frequency

* fmt

* feat(deployer): implement container memory limits (#411)

* feat(deployer): implement container memory limits

* test: fix warp integration test

* bug: `transport error` when trying to connect to provisioner (#416)

* bug: keep provisioner connections alive

* refactor: reduce scope provisioner client is running

* feat: gateway admin revive (#412)

* feat: gateway admin command (revive)

* fmt

* clippy

* refactor: revive deployers using GatewayService

* tests: add ContextArgs

* refactor: simplify passing around of fqdn

* tests: update test archive

* refactor: remove stray exec.rs file

* refactor: unused is_error()

Co-authored-by: chesedo <[email protected]>

* bug: timeout curl health check on deployer (#415)

* Article/beta article (#420)

* updated docs url

* beta article w/ damiens corrections

* Feat(www): shuttle beta signup (#421)

* feat(www): add beta signup form

* feat: add formspree endpoint

* feat: remove socials footer from beta page

* feat: add dummy text above sign up form

* feat: placeholder gif

* feat: align dummy text left

* feat: remove gif, add text

* feat: update beta page text

* feat: update the rest of the site

* bug: Fix thruster postgres example (#414)

* feat: shell completions (#343)

* bug: package Secrets.toml (#422)

* bug: package Secrets.toml

* refactor: clippy suggestion

* bug: big archives being cut off at 32 768 bytes (#423)

* refactor: set chunked header

* refactor: don't send stream across threads

Doing this is causing us to loose everything after 32 768 bytes. Don't
know the reason why 🤷

* refactor: fix health check

* refactor: remove unused use

* chore: v0.7.1 (#424)

* chore: v0.7.1

* chore: resources v0.7.1

* docs: v0.7.1

* tests: v0.7.1

* tests: fix warp test

* tests: fix warp test

* tests: fix secrets e2e test

* ci: remove hard coded DD env

* misc: unintended shell changes

* refactor: cargo check suggestion

* refactor: provide better context for errors (#430)

* refactor: do our own health checks on deployer containers (#427)

* refactor: do our own health checks on deployer containers

We are doing this because the docker health checks eats up 25% of a
single CPU core and we are spinning up many of these containers.

https://www.reddit.com/r/docker/comments/b68r53/healtchecks_add_high_cpu_load/
moby/moby#39102

* refactor: clippy suggestion

* bug: clear build folder before extracting (#428)

* refactor: make sure extract directory is created

* tests: test build folder is cleared

* bug: clear the build directory

* added some images (#435)

* feat(www): beta blog updates (#434)

* feat: add captioned image component (#440)

* feat: add captioned image component

* feat: update images

* feat: add header caption

* fix: quotes in header

* www: post small tweaks (#439)

* a few tweaks to the article

* feat: update gif

Co-authored-by: oddgrd <[email protected]>

* feat: replace cursed gif (#441)

* Feat/update contributing (#426)

* feat(docs): update contributing.md

* feat(e2e): make e2e admin user unique

* feat(e2e): clean up e2e test projects on test complete

* feat(e2e): don't fail on superuser conflict

* feat: add section about contributing to docs

* feat: remove unwrap, add comment to drop impl

* tests: update e2e test key

Co-authored-by: chesedo <[email protected]>

* chore: v0.7.2 (#442)

* chore: v0.7.2

* chore: resources v0.7.2

* docs: v0.7.2

* misc: v0.7.2

* misc: script to deploy all examples

* misc: targets for publishing crates

* misc: remove old scripts

* misc: instructions for next steps

* misc: get Cargo.lock changes

* misc: update .PHONY

* misc: show usage

* feat: link the tracing spans between services (#445)

* ci: DD environments

* refactor: try lowering broadcast channel for quicker feedback

* refactor: propagate tracing across threads in deployer

* refactor: propagate tracing between gateway and deployer

* refactor: trace account name

* refactor: associate project with each container

* feat: trace and propagate proxy in gateway

* feat: gateway record project

* refactor: tracing use 'error' for errors

* refactor: fix comment

* refactor: drop recording unused field

* Add docker-compose extra flags param in Makefile (#446)

* Add docker-compose extra flags param

* rename to avoid confusion

* misc: restructure repo (#453)

* misc: only run e2e on production

* misc: move www to shuttle-hq/www repo

* misc: move examples to shuttle-hq/examples repo

* refactor: update links to examples repo

* ci: remove checking fmt of examples

* ci: checkout submodules

* refactor: tf files have been moved to shuttle-hq/terraform-aws-shuttle

* fix: wrap around common::ProjectName for parsing (#451)

* fix: gateway state drifts, health checks and project recreation (#447)

* misc: add more helpful flags to Makefile

* misc: remove old migrator (#463)

* feat: add account_tier column (#458)

* feat: prefetch shuttle-service crates (#461)

* feat: prefetch shuttle-service crates

* refactor: add comment to prepare.sh files

* Feat: revive via gateway endpoint (#460)

* feat: revive via gateway api initial commit

* feat: cleanup, document gateway testing

* refactor: use post for revive endpoint

Co-authored-by: Damien <[email protected]>

* feat: send task to global queue

Co-authored-by: Damien <[email protected]>

* feat: create a new admin cli binary crate (#462)

* feat: init shuttle-admin

* feat: add args

* feat: simple client

* feat: read api_key

* refactor: hook it all together

* refactor: switch to post

* fix: broken link (#467)

* fix: add timeout to health checks (#468)

* fix: add timeout to health checks

* Parameterise the timeout

* rollback on inadvertent circleci change

* misc: log out a warning when a task has been running for a long time

* longer timeouts

* Address review

* Fmt

* feat(gateway): add custom domains table and routing (#465)

* WIP feat: count recent start events before restart (#469)

* feat: count recent start events before restart

* feat: try_collect and handle error in start counter

* fix: clippy

* refactor: gateway clippy

* feat: make deployer only answer its own project (#466)

* feat: make deployer only answer its own project

* bug: use correct project name

* refactor: make backwards compatible

* Fixed Links in Readme (#477)

* refactor: base client error off response status code (#470)

* refactor: base client error off response status code

* Update common/src/models/error.rs

Co-authored-by: Oddbjørn Grødem <[email protected]>

* refactor: use tracing

Co-authored-by: Oddbjørn Grødem <[email protected]>

* feat: verify project exists before sending destroy task (#474)

* feat: add a custom domains admin route (#473)

Co-authored-by: Alex Krantz <[email protected]>

* bug: deployer freezes (#478)

* bug: reduce spawning to have deployer lock up less

* refactor: don't return cargo logs

* ci: green (#482)

* tests: allow longer time for build

* refactor: trim dependencies on persist

* refactor: clippy suggestion

* feat: TLS acceptor with SNI resolver (#471)

* fix: custom domain routing (#484)

* refactor: more metrics (#475)

* feat: add more metric dimensions to deployer

* feat: add more metric dimensions to gateway

* refactor: common metrics code

* refactor: forward account name

* refactor: add backend feature to deployer

* refactor: standardize naming

* refactor: cargo sort

* misc: configurable deployment tags (#486)

* feat: gateway restores removed containers (#485)

* fix: backend bumps and hot fixes (#487)

* Feature/support actix web (#491)

* Support actix-web: impl Service

* Support actix-web: cli + boilerplate code

* Support actix-web: add framework to doc comment

* Support actix-web: add e2e test

* Support actix-web: fmt

* Support actix-web: sorting toml deps

* Support actix-web: add actix-web to ci

* Support actix-web: formatted boilerplate code for actix-web hello world example

* Support actix-web: formatted boilerplate code for actix-web hello world example
Support actix-web: update cargo lock zstd dep

* Support actix-web: simplify example

* Support actix-web: add test, change example dependency

* fix: e2e test assert, reset example module

Co-authored-by: maksim <[email protected]>
Co-authored-by: oddgrd <[email protected]>

* Doc: Fix command to prime database with docker-compose (#502)

Signed-off-by: Federico Guerinoni <[email protected]>

Signed-off-by: Federico Guerinoni <[email protected]>

* Doc: Improve contributing documentation (#499)

In the doc it mentions to go to subfolder of examples but it was not
there :).

Signed-off-by: Federico Guerinoni <[email protected]>

Signed-off-by: Federico Guerinoni <[email protected]>

* feat: static file support for a single folder (#501)

* feat: static-folder resource

* refactor: make local client work

* refactor: make local deployer work

* feat: storage_manager

* refactor: comments

* refactor: remove unwraps and expects

* refactor: update tests

* refactor: clippy suggestion

* refactor: update version

* refactor: update readme

* refactor: update comment

* refactor: change public to static

* ci: add static-folder

* refactor: code fixes

* refactor: update tests

* refactor: update description

Co-authored-by: Damien <[email protected]>

Co-authored-by: Damien <[email protected]>

* ci: build binary (#483)

* ci: build binary

* ci: make tar archive

* ci: make GH draft release with artifacts

* ci: add aarch64

* ci: get version from tag

* ci: rename to linux builds

* ci: add windows

* ci: add macos

* ci: test all

* ci: put all the artifacts under a command

* ci: only on tagged

* ci: restore old

* ci: fix windows

* ci: put archive in separate folder

* ci: switch to tar

* ci: restore old

* refactor: better `cargo-binstall` support

Co-authored-by: Isaiah Gamble <[email protected]>

* refactor: binary name

Co-authored-by: Isaiah Gamble <[email protected]>

Co-authored-by: Isaiah Gamble <[email protected]>

* feat: bump rust to 1.64, bump dependencies (#495)

* feat: bump rust to 1.64, correct contributing.md bug

* feat: bump service deps, change example branch

* fix: pin chrono to 4.22

* chore: upgrade workspace dependencies

* feat: remove aws-smithy-types

ref: smithy-lang/smithy-rs#1760

* feat: upgrade axum to 0.6 in common

* feat: upgrade to axum 0.6.0 in deployer

* feat: upgrade gateway to axum 0.6.0

* feat: upgrde sqlx to 0.6.2 in gateway

* feat: bump chrono to 0.4.23

* feat: replace deprecated chrono functions in deployer

* feat: bump chrono in common

* feat: ignore actix integration test

* feat: implement new state extractor in gateway

* feat: interactive project initialization (#498)

* feat: bump pinned rust version to 1.65 (#504)

* feat: bump pinned rust version to 1.65

 lockfile update is from binary dist PR #483

* ci: install newer protoc

* fix: special module name warning

* fix: clippy

* feat: install newer protoc in dockerfile as well

* misc: env updates (#509)

* ci: update toolchain for binaries

* misc: update dev environment

* misc: remove patches from Cargo.lock

* feat: make the folder configurable (#508)

* feat: make the folder configurable

* Update resources/static-folder/README.md

Co-authored-by: Damien <[email protected]>

Co-authored-by: Damien <[email protected]>

* feat: implement workspace inheritance (#506)

* refactor: switch away from cargo package (#507)

* feat: add spinner wait for `project new` and `project status --follow` (#503)

Signed-off-by: Federico Guerinoni <[email protected]>
Co-authored-by: oddgrd <[email protected]>

* fix: capitalise correctly (#511)

* fix: make nice (#512)

* feat: find (soon to be) invalid project names (#479)

* feat: find (soon to be) invalid project names

* refactor: move logic to admin client

* refactor: missed axum 0.6 update (#513)

* feat: build tests in release profile, limit build workers (#514)

* feat: build tests in release profile, limit build workers

* fix: typo in manifests

* feat: create `init` project from correct dir (#518)

* misc: interactive init gif (#519)

* Feat/set examples submodule to main (#520)

* feat: revert examples submodule back to main

* feat: un-ignore actix tests

* chore: 0.8.0 (#521)

* chore: bump examples (#522)

* bug: hacking static folders (#524)

* fix: actix integration with state (#523)

* feat(gateway,deployer): add more tracing events (#500)

* feat(deployer): add more tracing events

* feat(gateway): add more tracing events

* feat: canonicalize before trace (#531)

* feat: 'clean' subcommand (#530)

* feat: 'clean' subcommand

* refactor: output cleaning is done

* Feat/set cpu limit (#529)

* feat: remove redundant actix worker limit

* feat: remove redundant build job limit

* feat: set cpu limit for deployer container

* fix: limit actix worker and build jobs to 4

* feat: add panamax for mirroring crates.io (#525)

* feat: build queue (#532)

* feat: per-project parallelism (#533)

* feat: temp validation of project name in gateway (#534)

* fix(deployer): keep Cargo.lock between deployments (#517)

* ci: remove build and push req on build binaries (#535)

* refactor: don't crash when failing to release slot (#536)

* refactor: release build slot parse type correctly (#538)

* refactor: remove prefetch (#539)

* feat: add cron job for syncing mirror (#537)

* chore: bump cargo-shuttle to 0.8.1 (#540)

* chore: bump cargo-shuttle to 0.8.1

* chore: cargo.lock

* test: fixes (#554)

* tests: custom domains

* tests: end to end

* Refactor: remove deprecated auth command (#550)

* refactor: remove deprecated auth command

* docs: update main readme

* ci: add Makefile command for Windows to convert .sh files to LF format (#555)

* Add documentation for unblocking Windows CRLF

+ Add documentation for unblocking CRLF issues in Windows using git commands

* refactor: footnote for git link, remove extra newlines

Co-authored-by: oddgrd <[email protected]>

* Fix: dependencies compiled with incompatible versions of rustc (#545)

* feat: disable honor_rust_version

* feat: add rust toolchain override to deployer

* feat: remove rust-toolchain from deployer

* feat: add RUSTUP_TOOLCHAIN env to deployer container

* feat: get currently installed rustc version

* fix: better naming

* feat: set containerfile rust version via makefile

also set the RUSTUP_TOOLCHAIN env var via the dockerfile, and pass it to deployers

* refactor: remove unused dep

Co-authored-by: Pieter <[email protected]>

* refactor: remove toolchain env from deployer config

Co-authored-by: Pieter <[email protected]>

* feat: create subcommand to list all projects of calling account (#553)

* feat: create subcommand to list all projects of calling account

* fix: filter query by account name

* tests: iter_user_projects_detailed

Co-authored-by: chesedo <[email protected]>

* bug: deployer drifting state (#548)

* ci: fix bin builds (#546)

* ci: comment out other jobs than linux bins

* ci: try without tag

* feat: set git tag with git describe

* feat: echo tag in CI for debugging

* fix: set tag earlier for testing

* ci: echo artifacts directory

* ci: pass tag via parameters

* ci: move artifacts in target sub directory

* ci: create artifacts subdir

* ci: test draft release

* ci: authenticate ghr

* ci: try with symlink

* ci: revert symlink, cp bins from subdirs before release

* ci: flatten artifacts dir before releasing

* ci: set-tag command

* ci: fix invalid config

* fix: invalid config

* ci: test if artifacts flattening command works for win bin

* ci: enable linux bin build too

* ci: remove environment field

* ci: check artifacts dir structure

* ci: tweak ghr command and verify tag is set

* ci: include tag in make-artifact env

* fix: invalid command

* fix: incorrect path to bash.env

* ci: set tag in env in preceding step

* ci: clean up, add comments

* fix: rm path

* ci: re-enable all workflows, remove tag filters

* fix: formatting

* update contributing (#556)

Make docker compose commands consistent

* feat: Support Poise (#560)

* feat: impl Service for Poise

* feat(poise): add to ci

* doc(poise): add link to poise example

* feat(poise): add poise to cargo shuttle init

Co-authored-by: oddgrd <[email protected]>

* feat: add flag for router IP local run (#565)

* feat: add flag for router IP local run

* refactor: amend ipaddr setting, local run init fn

* refactor: delete unnecessary dependency

Forgot to delete a dependency and it looks like I can't squash commits, whoops 🤦

* Revert "refactor: delete unnecessary dependency"

This reverts commit 1794cc6.

* refactor: remove unnecessary dep (amendment)
Re-pushing this amendment but without the git diff. Probably should have changed that before submitting initially.

* clippy

* amendment from clippy warning (needless borrow)

* refactor: apply cargo fmt diffs

* refactor: return URL from run test setup fn
+ cargo_shuttle_run() now returns a string containing the full base URL instead of port
+ Relevant changes have been made to tests to account for this

* refactor: change flag name to be 'external'

Changed as per discussion on the PR.

* refactor: use passed args directly for struct

* test: make sure tests align to proper folders

Co-authored-by: Oddbjørn Grødem <[email protected]>

* bug: no networks (#541)

* refactor: connect to user network on startup

* feat: revive containers not connected to the user network

* refactor: attach network first

* refactor: revive created containers

* feat: attaching state

* refactor: resetting some code

Signed-off-by: Federico Guerinoni <[email protected]>
Co-authored-by: Oddbjørn Grødem <[email protected]>
Co-authored-by: Alexander Krantz <[email protected]>
Co-authored-by: Damien <[email protected]>
Co-authored-by: Ivan <[email protected]>
Co-authored-by: Peter Mertz <[email protected]>
Co-authored-by: XyLyXyRR <[email protected]>
Co-authored-by: Damien <[email protected]>
Co-authored-by: Nereuxofficial <[email protected]>
Co-authored-by: Maxim <[email protected]>
Co-authored-by: maksim <[email protected]>
Co-authored-by: Federico Guerinoni <[email protected]>
Co-authored-by: Isaiah Gamble <[email protected]>
Co-authored-by: Stijn ("stain") Seghers <[email protected]>
Co-authored-by: Joshua Mo <[email protected]>
Co-authored-by: Emma <[email protected]>
Co-authored-by: Oleks Gnatovskyi <[email protected]>
Co-authored-by: Anafabula <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants