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 2020/05/21 05:30:18 UTC

[GitHub] [bigtop] masatana opened a new pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

masatana opened a new pull request #641:
URL: https://github.com/apache/bigtop/pull/641


   This PR updates `get_roles.rb` to be faild if there is no such component which is specified in user config.
   
   Before:
   ```
   $ cat config.yaml
   docker:
           memory_limit: "4g"
           image: "bigtop/puppet:trunk-centos-7"
   
   repo: "http://repos.bigtop.apache.org/releases/1.4.0/centos/7/$basearch"
   distro: centos
   components: [foobar]
   enable_local_repo: false
   smoke_test_components: []
   $ ./docker-hadoop.sh -c 1
   (snip)
   (not failed)
   ```
   
   After:
   ```
   $ ./docker-hadoop.sh -c 1
   (snip)
   
   Error: get_roles(): No such component in roles_map. foobar on node 7541662d9555.bigtop.apache.org
   Error: get_roles(): No such component in roles_map. foobar on node 7541662d9555.bigtop.apache.org
   
   [LOG] Failed to provision container 7541662d95558968b99a07be71813b030a0f827243ca7a3d0f6c25c9d6cc784c with exit code 1
   
   ```
   
   Also I updated existed config files as I noticed that there is no such component `mapreduce` but `mapred`.


----------------------------------------------------------------
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] [bigtop] masatana commented on a change in pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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



##########
File path: provisioner/docker/config_centos-7.yaml
##########
@@ -19,6 +19,6 @@ docker:
 
 repo: "http://repos.bigtop.apache.org/releases/1.4.0/centos/7/$basearch"
 distro: centos
-components: [hdfs, yarn, mapreduce]
+components: [hdfs, yarn, mapred]
 enable_local_repo: false
 smoke_test_components: [hdfs, yarn, mapreduce]

Review comment:
       @iwasakims @evans-ye Thank you for the review. Updated cluster.pp




----------------------------------------------------------------
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] [bigtop] masatana commented on pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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


   @iwasakims Thanks! Will address your comments.


----------------------------------------------------------------
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] [bigtop] iwasakims commented on a change in pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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



##########
File path: provisioner/docker/config_centos-7.yaml
##########
@@ -19,6 +19,6 @@ docker:
 
 repo: "http://repos.bigtop.apache.org/releases/1.4.0/centos/7/$basearch"
 distro: centos
-components: [hdfs, yarn, mapreduce]
+components: [hdfs, yarn, mapred]
 enable_local_repo: false
 smoke_test_components: [hdfs, yarn, mapreduce]

Review comment:
       It would be nice if we can make it consistent between components and smoke_test_components (mapred vs. mapreduce). Can we change [the mapred label of the map defined in cluster.pp](https://github.com/apache/bigtop/blob/master/bigtop-deploy/puppet/manifests/cluster.pp#L57) to mapreduce?  




----------------------------------------------------------------
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] [bigtop] iwasakims merged pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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


   


----------------------------------------------------------------
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] [bigtop] evans-ye commented on a change in pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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



##########
File path: provisioner/docker/config_centos-7.yaml
##########
@@ -19,6 +19,6 @@ docker:
 
 repo: "http://repos.bigtop.apache.org/releases/1.4.0/centos/7/$basearch"
 distro: centos
-components: [hdfs, yarn, mapreduce]
+components: [hdfs, yarn, mapred]
 enable_local_repo: false
 smoke_test_components: [hdfs, yarn, mapreduce]

Review comment:
       +1. Let's get it rolled.




----------------------------------------------------------------
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] [bigtop] iwasakims commented on a change in pull request #641: BIGTOP-3358: get_roles() function should fail if a component is not in roles_map

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



##########
File path: bigtop-deploy/puppet/modules/bigtop-util/lib/puppet/parser/functions/get_roles.rb
##########
@@ -54,6 +54,8 @@
           temp_roles = component_map[role_type]
           roles.concat(temp_roles)
         end
+      else
+	fail Puppet::ParseError, "get_roles(): No such component in roles_map. #{component}"

Review comment:
       should be whitespace instead of tab.




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