Skip to content
This repository was archived by the owner on Aug 15, 2018. It is now read-only.

Conversation

d78ui98
Copy link
Contributor

@d78ui98 d78ui98 commented Jul 18, 2018

@d78ui98
Copy link
Contributor Author

d78ui98 commented Jul 18, 2018

These commits were done hours ago. I was trying to fix the test.sh script. In case of normal mode the vagrant box(ltsp server) server does not get ip address from the dhcp server. As a result the script fails in nornal mode.

Vagrantfile Outdated
@@ -6,7 +6,11 @@ Vagrant.configure("2") do |config|

config.vm.box = VB_IMAGE

config.vm.network "public_network", ip: LAN_IP, netmask: "255.255.255.0", bridge: LAN_IF
if $STANDALONE == "yes"

Choose a reason for hiding this comment

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

when comparing string always convert the case.

install.sh Outdated

# Fetching LAN_IP and network address
if [[ ${STANDALONE,,} != yes ]]; then
LAN_IP=$(ip addr | grep "inet\b" | cut -d" " -f6 | grep 192 | cut -d/ -f1)
Copy link

@akash0x53 akash0x53 Jul 18, 2018

Choose a reason for hiding this comment

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

You assumed LAN_IP will always class C 192.168.yyy.zzz. One can set 172.xxx.yyy.zzz

try
ip addr show <interface> | grep -Po 'inet \K[\d.]+'

Copy link
Contributor Author

@d78ui98 d78ui98 Jul 18, 2018

Choose a reason for hiding this comment

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

I am not sure if the interface name would be same everytime. Hardcoding interface does not seem like a good idea.
nice regex by the way :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, interface is the same every time, and it is LAN_IF.
The code proposed by Akash is correct

Copy link
Contributor Author

@d78ui98 d78ui98 Jul 18, 2018

Choose a reason for hiding this comment

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

Using this is also a good idea I think. Bcoz bridged interface will be always on 3 position.
ip addr | grep -Po -m3 'inet \K[\d.]+' | tail -n1
Let me know what are your thoughts on this.

Copy link
Member

Choose a reason for hiding this comment

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

No, it is not a good idea, because it never guarantied that the order of the interfaces will be the same. Do as Akash and me suggested: ip addr show $LAN_IF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. you are right.

install.sh Outdated
if [[ ${STANDALONE,,} != yes ]]; then
LAN_IP=$(ip addr | grep "inet\b" | cut -d" " -f6 | grep 192 | cut -d/ -f1)
sed -i /vagrant/settings.sh \
-e "/LAN_IP=""/ c LAN_IP=\"$LAN_IP\""
Copy link
Member

Choose a reason for hiding this comment

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

It is not a good idea to modify settings.sh from install.sh.
I also don't think you need to do that.

removed lines to modify settings.sh
install.sh Outdated

# Fetching LAN_IP and network address
if [[ ${STANDALONE,,} != yes ]]; then
LAN_IP=$(ip addr | grep -Po -m3 'inet \K[\d.]+' | tail -n1)

Choose a reason for hiding this comment

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

network interfaces at my end

1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
2: enp0s3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether 08:00:27:af:06:9a brd ff:ff:ff:ff:ff:ff
    inet 10.0.2.15/24 brd 10.0.2.255 scope global dynamic enp0s3
       valid_lft 85960sec preferred_lft 85960sec
    inet6 fe80::bc9e:d02b:c104:fcef/64 scope link 
       valid_lft forever preferred_lft forever
3: docker0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN group default 
    link/ether 02:42:68:9b:93:36 brd ff:ff:ff:ff:ff:ff
    inet 172.17.0.1/16 brd 172.17.255.255 scope global docker0
       valid_lft forever preferred_lft forever

above command gives me dockers ip address, which is not what we need.

Check if these snippet helps you. This will not require interface name, no grep, no assumption.

akash@skyfall:$ ip route get 8.8.8.8
8.8.8.8 via 10.0.2.2 dev enp0s3  src 10.0.2.15 
    cache 
akash@skyfall:$ ip route get 8.8.8.8|head -n1|cut -d ' ' -f8

Copy link
Member

Choose a reason for hiding this comment

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

above command gives me dockers ip address, which is not what we need.

Akash, Deepanshu was assuming that the command was executed inside the VB machine, not on the host. It seems that you are trying the command on the host.

ip route get 8.8.8.8

This does not work for the case of STANDALONE, because it will give the WAN_IP, not the LAN_IP. (By the way, ip route | grep default is better than this, when we need to get the WAN_IP).

Choose a reason for hiding this comment

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

Right, i ran command on my host machine. ip route | grep default is also good option to avoid hardcoding interface name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like ip addr output of host machine.(because of docker0)
In vagrant box also first is lo, second is enp0s3, third is enp0s8(bridged adapter the one we need)

Copy link
Contributor Author

@d78ui98 d78ui98 Jul 18, 2018

Choose a reason for hiding this comment

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

is also good option to avoid hardcoding interface name here.

Yes I was also trying to do the same. See my thoughts were. when we do a vagrant up. network adapters are created. Output is something like this -

==> default: Preparing network interfaces based on configuration...
    default: Adapter 1: nat
    default: Adapter 2: bridged

So I thought network adapter will always be created in this ordered mentioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, interface is the same every time, and it is LAN_IF.
The code proposed by Akash is correct

So should we merge this?


# Fetching LAN_IP and network address
if [[ ${STANDALONE,,} != yes ]]; then
LAN_IP=$(ip addr show enp0s8 | grep -Po 'inet \K[\d.]+')

Choose a reason for hiding this comment

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

Seems okay to use interface for now.

@d78ui98 d78ui98 merged commit 4efa9ca into bionic Jul 19, 2018
@d78ui98 d78ui98 deleted the new-ltsp branch July 19, 2018 18:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update virtual LTSP server project to new version of LTSP
3 participants