Topics

Community Feedback Required on PRs 1348 / 1359

Trevor.Conn@...
 

Hi all --


As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.


We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.


To summarize each approach:


1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]


2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.


The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.
Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.


Thanks.


Trevor Conn
Technical Staff Engineer
Core Working Group Chair of EdgeX Foundry
Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Goodell, Leonard
 

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Ian Johnson
 

Apologies for the long email, but there's a bit of background necessary to explain why I don't think it's a good idea to use this command line flag in the snap.


With the snap we are in a better position than Docker to use the configuration files more directly because the configuration files live on the user's host file system and not inside the container where modification is tricky. Due to this, we try to strike a nice balance in the snap between allowing the user to modify the configuration files directly and also enable using consul for more automated device management tasks. Our current plan for managing this can be read about in issue #922.


Basically, in the snap we want to have 2 ways to run config-seed, one automated way which runs without the overwrite flag, and one manual way which runs with the overwrite flag. This allows a developer or manual user to easily change configuration for the services by modifying the files locally on their file system directly and then run a single command to push the content of the configuration from the filesystem into Consul.


I don’t think the addition of this new command line option will really help the snap at all. To explain that, let me explain how using redis currently would work. If a user wants to use redis our current mechanism would be just to have the user run:


snap set dbtype=redisdb


(or they could use the snapd REST API to do the same thing)


That then triggers a configuration hook to run, which processes the configuration.toml files locally for redis using a tool called remarshal, then pushes that configuration into consul so that an automated configuration management system can interact with consul to get/set configuration afterwards. This has the combined benefit of allowing a local user or developer to know what and where the configuration items that are set come from. They either originated from local configuration files or from something driving consul. The “snap set” mechanism described above merely is used as a transparent external means to modify something internal to EdgeX, the configuration files and Consul. It’s not meant to store state about EdgeX configuration, just to conveniently trigger those internal changes.


If we now consider using the command line flag that Trevor has proposed, if we are to use that option in the snap, we now need to track internally in the snap what database was configured with the “snap set” command above, and more importantly use that config item to determine what flags to launch config-seed with (dbtype=redis -> --database=redis, etc.). This means that in the snap, EdgeX now has an external system controlling configuration of the system and someone using the snap has a third dimension for control-plane/configuration:


  • Configuration files

  • Consul

  • Snapd configuration items


Previously, the snapd configuration items were only used to track what services were on/off, which is no different to someone using docker-compose to start/stop certain services and is a natural way to track this.


Our intention with the snap is to minimize the extent to which the snap distribution differs from the other distribution methods, but also to add convenient features which the snap can provide that aren’t available to docker-compose and native packaging methods. While it may be slightly simpler to add this command line flag for Redis to config-seed and use it within the snap, I think that it is simpler from a user’s perspective (as well as a documentation perspective) to have all the EdgeX configuration live within EdgeX. Having configuration items live in multiple locations significantly contributes to user confusion and complexity as the software ages and so trying to minimize this from the 1.0 release is beneficial.




On Tue, May 21, 2019 at 11:13 AM Goodell, Leonard <leonard.goodell@...> wrote:

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA

Goodell, Leonard
 

Ian makes a good point in that in the non-docker scenarios, the configuration.toml is easily modified to specify use of Redis. So in those cases having a new command line switch is redundant.

 

I still think for the Docker scenario, the command line switch is preferred. I’d like the DB settings override be in code rather than in the Docker file. Have the switch doesn’t make it required in the non-docker scenarios. There the configuration.toml can still be modified, rather than use the switch.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Ian Johnson
Sent: Tuesday, May 21, 2019 11:47 AM
To: Goodell, Leonard <leonard.goodell@...>
Cc: Trevor.Conn@...; EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: Re: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Apologies for the long email, but there's a bit of background necessary to explain why I don't think it's a good idea to use this command line flag in the snap.

 

With the snap we are in a better position than Docker to use the configuration files more directly because the configuration files live on the user's host file system and not inside the container where modification is tricky. Due to this, we try to strike a nice balance in the snap between allowing the user to modify the configuration files directly and also enable using consul for more automated device management tasks. Our current plan for managing this can be read about in issue #922.

 

Basically, in the snap we want to have 2 ways to run config-seed, one automated way which runs without the overwrite flag, and one manual way which runs with the overwrite flag. This allows a developer or manual user to easily change configuration for the services by modifying the files locally on their file system directly and then run a single command to push the content of the configuration from the filesystem into Consul.

 

I don’t think the addition of this new command line option will really help the snap at all. To explain that, let me explain how using redis currently would work. If a user wants to use redis our current mechanism would be just to have the user run:

 

snap set dbtype=redisdb

 

(or they could use the snapd REST API to do the same thing)

 

That then triggers a configuration hook to run, which processes the configuration.toml files locally for redis using a tool called remarshal, then pushes that configuration into consul so that an automated configuration management system can interact with consul to get/set configuration afterwards. This has the combined benefit of allowing a local user or developer to know what and where the configuration items that are set come from. They either originated from local configuration files or from something driving consul. The “snap set” mechanism described above merely is used as a transparent external means to modify something internal to EdgeX, the configuration files and Consul. It’s not meant to store state about EdgeX configuration, just to conveniently trigger those internal changes.

 

If we now consider using the command line flag that Trevor has proposed, if we are to use that option in the snap, we now need to track internally in the snap what database was configured with the “snap set” command above, and more importantly use that config item to determine what flags to launch config-seed with (dbtype=redis -> --database=redis, etc.). This means that in the snap, EdgeX now has an external system controlling configuration of the system and someone using the snap has a third dimension for control-plane/configuration:

 

  • Configuration files
  • Consul
  • Snapd configuration items

 

Previously, the snapd configuration items were only used to track what services were on/off, which is no different to someone using docker-compose to start/stop certain services and is a natural way to track this.

 

Our intention with the snap is to minimize the extent to which the snap distribution differs from the other distribution methods, but also to add convenient features which the snap can provide that aren’t available to docker-compose and native packaging methods. While it may be slightly simpler to add this command line flag for Redis to config-seed and use it within the snap, I think that it is simpler from a user’s perspective (as well as a documentation perspective) to have all the EdgeX configuration live within EdgeX. Having configuration items live in multiple locations significantly contributes to user confusion and complexity as the software ages and so trying to minimize this from the 1.0 release is beneficial.

 

 

 

On Tue, May 21, 2019 at 11:13 AM Goodell, Leonard <leonard.goodell@...> wrote:

I prefer PR 1348 as it works for native and snaps, not just Docker. Also, I think it is more straight forward than modifying the toml file during the Docker build.

 

-Lenny

 

From: EdgeX-GoLang@... <EdgeX-GoLang@...> On Behalf Of Trevor.Conn@...
Sent: Tuesday, May 21, 2019 8:41 AM
To: EdgeX-GoLang@...; edgex-tsc-core@...; EdgeX-Devel@...
Subject: [Edgex-golang] Community Feedback Required on PRs 1348 / 1359

 

Hi all --

 

As you're aware from recent working group calls we need to develop a mechanism whereby the service configurations can be easily overridden to use Redis for the upcoming Edinburgh release. In a deployment scenario, we do not want to require an admin to go in and modify the configuration files for Redis to be enabled. It needs to be accomplished either via the Snap, docker-compose or script for automation.

 

We have two PRs for issue 1347 that seek to solve this problem. One of them is mine and I don't think it's appropriate for me to make a unilateral decision, so I'm seeking feedback from the community as to which approach should be accepted.

 

To summarize each approach:

 

1.) PR 1359 utilizes changes to the config-seed Dockerfile that creates copies of the existing service configuration.toml files. It replaces the necessary properties in the duplicated files with Redis values and then injects these into the config-seed image in a different directory. To use this, a Redis-specific docker-compose.yml would utilize the existing --cmd parameter to point at the root location of the Redis configuration files. For example: 

config-seed:
    image: edgexfoundry/docker-core-config-seed-go:0.7.1
    container_name: edgex-config-seed
    command: ["/edgex/cmd/config-seed/config-seed --profile=docker --cmd=/edgex/cmd-redis"]

 

2.) PR 1348 defines a new command line parameter on the config-seed (--database / -d) that defaults to Mongo. If the service is started with "-d=redis" then the database connectivity parameters are overridden from the existing configuration.toml files prior to population of Consul.

 

The essential differences are:

  • Definition of a new command line parameter on the config-seed versus not.
  • Assumptions about whether the config-seed should perform this function in native deployments utilizing Consul.
    • 1348 will accomplish this. 1359 will put this responsibility on the admin since it only touches the Dockerfile.

Since code freeze is on Tuesday the 28th, we need a consensus from the community rather quickly. You are welcome to provide feedback here or on either PR. If no clear preference has emerged by the time we hold the Core Working Group call this Thursday, we will make a decision then.

 

Thanks.

 

Trevor Conn
Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech
Trevor_Conn@...
Round Rock, TX  USA