You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bigtop.apache.org by GitBox <gi...@apache.org> on 2022/01/03 17:16:10 UTC

[GitHub] [bigtop] elukey opened a new pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

elukey opened a new pull request #851:
URL: https://github.com/apache/bigtop/pull/851


   This is a simple change to allow docker-compose to use a different
   config file when creating instances. This is useful when testing
   new settings and also to keep multiple versions of docker-compose.yml
   (if the same version cannot be re-used across multiple OS etc..).
   
   For example, from my recent tests the actual docker-compose.yml
   config seems not to work with cgroup v2 and systemd on Debian 10/11.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] iwasakims commented on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
iwasakims commented on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1011147493


   I cherry-picked this to branch-3.0 since this is useful for testing maintenance release (3.0.1).


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] evans-ye commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
evans-ye commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r781776837



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       Shall we error out earlier and explicitly log what's the problem when getting nodes?




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] elukey commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
elukey commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r781908136



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       I am open to any suggestion, I have basically wrapped what was there in a function.. What do you have in mind? I can incorporate it in the change :)




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] elukey edited a comment on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
elukey edited a comment on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1004809109


   @iwasakims great point, thanks a lot! Uploaded a new version of the script, currently under testing :)
   
   Nevermind, it needs more work, I'll do more extensive  tests and report back once done.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] iwasakims commented on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
iwasakims commented on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1004464593


   @elukey All invocation of `docker-compose` command should be replaced with `docker-compose -f $docker_compose_yamlconf`? I got following error since `docker-compose` without `-f` seeks the file named docker-compose.yml or docker-compose.yaml.
   
   ```
   $ mv docker-compose.yml docker-compose-renamed.yml
   $ ./docker-hadoop.sh \
      --docker-compose-yml docker-compose-renamed.yml \
      --create 1 \
      --image bigtop/puppet:trunk-centos-8 \
      --memory 8g \
      --repo file:///bigtop-home/output \
      --disable-gpg-check \
      --stack zookeeper
   Environment check...
   Check docker:
   Docker version 20.10.7, build f0df350
   Check docker-compose:
   docker-compose version 1.26.2, build eefe0d31
   Check ruby:
   ruby 2.5.5p157 (2019-03-15 revision 67260) [x86_64-linux]
   Creating 20220104_011930_r8500_bigtop_1 ... done
   ERROR:
           Can't find a suitable configuration file in this directory or any
           parent. Are you in the right directory?
   
           Supported filenames: docker-compose.yml, docker-compose.yaml
   ...
   ```


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] iwasakims merged pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
iwasakims merged pull request #851:
URL: https://github.com/apache/bigtop/pull/851


   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] evans-ye commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
evans-ye commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r783175301



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       My bad for not being clear. I was thinking that for the else case (PROVISION_ID is not set), we can error out earlier with a clear log message instead of being silence . But this is a minor comment as we didn't do this kind of early error out in other place, plus you're just wrapping up what originally written ;)




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] elukey commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
elukey commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r783184037



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       @evans-ye ah yes definitely! I can follow up with another commit :)




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] evans-ye commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
evans-ye commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r781776837



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       Shall we error out earlier and explicitly log what's the problem when we get correctly set nodes?




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] elukey commented on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
elukey commented on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1004809109


   @iwasakims great point, thanks a lot! Uploaded a new version of the script, currently under testing :)


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] elukey commented on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
elukey commented on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1007958623


   @iwasakims I added a few changes from your last review:
   1) A new file called `docker-compose-cgroupsv2.yml`
   2) An entry in the README file
   3) The possibility to use `-F` in the command line (as opposed to only `--docker-compose-yml` that is really long).
   
   Let me know if it is still ok :)


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] evans-ye commented on a change in pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
evans-ye commented on a change in pull request #851:
URL: https://github.com/apache/bigtop/pull/851#discussion_r783175301



##########
File path: provisioner/docker/docker-hadoop.sh
##########
@@ -284,12 +293,19 @@ log() {
 }
 
 configure-nexus() {
+    get_nodes
     for node in ${NODES[*]}; do
         docker exec $node bash -c "cd /bigtop-home; ./gradlew -PNEXUS_URL=$1 configure-nexus"
     done
     wait
 }
 
+get_nodes() {
+    if [ -n "$PROVISION_ID" ]; then
+        # shellcheck disable=SC2207
+        NODES=(`$DOCKER_COMPOSE_CMD -p $PROVISION_ID ps -q`)
+    fi

Review comment:
       My bad for not being clear. I was thinking that for the else case (PROVISION_ID is not set), we can error out earlier with a clear log message instead of being silence . But this is a minor comment as we didn't do this kind of early error out in other place, plus you're just wrapping what originally written ;)




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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



[GitHub] [bigtop] iwasakims commented on pull request #851: BIGTOP-3614: allow to change the docker-compose cfg in the provisioner

Posted by GitBox <gi...@apache.org>.
iwasakims commented on pull request #851:
URL: https://github.com/apache/bigtop/pull/851#issuecomment-1011120935


   @elukey I'm +1 on updated patch. It worked on Debian 11 and Fedora 33.
   
   ```
   ./docker-hadoop.sh \
    --docker-compose-yml docker-compose-cgroupv2.yml \
    --create 1 \
    --image bigtop/puppet:trunk-debian-11 \
    --memory 8g \
    --repo file:///bigtop-home/output/apt \
    --disable-gpg-check \
    --stack zookeeper
   ```
   ```
   ./docker-hadoop.sh \
    -F docker-compose-cgroupv2.yml \
    --create 1 \
    --image bigtop/puppet:trunk-fedora-33 \
    --memory 8g \
    --repo file:///bigtop-home/output \
    --disable-gpg-check \
    --stack zookeeper
   ```


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@bigtop.apache.org

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