Cleanup when sdn-plugin provider has gone away #92

Merged
merged 2 commits into from Oct 19, 2016

Conversation

Projects
None yet
3 participants
Collaborator

chuckbutler commented Oct 19, 2016

In the current code in master, the docker daemon is left in a broken
state if the SDN provider ever goes away.

  • renames the private method _reconfigure_docker_for_sdn to a much
    clearer indication of whats happening
  • Adds state detection to determine when we should stop workloads and
    reconfigure the daemon
  • Leverages the new methods added to the DockerOptsManager in
    charms.docker 1.11.0 for popping flags out of the dict.
Cleanup when sdn-plugin provider has gone away
In the current code in master, the docker daemon is left in a broken
state if the SDN provider ever goes away.

- renames the private method _reconfigure_docker_for_sdn to a much
  clearer indication of whats happening
- Adds state detection to determine when we should stop workloads and
  reconfigure the daemon
- Leverages the new methods added to the DockerOptsManager in
  charms.docker 1.11.0 for popping flags out of the dict.

@chuckbutler chuckbutler referenced this pull request in juju-solutions/charm-flannel Oct 19, 2016

Closed

juju remove-application cleanup not complete #19

reactive/docker.py
+ except KeyError:
+ hookenv.log('Unable to locate bip or mtu in Docker config.')
+ hookenv.log('Assuming no action required, and restarting the daemon.')
+
@wwwtyro

wwwtyro Oct 19, 2016

What if bip isn't in opts but mtu is? Seems like we'd fail to remove mtu -- is that acceptable?

@chuckbutler

chuckbutler Oct 19, 2016

Collaborator

hmmm, seems odd that you would have an interface MTU configuration without BIP. But i concede the point, that its possible a prior execution could have unintended side effects with checking both inline like this instead of independent checks.

Contributor

mbruzek commented Oct 19, 2016

For how I understand the DockerOpts object work, we need to pop the key/value off before running. This change looks good to me. +1

@mbruzek mbruzek merged commit f313c6a into juju-solutions:master Oct 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment