Astute must compare a sets of UIDs when collect data from netprobe.py

Bug #1284261 reported by Andrey Danin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fuel for OpenStack
Fix Committed
High
Vladimir Sharshov

Bug Description

For some reasons netprobe.py can provide an extra remote uids and Astute must ignore unknown uids when computes a status of network discowering.

Steps to reproduce:
1. Combine any tagged network with any untagged on one interface.
2. Run net_check procedure on a real hardware.
3. Expected result: all ok; real result: verify failed.

Because a NIC driver or NIC itself does not clear buffer between two frames, in case your frame is shorter than the previous one, when you collect raw data from socket, you can get a tail of the previous frame.
In our case netprobe.py does not manage a length of encapsulated data correctly and when it gets untagged frame just after tagged, it gets last four bytes of tagged frame in addition to untagged data.
And because we send UID at the end of our custom frame, netprobe.py recognize such extra data as a new UID and provide this information to Astute. In an attached snapshot you can see that for cluster with two nodes (uids 7 and 8) netprobe.py returns back a hash with UIDs ("6", "7", "7th 7") and Astute marks this link as failed link.

Broken line is:
https://github.com/stackforge/fuel-astute/blob/master/lib/astute/network.rb#L140

Good idea is to use Set(known_uids).subset? Set(remote_uids) instead of comparing arrays of uids.

Revision history for this message
Andrey Danin (gcon-monolake) wrote :
Mike Scherbakov (mihgen)
Changed in fuel:
importance: Critical → High
tags: added: low-hanging-fruit
Changed in fuel:
assignee: nobody → Vladimir Sharshov (vsharshov)
Mike Scherbakov (mihgen)
Changed in fuel:
milestone: 4.1 → 5.0
Revision history for this message
Vladimir Sharshov (vsharshov) wrote :

How we can reproduce this behavior?

I do not find any errors in logs and description with a potencial solution is useful, but without data about nature of bugs and steps to check /reproduce this i could not do anythings.

Please provide more information, thanks!

Changed in fuel:
status: Triaged → Incomplete
description: updated
Changed in fuel:
status: Incomplete → Triaged
Revision history for this message
Vladimir Sharshov (vsharshov) wrote :

Andrey, thanks!

What about such test case

{
  :uid => "1",
  :neighbours => {
    "eth0" => {
      "100" => {"1" => ["eth0"], "2" => ["eth0"]},
      "101" => {"1" => ["eth0"]}}}},
  :sender => "1"
}

and

{
  :uid => "2",
  :neighbours => {
    "eth0" => {
      "100" => {"1" => ["eth0"], "2" => ["eth0"]},
      "101" => {"1" => ["eth0"], "2" => ["eth0"]}
    }}},
  :sender => "2"
}

Expect before:
"nodes" => [{"networks"=>[{"iface"=>"eth0", "vlans"=>[100]}], "uid"=>"1"}, {"networks"=>[{"iface"=>"eth0", "vlans"=>[100, 101]}], "uid"=>"2"}]

Now:
"nodes" => [{"uid"=>"1", "networks"=>[{"iface"=>"eth0", "vlans"=>[100, 101]}]}, {"uid"=>"2", "networks"=>[{"iface"=>"eth0", "vlans"=>[100, 101]}]}]

Is it a real case? Is new answer correct?

If not, can we expect extra remote uids with a such format: any numbers and any non-numeric characters?

Revision history for this message
Andrey Danin (gcon-monolake) wrote :

No. Answer is incorrect. And case too.
In real case nodes will send:
{
  :uid => "1",
  :neighbours => {
    "eth0" => {
      "100" => {"1" => ["eth0"], "2" => ["eth0"]},
      "0" => {"1" => ["eth0"], "2" => ["eth0"], "2th 2" => ["eth0"]} }}},
  :sender => "1"
}
{
  :uid => "2",
  :neighbours => {
    "eth0" => {
      "100" => {"1" => ["eth0"], "2" => ["eth0"]},
      "0" => {"1" => ["eth0"], "2" => ["eth0"]} }}},
  :sender => "2"
}

and now answer will be:
"nodes" => [{"networks"=>[{"iface"=>"eth0", "vlans"=>[100]}], "uid"=>"1"}, {"networks"=>[{"iface"=>"eth0", "vlans"=>[100, 0]}], "uid"=>"2"}]
but it should be:
"nodes" => [{"networks"=>[{"iface"=>"eth0", "vlans"=>[100, 0]}], "uid"=>"1"}, {"networks"=>[{"iface"=>"eth0", "vlans"=>[100, 0]}], "uid"=>"2"}]

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to fuel-astute (master)

Fix proposed to branch: master
Review: https://review.openstack.org/76781

Changed in fuel:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to fuel-astute (master)

Reviewed: https://review.openstack.org/76781
Committed: https://git.openstack.org/cgit/stackforge/fuel-astute/commit/?id=97c91351062ed3f3bf745dc58a3c0e6387a9801a
Submitter: Jenkins
Branch: master

commit 97c91351062ed3f3bf745dc58a3c0e6387a9801a
Author: Vladimir Sharshov <email address hidden>
Date: Thu Feb 27 11:00:59 2014 +0400

    Network check should ignore incorrect uids from netprobe.py

    Change-Id: I835ef9cb49f94f7a43135bce5ab3c2a830e081e8
    Closes-Bug: #1284261

Changed in fuel:
status: In Progress → Fix Committed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.