You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by amandamoran <gi...@git.apache.org> on 2015/09/09 19:53:34 UTC

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

GitHub user amandamoran opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/76

    [[Trafodion 1133]] [[Trafodion 1088]] [[Trafodion 1481]]

    Trafodion 1133 and Trafodion 1088 allow installer to add a node.
    
    Trafodion 1481 seperates rpm installs into its own script so that it can be
    run during an upgrade (to make sure all new packages needed are added). Bug still needs to be fixed is that it runs too many times (is ran once per node) and doesn't need to be. Will create a new jira for that small bug. Would like to check this in to prevent issue when upgrading.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/amandamoran/incubator-trafodion bugFixes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/76.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #76
    
----
commit 330a6ea44d503d76b484c9a16550a9de72a583b2
Author: Amanda Moran <am...@apache.com>
Date:   2015-09-09T17:40:14Z

    [[Trafodion 1133]] [[Trafodion 1088]] [[Trafodion 1481]]
    
    Trafodion 1133 and Trafodion 1088 allow installer to add a node.
    
    Trafodion 1481 seperates rpm installs into its own script so that it can be
    run during an upgrade (to make sure all new packages needed are added). Bug still needs to be fixed is that it runs too many times (is ran once per node) and doesn't need to be. Will create a new jira for that small bug. Would like to check this in to prevent issue when upgrading.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39086354
  
    --- Diff: install/installer/traf_package_setup ---
    @@ -0,0 +1,224 @@
    +#!/bin/bash
    +
    +# @@@ START COPYRIGHT @@@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you under the Apache License, Version 2.0 (the
    +# "License"); you may not use this file except in compliance
    +# with the License.  You may obtain a copy of the License at
    +#
    +#   http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing,
    +# software distributed under the License is distributed on an
    +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +# KIND, either express or implied.  See the License for the
    +# specific language governing permissions and limitations
    +# under the License.
    +#
    +# @@@ END COPYRIGHT @@@
    +
    +#  Setup Trafodion environment on a Cluster System
    +
    +#==============================================
    +
    +timestamp=$(date +%F-%H-%M-%S)
    +source /etc/trafodion/trafodion_config
    +
    +cd $LOCAL_WORKDIR
    +
    +echo "***Log File: traf_package_setup script***" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +echo "Username: $TRAF_USER" >> $INSTALL_LOG
    +echo "Nodes: $NODE_LIST" >> $INSTALL_LOG
    +echo "Home directory: $HOME_DIR" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +
    +echo "***INFO: Starting Trafodion Package Setup ($timestamp)"
    +#========================================
    +#Setting up pdsh variables
    +if [ $node_count -eq 1 ]; then
    +    TRAF_PDSH=""
    +    TRAF_PDCP=""
    +else
    +    # use the -S option to cause pdsh to return largest of
    +    # the remote command return values so we can tell if one
    +    # or more of the remote commands failed
    +    TRAF_PDSH="pdsh -R exec $MY_NODES ssh -q -n %h"
    +    TRAF_PDCP="pdcp -R ssh $MY_NODES"
    +fi
    +#==========================================
    +echo "***INFO: Installing required packages"
    +echo "***INFO: Log file located in /var/log/trafodion"
    +YUM_LOG=/var/log/trafodion/trafodion_yum_$timestamp.log
    +ZYPP_LOG=/var/log/trafodion/trafodion_zypp_$timestamp.log
    +
    +#===============================
    +#Set internet access
    +internetAccess="true"
    +
    +#================================
    +#Checking to see if epel package is installed. 
    +if [[ $SUSE_LINUX == "false" ]]; then
    +   for node in $NODE_LIST;
    +   do
    +     EPEL_INSTALLED=$(ssh -q -n $node rpm -qa | grep epel | wc -l)
    +     if [[ $EPEL_INSTALLED == 0 ]]; then
    +        break;
    +     fi
    +   done
    +
    +   if [[ $EPEL_INSTALLED == 0 ]]; then
    +      echo "***INFO: ... EPEL rpm"
    +      if [[ "$EPEL_RPM" != "" ]]; then
    +         if [ $node_count -ne 1 ]; then
    +            for node in $NODE_LIST
    +            do
    +               scp -q $EPEL_RPM $(whoami)@$node:$HOME
    +            done
    +         else
    +            cp -rf $EPEL_RPM $HOME
    +         fi
    +      else
    +         if [[ $internetAccess == "true" ]]; then
    +            epel_rpm="epel-release-6-8.noarch.rpm"
    +            wget http://download.fedoraproject.org/pub/epel/6/x86_64/$epel_rpm
    +            if [ $node_count -ne 1 ]; then
    +               for node in $NODE_LIST
    +               do
    +                  scp -q $LOCAL_WORKDIR/$epel_rpm $(whoami)@$node:$HOME
    +               done
    +            else
    +               cp -rf $epel_rpm $HOME
    +            fi
    +
    +            if [ $? != 0 ]; then
    --- End diff --
    
    What happens if scp fails on the first node but succeeds on the remaining ones? What would the value of $? be in that case?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by amandamoran <gi...@git.apache.org>.
