Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for instantiating components multiple times #221

Closed
simu opened this issue Oct 19, 2020 · 9 comments · Fixed by #234
Closed

Support for instantiating components multiple times #221

simu opened this issue Oct 19, 2020 · 9 comments · Fixed by #234
Assignees
Labels
enhancement New feature or request RFC Request for Comments

Comments

@simu
Copy link
Member

simu commented Oct 19, 2020

Context

Currently Commodore components don't support installing multiple instances of the software they manage. There are already a some components for which installing multiple instances is a common use case, such as nfs-client-provisioner.

Potential solutions

Multi-instance aware components

The "simple" solution from the perspective of Commodore/Kapitan/Reclass is to simply leave multi-instance support to the component authors. For this solution, component authors would explicitly need to implement multi-instance support. This approach would most likely involve a parameter structure which exposes the multi-instance nature of the component in parameters.<component-name>.

Taking nfs-client-provisioner as an example, the parameters structure could be something like:

parameters:
  nfs_client_provisioner:
    namespace: ...
    common:
      host: ...
    instances:
      instanceA:
        path: ...
      instanceB:
        path: ...
      instanceC:
        host: ...
        path: ...

In this example structure, we allow users to define configurations which are shared between multiple (but maybe not all) instances in a key common. Instances are configured in key instances, and keys in instances are used as instance identifiers. Instance configuration overrides configurations in common.

The main downside of this approach is that it may not be feasible to implement for components which use Helm charts as their base, unless the Helm chart exposes a similar structure. The reason for this is that Kapitan's Helm templating is expressed in the reclass inventory, and reclass is not flexible enough to instantiate multiple copies of a part of the hierarchy based on another key in the inventory.

Commodore support for component instantiation

Another approach is to not make components multi-instance aware, but instead implement support for instantiating a component multiple times in Commodore. This approach would potentially scale better, as component authors won't have to reinvent the wheel for every component which they want to support multiple instances. Additionally, Commodore can probably juggle things so that Kapitan's helm templating can be used to instantiate a Helm-chart based component multiple times.

This approach would need logic in Commodore which identifies the component(s) to instantiate multiple times based on the included component classes according to some "instance naming scheme".

Commodore would then have to duplicate the relevant files (component class and defaults mainly), rewriting references to the component name in those files (input/output paths, parameters key, ...) to match the "instance name" which would be derived from the class "instance naming scheme".

Users wishing to instantiate a component multiple times would need to include the component using the "instance naming scheme", and provide parameters for each instance separately. Reclass references can potentially be used to share parameter values between instances.

Alternative approaches

Another approach which is not outlined in detail above is that when multi-instance support is required, the Commodore component must install an operator which supports instantiating the required software multiple times. This is naturally dependent on an operator existing for the software that needs to be instantiated multiple times, and is similar to the first proposed solution from the perspective of how multiple instances could then be configured via the hierarchy.

Out of scope

How to manage services for the customer on a Syn-enabled cluster is out of scope for this issue.

@simu simu added enhancement New feature or request RFC Request for Comments labels Oct 19, 2020
@simu simu added this to To do in Project Syn Focus Weeks Oct 2020 via automation Oct 19, 2020
@corvus-ch
Copy link
Contributor

All of the above approaches do have their raison d'être. And two out of three do not require any work on our side (Multi-instance aware components and operator based approach). They also allow us to achieve most if not all use cases.

As of Commodore support for component instantiation. I find this one quite tricky. I am not concerned about whether it can be implemented or not. I am more concerned about potential side effects of that approach.

A naive implementation would just create two instances of a component. Now two instances will create the same resources. The most simple way to deal with this, is to have their resources placed into different namespaces. But then there are resources not bound to a namespace. We would need to prefix them too but then their identity changes. This would then make like RBAC use less.

This all can be solved. But without special consideration from component authors, I can not see how this could work without any side effects and surprises.

If the only goal is to not have component maintainers think about multi instancing, then it is not worth to invest the time.

@srueg
Copy link
Contributor

srueg commented Oct 21, 2020

Implementing #223 might open up other possibilities or at least simplify the "Commodore Support" approach.

@simu
Copy link
Member Author

simu commented Oct 21, 2020

I disagree that the multi-instance aware component approaches do not require any work on our side.

The absolute minimum amount of work for those approaches is prominently documenting our stance on how to do multi-instance aware components. In reality, we'll additionally have to ensure that the components provided by Project Syn which could feasibly require multi-instance awareness actually are implemented in such a way, either proactively, or based on customer requests.

On the flip side, I'm aware that Commodore support for component instantiation is not trivial. If it were, it's likely it would have existed from the start.

In all likelihood, components will still need to have a minimal amount of logic to cleanly support Commodore component instantiation. However, I believe that with small adjustments in the component template and carefully written post-processing filters which can be applied to component instances, we can remove most of the burden from the component author.

