skupperproject / skupper-cli

A command-line tool for setting up and managing Skupper installations
https://skupper.io/
Apache License 2.0
29 stars 7 forks source link

An expose subcommand #3

Closed ssorj closed 4 years ago

ssorj commented 5 years ago

Of the form: skupper expose (\<deployment>|\<service>) [--protocol (http|tcp)]

$ skupper expose -h Expose a resource as a Kubernetes service on all namespaces connected to a Skupper network. [...]

Some thoughts on potential behavior:

ted-ross commented 4 years ago

All this would be doing is executing a single "kubectl annotate service ..." command. Is there sufficient value being added by creating this layer of abstraction to make it worthwhile?

ssorj commented 4 years ago

Based on recent discussions, it could be equivalent to more than one annotate call (or the annotate call gets longer).

$ skupper expose service/hello-world

is a lot nicer than

$ kubectl annotate service/hello-world skupper.io/proxy=http skupper.io/service-sync=on

The latter has a bunch of careful detail you can easily typo. It also means you can't easily default some things you might want to.

I personally also think it would be nice if expose created the local service and set up creation of the remote ones, in one go:

  1. kubectl run hello-world --image hello-world --port 8080 # Creates deployment
  2. skupper expose deploy/hello-world # Creates local service and sets up remote ones
ted-ross commented 4 years ago

What does skupper/io/service-sync=off do and why would it ever be used?

ssorj commented 4 years ago

If service sync is off by default, which @grs has discussed doing (and makes some sense to me as well), you would need service-sync=on to opt in for the service in question.

ted-ross commented 4 years ago

Sorry, I should have been more clear. I meant to ask what the value of proxy=http + service-sync=off would be. I'm not sure we need both annotations.

ssorj commented 4 years ago

I think you would use service-sync=off to mean "please don't sync this one" if sync is enabled by default.

(And there are more annotations, potentially. You may want service-sync-these-places-only=a,b. It seems pretty likely that will be a request at some point.)

ssorj commented 4 years ago

I should add: I think the higher level idea is that there may be cases where you want to control the service creation yourself, so you need the ability to inhibit Skupper doing it for you.

ted-ross commented 4 years ago

But in the more complex cases (selective sync, etc.), the complexity of what needs to be supplied by the user increases equally between the "kubectl annotate..." case and the "skupper expose..." case.

I'm arguing for not adding skupper expose now.