Add --rm mode for inletsctl create #45

utsavanand2 · 2020-02-03T18:11:13Z

The --rm flag (default true) will enable to point to
an upstream or a remote-tcp and will delete the exit-node
on a SIGINT (control + c)

Fixes #41

Signed-off-by: Utsav Anand [email protected]

Description

The --rm flag will enable developers to quickly expose locally running endpoints
and delete the exit nodes when they are done previewing with a ctrl + c

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Users will be able to use the --rm flag to delete the tunnel as soon as inletsctl is interrupted with
a (control + c). This will make sure any instances are deleted automatically after inletsctl exits with an interrupt.
A great feature for Web-Devs who want to give a preview of their projects with reviewers for a short while

Checklist:

I have:

updated the documentation and/or roadmap (if required)
read the CONTRIBUTION guide
signed-off my commits with git commit -s
added unit tests

alexellis

I think we can get the code a bit clearer before merging. Separation of concerns will make this easier to maintain

cmd/create.go

alexellis · 2020-02-05T20:02:56Z

./inletsctl create --rm --provider digitalocean --access-token  ~/Downloads/do-access-token
Using provider: digitalocean
Requesting host: recursing-leakey2 in , from digitalocean
2020/02/05 20:01:53 Provisioning host with DigitalOcean

I don't think this shouldn't work when the user hasn't specified an upstream port, can you show an error?

alexellis · 2020-02-05T20:03:40Z

Also can you think why I got an auth error?

POST https://api.digitalocean.com/v2/droplets: 401 Unable to authenticate you.

alexellis · 2020-02-05T20:05:17Z

Hmm it was --access-token vs --access-token-file

alexellis · 2020-02-05T20:06:28Z

  inletsctl create  \
	--provider [digitalocean|packet|ec2|scaleway|civo|gce] \
	--access-token-file $HOME/access-token \
	--region lon1

Please give an example showing --rm and how to invoke it with port in the CMD help text?

alexellis · 2020-02-05T20:07:11Z

The inlets-pro error is perhaps because the server takes a little while to come up, in which case you should retry the tunnel for let's say 5 times with a small pause between?

utsavanand2 · 2020-02-05T20:13:02Z

Thanks a lot! I'll keep this feedback in mind for the next commit push.

…

On Thu, Feb 6, 2020, 1:37 AM Alex Ellis ***@***.***> wrote: The inlets-pro error is perhaps because the server takes a little while to come up, in which case you should retry the tunnel for let's say 5 times with a small pause between? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AGAYDROYHXIIE5VG75MZA4DRBML7BA5CNFSM4KPJU6QKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK4ZPBY#issuecomment-582588295>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGAYDROF7IORBF3HR3UX2ZLRBML7BANCNFSM4KPJU6QA> .

utsavanand2 · 2020-02-05T20:16:08Z

./inletsctl create --rm --provider digitalocean --access-token  ~/Downloads/do-access-token
Using provider: digitalocean
Requesting host: recursing-leakey2 in , from digitalocean
2020/02/05 20:01:53 Provisioning host with DigitalOcean
I don't think this shouldn't work when the user hasn't specified an upstream port, can you show an error?

I'm using a default value for the upstream, http://127.0.0.1:3000, I think we should log it while creating the tunnel saying, using the default value for upstream http://127.0.0.1:3000

What do you think?

alexellis · 2020-02-05T20:32:03Z

The output needs some work too, we should be using StreamStdio so that we get a live printout of the child process.

Instead I only see this when I hit control + c:

ct to proxy\" error=\"dial tcp 159.65.83.139:8080: connect: connection refused\"\ntime=\"2020-02-05T20:08:15Z\" level=error msg=\"Failed to connect to proxy\" error=\"dial tcp 159.65.83.139:8080: connect: connection refused\"\ntime=\"2020-02-05T20:08:20Z\" level=info msg=\"Connecting to proxy\" url=\"ws://159.65.83.139:8080/tunnel\"\n", exit-code: -1
Deleting tunnel
exiting

alexellis · 2020-02-05T20:32:27Z

No, don't use a default, if --rm is given, the user must change the port or specify remote-tcp.

