vhive-serverless / vHive

vHive: Open-source framework for serverless experimentation
MIT License
265 stars 84 forks source link

Fix GetNodeIP to get the first ip #982

Closed lrq619 closed 1 month ago

lrq619 commented 2 months ago

Summary

Change the bash command for getting node's private ip address Now it would get the first ip address starting with 10.

Implementation Notes :hammer_and_pick:

Change bash command implementation to:

ip route | awk '{print $(NF)}' | awk '/^10\..*/ {print; exit}'

External Dependencies :four_leaf_clover:

Breaking API Changes :warning:

Simply specify none (N/A) if not applicable.

leokondrashov commented 2 months ago

Did you check it for operation in the case of two addresses in the subnet?

We should maybe try to let the user choose the IP out of the available ones, as was proposed in the issue.

lrq619 commented 2 months ago

Did you check it for operation in the case of two addresses in the subnet?

We should maybe try to let the user choose the IP out of the available ones, as was proposed in the issue.

Tested by replacing ip route to cat route_ip.txt, and change the contents in the txt file to simulate enviroments with multiple ip starting from 10.

Enabling user choosing NodeIP is good, but I haven't come up with any ideas. Most naive one would be pop out a prompt and let user input an ip_address, but I don't think adding such interaction operation would be good in a setup script.It would block there if no input is sent, and this setup script sometimes need to be run by automatical workflows like github actions instead of real person)

Do you have any suggestion?

lrq619 commented 2 months ago

@JooyoungPark73 I think the gvisor runner is failing again, could you please restart it? Thank you

yulinzou commented 2 months ago

Solving this issue, added error handling when ip route | awk '{print $(NF)}' | awk '/^10\..*/ {print; exit}' return empty IP, will fallback to get from hostname -I

lrq619 commented 1 month ago

@leokondrashov Hi, could you please review this branch so we can get this branch merged?