node-installer icon indicating copy to clipboard operation
node-installer copied to clipboard

fix: use the `ip` command

Open BoboTiG opened this issue 1 year ago • 2 comments

  • fixes #41
  • fix: remove usage of deprecated ifconfig

Now using ip (which is available since at least 2011).

  • fix: stick with ip for the route check

The new code is more readable and contains less lines actually. But I needed to keep the macOS compatibility, so that's why the patch is more important.

If the macOs support can be dropped, then I could simplify more, LMK.

BoboTiG avatar Jul 27 '24 12:07 BoboTiG

OK, the code quite changed since the first commit. It's is very simple on Linux while keeping the compat with macOS.

BoboTiG avatar Jul 27 '24 13:07 BoboTiG

Awesome @BoboTiG! Thanks a lot.

I'll put @herr-seppia as reviewer, as I can't test it properly this week. Looks very good though!

HDauven avatar Sep 02 '24 12:09 HDauven

Superseded by #123.

BoboTiG avatar Jan 03 '25 13:01 BoboTiG

@BoboTiG my PR still doesn't fix the underlying issue, we still rely on ifconfig and route. Ideally we'd upgrade to something more modern like ip and iproute2 in the near future to deal with the local IP. Feel free to tackle #41, and completely rid the script of ifconfig if you have time.

The most important thing is that the behavior that we currently have still works on DigitalOcean/Vultr/Hetzner//local. My PR fixes the behavior for Hetzner, which didn't work before, but we still have legacy in there.

Thank you for putting in the effort to help!

HDauven avatar Jan 03 '25 13:01 HDauven

@HDauven I'll see to do a second round after #123 is merged, and will try to split the new code in functions for an easier review.

BoboTiG avatar Jan 03 '25 14:01 BoboTiG