Expose open-port and close-port hook tools

[stub42]

The open-port and close-port Juju hook environment tools are not exposed by the operator framework.

It would be good if the framework handled closing the old port when a new one is opened. eg.

self.open_port('grpc', 1234)  # Remembers 'grpc' port
self.open_port('grpc', 4242)  # Automatically close-port 1234 before open-port 4242

[jameinel]

That assumes that any given charm never wants to open more than one port. Definitely we want to support ranges, not just singular ports (I believe 'open-port' already handles this. Also, there has been a design item on the Juju side for a while to link what binding (endpoint?) the port is for. So that someone can expose an attribute of an application without exposing everything it does. (There is further work to allow expose to a list of CIDR, but that doesn't impact how the charm needs to interact with Juju.) There is a desire as well to have charms declare 'internal' ports. Currently Juju models end up with a rule that machines in the same model can talk to each other on any port, which many find distasteful.

It feels like it would then end up on the Binding object. So something like:

binding = self.model.get_binding('db')
binding.ports = [PortRange(123)]

I'm not 100% sure how to express multiple ranges cleanly. Having it as an attribute makes it clear that you are replacing the existing ports, and can allow stuff like

binding.ports.append(PortRange(123, 456))

We could certainly allow for the trivial integer case binding.ports = [80]

On Wed, Mar 11, 2020 at 12:15 PM Stub [email protected] wrote:

The open-port and close-port Juju hook environment tools are not exposed by the operator framework.

It would be good if the framework handled closing the old port when a new one is opened. eg.

self.open_port('grpc', 1234) # Remembers 'grpc' port self.open_port('grpc', 4242) # Automatically close-port 1234 before open-port 4242

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/179?email_source=notifications&email_token=AABRQ7LNMJIMEFBMGZ2B3L3RG5CAZA5CNFSM4LFQCOH2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IUEDBVQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7MX7S4GMHYVZJ6AVHTRG5CAZANCNFSM4LFQCOHQ .

[facundobatista]

@stub42 the suggestion of including a "name" in the call, is to just assign it to the open port, so it can later be closed automatically? .close_port could use a name or the port number, too.

But is that name for any other use?

[stub42]

On Wed, 18 Mar 2020 at 07:00, Facundo Batista [email protected] wrote:

@stub42 https://github.com/stub42 the suggestion of including a "name" in the call, is to just assign it to the open port, so it can later be closed automatically? .close_port could use a name or the port number, too.

But is that name for any other use?

I only included the key to allow the port to be automatically closed. I don't see any other use.

[facundobatista]

Thanks @stub42. From your POV, changing ports for one service/purpose is so common that is a good thing for the framework to track them? I'm trying to understand if adding the complexity of a "name" to the method's signature is worthwhile (we need to explain why a name there, how to choose it, etc.).

@jameinel I don't understand why you say from @stub42 proposal that charms don't want to open new ports, a charm could totally do...

self.open_port('foo', 1111)  # open port 1111 for service/purpose 'foo'
self.open_port('bar', 2222)  # open port 2222 for service/purpose 'bar'
self.open_port('foo', 2222)  # it should raise an error, port 2222 is already used!
self.open_port('foo', 3333)  # open port 3333 for 'foo',  automatically closing 1111 (before or after?) 
self.close_port('foo')  # closes port 3333, removing foo/3333 from internal records
self.close_port(2222)  # closes port 2222, removing bar/2222 from internal records

@stub42 what do you think of this just described behaviour?

The "handling a range" conversation is slightly orthogonal to that one (not completely! "naming ranges" is what mix both talks). We could totally do self.open_port('foo', 6700, 6720) (as a third parameter is present, this is a range!), but my real concerns here is the defined behaviour for the cases of when ranges overlap.

What would happen in this case, for example?

self.open_port('foo', 6700, 6720)
self.open_port('bar', 6710, 6730)

Or in this case:

self.open_port('foo', 6700, 6720)
self.close_port(6715)

So, @stub42 @jameinel a question: is "opening ranges of ports" is a common situation? Are these ranges big or this is something the developer could do with a for?

Thanks!!!

[stub42]

On Thu, 19 Mar 2020 at 23:02, Facundo Batista [email protected] wrote:

Thanks @stub42 https://github.com/stub42. From your POV, changing ports for one service/purpose is so common that is a good thing for the framework to track them? I'm trying to understand if adding the complexity of a "name" to the method's signature is worthwhile (we need to explain why a name there, how to choose it, etc.).

With existing charms, it is extremely common to have ports specified as a charm configuration variables. Personally I consider this overused, with the usual rationale being 'what if someone wants it on a different port' and my answer 'why would someone want to avoid best practice and have a surprising deployment?'. But there are many cases where it is necessary, such as subordinates where common ports such as HTTP or monitoring may already be in use by the primary charm, or where a charm needs to expose many endpoints on different ports based on configuration, such as load balancers and caches placed in front of multiple backends.

@jameinel https://github.com/jameinel I don't understand why you say from @stub42 https://github.com/stub42 proposal that charms don't want to open new ports, a charm could totally do...

self.open_port('foo', 1111) # open port 1111 for service/purpose 'foo' self.open_port('bar', 2222) # open port 2222 for service/purpose 'bar' self.open_port('foo', 2222) # it should raise an error, port 2222 is already used! self.open_port('foo', 3333) # open port 3333 for 'foo', automatically closing 1111 (before or after?) self.close_port('foo') # closes port 3333, removing foo/3333 from internal records self.close_port(2222) # closes port 2222, removing bar/2222 from internal records

@stub42 https://github.com/stub42 what do you think of this just described behaviour?

Yes, I think this is the behavior charmers will find most helpful. I'm not concerned about how it is spelled. """ self.unit.ports['foo'] = 1111 """ may be more consistent with other operator framework features.

So, @stub42 https://github.com/stub42 @jameinel https://github.com/jameinel a question: is "opening ranges of ports" is a common situation? Are these ranges big or this is something the developer could do with a for?

I am not familiar with any charms opening ranges of ports.

[jameinel]

Juju 2.9 is planning on adding '--endpoints' as an argument to open-port to allow tying the port to a specific endpoint (so that you can 'juju expose mysql:db' and not expose all endpoints). They are also looking at a model config to have a strict firewall mode, so within a model, you wouldn't be able to talk between applications if the associated charm didn't define the ports that it was using internally. This will become more important once that goes through, as all charms that have any conversations need to open ports even if they won't be exposed.

[achilleasa]

Regarding the above questions, you can find the spec for the upcoming 2.9 juju changes wrt. managing open ports across application endpoints here.

TLDR: we are adding support for --endpoints to open-ports, close-ports and opened-ports hook tools. By default, these tools assume that operations apply to all endpoints (this is how things worked pre 2.9) so that we don't accidentally break any existing charms/frameworks that may attempt to parse the output of these commands.

However, interested charms can examine JUJU_VERSION and opt in to the new feature by providing --endpoints where needed. We actually landed the hook tool changes earlier today so you can actually try the new feature out. As the docs for this feature are not yet complete, you can, for the time being, take a look at the QA steps here for what kind of output and interaction with these tools that you will be able to use.

[jameinel]

There is a discourse post describing the functionality being added in 2.9: https://discourse.juju.is/t/granular-control-of-application-expose-parameters-in-the-upcoming-2-9-juju-release/3597

[johnsca]

In addition to the discussion about the new endpoint association feature for ports, it hasn't been mentioned that open-port also accepts a protocol for the port(s) in question, so that will need to be modeled as well (though I haven't really seen it used much that I can recall). Maybe something like public_binding.ports.tcp = [443, range(30000, 32767)]?

binding = self.model.get_binding('db')
binding.ports = [PortRange(123)]

I'd suggest that we use the built-in range() type rather than reinventing one, as it already does everything we need, including exposing the start and stop attributes.

They are also looking at a model config to have a strict firewall mode, so within a model, you wouldn't be able to talk between applications if the associated charm didn't define the ports that it was using internally.

Hrm, having it as a model config seems like it would make the transition more difficult. I haven't read the discussion yet, but I would think that a charm metadata field opting it in to the new behavior would allow for incremental adoption without breaking an entire model. But I guess for compliance purposes, an all-or-nothing setting makes sense.

[jameinel]

A model config would allow you to turn it on, and test what charms actually work in that situation. It needs to be done relatively high up, because it is a model level security group that actually lets each instance talk to each other instance, so you have to know what you are going to deploy before you start the instance. And then things like hulk-smash, colocating, late updating, etc. Or the primary machine supports it, but you deploy something into a container that doesn't, etc. It would be possible to do as charm metadata, but it would be quite a bit more complicated. This is something that is relatively easy to implement, and can become part of CI tests, etc if/when we actually care to have charms supporting it. It also works better from a compliance standpoint, because it is mandated from the top, rather than opt-in-if-convenient.

On Thu, Sep 24, 2020 at 9:33 AM Cory Johns [email protected] wrote:

In addition to the discussion about the new endpoint association feature for ports, it hasn't been mentioned that open-port also accepts a protocol for the port(s) in question, so that will need to be modeled as well (though I haven't really seen it used much that I can recall). Maybe something like public_binding.ports.tcp = [443, range(30000, 32767)]?

