You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/28 17:53:51 UTC

[GitHub] [accumulo-proxy] volmasoft opened a new issue #21: Provide overrides for proxy.properties in Docker deployments

volmasoft opened a new issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21


   @keith-turner and I briefly touched upon allowing `proxy.properties` to be overidden for a Docker instantiation of accumulo-proxy in this pull request: https://github.com/apache/accumulo-proxy/pull/20 
   
   Currently in accumulo-docker you can override properties by the following:
   ```
   export ACCUMULO_CL_OPTS="-o prop.1=var1 -o prop.2=var2"
   docker run -d --network="host" accumulo monitor $ACCUMULO_CL_OPTS
   ```
   This is documented in the docs here: https://github.com/apache/accumulo-docker 
   
   For standard docker approaches I would say this is a simplistic mechanism that works really cleanly however if you start to look at things like docker-compose or kubernetes I would say it would be advantageous to avoid overriding the CMD or ENTRYPOINT flags where possible.
   
   An alternative method would be to provide these as environment variables, therefore you could also use the Kubernetes secret APIs to handle things such as passwords and either map them to a file or environment variable for use within the application.
   
   Would this be acceptable? If so I'm also happy to look at providing the same change to accumulo-docker to retain consistency?
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo-proxy] keith-turner commented on issue #21: Provide overrides for proxy.properties in Docker deployments

Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21#issuecomment-621251205


   > Would this be acceptable? 
   
   How were you thinking of passing options via env vars? Seems like this could be done in multiple ways, I am curious what you are envisioning. Would it be possible to provide a few example commands that show what you are thinking?  


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo-proxy] volmasoft commented on issue #21: Provide overrides for proxy.properties in Docker deployments

Posted by GitBox <gi...@apache.org>.
volmasoft commented on issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21#issuecomment-621283417


   I think there's two ways 1 that goes towards Accumulo's approach, 1 that goes towards Nifi's approach.
   
   
   ### 1. accumulo-docker approach
   Take the variables straight off the command line e.g.:
   ```
   ACCUMULO_CL_OPTS="-o instance.name=uno -o instance.zookeepers=my.host.com:2181"
   docker run accumulo-proxy:latest  ${ACCUMULO_CL_OPTS}
   ```
   
   ### 2. Hybrid approach
   Provide a single environment variable e.g. similar to how accumulo-docker sets ACCUMULO_CL_OPTS
   ```
   ACCUMULO_CL_OPTS="-o instance.name=uno -o instance.zookeepers=my.host.com:2181"
   docker run -e EXTRA_PROPS=${ACCUMULO_CL_OPTS} accumulo-proxy:latest 
   ```
   Then inside our code we grab the PROPS environment variable and override any that are specified in proxy.properties.
   
   ### 3. NiFi's approach
   Provide individual environments variables for each property e.g. like NiFi does here: https://hub.docker.com/r/apache/nifi/ 
   So something like
   ```
   docker run -e INSTANCE_NAME="uno" -e INSTANCE_ZOOKEEPERS="my.host.com:2181"  accumulo-proxy:latest
   ```
   We would have to decide an approach to load these properties either in proxy-env or in the Java code.
   
   ---
   Personally I find the NiFi approach to be a cleaner approach especially if we look towards providing anything like a docker-compose or Kubernetes example configurations in the future. 
   
   Whichever approach we take we should be consistent so I think we'd have to dive into accumulo-docker and update it if we take a different approach, perhaps before we release the first container image on dockerhub.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo-proxy] keith-turner commented on issue #21: Provide overrides for proxy.properties in Docker deployments

Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21#issuecomment-621321862


   Personally, I would like to see accumulo-docker and accumulo-proxy be consistent w.r.t. passing properties.  As far as which should go first with something new, I am not quite sure. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo-proxy] keith-turner commented on issue #21: Provide overrides for proxy.properties in Docker deployments

Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21#issuecomment-621313606


   > Personally I find the NiFi approach to be a cleaner approach
   
   For that approach, it would be nice do it in such a way that code does not have to be written in accumulo-docker for each property.  What do you think about having a common env var name prefix with that approach?   Like `AP_INSTANCE_NAME`?  Code in accumulo-docker could look for all env vars with the prefix `AP_`. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo-proxy] volmasoft commented on issue #21: Provide overrides for proxy.properties in Docker deployments

Posted by GitBox <gi...@apache.org>.
volmasoft commented on issue #21:
URL: https://github.com/apache/accumulo-proxy/issues/21#issuecomment-621318269


   That's an interesting idea, to be fair I hadn't thought that far ahead yet but I like it 👍 
   
   I'm inclined not to implement this unless we're either:
   1. Happy with accumulo-proxy being inconsistent with accumulo-docker
   2. We at happy to update accumulo-docker to follow the same pattern
   
   Any views on who we should dial into that sort of conversation to gain consensus? 
   Looking at the accumulo-docker repo the two main commiters are @mikewalch and @karthick-rn


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org