FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

CLOUDSTACK-8799 fixed the defalut routes by bvbharatk · Pull Request #784 · apache/cloudstack · GitHub

  1. apache
  2. cloudstack

Repository navigation

CLOUDSTACK-8799 fixed the defalut routes #784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bvbharatk wants to merge 2 commits into from
Conversation 29 Commits 2 Checks 0 Files changed
Closed

CLOUDSTACK-8799 fixed the defalut routes #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3effdbe
CLOUDSTACK-8799 fixed the defalut routes
bvbharat Sep 4, 2015
da17c14
CLOUDSTACK-8799 fixed for vpc networks.
bvbharat Sep 10, 2015
File filter

Filter by extension

Filter by extension .py  (3) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
    • CsAddress.py
    • CsDatabag.py
    • CsRedundant.py
44 changes: 34 additions & 10 deletions systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,43 @@ def get_control_if(self):
return ip
return None

def check_if_link_exists(self,dev):
cmd="ip link show dev %s"%dev
result=CsHelper.execute(cmd)
if(len(result)!=0):
return True
else:
return False

def check_if_link_up(self,dev):
cmd="ip link show dev %s | tr '\n' ' ' | cut -d ' ' -f 9"%dev

yadvr Sep 8, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

can we use a better string processing here, instead of using hardcoded cut/trim limits?

wido Sep 9, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Indeed and also use the -o flag for 'ip':

ip -o link show dev

Then split the string using Python.

bvbharatk Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi,

the output of Ip link show command is not expected to changes so in this case i think hardcoding the limiters is not an issue.

wido Sep 10, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I still think that we should do the splitting in Python.

Execute ip with the -o flag so it is a one-liner and split with Python afterwards.

remibergsma Sep 10, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If you only want the state, why not simply do this? No magic needed.
cat /sys/class/net/eth0/operstate

bvbharatk Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi,

just out of curiosity why should we do the splitting in python and what is wrong in doing it this way.

wido Sep 11, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should keep as much logic in Python as possible. The performance difference is small, but with tr and grep you spawn subprocessess again.

But as @remibergsma says. Try to open that file in /sys and parse the contents. You can use the simple Python file functions.

No need to execute IP. The less subprocesses we execute, the better.

bvbharatk Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi,
we have used the subprocess command to find the state of the device in two more places. I have created a improvement ticket(CLOUDSTACK-8837) to track this. I will work on this later, until then
i guess the current changes will do.
result=CsHelper.execute(cmd)
if(result[0].lower()=="up"):
return True
else:
return False


def process(self):
route = CsRoute()
found_defaultroute = False

for dev in self.dbag:
if dev == "id":
continue
ip = CsIP(dev, self.config)

for address in self.dbag[dev]:
if(address["nw_type"]!="public"):
continue

#check if link is up
if (not self.check_if_link_exists(dev)):
logging.info("link %s does not exist, so not processing"%dev)
continue
if not self.check_if_link_up(dev):
cmd="ip link set %s up"%dev
CsHelper.execute(cmd)

gateway = str(address["gateway"])
network = str(address["network"])

ip.setAddress(address)
Expand All @@ -122,16 +147,15 @@ def process(self):
"Address %s on device %s not configured", ip.ip(), dev)
if CsDevice(dev, self.config).waitfordevice():
ip.configure()
route.add_route(dev, network)

if address["nw_type"] != "control":
route.add_route(dev, network)
# once we start processing public ip's we need to verify there
# is a default route and add if needed
if not route.defaultroute_exists():
cmdline=self.config.get_cmdline_instance()
if(cmdline.get_gateway()):
route.add_defaultroute(cmdline.get_gateway())

# once we start processing public ip's we need to verify there
# is a default route and add if needed
if address["nw_type"] == "public" and not found_defaultroute:
if not route.defaultroute_exists():
if route.add_defaultroute(gateway):
found_defaultroute = True


class CsInterface:
Expand Down
4 changes: 4 additions & 0 deletions systemvm/patches/debian/config/opt/cloud/bin/cs/CsDatabag.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,7 @@ def get_router_password(self):
md5 = hashlib.md5()
md5.update(passwd)
return md5.hexdigest()
def get_gateway(self):
if "gateway" in self.idata():
return self.idata()['gateway']
return False
17 changes: 3 additions & 14 deletions systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,9 @@ def set_master(self):

self.set_lock()
logging.debug("Setting router to master")
ads = [o for o in self.address.get_ips() if o.is_public()]
dev = ''
for o in ads:
if dev == o.get_device():
continue
cmd2 = "ip link set %s up" % o.get_device()
if CsDevice(o.get_device(), self.config).waitfordevice():
CsHelper.execute(cmd2)
dev = o.get_device()
logging.info("Bringing public interface %s up" %
o.get_device())
else:
logging.error(
"Device %s was not ready could not bring it up" % o.get_device())
self.address.process()
logging.info("added default routes")

# ip route add default via $gw table Table_$dev proto static
cmd = "%s -C %s" % (self.CONNTRACKD_BIN, self.CONNTRACKD_CONF)
CsHelper.execute("%s -c" % cmd)
Expand Down
Back | FazBrowse Home | New Git URL