Github user amandamoran commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39092192
  
    --- Diff: install/installer/trafodion_config_default ---
    @@ -125,6 +125,9 @@ export PASSWORD="admin"
     # hadoop cluster name
     export CLUSTER_NAME=""
     
    +#Trafodion Cluster name, normally the same as Cloudera cluster name
    +export CLUSTERNAME=""
    --- End diff --
    
    CLUSTERNAME is used through out Trafodion and it can not be changed. It was deleted awhile back, since it was believed to not being used and had some bad consequences. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39084442
  
    --- Diff: install/installer/traf_config_setup ---
    @@ -550,10 +562,10 @@ else
        exit -1
     fi
     
    -hadoopVersion=$(curl -su $username:$password http://$URL/api/v1/clusters | grep version | grep -c CDH)
    +hadoopVersion=$(curl -su admin:admin http://$URL/api/v1/clusters | grep version | grep -c CDH)
    --- End diff --
    
    Did you mean to hard-code admin:admin here (and below)? Seems there was a recent change to move away from this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by amandamoran <gi...@git.apache.org>.
Github user amandamoran commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39092241
  
    --- Diff: install/installer/traf_package_setup ---
    @@ -0,0 +1,224 @@
    +#!/bin/bash
    +
    +# @@@ START COPYRIGHT @@@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you under the Apache License, Version 2.0 (the
    +# "License"); you may not use this file except in compliance
    +# with the License.  You may obtain a copy of the License at
    +#
    +#   http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing,
    +# software distributed under the License is distributed on an
    +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +# KIND, either express or implied.  See the License for the
    +# specific language governing permissions and limitations
    +# under the License.
    +#
    +# @@@ END COPYRIGHT @@@
    +
    +#  Setup Trafodion environment on a Cluster System
    +
    +#==============================================
    +
    +timestamp=$(date +%F-%H-%M-%S)
    +source /etc/trafodion/trafodion_config
    +
    +cd $LOCAL_WORKDIR
    +
    +echo "***Log File: traf_package_setup script***" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +echo "Username: $TRAF_USER" >> $INSTALL_LOG
    +echo "Nodes: $NODE_LIST" >> $INSTALL_LOG
    +echo "Home directory: $HOME_DIR" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +
    +echo "***INFO: Starting Trafodion Package Setup ($timestamp)"
    +#========================================
    +#Setting up pdsh variables
    +if [ $node_count -eq 1 ]; then
    +    TRAF_PDSH=""
    +    TRAF_PDCP=""
    +else
    +    # use the -S option to cause pdsh to return largest of
    +    # the remote command return values so we can tell if one
    +    # or more of the remote commands failed
    +    TRAF_PDSH="pdsh -R exec $MY_NODES ssh -q -n %h"
    --- End diff --
    
    Comment is incorrect, I will delete it and check in. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39085350
  
    --- Diff: install/installer/traf_package_setup ---
    @@ -0,0 +1,224 @@
    +#!/bin/bash
    +
    +# @@@ START COPYRIGHT @@@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you under the Apache License, Version 2.0 (the
    +# "License"); you may not use this file except in compliance
    +# with the License.  You may obtain a copy of the License at
    +#
    +#   http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing,
    +# software distributed under the License is distributed on an
    +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +# KIND, either express or implied.  See the License for the
    +# specific language governing permissions and limitations
    +# under the License.
    +#
    +# @@@ END COPYRIGHT @@@
    +
    +#  Setup Trafodion environment on a Cluster System
    +
    +#==============================================
    +
    +timestamp=$(date +%F-%H-%M-%S)
    +source /etc/trafodion/trafodion_config
    +
    +cd $LOCAL_WORKDIR
    +
    +echo "***Log File: traf_package_setup script***" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +echo "Username: $TRAF_USER" >> $INSTALL_LOG
    +echo "Nodes: $NODE_LIST" >> $INSTALL_LOG
    +echo "Home directory: $HOME_DIR" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +
    +echo "***INFO: Starting Trafodion Package Setup ($timestamp)"
    +#========================================
    +#Setting up pdsh variables
    +if [ $node_count -eq 1 ]; then
    +    TRAF_PDSH=""
    +    TRAF_PDCP=""
    +else
    +    # use the -S option to cause pdsh to return largest of
    +    # the remote command return values so we can tell if one
    +    # or more of the remote commands failed
    +    TRAF_PDSH="pdsh -R exec $MY_NODES ssh -q -n %h"
    --- End diff --
    
    The comment says, "use the -S option" but the commands don't seem to use it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by amandamoran <gi...@git.apache.org>.
Github user amandamoran commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39092287
  
    --- Diff: install/installer/traf_package_setup ---
    @@ -0,0 +1,224 @@
    +#!/bin/bash
    +
    +# @@@ START COPYRIGHT @@@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you under the Apache License, Version 2.0 (the
    +# "License"); you may not use this file except in compliance
    +# with the License.  You may obtain a copy of the License at
    +#
    +#   http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing,
    +# software distributed under the License is distributed on an
    +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +# KIND, either express or implied.  See the License for the
    +# specific language governing permissions and limitations
    +# under the License.
    +#
    +# @@@ END COPYRIGHT @@@
    +
    +#  Setup Trafodion environment on a Cluster System
    +
    +#==============================================
    +
    +timestamp=$(date +%F-%H-%M-%S)
    +source /etc/trafodion/trafodion_config
    +
    +cd $LOCAL_WORKDIR
    +
    +echo "***Log File: traf_package_setup script***" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +echo "Username: $TRAF_USER" >> $INSTALL_LOG
    +echo "Nodes: $NODE_LIST" >> $INSTALL_LOG
    +echo "Home directory: $HOME_DIR" >> $INSTALL_LOG
    +echo >> $INSTALL_LOG
    +
    +echo "***INFO: Starting Trafodion Package Setup ($timestamp)"
    +#========================================
    +#Setting up pdsh variables
    +if [ $node_count -eq 1 ]; then
    +    TRAF_PDSH=""
    +    TRAF_PDCP=""
    +else
    +    # use the -S option to cause pdsh to return largest of
    +    # the remote command return values so we can tell if one
    +    # or more of the remote commands failed
    +    TRAF_PDSH="pdsh -R exec $MY_NODES ssh -q -n %h"
    +    TRAF_PDCP="pdcp -R ssh $MY_NODES"
    +fi
    +#==========================================
    +echo "***INFO: Installing required packages"
    +echo "***INFO: Log file located in /var/log/trafodion"
    +YUM_LOG=/var/log/trafodion/trafodion_yum_$timestamp.log
    +ZYPP_LOG=/var/log/trafodion/trafodion_zypp_$timestamp.log
    +
    +#===============================
    +#Set internet access
    +internetAccess="true"
    +
    +#================================
    +#Checking to see if epel package is installed. 
    +if [[ $SUSE_LINUX == "false" ]]; then
    +   for node in $NODE_LIST;
    +   do
    +     EPEL_INSTALLED=$(ssh -q -n $node rpm -qa | grep epel | wc -l)
    +     if [[ $EPEL_INSTALLED == 0 ]]; then
    +        break;
    +     fi
    +   done
    +
    +   if [[ $EPEL_INSTALLED == 0 ]]; then
    +      echo "***INFO: ... EPEL rpm"
    +      if [[ "$EPEL_RPM" != "" ]]; then
    +         if [ $node_count -ne 1 ]; then
    +            for node in $NODE_LIST
    +            do
    +               scp -q $EPEL_RPM $(whoami)@$node:$HOME
    +            done
    +         else
    +            cp -rf $EPEL_RPM $HOME
    +         fi
    +      else
    +         if [[ $internetAccess == "true" ]]; then
    +            epel_rpm="epel-release-6-8.noarch.rpm"
    +            wget http://download.fedoraproject.org/pub/epel/6/x86_64/$epel_rpm
    +            if [ $node_count -ne 1 ]; then
    +               for node in $NODE_LIST
    +               do
    +                  scp -q $LOCAL_WORKDIR/$epel_rpm $(whoami)@$node:$HOME
    +               done
    +            else
    +               cp -rf $epel_rpm $HOME
    +            fi
    +
    +            if [ $? != 0 ]; then
    --- End diff --
    
    Ah! Good thinking! Let me put in error checking! Thanks! 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/76


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by amandamoran <gi...@git.apache.org>.
Github user amandamoran commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39092032
  
    --- Diff: install/installer/traf_config_setup ---
    @@ -550,10 +562,10 @@ else
        exit -1
     fi
     
    -hadoopVersion=$(curl -su $username:$password http://$URL/api/v1/clusters | grep version | grep -c CDH)
    +hadoopVersion=$(curl -su admin:admin http://$URL/api/v1/clusters | grep version | grep -c CDH)
    --- End diff --
    
    Sorry, I guess the merge didn't happened as I thought it would. Let me fix and check in. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [[Trafodion 1133]] [[Trafodion 1...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/76#discussion_r39084928
  
    --- Diff: install/installer/trafodion_config_default ---
    @@ -125,6 +125,9 @@ export PASSWORD="admin"
     # hadoop cluster name
     export CLUSTER_NAME=""
     
    +#Trafodion Cluster name, normally the same as Cloudera cluster name
    +export CLUSTERNAME=""
    --- End diff --
    
    Glad I saw this comment otherwise I'd have no clue about the difference between CLUSTER_NAME and CLUSTERNAME. Maybe these should be named differently? (Perhaps TRAFODION_CLUSTER_NAME for one of them?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---