alexellis · 2020-02-05T20:33:21Z

The behaviour looks different between create + delete, vs --rm then control+c - the ID wasn't printed out.

Deleting tunnel
exiting
kosmos:inletsctl alex$ ./inletsctl delete --provider digitalocean --access-token-file  ~/inlets-token --id 178924410
Using provider: digitalocean
Deleting host: 178924410 from digitalocean

alexellis · 2020-02-05T20:34:46Z

Your version of github.com/alexellis/go-execute is too old, please update it?

Also - you'll need to use Command for only the first part of the text, and Args[] for everything else. Please can you change that?

utsavanand2 · 2020-02-05T20:36:22Z

Sure @alexellis I'll do this!

utsavanand2 · 2020-02-05T20:36:55Z

Thanks a lot for the feedback!

alexellis · 2020-02-05T20:38:08Z

You're going to need a rebase now, because I've added go modules.

After rebasing from master, run:

go mod tidy

alexellis · 2020-02-09T08:38:37Z

Hi @utsavanand2 I noticed the issue title now says "fix issue with GCE running inlets-pro"

Didn't I merge that fix via another PR? If so, please could you rebase this PR against master and then edit the title to remove that part?

utsavanand2 · 2020-02-09T09:12:05Z

@alexellis Sir I think I just need to change the title because this PR changes opens up all ports for running inlets-pro (specifically allowing all ports a user can use with --tcp-ports flag)

alexellis · 2020-02-09T09:51:40Z

I've edited the title for you on this PR to (WIP) Add --rm mode for inletsctl create -> it should not include any other changes.

You also need a rebase. Could I ask you to prioritize fixing the following PRs #56 #44 #55? A separate branch might be a good idea.

Thank you for contributing 👍

utsavanand2 · 2020-02-09T09:52:48Z

Sure!

utsavanand2 · 2020-03-22T18:05:56Z

cmd/create.go

+				fmt.Printf("Your IP is: %s\n", hostStatus.IP)
+
+				var err error = nil
+				ctx, cancelFunction := context.WithCancel(context.Background())


@viveksyngh I would love to have your feedback on this. Even after passing a req context to checkServiceUp() we're not able to cancel our request and exit out.

utsavanand2 · 2020-03-22T18:06:15Z

@viveksyngh I would love to have your feedback on the code, especially where we're creating and passing the context https://github.com/inlets/inletsctl/pull/45/files#r396123320

alexellis · 2020-04-09T13:46:53Z

@viveksyngh - I think there's been two members meetings where you've agreed to take a look at this, are you still able to assist or should we find someone else?

viveksyngh · 2020-04-09T13:47:56Z

I will have look at it this weekend.

viveksyngh · 2020-04-09T20:09:44Z

I have created this PR to utsav branch to fix the issue which @utsavanand2 is facing.

cc @alexellis

utsavanand2#1

viveksyngh · 2020-04-09T19:55:37Z

cmd/create.go

