Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

[WIP] Bug fix: only remove states when last remote unit departs. #2

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ def joined_or_changed(self):

@hook('{provides:kube-control}-relation-{broken,departed}')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -broken hook completely fails with the current conversation model for interface layers because there is no REMOTE_UNIT and that's not handled well. It also doesn't have a real use in the reactive pattern because you should be removing the states on a per-unit basis anyway and -broken just becomes the state being removed from the last conversation and thus starting to match @when_not again.

TL;DR: interface layers should only ever use -departed and using -broken can cause some strange issues.

def departed(self):
"""Remove all states.
"""Remove all states when the last remote unit leaves the relation.

"""
conv = self.conversation()
conv.remove_state('{relation_name}.connected')
conv.remove_state('{relation_name}.gpu.available')
if not self.conversations():
Copy link
Contributor

@johnsca johnsca Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an @hook context, there will always be exactly one conversation, for the REMOTE_UNIT of the hook. I think that this interface layer is actually correct (save for the -broken hook issue mentioned in the other comment) and the issue may be in how it's being used in the charm layer.

self.remove_state('{relation_name}.connected')
self.remove_state('{relation_name}.gpu.available')

def set_dns(self, port, domain, sdn_ip):
"""Send DNS info to the remote units.
Expand Down