Opened 4 years ago

Closed 4 years ago

#4249 closed defect (fixed)

Address distcheck failures after merge of #4224

Reported by: marcin Owned by: marcin
Priority: high Milestone: Kea1.0
Component: tests Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no


The #4224 introduced identifying process using PID file. When the shell test is doing a cleanup it needs to identify all processes by PID files and kill them. kea-lfc is using a different PID file name format than other processes and the current code doesn't kill LFC during cleanup. My suspicion is that the additional lease files (generated by LFC) can be removed during cleanup because the process still writes to them. In any case, the first thing to address is to make sure that the LFC process is killed after the test.

This is regression so I put it to 1.0 on my discretion.


Change History (5)

comment:1 Changed 4 years ago by marcin

  • Owner set to marcin
  • Status changed from new to accepted

comment:2 Changed 4 years ago by marcin

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

The issue seems to be because of the race conditions when killing processes during the cleanup after test. The 'kea-lfc' instances are now identified using 'pgrep' rather than PID file. Note that 'kea-lfc' creates PID file after it is started and removes the PID file when it shuts down. Thus, there are situations when the process is running (perhaps starting) and the PID file doesn't exist yet. The cleanup code would not find the process (because of lack of PID file) and thus the process will not be terminated and the cleanup will continue. The kea-lfc may terminate when the cleanup is over and leave the products of LFC (lease files) not removed. The use of pgrep seems to be good remedy for that.

Proposed ChangeLog? entry:

10XX.	[bug]		marcin
	Addressed regression in distcheck after merge of #4224.
	(Trac #4224, git abcd)

comment:3 Changed 4 years ago by tmark

  • Owner changed from UnAssigned to tmark

comment:4 Changed 4 years ago by tmark

  • Owner changed from tmark to marcin

Your changes look great, exept they don't work. ;)

You renamed the kill_pids function to kill_processes but did not change the invocation. Since we're renaming things, I renamed kill_processes to kill_processes_by_name and fixed the invocation.

The tests pass under OS_X as well as make -j8 distcheck.

I think your change log entry should briefly describe the regression that was fixed.

Pull my changes and the feel free to merge.

comment:5 Changed 4 years ago by marcin

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

Merged with commit 04aa9b95bf8c4dd8b555dd78cc8cd57126473800

Note: See TracTickets for help on using tickets.