add option to not HUP on reload

[cglewis]

This closes #22.

It needs to wait for #25 to be merged first for the linting to pass.

[lantz]

I'd recommend fixing the pylint error in this PR even though it may lead to a merge conflict.

Note that you can update a commit with git --amend and then git push --force to update this PR.

However, even if you fix the code check issue I don't think it will work as expected, since it looks like FAUCET doesn't update config_hash_info in this situation, so we'll have to fix that first. I implemented something (very) similar and ran into that problem.

[cglewis]

Ok, those tests are not working as I expected. I'll have to mess with this some more.

[lantz]

Nice - this is getting there; I think that FAUCET_CONFIG_STAT_RELOAD should be set appropriately in the FAUCET controller class. Usually the idiom is to add an initialization parameter to the controller constructor, and then pass it into the network constructor, something like:

faucet = partial(FAUCET, config_stat_reload=1)
net = Mininet(topo=TestTopo(), controller=faucet, autoSetMacs=True)

I think something like this will be good on the agent side; then we'll need to change FAUCET to make sure that faucet_config_hash_info is updated properly when a reload is triggered due to FAUCET_CONFIG_STAT_RELOAD. Once that's done and merged in, then the --nohup test should pass. I will take a look at it when I get the chance.

[cglewis]

Ok, I think this is closer, though it's still not applying the environment variable to the config_stat_reload so something is not quite right. There's also something still causing the config_files error in the tests that I haven't tracked down yet. I'll have to get back to this a bit later.

[cglewis]

tests for this should pass now once https://github.com/faucetsdn/faucet/pull/2979 is merged in. I had to remove the writing an empty config file, as Faucet never gets out of the error state when using CONFIG_STAT_RELOAD. I'm not entirely sure why that behavior is different then HUP, but since it's an error that's getting thrown in both cases, it seemed like it was cleaner to remove anyway.

[lantz]

If the FAUCET patch generates the prometheus var when a config load fails does this mean the original agent test will pass?

[cglewis]

@lantz yes, that is my understanding, if I'm tracking what you mean. Even with the patch to Faucet the tests will still fail with an empty config file using HUP.

[cglewis]

@lantz is this good to go, or does it need anything else before it can be merged?

[lantz]

p.s. I still think we should start with clearing the config file.

[cglewis]

@lantz i can add back in clearing the config file, but the tests will fail and I'm not sure how to fix them, but we can add it back in until we figure out how to fix the failure, if you prefer?

[lantz]

Yes, I would like to figure out how to fix it.

[lantz]

Actually we've messed with this enough. I think we can just move forward for now.