FraunhoferIOSB / FROST-Server

A Complete Server implementation of the OGC SensorThings API
https://fraunhoferiosb.github.io/FROST-Server/
GNU Lesser General Public License v3.0
199 stars 74 forks source link

Kubernetes deployment (&environment variables) #61

Closed abourdon closed 6 years ago

abourdon commented 6 years ago

Hi,

I'm working on a Kubernetes deployment of the full FROST-Server stack (server, db & mqtt). To be as much reusable as possible, I use the official FROST-Server Docker images (frost-server-http & frost-server-mqtt).

I need to override some configuration within containers (e.g., frost-server-http's bus.mqttBroker). The simple (and desired) way to do this would be to use environment variables, but I can see some of them cannot be overridden by default (especially mqtt and persistence related configuration):

https://github.com/FraunhoferIOSB/FROST-Server/blob/master/FROST-Server.Core/src/main/java/de/fraunhofer/iosb/ilt/sta/settings/CoreSettings.java#L158

Is there any reason, by default, to prevent configuration overriding from environment variables?

PS 1: I don't want to override the full context.xml file via volume mounting. PS 2: I want to use the official FROST-Server's images. I don't want to create my own Docker image based on them to apply additional configuration.

mjacoby commented 6 years ago

The docker images already support setting configuration properties via environment variables. You can find the documentation here.

abourdon commented 6 years ago

The docker images already support setting configuration properties via environment variables.

Yes I know :-) But some of them seem to be not overridable.

Actually, it was a bug from my side because I used dots in property names instead of underscores, as explained in the documentation:

Environment variables are not allowed to have the dot (.) character in the name. You can replace all of the dots in the names with an underscore (_).

It should be written You must instead of You can ;-)

phertweck commented 6 years ago

@abourdon: I am very pleased to hear that you're working on a kubernetes deployment of the FROST-Server. This is something we're also planning to do in the future.

Do you use static deployment files or are you creating a Helm-Chart to have a configurable/reusable solution?

abourdon commented 6 years ago

Hi @phertweck,

Glad to hear that this is something that could be useful for the project!

Actually, the idea is to provide a fully configurable and reusable SensorThings API stack that could be deployed in "one click". And of course, we are using the FROST-Server implementation to do that :-)

We are still working on the definition of the best architecture that could provide the following features:

Hereafter the link to this project: https://github.com/StoreConnect/sensorthings-api-deployment

Concerning Kubernetes, the first implementation is using static deployment files. More precisely, template deployment files which are rendered before sending them to kubectl. This way, we can create a configurable and reusable Kubernetes stack without being dependent to any additional complex layer as Helm. But this is the first implementation. Maybe it could be great in the future to have a Helm support. You can have a look at this first implementation here.

If you are interested, feel free to contribute :-) Instructions about contribution can be found here.

hylkevds commented 6 years ago

Since this is an interesting topic, I've renamed the issue, labelled it "documentation" and re-opened it, to make it more visible.

hylkevds commented 6 years ago

Looking at your Kubernetes files, it looks like you use the FROST-Mqtt component itself as the message bus that FROST components use internally. That works fine as long as you have only 1 instance of the FROST-Mqtt component, but doesn't work any more when you start to horizontally scale FROST, by using multiple MQTT instances.

Do you foresee the need for horizontal scaling in your project?

abourdon commented 6 years ago

Hi @hylkevds, actually I used configuration from your docker-compose-separated. If there is another way to do it more better, please let me know

hylkevds commented 6 years ago

An image of the architecture is here

In the docker-compose-separated there is a separate Mosquitto MQTT instance that is used as the internal message bus. I can't find that in your set of Kubernetes files. In frost-http-deployment.template.yml you set the bus_mqttBroker to the name used for the FROST-Mqtt container. So the http instance will use the FROST-Mqtt instance as a message bus. In frost-mqtt-deployment.template.yml you don't have the variable bus_mqttBroker yet, so it might be that MQTT doesn't work when entities are posted to the HTTP instance.

So it seems to be a bit different from the docker-compose-separated example.

abourdon commented 6 years ago

In the docker-compose-separated there is a separate Mosquitto MQTT instance that is used as the internal message bus. I can't find that in your set of Kubernetes files.

You right, I missed this point.

