Opened 8 years ago

Closed 8 years ago

#1503 closed defect (fixed)

dhcp4 and dhcp6 daemons must have option to specify port number

Reported by: tomek Owned by: tomek
Priority: medium Milestone: DHCP-Sprint-20120611
Component: dhcp Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Medium
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Both b10-dhcp4 and b10-dhcp6 components must have an option that specifies port number. This should be clearly marked in man pages that this feature is only useful for testing purposes.

Subtickets

Change History (10)

comment:1 Changed 8 years ago by tomek

src/bin/dhcp6/tests/dhcp6_test.py should be improved once this is implemented.
Similar test (src/bin/dhcp4/tests/dhcp4_test.py) should be implemented for dhcp4.

comment:2 Changed 8 years ago by stephen

  • Milestone changed from DHCP 2012 to Sprint-DHCP-20120305

comment:3 Changed 8 years ago by tomek

  • Owner changed from UnAssigned to tomek
  • Status changed from new to accepted

comment:4 Changed 8 years ago by tomek

  • Owner changed from tomek to UnAssigned
  • Status changed from accepted to reviewing

I do not have much Python experience, so I do not know how to fix one particular issue. There is runDhcp4() method that is roughly based on code from src/bin/bind10/tests/bind10_test.py. It does exhibit the same problem. It manifests itself every time a new process is created, its output read and process is killed. There is one file open somewhere. Framework complains about this. Example warning:

src/bin/dhcp4/tests/dhcp4_test.py:131: ResourceWarning?: unclosed file <_io.FileIO name=9 mode='wb'>

Implemented changes for b10-dhcp4 only. Once the review is concluded, I will implement the same way for b10-dhcp6. Please advise if you prefer this to be done within the same ticket or should a separate ticket be created for that purpose.

Please review.

comment:5 Changed 8 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:6 Changed 8 years ago by stephen

  • Owner changed from stephen to tomek

Checked commit b35d3159e812712e47d59db80ae0115e58cd5c82

All OK (apart from the incorrect spelling of "unprivileged" in dhcp4_test.py :-). I suggest implementing the changes for b10-dhcp6 on this ticket.

Regarding the ResourceWarning. According to the docs.python.org article What's New in Python 3.2:

A ResourceWarning is also issued when a file object is destroyed
without having been explicitly closed. While the deallocator for
such object ensures it closes the underlying operating system
resource (usually, a file descriptor), the delay in deallocating
the object could produce various issues, especially under Windows.

There does not appear to be anything obvious, so I guess it is a problem in Python, somewhere in the "subprocess" class.

comment:7 Changed 8 years ago by tomek

  • Owner changed from tomek to stephen

Implemented identical tests (and the functionality itself) for dhcp6. Also, fixed spelling of "unpriviledged". Changes pushed to trac1503. Please review.

comment:8 Changed 8 years ago by stephen

  • Owner changed from stephen to tomek

Reviewed commit e60af9fa16a6094d2204f27c40a648fae313bdae

The only question I have is why the equivalent of "runDhcp4" in the dhcp6 version of the test program is called "runCommand" and not "runDhcp6".

However, all is OK, please merge.

comment:9 Changed 8 years ago by tomek

The reason is that this is a step towards unification. Actually, I plan to rename runDhcp4 method to runCommand as well. We need to define a common base class one day.

Will merge the code tomorrow morning. It is too late today.

comment:10 Changed 8 years ago by tomek

  • Resolution set to fixed
  • Status changed from reviewing to closed

Code merged. Closing ticket.

Note: See TracTickets for help on using tickets.