Implementing #223 might open up other possibilities or at least simplify the "Commodore Support" approach.

I definitely think that we'll want to tackle this issue after #223, as having a target per component will make the "Commodore Support" approach significantly simpler to implement.

@corvus-ch
Copy link
Contributor

However, I believe that with small adjustments in the component template and carefully written post-processing filters which can be applied to component instances, we can remove most of the burden from the component author.

Yes, most but not all. And I am not convinced, that we are able to cover all edge cases. I am afraid of those edge cases to fail spectacularly.

@simu
Copy link
Member Author

simu commented Oct 21, 2020

Yes, most but not all. And I am not convinced, that we are able to cover all edge cases. I am afraid of those edge cases to fail spectacularly.

I'm not sure what the remaining edge/corner cases would be if we manage to come up with a robust solution for making non-namespaced resources unique and deduplicating CRDs. If you have any particular cases in mind, can you document them here?


As a follow-up regarding the approaches which delegate multi-instance support to the components (note that those two approaches are not mutually exclusive):

  • For Helm-chart based components, the big issue will be that Kapitan's Helm chart templating won't be usable for templating a chart multiple times for each instance defined in parameters.<component-name>.instances, as the reclass references are not an actual templating language where we can spit out an array entry for parameters.kapitan.compile per instance.

    To make this approach work, we'll need to implement some support for it in Commodore, which transforms the component's parameters.kapitan.compile according to the defined instances in parameters.<component-name>.instances. This most likely will need some marker (an annotation, or special key) for the component's parameters.kapitan.compile entries which need to be duplicated per instance. This is not a problem per se, but will impose restrictions on how a multi-instance aware component can be structured, as Commodore will require instances to be defined in a well-known key in the component parameters, or another meta-parameter will be required.

  • The operator approach sounds like a nice alternative, iff an operator exists for the infrastructure tool packaged by the component. However, operators don't exist (yet?) for all infrastructure tooling, not even for the tools we're using. For example, there doesn't seem to be an operator to manage nfs-client-provisioner instances (my impression after googling for 5 minutes). Therefore we'd need to first develop an nfs-client-provisioner-operator before we can make the component multi-instance aware.

  • Both approaches will be prone to "reinventing the wheel", unless we provide a robust set of library functions for the boilerplate of creating instances (be that CRs for the operator-based approach, or other manifests for Jsonnet- or Helm-based components).

@corvus-ch
Copy link
Contributor

Consider the following cases:

  • A component manages a ClusterRole and a ClusterRoleBinding. When instantiating this component multiple times we have a name collision. This we can fix with a filter by altering their name (for example prefix or postfix). By doing so, we alter their identity. Since the ClusterRoleBinding contains a reference to the ClusterRole. So we would also need to alter the reference within the role binding.

    How can we ensure consistency for such scenarios? Do we now of and can handle all cases like this?

    Future release of Kubernetes and or operators can change this and we miss something. I dislike the idea, that we would have to handle all those special cases.

  • appuio/component-openshift4-version manages the custom resource ClusterVersion on OpenShift 4 clusters. There is and should only be one instance of that custom resource (named version). If someone — for what ever reason — instantiate that component multiple times, things will not work as expected.

I do get the impression that multi instantiation of components is supposed to magically fix all the issues and component authors do not have to think about it. For me this certainly is not the case. It adds complexity and I question the value of investing time into that feature.

However, if we do implement that feature, I suggest to follow the following rules:

  • A component must explicitly state if it is ready to be instantiated multiple times
  • Post processing to resolve resource conflicts (naming of resources that are not namespaces) must be within the responsibility of a component.
  • Syn can provide libraries to help with the later

@srueg
Copy link
Contributor

srueg commented Oct 22, 2020

As discussed:
I also prefer the "Commodore support" solution with the added feature that components need to opt-in to be instantiated multiple times.
Similar to Helm charts, the components themselves must make sure to not cause any naming collisions of objects (also in the same namespace). Since some way of aliasing (i.e. release name in Helm lingo) is required anyway to instantiate a component multiple times, this could be used for having unique names per instance of a component.

@simu
Copy link
Member Author

simu commented Oct 22, 2020

As discussed in the call: The "Commodore Support" approach needs the following restrictions:

  • Components must explicitly support instantiation
  • Components must make sure that there's no name conflicts of resources of different instances.
  • Commodore provides a way to provide component parameters per instance, shared defaults can be injected via reclass references.
    • Potentially we can build more full-fledged Commodore support for defining defaults which can be injected into the component instance's parameters when the component target is generated (depends on Define a Kapitan target per component #223).

@tobru
Copy link
Contributor

tobru commented Oct 23, 2020

This discussion here should probably end up in an SDD, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request RFC Request for Comments
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants