airyhq / airy

💬 Open Source App Framework to build streaming apps with real-time data - 💎 Build real-time data pipelines and make real-time data universally accessible - 🤖 Join historical and real-time data in the stream to create smarter ML and AI applications. - ⚡ Standardize complex data ingestion and stream data to apps with pre-built connectors
https://airy.co/docs/core
Apache License 2.0
369 stars 44 forks source link

CLI doesn't remove configMaps #740

Closed ljupcovangelski closed 3 years ago

ljupcovangelski commented 3 years ago

If configuration has been added to the airy.yaml file, then the cli creates the appropriate configMaps when running airy config apply.

However, if we then delete the configuration for a particular source, then the configMap of the particular source will not be removed and the pods for the source will continue to run.

Probably the solution would involve refactoring the logic and the configuration.

ljupcovangelski commented 3 years ago

In order to delete a configMap when a configuration is removed, we must have a refference point where all the components of the Airy platform are listed, so that we can iterate over them to sync the configuratio between the config file and the kubernetes cluster.

One way would be to hardcode them in the cli, for a particular version. Another way would be to hardcode the components inside the ServiceDiscovery app, in the backend (as we have at this moment) for the airy status cli.

I think a good approach would be to have a configMap mounted on the ServiceDiscovery app, presenting a json configuraiton file, where all the components and their configurationItems are listed. The ServiceDiscovery app can read from there and the configu=

{
  "components": {
    "core": [
      {
        "name": "facebook",
        "type": "source",
        "configItems": {
          "appId": null,
          "appKey": null
        }
      },
      {
        "name": "twilio",
        "type": "source",
        "configItems": {
          "number": 0,
          "twilioKey": null
        }
      },
      {
        "name": "webhook",
        "type": "integration",
        "configItems": "siteName"
      },
      {
        "name": "media-resolver",
        "type": "storage",
        "configItems": {
          "s3Bucket": null,
          "accessKey": null
        }
      }
    ],
    "enterprise": [
      {
        "name": "search",
        "type": "ui",
        "configItems": "key"
      }
    ]
  }
}

This way, we would also be able to generate the airy.yaml file, as the cli will have from where to fetch all the components and their configuration items. Also we can introduce a wizzard, to guide users through the setup process of a component, as we know what are the expected configuration Items for every component.

If we change the configmap on the fly, we need to reload the serviceDiscovery app. Also we can have a convention for the components, for the configMaps and names of the apps - {type}-{name} (source-facebook)

This concept can work at the moment and if we switch to CustomResourcesDefinitions in the future.

lucapette commented 3 years ago

I think I like the idea the "only central" place is a config map (something called components-config?). That way, as you say, we can refactor the service discovery class in the client.config endpoint

ljupcovangelski commented 3 years ago

As we are going to use helm on the long run, I'm just thinking out loud if it makes sense to also use it for the things that the controller does? Because if we change the configuration part we do in config apply and run helm upgrade all the configuration will be updated, configMaps created or deleted and the affected pods will be restarted. Not sure what will not work. It has this out of the box and then we don't even need to do this issue for deleting the configmaps.

I guess I always had a sense that helm is temporary, that in the future we would want to have our own Airy CRD + operator and didn't consider this option at all so far. But going this direction, if we make use of all of the helm possibilities in a proper way - everything will work out of the box with very little code on our side. Perhaps best option would be to to create a kubernetes job to run helm inside the cluster and apply the configuration.

We can also use the it for the airy create command and it will be consistent:

Maybe helm upgrade is not intended in the first place to apply configuration, but at this moment it looks to me as everything will just work properly. Also the other issue that we have for restarting statefulsets will also be solved. Maybe it will be good to split the charts

helm install prerequisites
helm install core
helm upgrade core

so that we can upgrade only the components and not the prerequisites. But this is a different discussion.

This is the way we can use helm to restart the deployments/statefulsets on configMap changes, by adding an annotation to the deployment/statefulset:

spec:
  template:
    metadata:
      annotations:
        checksum/config: {{ include (print $.Template.BasePath "/configMaps.yaml") . | sha256sum }}

For whether to create or not a particular component, we can use an if block


{{- if .Values.comonent.enabled }}
... (kubernetes resources) ...
{{- end }}
lucapette commented 3 years ago

While it's appealing to "delete the controller" and just rely on some other component doing that, it feels really wrong to rely on helm upgrade. Helm is a package manager after all and we do rely on it a lot to install things right now. Soon enough, we'll have to provide upgrade guides (hopefully people will be using airy core in production and want the shiny new features a new version brings). In that context, I'm relatively sure we'll end up using helm upgrade the "right way". Once we do this, I'm not sure we'll like the idea we mixed into a package upgrade our own knowledge of how the different components can/should be configured.

tl;dr I think we should keep helm out of Airy Core configuration because 1) we'd be heavily misusing an helm feature we plan to use 2) we should keep ourselves in control (pun intended) of how airy config exactly works

ljupcovangelski commented 3 years ago

I also think that it is better that we have better control over the config process and do it ourselves. Also having our own K8s controller can help us for sure in fine-tuning our configuration process.

The main reason why I brought this up is because at this moment I am not aware of any features which our Airy controller should have in the foreseeing future (configuration-wise) and which cannot be covered with helm upgrade. Also as things are now, we cannot really use airy config unless we do helm install first, so that is why helm upgrade did not sound that wrong to me.

But I also have a hunch that relying on our own controller will be better on the long run and I vote for sticking with the Airy k8s controller for configuration purposes :+1:

ljupcovangelski commented 3 years ago

We will create one configMap with all the mandatory and optional components with helm, as part of the airy create process.

ljupcovangelski commented 3 years ago

After reviewing the PR https://github.com/airyhq/airy/pull/1510 and discussing the concept, me, @lucapette, @paulodiniz and @chrismatix agreed on the following:

ljupcovangelski commented 3 years ago

For the jwtsecret, token, allowedOrigins and similar configuration values, we agree do put them under a top-level secrtion security, because they are relevant for more components.