Bigtop
  1. Bigtop
  2. BIGTOP-596

move service configuration from groovy code to package xml manifest file

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.4.0
    • Fix Version/s: 0.4.0
    • Component/s: Tests
    • Labels:
      None

      Description

      the current package test doesn't format namenode, so that hadoop-hdfs-namenode daemon cannot be started

      1. BIGTOP_596-4.txt
        7 kB
        Johnny Zhang
      2. BIGTOP_596-3.txt
        6 kB
        Johnny Zhang
      3. BIGTOP_596-2.txt
        4 kB
        Johnny Zhang
      4. BIGTOP_596.txt
        2 kB
        Johnny Zhang

        Activity

        Hide
        Roman Shaposhnik added a comment -

        Perfect!

        +1 and committed – thanks so much for your patience!

        Show
        Roman Shaposhnik added a comment - Perfect! +1 and committed – thanks so much for your patience!
        Hide
        Johnny Zhang added a comment -

        the config() function in StateVerifierHBase and StateVerifierHDFS is used to edit hbase-site.xml and hdfs-site.xml in hard code way, which is now taken care by the new function configService, which copy <config> block from package_data.xml to corresponding config file. deleted them in new patch BIGTOP_596-4.txt

        Show
        Johnny Zhang added a comment - the config() function in StateVerifierHBase and StateVerifierHDFS is used to edit hbase-site.xml and hdfs-site.xml in hard code way, which is now taken care by the new function configService, which copy <config> block from package_data.xml to corresponding config file. deleted them in new patch BIGTOP_596-4.txt
        Hide
        Roman Shaposhnik added a comment -

        I really do apologize for not catching this earlier, but when I was about to apply the patch I've noticed that you comment out blocks of code. Please let us know why and if the code is indeed not needed – just remove it.

        Show
        Roman Shaposhnik added a comment - I really do apologize for not catching this earlier, but when I was about to apply the patch I've noticed that you comment out blocks of code. Please let us know why and if the code is indeed not needed – just remove it.
        Hide
        Johnny Zhang added a comment -

        change a better title

        Show
        Johnny Zhang added a comment - change a better title
        Hide
        Johnny Zhang added a comment - - edited

        please review BIGTOP_596-3.txt, it includes parts:
        1. move the service configurations from groovy code to manifest xml
        2. copy configurations inside of <config> block into the config file
        3. add a new init() function in Service class, and if <init> is not empty, do 'service $svc_name init' to init service

        I verified it in centos5_64 VM

        Show
        Johnny Zhang added a comment - - edited please review BIGTOP_596-3.txt, it includes parts: 1. move the service configurations from groovy code to manifest xml 2. copy configurations inside of <config> block into the config file 3. add a new init() function in Service class, and if <init> is not empty, do 'service $svc_name init' to init service I verified it in centos5_64 VM
        Hide
        Johnny Zhang added a comment -

        the current code parse the <service> block into a map of key-value pairs, so store the config file name in attribute will be lost. I change the schema a little bit into

        <config>
          <configfile>/etc/hbase/conf/hbase-site.xml</configfile>
          <property>
            <name>hbase.rootdir</name>
            <value>hdfs://localhost/hbase</value>
          </property>
        </config>
        

        bases on this, I give new patch BIGTOP_596-3.txt. I tested it on centos5_64, it copy the corresponding config to hbase-site.xml

        seems we don't need to worry about hadoop-hdfs-namenode config (dfs.safemode.min.datanodes and dfs.safemode.extension), it has been taken cared by packaging.

        Show
        Johnny Zhang added a comment - the current code parse the <service> block into a map of key-value pairs, so store the config file name in attribute will be lost. I change the schema a little bit into <config> <configfile>/etc/hbase/conf/hbase-site.xml</configfile> <property> <name>hbase.rootdir</name> <value>hdfs://localhost/hbase</value> </property> </config> bases on this, I give new patch BIGTOP_596-3.txt. I tested it on centos5_64, it copy the corresponding config to hbase-site.xml seems we don't need to worry about hadoop-hdfs-namenode config (dfs.safemode.min.datanodes and dfs.safemode.extension), it has been taken cared by packaging.
        Hide
        Roman Shaposhnik added a comment -

        Johnny, please put this issue into Patch Available state once you have resolved all the issues and the new patch is ready

        Show
        Roman Shaposhnik added a comment - Johnny, please put this issue into Patch Available state once you have resolved all the issues and the new patch is ready
        Hide
        Johnny Zhang added a comment -

        I did the test for BIGTOP-596-2.txt, it executes configServices(Node rootNode) before checking each packages, which is a little bit confusing to me. Still looking at it.

        Show
        Johnny Zhang added a comment - I did the test for BIGTOP-596 -2.txt, it executes configServices(Node rootNode) before checking each packages, which is a little bit confusing to me. Still looking at it.
        Hide
        Johnny Zhang added a comment - - edited

        @Roman, how about BIGTOP_596-2.txt. Three parts:
        1. added the <config> section into the package_data.xml for 'hadoop-hdfs-namenode'
        2. there is a new function called configService(Node rootNode), which parse package_data.xml and apply <config> node to all the configuration files.
        3.the config() function in StateVerifierHDFS is deleted. If this is OK, will delete config() function for all the other StateVerifier

        Show
        Johnny Zhang added a comment - - edited @Roman, how about BIGTOP_596-2.txt. Three parts: 1. added the <config> section into the package_data.xml for 'hadoop-hdfs-namenode' 2. there is a new function called configService(Node rootNode), which parse package_data.xml and apply <config> node to all the configuration files. 3.the config() function in StateVerifierHDFS is deleted. If this is OK, will delete config() function for all the other StateVerifier
        Hide
        Johnny Zhang added a comment -

        agree! working on a patch for this change

        Show
        Johnny Zhang added a comment - agree! working on a patch for this change
        Hide
        Roman Shaposhnik added a comment -

        I guess my only concern is that configured then feels redundant. Basically does the presence/absence of the configuration block serve the same purpose as the configured property? IOW configured==true IFF configuration property is defined. Or am I missing something?

        Finally, as a small nitpick, I'd recommend the following schema for the file property inside of a configuration block:

        <config type="xml-file" name="/etc/hadoop/conf/hdfs-site.xml">
           <property>
              <name>dfs.safemode.min.datanodes</name>
              <value>1</value>
           </property>
        
           <property>
              <name>dfs.safemode.extension</name>
              <value>0</value>
           </property>
        </config>
        

        Makes sense?

        Show
        Roman Shaposhnik added a comment - I guess my only concern is that configured then feels redundant. Basically does the presence/absence of the configuration block serve the same purpose as the configured property? IOW configured==true IFF configuration property is defined. Or am I missing something? Finally, as a small nitpick, I'd recommend the following schema for the file property inside of a configuration block: <config type="xml-file" name="/etc/hadoop/conf/hdfs-site.xml"> <property> <name>dfs.safemode.min.datanodes</name> <value>1</value> </property> <property> <name>dfs.safemode.extension</name> <value>0</value> </property> </config> Makes sense?
        Hide
        Johnny Zhang added a comment -

        <configured> means whether the service need additional configuration to start, and <configcontent> section will be inserted into file <configname> to complete this. <init> means whether the service need 'init' command before start the service.

        if <configured> is false, then <configuration> section is empty

        here is an example for 'hadoop-hdfs-namenode', which can take place the config() function in StateVerifierHDFS.groovy

        <services>
          <init>true</init>
          <configured>true</configured>
          <configuration>
            <file>
              <configname>/etc/hadoop/conf/hdfs-site.xml</configname>
              <configcontent>
                <property>dfs.safemode.min.datanodes</property>
                <value>1</value>
                <property>dfs.safemode.extension</property>
                <value>0</value>
              </configcontent>
            </file>
          </configuration>
        </services>
        
        Show
        Johnny Zhang added a comment - <configured> means whether the service need additional configuration to start, and <configcontent> section will be inserted into file <configname> to complete this. <init> means whether the service need 'init' command before start the service. if <configured> is false, then <configuration> section is empty here is an example for 'hadoop-hdfs-namenode', which can take place the config() function in StateVerifierHDFS.groovy <services> <init>true</init> <configured>true</configured> <configuration> <file> <configname>/etc/hadoop/conf/hdfs-site.xml</configname> <configcontent> <property>dfs.safemode.min.datanodes</property> <value>1</value> <property>dfs.safemode.extension</property> <value>0</value> </configcontent> </file> </configuration> </services>
        Hide
        Roman Shaposhnik added a comment -

        Looks good! Two questions though:

        1. what's the semantic of configured?
        2. do you plan to have something like <init> to tell the framework to call init on the service?
        Show
        Roman Shaposhnik added a comment - Looks good! Two questions though: what's the semantic of configured? do you plan to have something like <init> to tell the framework to call init on the service?
        Hide
        Johnny Zhang added a comment - - edited

        Rorman, agree to put config in manifest xml instead of groovy code, which separates the logic and data.

        how about something like

        <services>
          <configured>true</configured>
          <configuration>
            <file>
              <configname>/etc/hadoop/conf/hdfs-site.xml</configname>
              <configcontent>
                <property>pro1</property>
                <value>val1</value>
              </configcontent>
            </file>
          <configuration>
        </services>
        
        Show
        Johnny Zhang added a comment - - edited Rorman, agree to put config in manifest xml instead of groovy code, which separates the logic and data. how about something like <services> <configured>true</configured> <configuration> <file> <configname>/etc/hadoop/conf/hdfs-site.xml</configname> <configcontent> <property>pro1</property> <value>val1</value> </configcontent> </file> <configuration> </services>
        Hide
        Roman Shaposhnik added a comment -

        Johnny, thinking about it some more I've come to a realization that the easiest way to accomplish this would be to modify the schema for our manifest xml and have a property there that would tell whether the service needs to be initialized before it can be used. We can also add any needed configs properties in there as well.

        This will be much better than adding this knowledge directly to our Groovy code.

        Thoughts?

        Show
        Roman Shaposhnik added a comment - Johnny, thinking about it some more I've come to a realization that the easiest way to accomplish this would be to modify the schema for our manifest xml and have a property there that would tell whether the service needs to be initialized before it can be used. We can also add any needed configs properties in there as well. This will be much better than adding this knowledge directly to our Groovy code. Thoughts?
        Hide
        Roman Shaposhnik added a comment -

        Thanks for the patch! A couple of comments:

        1. I really would like us to hook this to StateVerifier class functionality instead of introducing a switch statement
        2. for HDFS you should do exactly what is done for zookeeper: service hadoop-hdfs-namenode init
        3. why are you adding a fully distributed config to the hbase master? in the context of package tests this is unlikely to work
        Show
        Roman Shaposhnik added a comment - Thanks for the patch! A couple of comments: I really would like us to hook this to StateVerifier class functionality instead of introducing a switch statement for HDFS you should do exactly what is done for zookeeper: service hadoop-hdfs-namenode init why are you adding a fully distributed config to the hbase master? in the context of package tests this is unlikely to work
        Hide
        Johnny Zhang added a comment -

        this patch does:

        • formatnamenode
        • init zookeeper
        • config hbase-site.xml for hbase-regionserver to start

        if the way of this patch doing is acceptable, will resolve other services initialization in another jira

        Show
        Johnny Zhang added a comment - this patch does: formatnamenode init zookeeper config hbase-site.xml for hbase-regionserver to start if the way of this patch doing is acceptable, will resolve other services initialization in another jira
        Hide
        Roman Shaposhnik added a comment -

        Note: same applies to Zookeeper and Oozie (they can't be started unless inited).

        Now, initing is half of the battle – you also have to create configs.

        I'd suggest taking a look at upgrade tests and how they create minimalist configs/state. Then it would be useful to have those steps available for service testing in package scripts.

        Show
        Roman Shaposhnik added a comment - Note: same applies to Zookeeper and Oozie (they can't be started unless inited). Now, initing is half of the battle – you also have to create configs. I'd suggest taking a look at upgrade tests and how they create minimalist configs/state. Then it would be useful to have those steps available for service testing in package scripts.

          People

          • Assignee:
            Johnny Zhang
            Reporter:
            Johnny Zhang
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development