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

Configuration via environment variables #1094

Closed
mateuszdrab opened this issue Mar 8, 2022 · 13 comments · Fixed by #1099
Closed

Configuration via environment variables #1094

mateuszdrab opened this issue Mar 8, 2022 · 13 comments · Fixed by #1099

Comments

@mateuszdrab
Copy link

I am trying to deploy room assistant as a daemonset on my RPIs via Kubernetes.

I understand that it was possible to configure previous version on RA via env variables - perfect for K8s.
However, it looks like the feature is now removed and the only option is to supply a YAML/JSON file via an environment variable or in the file system.
This means, supplying separate file for each instance because there is no easy way to override the room name and the default is the hostname, which isn't appropriate for my use case.

Is is possible to at least provide an environment variable based configuration of the room name?

The yaml is uniform for all nodes, except the room name and I could have the environment variable populated from the label of the kubernetes node which bears the desired room name.

Thanks

@mKeRix
Copy link
Owner

mKeRix commented Mar 10, 2022

room-assistant utilizes node-config under the hood for its configuration interface in the current version, which does not expose these flat environment variable configurations anymore as you suggested. I think your use case is a valid one though - and while I don't think that exposing all options via env variables is possible anymore due to the complex structure, some basic options can be made available.

I'll take a look at configuring this for a few basic options, such as the room name.

@mateuszdrab
Copy link
Author

Perfect, looks like quite an easy addition based on the fact node config supports it already.
I think apart from the room name, I'd also recommend exposing the mqtt connection broker details, at least the username and password so that they can also be injected from Kubernetes' native secrets.
I think pretty much the rest of the config should in fact stay unform across nodes.

mKeRix added a commit that referenced this issue Mar 10, 2022
mKeRix added a commit that referenced this issue Mar 10, 2022
@mKeRix mKeRix linked a pull request Mar 10, 2022 that will close this issue
4 tasks
@mKeRix
Copy link
Owner

mKeRix commented Mar 10, 2022

I whipped up a quick PR that implements these environment variables. Seems to work fine on my dev machine, but I want to make sure it works on a real world installation before merging & releasing.

@mateuszdrab
Copy link
Author

That was quick!
I'm about to try, building the container on the pi is taking forever...

@mateuszdrab
Copy link
Author

mateuszdrab commented Mar 10, 2022

Hmm, so is it supposed to work when loading the config from the file but the hostname from the variable?
Can't get it to work, even with NODE_CONFIG used without a configuration file.

room-assistant  | 3/10/2022, 10:35:20 PM - info - RoutesResolver: PrometheusController {/metrics}:
room-assistant  | 3/10/2022, 10:35:20 PM - info - RouterExplorer: Mapped {/metrics, GET} route
room-assistant  | 3/10/2022, 10:35:20 PM - info - ConfigService: Loading configuration from /usr/local/lib/node_modules/room-assistant/dist/config/definitions/default.js, $NODE_CONFIG (Current: /room-assistant)
room-assistant  | 3/10/2022, 10:35:20 PM - info - ClusterService: rpi-a-lr has been elected as leader

docker-compose.yml contains:

    environment:
      - RA_GLOBAL_INSTANCE_NAME=livingroom
      - NODE_CONFIG=... (removed)

mKeRix added a commit that referenced this issue Mar 13, 2022
Includes global and some MQTT options for now.

Closes #1094
@mKeRix
Copy link
Owner

mKeRix commented Mar 13, 2022

I've now tested this successfully on a real world npm installation, as well as my dev machine. In all cases the custom env variable took precedence, even when configuring an instance name both via RA_GLOBAL_INSTANCE_NAME and NODE_CONFIG. I'll check again by building the Docker image locally to try and reproduce your issue.

@mKeRix
Copy link
Owner

mKeRix commented Mar 13, 2022

My Docker test was also successful - maybe something is off in your custom image build? I can put out a new release with these changes soon, so you can test it again with the official build.

@mateuszdrab
Copy link
Author

mateuszdrab commented Mar 13, 2022

@mKeRix Yeah, please do. I keep getting the same result locally.

@mateuszdrab
Copy link
Author

@mKeRix any update on the new release builds?

Thanks

@mKeRix
Copy link
Owner

mKeRix commented Mar 20, 2022

I'll put out new releases later today.

github-actions bot pushed a commit that referenced this issue Mar 20, 2022
# [2.20.0](v2.19.1...v2.20.0) (2022-03-20)

### Bug Fixes

* **bluetooth-classic:** increase change detection speed ([6325937](6325937)), closes [#775](#775)
* **docker:** include heatmap dependencies in image ([7cf5de1](7cf5de1)), closes [#826](#826)

### Features

* **bluetooth:** add configuration options for noise filter ([85981ee](85981ee)), closes [#775](#775)
* expose some config options as env variables ([#1099](#1099)) ([3ae8e9a](3ae8e9a)), closes [#1094](#1094)
@github-actions
Copy link

🎉 This issue has been resolved in version 2.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue Mar 20, 2022
# [3.0.0-beta.5](v3.0.0-beta.4...v3.0.0-beta.5) (2022-03-20)

### Bug Fixes

* **bluetooth-classic:** increase change detection speed ([6325937](6325937)), closes [#775](#775)
* **docker:** include heatmap dependencies in image ([7cf5de1](7cf5de1)), closes [#826](#826)

### Features

* **bluetooth:** add configuration options for noise filter ([85981ee](85981ee)), closes [#775](#775)
* expose some config options as env variables ([#1099](#1099)) ([3ae8e9a](3ae8e9a)), closes [#1094](#1094)
@github-actions
Copy link

🎉 This issue has been resolved in version 3.0.0-beta.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mateuszdrab
Copy link
Author

@mKeRix Thanks! The feature works fine and I've been able to feed in the MQTT creds from a secret plus the instance name from the K8s node label :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants