/flannel-docker-charm

Make NetworkRelation resilient when related to multiple services #16

Merged
merged 2 commits into from May 18, 2015

Conversation

Reviewers
No reviews
Assignees

@chuckbutler chuckbutler

Labels
enhancement help wanted
Projects
None yet
Milestone
No milestone
3 participants
@chuckbutler @mbruzek @whitmo
@chuckbutler
Owner

chuckbutler commented May 14, 2015

I'd like a review on this w/ a possible change in direction on how we can better sniff if we're attached to another host. This might mean implementing a new host type juju-info relation, but this was the simplest path to completion for the PoC work

DO NOT MERGE

This will have a side-effect if we relate to a docker host, and we encounter a transient failure, we have to dig deep in the logs to find out why. And I don't like silently failing on a core component of this relationship model that we established to have docker manage its own config.

@chuckbutler chuckbutler added enhancement help wanted question labels May 14, 2015

@chuckbutler chuckbutler self-assigned this May 14, 2015

@chuckbutler
Owner

chuckbutler commented May 14, 2015

@mbruzek @whitmo pinging to start the discussion
@chuckbutler
Owner

chuckbutler commented May 15, 2015

It was suggested to implement a variable being sent from the kubes_master unit to denote we can safely ignore the error. This seems like a reasonable path forward.

@chuckbutler chuckbutler locked and limited conversation to collaborators May 15, 2015

@chuckbutler chuckbutler unlocked this conversation May 15, 2015

@chuckbutler
Changes the lookup path from shell "relation-ids" to using the scoped 
databag provided by ansible

- This is an untested evaluation of this method to access the data. It
  calls an implicit assumption to array index 0 - this may bite us later
4b32969

@chuckbutler chuckbutler changed the title from Ignore network relation errors, this is wrt Kubernetes network relation to Ignore network relation data when not on a docker host May 15, 2015

@chuckbutler
Owner

chuckbutler commented May 15, 2015

I'm satisfied the current implementation is a better path moving forward than what we have in the charm code today. This is ready for a review @whitmo and @mbruzek

@chuckbutler chuckbutler removed the question label May 15, 2015

@chuckbutler chuckbutler changed the title from Ignore network relation data when not on a docker host to Make NetworkRelation resilient when related to multiple services May 18, 2015

@mbruzek
Collaborator

mbruzek commented May 18, 2015

@chuckbutler explained the problem to me. My understanding is this fix is necessary when there are 2 "network" relations to this charm it can not tell which one to call the relation-set command with.

I was concerned about the array reference of zero but I see the command is short circuited with an '| default("")'. This code change looks good to me!
@whitmo
Collaborator

whitmo commented May 18, 2015

LGTM 👍

chuckbutler added a commit that referenced this pull request May 18, 2015

@chuckbutler
Merge pull request #16 from chuckbutler/kubernetes-network-patch 
Make NetworkRelation resilient when related to multiple services
79c9342

@chuckbutler chuckbutler merged commit 79c9342 into master May 18, 2015

chuckbutler added a commit to whitmo/kubernetes that referenced this pull request May 18, 2015

@chuckbutler
Repoint the flannel-docker charm to github pending charmstore publish 
- Note: as of the writing of this Commit, the PR has not been accepted
  that addresses the multi-network relationship id bug that was squashed
  with PR chuckbutler/flannel-docker-charm#16
8320102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment