Skip to content
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.

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

fix/5075 #5088

Merged
merged 4 commits into from
Mar 2, 2020
Merged

fix/5075 #5088

merged 4 commits into from
Mar 2, 2020

Conversation

jrouzierinverse
Copy link
Member

Description

Node deletion in consistent between the the API and pf::node::node_delete

Impacts

API

NEWS file entries

Enhancements

  • Node deletion in consistent between the the API and pf::node::node_delete

Issue

fixes #5075

Delete branch after merge

NO

Checklist

  • Document the feature
  • Add unit tests
  • Add acceptance tests (TestLink)

lib/pf/node.pm Show resolved Hide resolved
lib/pf/node.pm Show resolved Hide resolved
lib/pf/node.pm Outdated Show resolved Hide resolved
@nqb
Copy link
Contributor

nqb commented Feb 3, 2020

@jrouzierinverse,

Another thing I noticed. Now when you remove a node using pfcmd, you got two lines in logs:

Feb  3 09:50:06 pfcen7stable packetfence: WARN pfcmd.pl(29687): 11:11:11:11:11:11 has an open locationlog entry. Node deletion prohibited (pf::node::node_delete)
Feb  3 09:50:06 pfcen7stable packetfence: ERROR pfcmd.pl(29687): Cannot delete node 11:11:11:11:11:11 since there are some records in locationlog table indicating that this node might still be connected and active on the network  (pf::cmd::pf::node::action_delete)

and you get "Cannot delete node 11:11:11:11:11:11 .." on STDERR.

IMO, we should adjust action_delete function in pf::cmd::pf::node to:

  • use message returns by API in place of this different message
  • display message returns by API on STDERR

@nqb
Copy link
Contributor

nqb commented Feb 28, 2020

@jrouzierinverse, what about my last comment ?

@nqb
Copy link
Contributor

nqb commented Mar 2, 2020

It works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: delete of a node is possible even with a locationlog entry opened
2 participants