In frost-http-deployment.template.yml you set the bus_mqttBroker to the name used for the FROST-Mqtt container. So the http instance will use the FROST-Mqtt instance as a message bus. In frost-mqtt-deployment.template.yml you don't have the variable bus_mqttBroker yet, so it might be that MQTT doesn't work when entities are posted to the HTTP instance.

So it seems to be a bit different from the docker-compose-separated example.

Ok I will rework this point. Actually, I'm not very familiar with MQTT and I don't use this feature in my SensorThings use cases. I will read your architecture documentation carefully and go back with a better implementation. Thanks!

hylkevds commented 6 years ago

If you don't use MQTT, you can also leave the MQTT parts out completely. Since the HTTP parts doesn't allow for subscriptions, you don't even need the message bus in that case. Only the database and HTTP part is needed, and then the HTTP part itself can scale horizontally with no problems.

abourdon commented 6 years ago

Yes of course. But the aim of sensorthings-api-deployment is to provide a deployment of the full SensorThings API stack to fit with any kind of use cases.

phertweck commented 6 years ago

Thank you for the link to your project.

The template syntax looks very similar to Helm (I was not able to figure out if it's the same). I will try to create a Helm chart based on your .yml files.

We're also setting up a Rancher based Kubernetes cluster with the aim to start FROST-Servers through the build in catalog mechanism. As far as I've understood this is based on Helm charts, so you might have done already a big step into this direction.

abourdon commented 6 years ago

The template syntax looks very similar to Helm (I was not able to figure out if it's the same). I will try to create a Helm chart based on your .yml files.

Great! Feel free to contribute!

We're also setting up a Rancher based Kubernetes cluster with the aim to start FROST-Servers through the build in catalog mechanism. As far as I've understood this is based on Helm charts, so you might have done already a big step into this direction.

Again, feel free to contribute if you want to share your work on the Rancher configuration.

abourdon commented 6 years ago

@hylkevds, now the sensorthings-api-deployment's Kubernetes part defines a standalone MQTT broker with which the FROST-Server's http and mqtt resources communicate.

@mjacoby I also add a way to fully configure FROST-Server's resources, by adding all settings variables from here. I tried to harmonize default values found in properties files or source code to define a default configuration. Let me know if it is coherent for a common use.

abourdon commented 6 years ago

Re,

I just created a Helm chart of the FROST-Server stack (you were right @phertweck, it was practically a Helm chart definition).

At the moment, this chart is pushed into a personal Helm repository. To give a try you can execute:

$ helm repo add storeconnect https://storeconnect.github.io/helm-charts
$ helm install --set externalIp=<Kubernetes cluster ip> storeconnect/frost-server

Documentation about how to configure this chart can be found here.

If you are interested, I could merge this work into your FROST-Server code base. Let me kow.

phertweck commented 6 years ago

@abourdon: Thank you for the great Helm chart! One point is not clear to me: why do you need to set the external ip?

You're using them in your db, http and mqtt service. I think none if this services needs an external IP. The db should be accessible only inside the cluster, for security reasons. The HTTP service should be exposed by the K8 built in Ingress-Component and mqtt should be exposed as Nodeport service (if needed).

abourdon commented 6 years ago

Thank you @phertweck.

why do you need to set the external ip?

I had the idea to expose services outside of the cluster by using common port values (80 for HTTP, 1883 for MQTT...) instead of using a range of valid ports used by a NodePort service. But when I write that, I realize that this configuration is not portable if you want to deploy this chart into an existing cluster that already exposes these ports. I will fix it by using NodePorts.

The db should be accessible only inside the cluster, for security reasons

Yes you right, the database is an internal FROST component and FROST data must be fetched through the HTTP and MQTT API only. I will fix it.

The HTTP service should be exposed by the K8 built in Ingress-Component

I hesitated to define an Ingress service because this feature is in beta and not available in any Kubernetes release prior to 1.1. WDYT?

phertweck commented 6 years ago

@abourdon Great! I had a look at the official Helm charts (e.g. Owncloud, Artifactory, ...). All of them are using Ingress for their HTTP services. Therfore I think you can assume that Ingress service is available on most clusters. If not, the Ingress service could be disabled like you've done it in the mqtt service.

Binding one application of the cluster to port 80 of the public IP adress can also be very critical, since no other application (even the K8 ingress) is reachable in the cluster through HTTP.

abourdon commented 6 years ago

Another argument to why I used an ExternalIP service: to set the serviceRootUrl configuration property. We need to set it with an ip (or a host) that is accessible from the outside of the cluster.

By using ExternalIP it works fine, because it is designed to do that: to expose a service from a given external ip. And as this ip needs to be known in advance, then we can easily configure our serviceRootUrl with it. By using Ingress, it works also fine for the same reasons and also because our service serves on HTTP. But, if Ingress is disabled and if we cannot use an ExternalIP service, then how to dynamically know the external ip (or host) to use in serviceRootUrl?

Any idea @phertweck?

phertweck commented 6 years ago

@abourdon Yes, the serviceRootURL must be provided to generate the correct links. However it's very dangerous to link this property to the externalIp. Like mentioned before: You can only bind one service to your external ip adress on port 80/443. This means if you deploy FROST-Server in your K8 cluster, exposed on port 80 on your public ip adress, you can't deploy any other web application in your cluster (unless you have multpile external ips). So you loose the flexibility of the cluster.

Therefore I think you won't use your cluster without any L8 reverse proxy/load balancer. This can be done by ingress or you deploy your own service doing this multiplexing.

I hope my explanation was clear enough. I think this this article gives a good overview about those K8 concepts.

To sum up, I suggest the following: Remove all externalIp settings. Expose Mqtt as node port service and make it deactivatable. Expose the FROST-HTTP through an ingress resource. Make the serviceRootUrl independantly settable, since I don't know if you have access to the domain name or external ip at this point.

abourdon commented 6 years ago

@phertweck, Thanks. To sum up our ideas, I propose this kind of configuration (simplified):

values.yaml

[...]
cluster:
  host: frost-server

ingress:
  enabled: [true|false]

components:
  http:
    ports:
       http:
         internal: 80
         external: 30080 # for instance
[...]

ingress.yaml

[...]
spec:
  rules:
  - host: {{ .Values.cluster.host }}
    http:
      paths:
      - path: /
        backend:
          serviceName: frost-server-http
          servicePort: {{ .Values.components.http.ports.http.internal }}
[...]

frost-server-http-service.yaml (simplified)

[...]
kind: Service
metadata:
  name: frost-server-http
[...]
spec:
  {{- if ne .Values.ingress.enabled }}
  type: NodePort
  {{- end }}
  ports:
    - name: http
      port: {{ .Values.components.http.ports.http.internal }}
      {{- if ne .Values.ingress.enabled }}
      nodePort: {{ .Values.components.http.ports.http.external }}
      {{- end }}
[...]

This way:

And then serviceRootUrl can be statically set whatever if Ingress is enabled or disabled.

WDYT?

phertweck commented 6 years ago

@abourdon Thanks for this solution. For me it looks very good and I don't see any problem at the moment. After you've done the change I'll test it on our environment and check if it's working like expected.

I've two small naming hints. I suggest to rename:

I think this makes the meaning of the values a bit more clear.

abourdon commented 6 years ago

Ok I will use it. I also want to rename components to frost plus some minor changes.

I go back as soon as a stable version is available.

abourdon commented 6 years ago

The new frost-server-1.7.4 Helm chart is now available with the following features:

Documentation has also been updated following these changes.

Waiting for your feedback.

abourdon commented 6 years ago

One more thing. I tried to simplify the configuration by not duplicating configuration keys. For instance, instead of having one separated configuration for the bus_sendWorkerPoolSize environment variable (one for the http module and one for the mqtt one), I decided to only define it in the mqtt broker module and to use it from the http and mqtt ones.

In other words: There is only one configuration for the bus_sendWorkerPoolSize environment variable, which is shared with the mqtt and http modules. And as this configuration is associated to the mqtt broker module, then the configuration key is contained into the values.yaml's frost.mqtt.broker part:

frost:
  mqtt:
    broker:
       sendWorkerPoolSize: 10

Let me know if it is a correct approach, if not, gives me keys which need to be duplicated by module and I will do the change.

phertweck commented 6 years ago

Thank you for your work! I managed to install FROST-Server using helm from the command line. I also tried to add your repository to our rancher catalog. Unfortunately rancher is not able to pull the chart. I checked the rancher issues and I assume it's because you're using a relative url to the tgz file. Can you please try using the absolute URL to frost-server-1.7.6.tgz?

abourdon commented 6 years ago

Hi @phertweck, I just updated the index to point to absolute url of frost-server-1.7.6.tgz. Is it ok now?

phertweck commented 6 years ago

Yes! I was able to deploy the FROST-Server on our rancher environment using the rancher catalog.

I noticed one slight problem. All entities use frost-server-* as name. When deploying multiple FROST-Server instances there's a name conflict. I my case the problem was the second frost-server-db-volume-claim was not bound. I had a look at other existing Helm charts. They are respecting the name which is entered when launching the app from rancher catalog. They use the fullName for every entity name (e.g. defined here).

I suggest to adopt this naming scheme to allow multiple FROST-instances in one environment.

abourdon commented 6 years ago

You can allow multiple FROST-instances in one environment by overriding the app configuration key which is by default set to frost-server.

In your example, I can see that fullName is actually the concatenation between the release name and the name. As the release name is unique, then the fullName is unique. So I think this is not related to Rancher, but simply a way to name deployment's resources with unique name. Not also you can retrieve the release name in the label of each deployment's resources.

I hesitated to make names unique (by suffixing by the release name as it can be done in fullName) because I don't know what should be use cases that would use multiple FROST-instances in one environment. Especially if you are using a local PersistentVolume that cannot be created twice if it is pointing to the same volume mount path.

But I can make the change if it is the "standard" way to define charts.

abourdon commented 6 years ago

@phertweck, the new 1.7.7 release is available and integrates these changes. Now any Kubernetes resource is prefixed with the release name.

Let me know

abourdon commented 6 years ago

@phertweck Thinking about this further. I would also like to set the cluster.host to use the release name. This way, there will be no conflict, especially with Ingress, when deploying multiple instances. But I can't see this kind of configuration in standard charts (from https://github.com/helm/charts). indeed, all of them are using static (but configurable) values for their Ingress' hosts. So it depends to your needs. What do you suggest?

phertweck commented 6 years ago

@abourdon Now it's working perfectly fine!

I don't know what's the best way to specify the host name. I think the current solution is pretty good and useful. Maybe I find a better solution when using your Helm chart more intensively. I'll keep my eye open to look how this is solved in other helm charts.

If you're looking for further improvement: Did you have a look how the documentation (readme) is presented in rancher? I think it's very difficult for a new one not familiar with the chart to find the important configuration values (for me its: cluster.host, frost.db.volume.enabled and ingress.enabled).

abourdon commented 6 years ago

Even if it is subjective, I tried to write a complete, generic (regarding use cases) and as simple as possible documentation explaining the main concepts behind this chart, without pretending that user has any experience in Helm or Kubernetes. That's why you have a Getting started section to quickly install the chart, and also a Configuration section for main configuration parts (including volume and Ingress configuration). In addition, this documentation does not assume you already have minimal requirements to install this chart. That's also why you have a Requirements section that describes the minimal prerequisites to met in order to install this chart by pointing to official documentation for any required tool. On top of that, you have a ToC to quickly access to the whole documentation.

I never used Rancher and it is not a prerequisite to install any Helm chart. So the documentation should not be focused to it. However, I will have a look at Rancher to see how to adapt it for Rancher while being generic enough to not be constrained to only use it.

Otherwise, I'm glad to hear you are happy with this implementation. If you are interested, I'm ready to create a PR to merge it with your source code. But I'll need some precision about where to put sources. Do you want to create a directory at the root of FROST-Server (e.g., helm), or create an independent project, or something else?

phertweck commented 6 years ago

@abourdon Thank you for the explanation of your ideas. I know writing documentation for several usercases and users with different background knowledge is hard. What do you think about splitting the documentation?

I think you've already covered everything very well, so it's just a bit of restructuting. I was looking for your configuration parameters table. Somehow it's not displayed in rancher and I didn't notice that's in already in you repo. Sorry for that.

What do you think about this documentation structure?

Yes, of course we would appreciate a PR! I think creating a new helm folder in the FROST-Server repository is good. If we face any problems I think we can extract it to a separate repository in the future. I'll have a look how to build the chart releases using TravisCI. I think it's very good to have an automated build also for the chart artifact. Can you shortly name the commands you executed to build the chart (I've no experience with this)?

Regarding your question about the bus_sendWorkerPoolSize I talked to @hylkevds . We think it's better to split the configuration for mqtt and http since it's possible to have different values. Also it might be a bit confusing that the http value is derived from the mqtt value. One last thing: is it possible to rename frost.mqtt.**broker**.sendWorkerPoolSize to frost.mqtt.**bus**.sendWorkerPoolSize, since they are mapped to the bus configuration?

hylkevds commented 6 years ago

is it possible to rename frost.mqtt.broker.sendWorkerPoolSize to frost.mqtt.bus.sendWorkerPoolSize, since they are mapped to the bus configuration?

Thinking about it a bit more, it should probably be frost.bus.sendWorkerPoolSize, since it's not specific to the MQTT Component, and in the future there could be non-mqtt bus implementations.

abourdon commented 6 years ago

@phertweck:

What do you think about splitting the documentation?

GettingStarted part: Describing the requirements and basic Kubernetes/Helm concepts, commands and configurations. This could be your Requirements and Getting started chapter. So this part can be chart independet.

Chart Description: Assuming the reader knows the basic concepts (otherwise there is a reference to the GettingStarted part) this describes the chart specific configuration like your Configuration part. Maybe you also can allign to the sections of the official charts:

One sentence description of the application

  • TL;DR; part helm install --name storeconnect/frost-server
  • Introduction
  • Prerequisites
  • Installing the Chart
  • Uninstalling the Chart
  • Configuration
  • Persistence

In this case, the Getting started part could be integrated in the Chart Description part:

This way, you will have the same structure as official charts and you will avoid redundancy. Ok?

Can you shortly name the commands you executed to build the chart (I've no experience with this)

There are mainly two phases: build the chart and upload the chart to a Helm repository.

To build the chart, I:

To upload the chart, I:

Regarding your question about the bus_sendWorkerPoolSize I talked to @hylkevds . We think it's better to split the configuration for mqtt and http since it's possible to have different values.

Ok so I will do it.

@phertweck, @hylkevds:

is it possible to rename frost.mqtt.broker.sendWorkerPoolSize to frost.mqtt.bus.sendWorkerPoolSize, since they are mapped to the bus configuration?

Thinking about it a bit more, it should probably be frost.bus.sendWorkerPoolSize, since it's not specific to the MQTT Component, and in the future there could be non-mqtt bus implementations.

In this case, I propose to rename the whole mqtt.broker module to just bus. This way, you break the link with mqtt. So, the bus configuration will be at the same level as mqtt configuration, and the mqtt-broker-.* yaml files will be renamed bus-.*. WDYT?

hylkevds commented 6 years ago

In this case, I propose to rename the whole mqtt.broker module to just bus. This way, you break the link with mqtt. So, the bus configuration will be at the same level as mqtt configuration, and the mqtt-broker-. yaml files will be renamed bus-.. WDYT?

Yes, that's a good idea.

abourdon commented 6 years ago

The new frost-server-1.7.9 version is available and integrates these changes. Documentation has also been updated to match with official charts structure.

Here is a summary of what has been done:

@phertweck: as these are structural changes, could you please check on your side if chart can be correctly installed and runs as expected?

Waiting for your return, Aurélien

phertweck commented 6 years ago

Documentation has also been updated to match with official charts structure.

Great! This looks very good now. (And now the table with the configuration values is shown in rancher)

Here is a summary of what has been done:

I agree to all of your changes. I've nothing to add.

Yes, the new chart version is working on my side. While working with the chart I noticed two things:

abourdon commented 6 years ago

@phertweck

The repository contains only the latest version of the chart. Is it possible to keep the old versions as well to allow a rollback in case of any problem?

Since we are still working on the chart structure, there is no need to keep old versions. Once chart will be stable, then I could merge it to the FROST-Server base code. From there, it will be more consistent to create the official FROST helm chart repository and you could manage your chart version as you want.

When enabling the persistence (without specifying a storage class) a new local volume is created. This can be a bit confusing, because in our cluster we have a storage class which is used by default. Since you set an explicit storage class this default value is ignored when not passing the storage class name to the chart. I suggest to rely on the default storage class of the cluster if the persistence is enabled. To allow an easy use if no default storage class is defined in the cluster the user can explicitly select your local storage class.

Ok, so what do you think to have something like this:

persistence:
  enabled: false
  storageClassNameOverride: # if empty, then rely to the default StorageClassName
  capacity: 10Gi
  #local:
  #  nodeMountPath: /mnt/frost-server-db

Then by default no storage class name will be used, but you can override this behaviour by setting the persitence.storageClassNameOverride value. And if you define the local one, then you could use the builtin local storage class.

phertweck commented 6 years ago

Then by default no storage class name will be used, but you can override this behaviour by setting the persitence.storageClassNameOverride value. And if you define the local one, then you could use the builtin local storage class.

Sounds good 👍

abourdon commented 6 years ago

@phertweck the new frost-server-1.7.10 has been published with these changes. Little change according to what we said: storageClassNameOverride has been renamed simply storageClassName as there is no storage class name by default.

phertweck commented 6 years ago

@abourdon 👍

While testing the chart I noticed one more thing (which made the upgrade to the new version a bit more complicated due to our cluster setup):

abourdon commented 6 years ago

@phertweck, the frost-server-1.7.12 is now available and makes accessModes overridable. It also simplifies HTTP component related configuration, especially for the mandatory serviceRootUrl value, by moving all HTTP related configuration (cluster.host, ingress) to frost.http.

Same work has been done for the Database component, by moving the global persistence configuration to frost.db. In addition, the StorageClassName has been renamed frost-server-db-local to avoid possible conflicts with the simple local name.

phertweck commented 6 years ago

@abourdon Thank you very much!

I'm sorry, but while working with the chart two new suggestions for improvement came up:

abourdon commented 6 years ago

@phertweck,

Is it possible to make the name of the volume claim configurable? This offers the possibility to provide an existing volume and mount it to the FROST server.

Do you mean to be able to configure the volume claim name when used by a pod ? https://github.com/StoreConnect/sensorthings-api-deployment/blob/master/deployment/helm/frost-server/templates/db-deployment.yaml#L59

Is it possible to leave the MQTT nodePorts empty when not explicitly overriding it? By this kubernetes will automatically assign a free port and you can be sure that there is no problem with a double use of a port.

Since the HTTP module also needs to have a predefined node port (due to the mandatory serviceRootUrl parameter that constraints to know the complete URL in advance), you also have to be sure there is no double use of a port for it. So I don't think that's a problem to have this same behavior with other exposed services, even if we could do differently for them.

phertweck commented 6 years ago

Do you mean to be able to configure the volume claim name when used by a pod ? https://github.com/StoreConnect/sensorthings-api-deployment/blob/master/deployment/helm/frost-server/templates/db-deployment.yaml#L59

Yes exactly. By this I can provide an already existing volume to the db pod: e.g. because the db is imported from somewhere else or (like in my case) I needed to delete whole whole instance and start a new one.

Since the HTTP module also needs to have a predefined node port (due to the mandatory serviceRootUrl parameter that constraints to know the complete URL in advance), you also have to be sure there is no double use of a port for it. So I don't think that's a problem to have this same behavior with other exposed services, even if we could do differently for them.

Yes, for the HTTP part you're right. That's the reason why I always use ingress. At least for the MQTT port you don't need to know it when starting the service. I know that you afterwards need the knowledge to determine which port was choosen by kubernetes. Still I think is better not to specify the node port, because I think it should be possible to instantiate multiple FROST instances in one cluster with as little configuration as possible.

I know it's hard to find the best default values, because they're highly depending on the available infrastructure/cluster and the use case. Since you've depveloped the chart the decision is up to you.

phertweck commented 6 years ago

@abourdon One more thing. Can you please enable sticky sessions in the mqtt service? service.spec.sessionAffinity to “ClientIP” see here. Since the mqtt server stores the subscriptions and the client should connect to the same server after the connection is lost.

abourdon commented 6 years ago

I think it should be possible to instantiate multiple FROST instances in one cluster with as little configuration as possible.

I agree, but configuration should also be coherent between modules. And as Ingress is not enabled by default (in regard with how official charts are using it), then I think we can keep it this way. At least for the moment, and wait for user feedback when chart will be merged to FROST-Server

abourdon commented 6 years ago

@phertweck, the frost-server-1.7.13 is now available and includes:

Let me know