binding = self.model.get_binding('db')binding.ports = [PortRange(123)]

I'd suggest that we use the built-in range() https://docs.python.org/3/library/stdtypes.html#ranges type rather than reinventing one, as it already does everything we need, including exposing the start and stop attributes.

They are also looking at a model config to have a strict firewall mode, so within a model, you wouldn't be able to talk between applications if the associated charm didn't define the ports that it was using internally.

Hrm, having it as a model config seems like it would make the transition more difficult. I haven't read the discussion yet, but I would think that a charm metadata field opting it in to the new behavior would allow for incremental adoption without breaking an entire model. But I guess for compliance purposes, an all-or-nothing setting makes sense.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/179#issuecomment-698384011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7JIX3HDMYR7D7OAI73SHNKD3ANCNFSM4LFQCOHQ .

[benhoyt]

Oops, I accidentally opened a dupe of this at https://github.com/canonical/operator/issues/883 (closing that one now).

However, as noted there: especially now that open-port and so on are now supported in K8s (sidecar) charms (see https://github.com/juju/juju/pull/14975), we should probably bump up the priority of this.

From Kelvin (who implemented the K8s sidecar version of this in Juju):

for k8s sidecar, only leader unit can call open/close-port hook command, but IAAS, we don't have this restriction.

ops does not have to know this knowledge, it's the charmer's responsibility to do leadership check if they are calling the command in a k8s sidecar charm. So if we want to implement something in ops, just a generic command exec helper function is sufficient.

[benhoyt]

Just adding these notes from internal chat here:

Guillaume Belanger:

FYI open-port with subprocess fails on juju 3.0 with the following message: unit-test-juju30-0: 16:40:14 ERROR juju.worker.uniter.operation hook "install" (via hook dispatching script: dispatch) failed: cannot retrieve ports for unit "test-juju30/0": unit "test-juju30/0" is not assigned to a machine

The error message is somewhat confusing but at least it throws an error message.

Jon Seager:

Thanks @Guillaume Belanger.

@Ben Hoyt - we were discussing making sure we raise a decent error if someone tries to use open-port on a sidecar charm on juju < 3.1, so that we could have a flow like:

try:
    subprocess.check_call(["open-port", "3000/tcp"])
    # or the new ops equivalent...
except:
    KubernetesServicePatch(...)

Sadly it seems like open-port raises a pretty weird message on earlier versions, but worthy of consideration either way :slightly_smiling_face:

[rbarry82]

I would love to have an event for this. Maybe a WorkloadEvent (it's only Pebble so far, but this seems appropriate even if it's a hook), or a way to introspect 'what ports have been opened'?

This may be pre-emptive or unnecessary, but in order for Traefik to actually route traffic to the appropriate backend service, it needs to know which port it's running on (and inversely, if it's a non-HTTP(s) port for a raw TCP route Traefik is proxying, to add that port to its own definition so MetalLB will send traffic to Traefik). An event to watch or property which could be monitored would ease potential maintenance burden for charmers and make it less likely for changes in status to get missed.

[benhoyt]

@rbarry82 Can you clarify what you mean by an event for this? I definitely intend to include a way to list the opened ports (the opened-ports hook tool), but not sure how an event fits in.

[rbarry82]

Basically, that there is a need for an asynchronous way to wake a charm up and propagate the information up the stack. The venerable KubernetesServicePatch has all of the information needed as part of the constructor for the charm, but in order for traffic to actually make it to a charm through any given ingress:

• The port must be properly set (hopefully in the k8s object for pure k8s, but we're sending it across in Traefik regardless, so either mechanism works), so it can be forwarded along an ingress[*] relation in order for the appropriate service (or IP/FQDN, depending on the ingress) to "know" where to go.
  • In the case of Traefik, for example, there is a loadBalancers element in the config, which points back to {ip|fqdn}:{port} when a request matches that route. If Traefik doesn't know the port from the charm/workload, it can't route the request.
• For non-HTTP(s) cases, the ingress must also bind to a new port (not strictly necessary in the case of TLS connections where the backend can be looked up via SNI from the cert), and add it to its own service definition also.
  • If the ingress does not also have the port in its service definition, incoming traffic from a k8s load balancer to some port will not be forwarded.
  • Alternatively, if we ever get to a point of directly charming loadbalancers and exposing individual non-HTTP(S) on their own IPs (LBs do not do SNI lookup, so no possible bypass there), the LB must also know which port to map.

In an example case of a MySQL/MariaDB charm where open-port has been invoked (by a Juju admin or the charm), and the admin wants access to it externally on port 3306, the fact that port 3306 has been opened must be propagated back up to Traefik, which must add a new port to bind in the workload, and the new port to its service definition, or users will not be able to reach it.

Hence the request for an event. As for what kind of event and the lifecycle around it, the world is your oyster. WorkloadEvent seemed to be a semi-logical fit (even if it's not from the workload, it seems at least most conceptually related to workload operations than being a HookEvent, despite the fact that it's strictly a hook), but any kind fits.

Waiting for pinger|update-status is not reliable depending on the model configuration, and almost certainly not as timely as admins would expect "open this port and relate it to ingress, now I can reach my application" to be.

It's an unfortunate artifact of the way traffic from outside a k8s cluster flows, more or less.

[benhoyt]

For reference, here is my proposal for the API to add to ops (this iteration doesn't address @rbarry82's idea for a "port opened" event). Feel free to review and comment there.

[benhoyt]

FYI, I'm worked on (and have mostly completed) this in https://github.com/canonical/operator/pull/905