+
+				go func() {
+					sigval, ok := <-sig
+					fmt.Printf("\n%v\n", sigval)


I think after receiving the signal, you need to call the cancel function as soon as possible, so that context can be canceled and http request being made in checkServiceUp should pause and return from there. For that you might need to move cancelFunction() inside this go routine. There will other changes required as well for that.

Since function will also return from there, you need to make sure that you delete the instance as well. for that you will have to one more goroutine to delete that. I have these changes in my local and I will raise a PR to this branch .

derek · 2020-04-10T16:33:13Z

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@viveksyngh

an upstream or a remote-tcp and will delete the exit-node on a SIGINT (control + c) Huge thanks to @viveksyngh for helping me out with the context cancellation bug! Fixes inlets#41 Signed-off-by: Utsav Anand <[email protected]>

utsavanand2 · 2020-04-15T11:50:10Z

@alexellis I think we're ready to merge this PR now so we can take advantage of the new feature

utsavanand2 · 2020-04-15T11:50:44Z

@viveksyngh Thanks a lot for helping me out!

utsavanand2 requested a review from alexellis February 3, 2020 18:11

utsavanand2 mentioned this pull request Feb 3, 2020

Fix issue with the opening of TCP ports for inlets-pro on GCE #44

Closed

alexellis requested changes Feb 4, 2020

View reviewed changes

cmd/create.go Show resolved Hide resolved

cmd/create.go Show resolved Hide resolved

utsavanand2 changed the title ~~(WIP) Add --rm flag to "inletsctl create" and fix issue with GCE running inlets-pro~~ (WIP) Add --rm flag to "inletsctl create" and fix issue with GCE running inlets-pro by opening all TCP ports Feb 9, 2020

alexellis changed the title ~~(WIP) Add --rm flag to "inletsctl create" and fix issue with GCE running inlets-pro by opening all TCP ports~~ (WIP) Add --rm mode for inletsctl create Feb 9, 2020

utsavanand2 changed the title ~~(WIP) Add --rm mode for inletsctl create~~ Add --rm mode for inletsctl create Feb 9, 2020

utsavanand2 force-pushed the tmp-tunnel branch 2 times, most recently from b4bba83 to 13b7f9d Compare February 9, 2020 17:14

utsavanand2 force-pushed the tmp-tunnel branch 4 times, most recently from 70e02da to 98f3cbe Compare February 24, 2020 15:08

utsavanand2 force-pushed the tmp-tunnel branch 12 times, most recently from 80c67b9 to c1fa6b8 Compare February 27, 2020 08:17

utsavanand2 force-pushed the tmp-tunnel branch 4 times, most recently from 12cba05 to ebdf87e Compare March 10, 2020 12:47

utsavanand2 commented Mar 22, 2020

View reviewed changes

viveksyngh reviewed Apr 9, 2020

View reviewed changes

derek bot added the no-dco label Apr 10, 2020

The --rm flag will enable to point to

4163b8c

an upstream or a remote-tcp and will delete the exit-node on a SIGINT (control + c) Huge thanks to @viveksyngh for helping me out with the context cancellation bug! Fixes inlets#41 Signed-off-by: Utsav Anand <[email protected]>

utsavanand2 force-pushed the tmp-tunnel branch from 8bf5c63 to 4163b8c Compare April 15, 2020 11:48

derek bot removed the no-dco label Apr 15, 2020

alexellis closed this Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --rm mode for inletsctl create #45

Add --rm mode for inletsctl create #45

utsavanand2 commented Feb 3, 2020 •

edited

Loading

alexellis left a comment

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020 via email

utsavanand2 commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 9, 2020

utsavanand2 commented Feb 9, 2020

alexellis commented Feb 9, 2020

utsavanand2 commented Feb 9, 2020

utsavanand2 Mar 22, 2020

utsavanand2 commented Mar 22, 2020

alexellis commented Apr 9, 2020

viveksyngh commented Apr 9, 2020

viveksyngh commented Apr 9, 2020

viveksyngh Apr 9, 2020

viveksyngh Apr 9, 2020

derek bot commented Apr 10, 2020

utsavanand2 commented Apr 15, 2020

utsavanand2 commented Apr 15, 2020

Add --rm mode for inletsctl create #45

Add --rm mode for inletsctl create #45

Conversation

utsavanand2 commented Feb 3, 2020 • edited Loading

Description

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

alexellis left a comment

Choose a reason for hiding this comment

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020 via email

utsavanand2 commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020

utsavanand2 commented Feb 5, 2020

alexellis commented Feb 5, 2020

alexellis commented Feb 9, 2020

utsavanand2 commented Feb 9, 2020

alexellis commented Feb 9, 2020

utsavanand2 commented Feb 9, 2020

utsavanand2 Mar 22, 2020

Choose a reason for hiding this comment

utsavanand2 commented Mar 22, 2020

alexellis commented Apr 9, 2020

viveksyngh commented Apr 9, 2020

viveksyngh commented Apr 9, 2020

viveksyngh Apr 9, 2020

Choose a reason for hiding this comment

viveksyngh Apr 9, 2020

Choose a reason for hiding this comment

derek bot commented Apr 10, 2020

utsavanand2 commented Apr 15, 2020

utsavanand2 commented Apr 15, 2020

utsavanand2 commented Feb 3, 2020 •

edited

Loading