search

juju-solutions / interface-http

Interface layer for the basic http interface protocol
4 stars 7 forks source link

Adding network space support for http interface. #12

Closed hyperbolic2346 closed 6 years ago

hyperbolic2346 commented 6 years ago

This consists of using network_get instead of hookenv.unit_get('private-address'). We fall back to private-address if network_get doesn't include ingress information.

Cynerva commented 6 years ago

Looks good to me.

@kwmonroe @johnsca could y'all review this too? interface-http is widely used so it'd definitely be good to have extra eyes on this in case I'm missing something. Thanks.

johnsca commented 6 years ago

Is ingress-address what we want here? I would assume that we use the private address by default to keep all traffic for the relation cloud-internal. Is the ingress-address the public-facing address? (Note: I'm not terribly familiar with the new network spaces data.)

If ingress-address is the correct thing, then I'm +1 on this PR.

Cynerva commented 6 years ago

Thanks @johnsca. In my testing, ingress-address has always been the same as private-address. My understanding from the Charm Network Primitives doc is that it only becomes the public-address if you pass a specific relation ID to network-get, and that relation is cross-model.

We're not trying to add cross-model relation support in this PR, we just want interface-http to respect network space bindings. For that, using either bind-address or ingress-address should work, but the intention seems to be for charm developers to use ingress-address when advertising an address to other units.

hyperbolic2346 commented 6 years ago

How does that look, @johnsca ?