Add basic proxy relation #10

Merged
merged 1 commit into from Jul 8, 2015

Conversation

Projects
None yet
2 participants
Contributor

Lukasa commented Jun 16, 2015

For Project Calico's OpenStack charms, we deploy etcd proxies on every node. These need access to the cluster token in order to function correctly, but they aren't raft peers. Rather than join on the cluster relation, it seemed like the better idea was to create a new relation expressly for proxies.

I have not yet tested this, but I wanted to open for review feedback as soon as possible.

Collaborator

chuckbutler commented Jun 16, 2015

This looks good @Lukasa

I'd like to see some tests added for this, a simple standup, relation add and validation of whats sent across the wire would be perfect to ensure the function stays behaving as expected while we continue to iterate on the ETCD charm would be most welcome.

Contributor

Lukasa commented Jun 16, 2015

@chuckbutler You bet.

In the short term I'm going to do some quick live test to confirm it plays nice with my other charms, and then I'll swing back around to this and add some test.

Collaborator

chuckbutler commented Jun 16, 2015

Sounds great. Thanks for the contribution. If you need any assistance in getting tests landed lmk and i'll be happy to dedicate some time to this.

Contributor

Lukasa commented Jun 18, 2015

Alright cool, this works live. I'll take a look at arranging tests today.

Contributor

Lukasa commented Jun 18, 2015

So based on my understanding of Amulet, the way this needs to work is actually that I need to deploy a charm that satisfies the proxy relation, which is currently only my in-development Calico charms, which are probably a bit heavy to deploy in this kind of testing.

Is that understanding wrong? Can I test this in Amulet without deploying an extra charm? If I cannot, is the best thing to do to write an etcd proxy charm (despite its limited utility)?

Collaborator

chuckbutler commented Jul 6, 2015

Actually @Lukasa, it would be great if your Calico charms had the amulet test included - and we could simply port that bundle/test over to the ETCD charm here. While I was under the impression we could stub some things out and test it - that doesn't appear to be the case from the notes I read on the mailing list last week.

Is there a minimal configuration for those charms we can deploy to flex this relationship?

Contributor

Lukasa commented Jul 6, 2015

Deploying neutron-calico is a bit painful because you have to deploy nova-compute, but if you're happy to do that there's no extra config required. Alternatively, you can deploy neutron-api from my branch, which only requires this tiny amount of config.

Collaborator

chuckbutler commented Jul 6, 2015

@Lukasa i can understand that. I think that if you have the openstack level tests in the calico bundle, we can create a mock charm that flexes this relationship and doesn't have the dependency chain of a full openstack standup. You would own the mock charm, and ensure it conforms to what you're expecting to send/receive over the interface.

That will keep the etcd charm tests light weight, and satisfy this dependency. Lets link up over IRC and I can send you a repository w/ the charm to satisfy my request, once you sign off on that, publish it in your namespace - we'll get this merged up and march on towards the future.

does this sound acceptable to you? (i hate that you're maintaining the details in more than one place, but i cannot think of a better method to date today - and will bring this up at our next ~charmer meeting)

Collaborator

chuckbutler commented Jul 8, 2015

After a discussion in IRC - there's now a test suite and a mock charm to validate we dont break the proxy relationship moving forward.

https://github.com/chuckbutler/etcd-proxy-mock-charm/blob/master/tests/10-verify-etcd-proxy

Once this has landed in @Lukasa's launchpad repo (or we can put this in ~zoology perhaps @whitmo and add @Lukasa to the members list) - we'll have a charm store copy, and get this under CI

Thanks for the contribution!

chuckbutler added a commit that referenced this pull request Jul 8, 2015

Merge pull request #10 from Lukasa/master
Add basic proxy relation

@chuckbutler chuckbutler merged commit 22a3aba into whitmo:master Jul 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment