Skip to content
This repository has been archived by the owner on Mar 13, 2021. It is now read-only.

Change stream.provider to stream.provisionerHost #86

Closed
ericbottard opened this issue Sep 17, 2019 · 6 comments · Fixed by projectriff/cli#180
Closed

Change stream.provider to stream.provisionerHost #86

ericbottard opened this issue Sep 17, 2019 · 6 comments · Fixed by projectriff/cli#180
Assignees
Milestone

Comments

@ericbottard
Copy link
Contributor

This better reflects what that field really is while preserving loose coupling

@markfisher
Copy link
Contributor

markfisher commented Nov 7, 2019

I think we should consider inverting this thinking... rather than renaming the field to match the lower level implementation detail, we should consider "provider" to be a higher level concept and all that a user should be thinking about when creating a stream. How that maps to "provisioning" (which might vary widely when we add other types of streams, and which might move into the scope of the "gateway" itself). Along these same lines, we should consider whether "gateway" is a more appropriate name than "provider", e.g. change the KafkaProvider CRD to KafkaGateway.

@ericbottard
Copy link
Contributor Author

I totally agree with that, but the problem now that we have loose providers, is that there is no (easy) way to identify such a provider.

IMO, doing the following would be an acceptable compromise:
Change the CLI to

riff streaming stream foo --content-type text/plain --provider <kind>/<name>
   or
riff streaming stream foo --content-type text/plain --providerType <kind> --providerName <name>

and require provider to have a .Status.provisionerServiceName that points to something that adheres to the (TBW) provisioner contract.

Feel free to mentally s/provider/gateway/ in my comment, that's not the main point, but rather how to get from a provisioner "name" to the thing that eventually knows how to provision (and treating that as a black box to the best extent)

PS: created bsideup/liiklus#220 to track provisioning as a liiklus responsibility

@markfisher
Copy link
Contributor

markfisher commented Nov 7, 2019

all good points @ericbottard - I guess rather than renaming the provider CRDs we could add a StreamGateway CRD that is itself namespaced and handles that kind + name uniqueness. That said, I don't want to address every problem by adding yet another CRD... should only do so if there are other common concerns/configurations (such as configuration for liiklus itself, especially as it evolves) that we could include at that level instead of duplicating across multiple "provider" CRDs.

@ericbottard
Copy link
Contributor Author

What should the next steps for this be?

  • There's the "simple" proposal in my comment above
  • There's also something more complex introduced by @markfisher , which would require a more detailed proposal I think.
  • Lastly, I think we agree the current situation, even with a rename to provisionerHost is not ideal

@scothis
Copy link
Member

scothis commented Nov 18, 2019

I don't see --provider <kind>/<name> as "simple":

  1. the user has to know about the provider's kind (not terrible, but not top of mind for CLI user)
  2. the controller will need to get the provider. So kind is woefully insufficient. A group, version and resource (resource != kind, but is similar in most cases) will be needed to get the resource with a rest client
  3. the controller will need permission to get the provider. So we'l need to setup an aggregate cluster role and new providers will need to expose a cluster role that is matched by the aggregator
  4. the controller won't know anything about the structure of the provider resource. So we'll need to define a duck type to resolve all information about the provisioner.

@scothis
Copy link
Member

scothis commented Nov 18, 2019

At first glance StreamGateway is similar to the previous Provider CRD. The advantage is that it defines a flat naming scope for providers to register under and it a bit nicer for discovery than a label selector query for a Service. Going from many-to-one logical naming scope will always have this tension.

My main question is who create the StreamGateway?

If users are responsible for creating the StreamGateway resource, it could in turn create a Service with a matching selector and the provider is then responsible for defining the deployment that matches the selector.

If the provider is responsible for creating the StreamGateway, it's logically not much different than the current situation with the provisioner Service.

In both cases, what should happen if a StreamGateway is removed and replaced with a new resource of the same name? Should each stream detect this change and re-provision? This is not possible with the current model.

@scothis scothis self-assigned this Jan 17, 2020
scothis added a commit that referenced this issue Jan 17, 2020
Adding new Gateway and *Gateway resources along side the existing *Providers. Updated the Stream resource to be able to specify and track the Gateway in addition to the provisioner. Deprecated all providers.

Refs #86 and RFC 0011

Co-Authored-By: Eric Bottard <[email protected]>
scothis added a commit to scothis/cli that referenced this issue Jan 17, 2020
When creating a stream, the --provisioner flag is now --gateway, and
its value is the same name the stream gateway was created with.

Refs rfc 0011
Fixes projectriff/system#86
scothis added a commit to projectriff/cli that referenced this issue Jan 17, 2020
When creating a stream, the --provisioner flag is now --gateway, and
its value is the same name the stream gateway was created with.

Refs rfc 0011
Fixes projectriff/system#86
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants