faucetsdn / faucetagent

gNMI agent for faucet configuration
5 stars 2 forks source link

Logging set failed when Faucet isn't running, but the config is changed nonetheless #21

Closed cglewis closed 5 years ago

cglewis commented 5 years ago
DEBUG:faucetagent:Writing FAUCET config
DEBUG:faucetagent:Reloading FAUCET config
DEBUG:faucetagent:Sending HUP (config reload signal) to FAUCET
ERROR:grpc._server:Exception calling application: Command '['fuser', '-k', '-HUP', '9302/tcp']' returned non-zero exit status 1
Traceback (most recent call last):
  File "/home/.local/lib/python3.5/site-packages/grpc/_server.py", line 392, in _call_behavior
    return behavior(argument, context), True
  File "./faucetagent.py", line 280, in Set
    self.faucet.write_config(replace.val.string_val)
  File "./faucetagent.py", line 202, in write_config
    self.reload(config=data)
  File "./faucetagent.py", line 175, in reload
    run(cmd.split(), check=True)
  File "/usr/lib/python3.5/subprocess.py", line 708, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['fuser', '-k', '-HUP', '9302/tcp']' returned non-zero exit status 1
F0520 19:45:01.454529   18522 gnmi_set.go:160] Set failed: rpc error: code = Unknown desc = Exception calling application: Command '['fuser', '-k', '-HUP', '9302/tcp']' returned non-zero exit status 1

The config file is successfully set, but the HUP is failing because Faucet isn't running. Not sure what the best action for improving this is, since it is working as intended/expected, but it's somewhat misleading the way the errors are logged.

lantz commented 5 years ago

The semantics are whether the config is accepted by FAUCET. If FAUCET crashes or isn't running, then the result is not successful. I used to have a separate error condition but it seemed redundant so I removed it since it raises an exception anyway.

Do you think it should be added back in?

cglewis commented 5 years ago

I guess the question is what state the config file should be in if a "set" fails for any reason? Or should it just be assumed that the config file can no longer be trusted if a "set" fails?

lantz commented 5 years ago

I think we want to fail if we are sending a HUP but FAUCET isn't running. Since we now have --nohup it seems like this shouldn't be a huge issue. If you're OK with the way things are now, we can close this issues as resolved.