knorrie / network-examples

Linux networking examples and tutorials
Other
950 stars 80 forks source link

fix minor problems #5

Closed yvolchkov closed 6 years ago

yvolchkov commented 6 years ago

This is basically my feedback on this amazing tutorial. Changes in this patches are tiny little things which would save me quite a bit of time.

knorrie commented 6 years ago

Hi,

These are some nice improvements.

I dragged the commits into my branch and made a few small changes along the way. They're mentioned in the commit messages. You still get the credits. :-)

Thanks for doing the tutorial and contributing.

Hans

knorrie commented 6 years ago

Don't be discouraged by the red github traffic sign. It's just that I prefer merging myself instead of via github.

knorrie commented 6 years ago

Ah, I changed the check_connectivity.sh to display all errors at once instead of stopping at the first one, because that can provide a better view on all the things that are failing. But then I messed it up a bit by removing the "success" line at the end, which caused the poor thing to not say anything at all when every test succeeds.

Instead of adding back a "Yay!!" or some cowsay I just made it also output all [OK] now, since it gives an idea about how many different spots in the infrastructure need to be able to talk to each other. The total amount is not that big that it would result in a too long list.

Hope you like it o/

yvolchkov commented 6 years ago

Don't be discouraged by the red github traffic sign. It's just that I prefer merging myself instead of via github.

I do hate merge commits too. And do merge manually too. This is not a problem

However, I disagree with one modification you made to my patches. Just tell me next time what you do not like. I will fix it. Or introduce more overhead by arguing :)

-    for dst in $routers
-    do
-       lxc-attach -n R$src -- ping6 -c 1 r$dst
-       if [ $? -ne 0 ]
-       then
-          echo
-          echo "Connectivity test failed: R$src -> R$dst"
-          exit 1
-       fi
-    done
+       for dst in $routers
+       do
+               lxc-attach -n R$src -- ping6 -c 1 r$dst >/dev/null 2>&1
+               if [ $? -ne 0 ]
+               then
+                       echo "[FAIL] R$src -> R$dst"
+               #else
+               #       echo "[OK] R$src -> R$dst"
+               fi
+       done

So you just deleted my "success" notification, silenced everything, added OK or FAIL to every output... and commented the OK branch (??). And then you are "fixing" my mistakes in your follow-up patch, by uncommenting the OK branch back. Now my patch looks a bit silly. UPDATE: Ok I basically repeated what you already said in the comment above. Shit happens. And this is not a terrible thing. My modifications/patches are never perfect either (I guess you can tell).

-        cp ~/network-examples/ospf-intro/lxc/R* . -r
+        cp ~/network-examples/ospf-intro/lxc/[RH]* . -r

Crap, that's why I had to configure hosts by myself :). I thought it is a part of an assignment.

knorrie commented 6 years ago

Yeah, sorry about that. I already pushed the master branch before I did that last one. I should have waited a bit for your feedback on my additional changes.