Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.2.0
    • Component/s: tests
    • Labels:

      Description

      The next step of the QFS integration into BigTop is to add smoke tests for QFS.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fsareshwala closed the pull request at:

          https://github.com/apache/bigtop/pull/86

          Show
          githubbot ASF GitHub Bot added a comment - Github user fsareshwala closed the pull request at: https://github.com/apache/bigtop/pull/86
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fsareshwala commented on the issue:

          https://github.com/apache/bigtop/pull/86

          This change was committed to the bigtop repository already. Closing out the pull request.

          Show
          githubbot ASF GitHub Bot added a comment - Github user fsareshwala commented on the issue: https://github.com/apache/bigtop/pull/86 This change was committed to the bigtop repository already. Closing out the pull request.
          Hide
          kstinson Kevin Stinson added a comment -

          Sorry. Commented on the wrong ticket.

          Show
          kstinson Kevin Stinson added a comment - Sorry. Commented on the wrong ticket.
          Hide
          kstinson Kevin Stinson added a comment -

          Please reassign this one to me. I work on QFS and am looking at added the tests.

          Show
          kstinson Kevin Stinson added a comment - Please reassign this one to me. I work on QFS and am looking at added the tests.
          Hide
          cos Konstantin Boudnik added a comment -

          Pushed to the master, thanks Faraaz Sareshwala

          Show
          cos Konstantin Boudnik added a comment - Pushed to the master, thanks Faraaz Sareshwala
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fsareshwala commented on a diff in the pull request:

          https://github.com/apache/bigtop/pull/86#discussion_r61629748

          — Diff: bigtop-tests/smoke-tests/qfs/build.gradle —
          @@ -0,0 +1,40 @@
          +/**
          + * 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
          + * <p/>
          + * http://www.apache.org/licenses/LICENSE-2.0
          + * <p/>
          + * 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.
          + */
          +def tests_to_include()

          { + return [ + "TestHadoopExamples.groovy" + ]; +}

          +
          +sourceSets {
          + test {
          + groovy {
          + srcDirs = [
          + "$

          {BIGTOP_HOME}

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce"
          + ]
          + exclude

          { + FileTreeElement elem -> (doExclude(elem.getName())) + }

          + }
          + }
          +}
          +
          +test.doFirst {
          + environment("HADOOP_COMMAND", "hadoop-qfs")
          — End diff –

          Sure thing @c0s. I still think it's valuable to check for the `SMOKE_TESTS` environment variable inside the qfs `build.gradle`. I've left that in. I think if you are ready for the merge, this is ready to go. Let me know if there's anything else you want me to change.

          Show
          githubbot ASF GitHub Bot added a comment - Github user fsareshwala commented on a diff in the pull request: https://github.com/apache/bigtop/pull/86#discussion_r61629748 — Diff: bigtop-tests/smoke-tests/qfs/build.gradle — @@ -0,0 +1,40 @@ +/** + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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. + */ +def tests_to_include() { + return [ + "TestHadoopExamples.groovy" + ]; +} + +sourceSets { + test { + groovy { + srcDirs = [ + "$ {BIGTOP_HOME} /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce" + ] + exclude { + FileTreeElement elem -> (doExclude(elem.getName())) + } + } + } +} + +test.doFirst { + environment("HADOOP_COMMAND", "hadoop-qfs") — End diff – Sure thing @c0s. I still think it's valuable to check for the `SMOKE_TESTS` environment variable inside the qfs `build.gradle`. I've left that in. I think if you are ready for the merge, this is ready to go. Let me know if there's anything else you want me to change.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on a diff in the pull request:

          https://github.com/apache/bigtop/pull/86#discussion_r61629098

          — Diff: bigtop-tests/smoke-tests/qfs/build.gradle —
          @@ -0,0 +1,40 @@
          +/**
          + * 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
          + * <p/>
          + * http://www.apache.org/licenses/LICENSE-2.0
          + * <p/>
          + * 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.
          + */
          +def tests_to_include()

          { + return [ + "TestHadoopExamples.groovy" + ]; +}

          +
          +sourceSets {
          + test {
          + groovy {
          + srcDirs = [
          + "$

          {BIGTOP_HOME}

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce"
          + ]
          + exclude

          { + FileTreeElement elem -> (doExclude(elem.getName())) + }

          + }
          + }
          +}
          +
          +test.doFirst {
          + environment("HADOOP_COMMAND", "hadoop-qfs")
          — End diff –

          I guess you right. You know - let's scratch my initial suggestion and leave it as it was. After all other components don't do any checks like that and so far we haven't blown anything Thanks!

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/86#discussion_r61629098 — Diff: bigtop-tests/smoke-tests/qfs/build.gradle — @@ -0,0 +1,40 @@ +/** + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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. + */ +def tests_to_include() { + return [ + "TestHadoopExamples.groovy" + ]; +} + +sourceSets { + test { + groovy { + srcDirs = [ + "$ {BIGTOP_HOME} /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce" + ] + exclude { + FileTreeElement elem -> (doExclude(elem.getName())) + } + } + } +} + +test.doFirst { + environment("HADOOP_COMMAND", "hadoop-qfs") — End diff – I guess you right. You know - let's scratch my initial suggestion and leave it as it was. After all other components don't do any checks like that and so far we haven't blown anything Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fsareshwala commented on a diff in the pull request:

          https://github.com/apache/bigtop/pull/86#discussion_r61158128

          — Diff: bigtop-tests/smoke-tests/qfs/build.gradle —
          @@ -0,0 +1,40 @@
          +/**
          + * 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
          + * <p/>
          + * http://www.apache.org/licenses/LICENSE-2.0
          + * <p/>
          + * 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.
          + */
          +def tests_to_include()

          { + return [ + "TestHadoopExamples.groovy" + ]; +}

          +
          +sourceSets {
          + test {
          + groovy {
          + srcDirs = [
          + "$

          {BIGTOP_HOME}

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce"
          + ]
          + exclude

          { + FileTreeElement elem -> (doExclude(elem.getName())) + }

          + }
          + }
          +}
          +
          +test.doFirst {
          + environment("HADOOP_COMMAND", "hadoop-qfs")
          — End diff –

          I worry that checking for a binary existing might hide other problems with installing the binary itself. That's why I used the environment variable. One option would be to pass in the list of components to the `smoke-tests.sh` script from `docker-hadoop.sh` and I can search through that instead. What do you think about that? If that's not a good solution, can you give me a suggestion on how I might go about doing what you are asking for?

          Show
          githubbot ASF GitHub Bot added a comment - Github user fsareshwala commented on a diff in the pull request: https://github.com/apache/bigtop/pull/86#discussion_r61158128 — Diff: bigtop-tests/smoke-tests/qfs/build.gradle — @@ -0,0 +1,40 @@ +/** + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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. + */ +def tests_to_include() { + return [ + "TestHadoopExamples.groovy" + ]; +} + +sourceSets { + test { + groovy { + srcDirs = [ + "$ {BIGTOP_HOME} /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce" + ] + exclude { + FileTreeElement elem -> (doExclude(elem.getName())) + } + } + } +} + +test.doFirst { + environment("HADOOP_COMMAND", "hadoop-qfs") — End diff – I worry that checking for a binary existing might hide other problems with installing the binary itself. That's why I used the environment variable. One option would be to pass in the list of components to the `smoke-tests.sh` script from `docker-hadoop.sh` and I can search through that instead. What do you think about that? If that's not a good solution, can you give me a suggestion on how I might go about doing what you are asking for?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user c0s commented on a diff in the pull request:

          https://github.com/apache/bigtop/pull/86#discussion_r61020136

          — Diff: bigtop-tests/smoke-tests/qfs/build.gradle —
          @@ -0,0 +1,40 @@
          +/**
          + * 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
          + * <p/>
          + * http://www.apache.org/licenses/LICENSE-2.0
          + * <p/>
          + * 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.
          + */
          +def tests_to_include()

          { + return [ + "TestHadoopExamples.groovy" + ]; +}

          +
          +sourceSets {
          + test {
          + groovy {
          + srcDirs = [
          + "$

          {BIGTOP_HOME}

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce"
          + ]
          + exclude

          { + FileTreeElement elem -> (doExclude(elem.getName())) + }

          + }
          + }
          +}
          +
          +test.doFirst {
          + environment("HADOOP_COMMAND", "hadoop-qfs")
          — End diff –

          This isn't what I meant. Someone can set the SMOKE_TESTS variable to qfs as he is intent to run such tests. However, the cluster might not be configured with qfs, i.e. the packages aren't installed, etc. This will lead to a false negative. The reason I am being overly cautious is because in case of HDFS it is always there, so there's no need to specifically check for its presence. QFS being an optional alternative won't be implicitly required by other downstream components like YARN, hence it configuration might be omitted. See where I am going with this?

          Show
          githubbot ASF GitHub Bot added a comment - Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/86#discussion_r61020136 — Diff: bigtop-tests/smoke-tests/qfs/build.gradle — @@ -0,0 +1,40 @@ +/** + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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. + */ +def tests_to_include() { + return [ + "TestHadoopExamples.groovy" + ]; +} + +sourceSets { + test { + groovy { + srcDirs = [ + "$ {BIGTOP_HOME} /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce" + ] + exclude { + FileTreeElement elem -> (doExclude(elem.getName())) + } + } + } +} + +test.doFirst { + environment("HADOOP_COMMAND", "hadoop-qfs") — End diff – This isn't what I meant. Someone can set the SMOKE_TESTS variable to qfs as he is intent to run such tests. However, the cluster might not be configured with qfs, i.e. the packages aren't installed, etc. This will lead to a false negative. The reason I am being overly cautious is because in case of HDFS it is always there, so there's no need to specifically check for its presence. QFS being an optional alternative won't be implicitly required by other downstream components like YARN, hence it configuration might be omitted. See where I am going with this?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fsareshwala commented on a diff in the pull request:

          https://github.com/apache/bigtop/pull/86#discussion_r61018978

          — Diff: bigtop-tests/smoke-tests/qfs/build.gradle —
          @@ -0,0 +1,40 @@
          +/**
          + * 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
          + * <p/>
          + * http://www.apache.org/licenses/LICENSE-2.0
          + * <p/>
          + * 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.
          + */
          +def tests_to_include()

          { + return [ + "TestHadoopExamples.groovy" + ]; +}

          +
          +sourceSets {
          + test {
          + groovy {
          + srcDirs = [
          + "$

          {BIGTOP_HOME}

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce"
          + ]
          + exclude

          { + FileTreeElement elem -> (doExclude(elem.getName())) + }

          + }
          + }
          +}
          +
          +test.doFirst {
          + environment("HADOOP_COMMAND", "hadoop-qfs")
          — End diff –

          That's a good suggestion, @c0s. I've updated the code to check the environment variable `SMOKE_TESTS` before running the qfs smoke tests.

          Show
          githubbot ASF GitHub Bot added a comment - Github user fsareshwala commented on a diff in the pull request: https://github.com/apache/bigtop/pull/86#discussion_r61018978 — Diff: bigtop-tests/smoke-tests/qfs/build.gradle — @@ -0,0 +1,40 @@ +/** + * 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 + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * 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. + */ +def tests_to_include() { + return [ + "TestHadoopExamples.groovy" + ]; +} + +sourceSets { + test { + groovy { + srcDirs = [ + "$ {BIGTOP_HOME} /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/mapreduce" + ] + exclude { + FileTreeElement elem -> (doExclude(elem.getName())) + } + } + } +} + +test.doFirst { + environment("HADOOP_COMMAND", "hadoop-qfs") — End diff – That's a good suggestion, @c0s. I've updated the code to check the environment variable `SMOKE_TESTS` before running the qfs smoke tests.
          Hide
          fsareshwala Faraaz Sareshwala added a comment -

          Hi guys,

          Just wanted to follow up on this ticket. Any chance this can get reviewed and merged? Is there anything else you'd like me to focus on regarding this code?

          Show
          fsareshwala Faraaz Sareshwala added a comment - Hi guys, Just wanted to follow up on this ticket. Any chance this can get reviewed and merged? Is there anything else you'd like me to focus on regarding this code?
          Hide
          cos Konstantin Boudnik added a comment -

          You only need to explicitly specify helper packages like bigtop-utils. This is the list of components for puppet. Basically, Puppet will try to find the module with that name and install it, which will clearly fail. I suppose, removing bigtop-utils from your example should do the trick.

          Show
          cos Konstantin Boudnik added a comment - You only need to explicitly specify helper packages like bigtop-utils. This is the list of components for puppet. Basically, Puppet will try to find the module with that name and install it, which will clearly fail. I suppose, removing bigtop-utils from your example should do the trick.
          Hide
          oflebbe Olaf Flebbe added a comment -

          there seems to be issues with our smoke test infrastructure. Since I am not too familiar with it, I like to iron it with your tests as an example.

          Right now I am using this CI script:

          # setup configuration file
          cat > bigtop-deploy/vm/vagrant-puppet-docker/vagrantconfig_Bigtop-Docker-provisioner-debian-8.yaml <<-__EOT__
          docker:
                  memory_size: "4096"
                  image:  "bigtop/deploy:debian-8"
          boot2docker:
                  memory_size: "4096"
                  number_cpus: "2"
          repo: "https://ci.bigtop.apache.org/job/Bigtop-trunk/BUILD_ENVIRONMENTS=debian-8%2clabel=docker-slave/lastSuccessfulBuild/artifact/output/apt/"
          distro: debian
          components: [hadoop, qfs, bigtop-utils]
          namenode_ui_port: "56110"
          yarn_ui_port: "8211"
          hbase_ui_port: "61110"
          enable_local_repo: false
          smoke_test_components: [qfs, hadoop]
          jdk: "openjdk-7-jdk"
          __EOT__
          
          # destroy previous cluster
          ./gradlew docker-provisioner-destroy
          # provision
          ./gradlew -Pconfig=vagrantconfig_Bigtop-Docker-provisioner-debian-8.yaml -Pnum_instances=3 -Prun_smoke_tests=true  docker-provisioner
          # destroy provisinoed cluster
          ./gradlew docker-provisioner-destroy
          

          with code from https://github.com/oflebbe/bigtop.git branch BIGTOP-2366

          Right now it fails with a silly error message that bigtop-utils is not installed ?!?

          Show
          oflebbe Olaf Flebbe added a comment - there seems to be issues with our smoke test infrastructure. Since I am not too familiar with it, I like to iron it with your tests as an example. Right now I am using this CI script: # setup configuration file cat > bigtop-deploy/vm/vagrant-puppet-docker/vagrantconfig_Bigtop-Docker-provisioner-debian-8.yaml <<-__EOT__ docker: memory_size: "4096" image: "bigtop/deploy:debian-8" boot2docker: memory_size: "4096" number_cpus: "2" repo: "https: //ci.bigtop.apache.org/job/Bigtop-trunk/BUILD_ENVIRONMENTS=debian-8%2clabel=docker-slave/lastSuccessfulBuild/artifact/output/apt/" distro: debian components: [hadoop, qfs, bigtop-utils] namenode_ui_port: "56110" yarn_ui_port: "8211" hbase_ui_port: "61110" enable_local_repo: false smoke_test_components: [qfs, hadoop] jdk: "openjdk-7-jdk" __EOT__ # destroy previous cluster ./gradlew docker-provisioner-destroy # provision ./gradlew -Pconfig=vagrantconfig_Bigtop-Docker-provisioner-debian-8.yaml -Pnum_instances=3 -Prun_smoke_tests= true docker-provisioner # destroy provisinoed cluster ./gradlew docker-provisioner-destroy with code from https://github.com/oflebbe/bigtop.git branch BIGTOP-2366 Right now it fails with a silly error message that bigtop-utils is not installed ?!?
          Hide
          fsareshwala Faraaz Sareshwala added a comment -

          Does anyone else have any comments regarding this code? Can we merge it?

          Show
          fsareshwala Faraaz Sareshwala added a comment - Does anyone else have any comments regarding this code? Can we merge it?
          Hide
          fsareshwala Faraaz Sareshwala added a comment -

          Thanks Olaf Flebbe. I've put a patch in for BIGTOP-2367.

          Show
          fsareshwala Faraaz Sareshwala added a comment - Thanks Olaf Flebbe . I've put a patch in for BIGTOP-2367 .
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Errr, I meant: The puppet recipies worked and triggered bugs in packaging: -> new ticket

          Show
          oflebbe Olaf Flebbe added a comment - - edited Errr, I meant: The puppet recipies worked and triggered bugs in packaging: -> new ticket
          Hide
          fsareshwala Faraaz Sareshwala added a comment -

          Thanks for reviewing the code Cos! I've made the requested changes and updated the pull request. Please take another look when you have a moment.

          Show
          fsareshwala Faraaz Sareshwala added a comment - Thanks for reviewing the code Cos! I've made the requested changes and updated the pull request. Please take another look when you have a moment.
          Hide
          cos Konstantin Boudnik added a comment -

          A few comments:

          • you don't need to write an essay for a git commit. There's a JIRA ticket, where you can explain anything that has to be explain. So, please limit the commit message to simply "JIRA ###. JIRA title"
          • set HADOOP_COMMAND to default hadoop, so you don't need to string it all the way from mapreduce smoke. Which is orthogonal to your change. In general, concerns like this should be separated and isolated, which leads to a way cleaner code.
          • same in the TestHadoopExemples: instead of the assert, just initialize that HADOOP_COMMAND constant with default value, to guarantee preserve the original behavior.
          • in Groovy you can avoid String concatenation like {{sh.exec(HADOOP_COMMAND + " fs -put $ {source}/. .");}} Instead of you can simply {{sh.exec("${HADOOP_COMMAND} fs -put ${source}

            /. .");}}

          Thanks!

          Show
          cos Konstantin Boudnik added a comment - A few comments: you don't need to write an essay for a git commit. There's a JIRA ticket, where you can explain anything that has to be explain. So, please limit the commit message to simply "JIRA ###. JIRA title" set HADOOP_COMMAND to default hadoop , so you don't need to string it all the way from mapreduce smoke. Which is orthogonal to your change. In general, concerns like this should be separated and isolated, which leads to a way cleaner code. same in the TestHadoopExemples: instead of the assert, just initialize that HADOOP_COMMAND constant with default value, to guarantee preserve the original behavior. in Groovy you can avoid String concatenation like {{sh.exec(HADOOP_COMMAND + " fs -put $ {source}/ . .");}} Instead of you can simply {{sh.exec("${HADOOP_COMMAND} fs -put ${source} / . .");}} Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user fsareshwala opened a pull request:

          https://github.com/apache/bigtop/pull/86

          BIGTOP-2317. Add smoke tests for QFS

          This change adds some simple testing for qfs running under hadoop. The bigtop
          repository already has smoke tests for the hadoop mapreduce framework. It simply
          runs the examples through the hadoop infrastructure and ensures that the output
          is valid and expected files within the filesystem are present. This change
          modifies the bigtop code to support running the same examples but using qfs as
          the backend filesystem. This is done through dynamically supplying the hadoop
          command to use. In the hadoop hdfs mapreduce scenario, we use the standard
          hadoop command that uses hdfs as the backend filesystem. In the hadoop qfs
          mapreduce scenario, we use the hadoop-qfs command that uses qfs as the backend
          filesystem. The qfs tests simply reference the mapreduce tests and tell it to
          use the hadoop-qfs command instead of the hadoop command.

          I have tested this works by running the qfs smoke tests using the BigTop
          provisioner under docker. The build and tests pass without issue.

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

          $ git pull https://github.com/fsareshwala/bigtop qfs-smoke-tests

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

          https://github.com/apache/bigtop/pull/86.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 #86



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user fsareshwala opened a pull request: https://github.com/apache/bigtop/pull/86 BIGTOP-2317 . Add smoke tests for QFS This change adds some simple testing for qfs running under hadoop. The bigtop repository already has smoke tests for the hadoop mapreduce framework. It simply runs the examples through the hadoop infrastructure and ensures that the output is valid and expected files within the filesystem are present. This change modifies the bigtop code to support running the same examples but using qfs as the backend filesystem. This is done through dynamically supplying the hadoop command to use. In the hadoop hdfs mapreduce scenario, we use the standard hadoop command that uses hdfs as the backend filesystem. In the hadoop qfs mapreduce scenario, we use the hadoop-qfs command that uses qfs as the backend filesystem. The qfs tests simply reference the mapreduce tests and tell it to use the hadoop-qfs command instead of the hadoop command. I have tested this works by running the qfs smoke tests using the BigTop provisioner under docker. The build and tests pass without issue. You can merge this pull request into a Git repository by running: $ git pull https://github.com/fsareshwala/bigtop qfs-smoke-tests Alternatively you can review and apply these changes as the patch at: https://github.com/apache/bigtop/pull/86.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 #86

            People

            • Assignee:
              fsareshwala Faraaz Sareshwala
              Reporter:
              fsareshwala Faraaz Sareshwala
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development