Details

    • Type: Task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.1
    • Component/s: deployment
    • Labels:
      None

      Description

      Charms for bigtop-1.2 have a minimum requirement of Xenial and Juju 2.0. The following source changes are needed to support this:

      READMEs/Metadata:

      • remove references to Juju 1.x
      • include component version numbers
      • include issue section to help people find jira

      Actions/Benchmarks:

      • consistent action output
      • use the new(er) charms.benchmark python library

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user evans-ye commented on the issue:

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

          Thanks for the patch and review @kwmonroe @johnsca.

          Show
          githubbot ASF GitHub Bot added a comment - Github user evans-ye commented on the issue: https://github.com/apache/bigtop/pull/197 Thanks for the patch and review @kwmonroe @johnsca.
          Hide
          kwmonroe Kevin W Monroe added a comment -

          All 11 charms and 5 bundles touched by this patch tested well across aws/gce/azure/lxd.

          Show
          kwmonroe Kevin W Monroe added a comment - All 11 charms and 5 bundles touched by this patch tested well across aws/gce/azure/lxd.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/bigtop/pull/197
          Hide
          johnsca Cory Johns added a comment -

          The GitHub bot didn't bring over my final review approval, but given the clarifications, this PR has my +1

          Show
          johnsca Cory Johns added a comment - The GitHub bot didn't bring over my final review approval, but given the clarifications, this PR has my +1
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112996574

          — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test —
          @@ -43,6 +43,6 @@ smoke_env = {
          bigtop = Bigtop()
          result = bigtop.run_smoke_tests(smoke_components, smoke_env)
          if result == 'success':

          • hookenv.action_set( {'outcome': result}

            )
            + hookenv.action_set(

            {'outcome': 'success'}

            )
            else:

          • fail('{} smoke tests failed'.format(smoke_components), result)
            + fail('{} smoke tests failed with: {}'.format(smoke_components, result))
              • End diff –

          Fair enough. +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112996574 — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test — @@ -43,6 +43,6 @@ smoke_env = { bigtop = Bigtop() result = bigtop.run_smoke_tests(smoke_components, smoke_env) if result == 'success': hookenv.action_set( {'outcome': result} ) + hookenv.action_set( {'outcome': 'success'} ) else: fail('{} smoke tests failed'.format(smoke_components), result) + fail('{} smoke tests failed with: {}'.format(smoke_components, result)) End diff – Fair enough. +1
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112996525

          — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml —
          @@ -29,21 +29,21 @@ services:

          • "2"
          • "3"
            plugin:
          • charm: "cs:xenial/hadoop-plugin-13"
            + charm: "cs:xenial/hadoop-plugin-14"
            annotations:
            gui-x: "1000"
            gui-y: "400"
            client:
            charm: "cs:xenial/hadoop-client-3"
          • constraints: "mem=3G"
            + constraints: "mem=7G root-disk=32G"
              • End diff –

          Ah, that makes sense. Thanks for the info!

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112996525 — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml — @@ -29,21 +29,21 @@ services: "2" "3" plugin: charm: "cs:xenial/hadoop-plugin-13" + charm: "cs:xenial/hadoop-plugin-14" annotations: gui-x: "1000" gui-y: "400" client: charm: "cs:xenial/hadoop-client-3" constraints: "mem=3G" + constraints: "mem=7G root-disk=32G" End diff – Ah, that makes sense. Thanks for the info!
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112990603

          — Diff: bigtop-packages/src/charm/spark/layer-spark/actions/restart-spark-job-history-server —
          @@ -15,22 +15,22 @@

          1. limitations under the License.
            import sys

          +from charmhelpers.core import hookenv, host
          +from charms.reactive import is_state

          -try:

          • from charmhelpers.core import host, hookenv, unitdata
          • from jujubigdata import utils
          • charm_ready = True
            -except ImportError:
          • charm_ready = False

          -if not charm_ready:

          • from subprocess import call
          • call(['action-fail', 'Spark service not yet ready'])
          • sys.exit(1)
              • End diff –

          Thanks for the history. I originally assumed this came in before `is_state` was a thing.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112990603 — Diff: bigtop-packages/src/charm/spark/layer-spark/actions/restart-spark-job-history-server — @@ -15,22 +15,22 @@ limitations under the License. import sys +from charmhelpers.core import hookenv, host +from charms.reactive import is_state -try: from charmhelpers.core import host, hookenv, unitdata from jujubigdata import utils charm_ready = True -except ImportError: charm_ready = False -if not charm_ready: from subprocess import call call( ['action-fail', 'Spark service not yet ready'] ) sys.exit(1) End diff – Thanks for the history. I originally assumed this came in before `is_state` was a thing.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112990094

          — Diff: bigtop-packages/src/charm/pig/layer-pig/actions/smoke-test —
          @@ -63,8 +62,8 @@ else:

          1. The smoke tests analyze /etc/passwd. Successful runs will have an 'ubuntu'
          2. passwd entry in the output.
            +hookenv.action_set( {'raw': output}

            )

              • End diff –

          @johnsca, proper benchmarks (using `charms.benchmark`) set benchmark-related bits like start/stop/composite/raw under the `meta` key. Non-benchmark actions may still have helpful `raw` data, but I chose to display that as `results.raw` instead of creating a new `meta` key that only included `results.meta.raw`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112990094 — Diff: bigtop-packages/src/charm/pig/layer-pig/actions/smoke-test — @@ -63,8 +62,8 @@ else: The smoke tests analyze /etc/passwd. Successful runs will have an 'ubuntu' passwd entry in the output. +hookenv.action_set( {'raw': output} ) End diff – @johnsca, proper benchmarks (using `charms.benchmark`) set benchmark-related bits like start/stop/composite/raw under the `meta` key. Non-benchmark actions may still have helpful `raw` data, but I chose to display that as `results.raw` instead of creating a new `meta` key that only included `results.meta.raw`.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112989226

          — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test —
          @@ -43,6 +43,6 @@ smoke_env = {
          bigtop = Bigtop()
          result = bigtop.run_smoke_tests(smoke_components, smoke_env)
          if result == 'success':

          • hookenv.action_set( {'outcome': result}

            )
            + hookenv.action_set(

            {'outcome': 'success'}

            )
            else:

          • fail('{} smoke tests failed'.format(smoke_components), result)
            + fail('{} smoke tests failed with: {}'.format(smoke_components, result))
              • End diff –

          @johnsca, I merged the output with the message to be consistent with other bigtop charms. When an action fails, users expect to see the reason in the `message` key. Without this change, they would see the abbreviated failure message and need to know to look elsewhere for an `output` key.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112989226 — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test — @@ -43,6 +43,6 @@ smoke_env = { bigtop = Bigtop() result = bigtop.run_smoke_tests(smoke_components, smoke_env) if result == 'success': hookenv.action_set( {'outcome': result} ) + hookenv.action_set( {'outcome': 'success'} ) else: fail('{} smoke tests failed'.format(smoke_components), result) + fail('{} smoke tests failed with: {}'.format(smoke_components, result)) End diff – @johnsca, I merged the output with the message to be consistent with other bigtop charms. When an action fails, users expect to see the reason in the `message` key. Without this change, they would see the abbreviated failure message and need to know to look elsewhere for an `output` key.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112987956

          — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml —
          @@ -29,21 +29,21 @@ services:

          • "2"
          • "3"
            plugin:
          • charm: "cs:xenial/hadoop-plugin-13"
            + charm: "cs:xenial/hadoop-plugin-14"
            annotations:
            gui-x: "1000"
            gui-y: "400"
            client:
            charm: "cs:xenial/hadoop-client-3"
          • constraints: "mem=3G"
            + constraints: "mem=7G root-disk=32G"
              • End diff –

          @johnsca, this is correct - in this rev of the bundle, spark is colocated with client. I did this because in yarn-client mode, spark doesn't need a whole new unit by itself. The driver process can tax the unit a bit more, so I bumped ram to 7g. This also benefits users because they can run `spark-submit` and `hadoop jar` on the same unit. Previously, they'd have to run spark jobs on the spark unit and hadoop jobs on the client unit.

          Yes, app-level constraints are required in addition to machine constraints. Without them, the initial deployment would use the machine constraints, but a subsequent `add-unit` would fall back to the provider default. Ideally, we could remove the machine constraints and solely rely on app-constraints. We'll do that once https://bugs.launchpad.net/juju/+bug/1676986 is fixed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user kwmonroe commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112987956 — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml — @@ -29,21 +29,21 @@ services: "2" "3" plugin: charm: "cs:xenial/hadoop-plugin-13" + charm: "cs:xenial/hadoop-plugin-14" annotations: gui-x: "1000" gui-y: "400" client: charm: "cs:xenial/hadoop-client-3" constraints: "mem=3G" + constraints: "mem=7G root-disk=32G" End diff – @johnsca, this is correct - in this rev of the bundle, spark is colocated with client. I did this because in yarn-client mode, spark doesn't need a whole new unit by itself. The driver process can tax the unit a bit more, so I bumped ram to 7g. This also benefits users because they can run `spark-submit` and `hadoop jar` on the same unit. Previously, they'd have to run spark jobs on the spark unit and hadoop jobs on the client unit. Yes, app-level constraints are required in addition to machine constraints. Without them, the initial deployment would use the machine constraints, but a subsequent `add-unit` would fall back to the provider default. Ideally, we could remove the machine constraints and solely rely on app-constraints. We'll do that once https://bugs.launchpad.net/juju/+bug/1676986 is fixed.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112970809

          — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml —
          @@ -29,21 +29,21 @@ services:

          • "2"
          • "3"
            plugin:
          • charm: "cs:xenial/hadoop-plugin-13"
            + charm: "cs:xenial/hadoop-plugin-14"
            annotations:
            gui-x: "1000"
            gui-y: "400"
            client:
            charm: "cs:xenial/hadoop-client-3"
          • constraints: "mem=3G"
            + constraints: "mem=7G root-disk=32G"
              • End diff –

          Is this right? Does the client / gateway node really need such large constraints?

          Actually, do we need any application-level constraints if we're already specifying them in the machines section?

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112970809 — Diff: bigtop-deploy/juju/hadoop-spark/bundle.yaml — @@ -29,21 +29,21 @@ services: "2" "3" plugin: charm: "cs:xenial/hadoop-plugin-13" + charm: "cs:xenial/hadoop-plugin-14" annotations: gui-x: "1000" gui-y: "400" client: charm: "cs:xenial/hadoop-client-3" constraints: "mem=3G" + constraints: "mem=7G root-disk=32G" End diff – Is this right? Does the client / gateway node really need such large constraints? Actually, do we need any application-level constraints if we're already specifying them in the machines section?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112973155

          — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test —
          @@ -43,6 +43,6 @@ smoke_env = {
          bigtop = Bigtop()
          result = bigtop.run_smoke_tests(smoke_components, smoke_env)
          if result == 'success':

          • hookenv.action_set( {'outcome': result}

            )
            + hookenv.action_set(

            {'outcome': 'success'}

            )
            else:

          • fail('{} smoke tests failed'.format(smoke_components), result)
            + fail('{} smoke tests failed with: {}'.format(smoke_components, result))
              • End diff –

          Why combining the output and message here? Seems like it would make the output harder to handle separately. Is it just because the message is more important for CI?

          I do like the consistent `outcome` field.

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112973155 — Diff: bigtop-packages/src/charm/hadoop/layer-hadoop-resourcemanager/actions/smoke-test — @@ -43,6 +43,6 @@ smoke_env = { bigtop = Bigtop() result = bigtop.run_smoke_tests(smoke_components, smoke_env) if result == 'success': hookenv.action_set( {'outcome': result} ) + hookenv.action_set( {'outcome': 'success'} ) else: fail('{} smoke tests failed'.format(smoke_components), result) + fail('{} smoke tests failed with: {}'.format(smoke_components, result)) End diff – Why combining the output and message here? Seems like it would make the output harder to handle separately. Is it just because the message is more important for CI? I do like the consistent `outcome` field.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112975062

          — Diff: bigtop-packages/src/charm/spark/layer-spark/actions/restart-spark-job-history-server —
          @@ -15,22 +15,22 @@

          1. limitations under the License.
            import sys

          +from charmhelpers.core import hookenv, host
          +from charms.reactive import is_state

          -try:

          • from charmhelpers.core import host, hookenv, unitdata
          • from jujubigdata import utils
          • charm_ready = True
            -except ImportError:
          • charm_ready = False

          -if not charm_ready:

          • from subprocess import call
          • call(['action-fail', 'Spark service not yet ready'])
          • sys.exit(1)
              • End diff –

          This was originally intended to handle the case where the action somehow ran before the bootstrap code, but I don't think that's actually possible, so +1 to this change.

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112975062 — Diff: bigtop-packages/src/charm/spark/layer-spark/actions/restart-spark-job-history-server — @@ -15,22 +15,22 @@ limitations under the License. import sys +from charmhelpers.core import hookenv, host +from charms.reactive import is_state -try: from charmhelpers.core import host, hookenv, unitdata from jujubigdata import utils charm_ready = True -except ImportError: charm_ready = False -if not charm_ready: from subprocess import call call( ['action-fail', 'Spark service not yet ready'] ) sys.exit(1) End diff – This was originally intended to handle the case where the action somehow ran before the bootstrap code, but I don't think that's actually possible, so +1 to this change.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/bigtop/pull/197#discussion_r112974240

          — Diff: bigtop-packages/src/charm/pig/layer-pig/actions/smoke-test —
          @@ -63,8 +62,8 @@ else:

          1. The smoke tests analyze /etc/passwd. Successful runs will have an 'ubuntu'
          2. passwd entry in the output.
            +hookenv.action_set( {'raw': output}

            )

              • End diff –

          Why do some spots use `meta.raw` and others just use `raw`?

          Show
          githubbot ASF GitHub Bot added a comment - Github user johnsca commented on a diff in the pull request: https://github.com/apache/bigtop/pull/197#discussion_r112974240 — Diff: bigtop-packages/src/charm/pig/layer-pig/actions/smoke-test — @@ -63,8 +62,8 @@ else: The smoke tests analyze /etc/passwd. Successful runs will have an 'ubuntu' passwd entry in the output. +hookenv.action_set( {'raw': output} ) End diff – Why do some spots use `meta.raw` and others just use `raw`?
          Hide
          kwmonroe Kevin W Monroe added a comment -

          Juju CI will run over the weekend to test the linked PR.

          Show
          kwmonroe Kevin W Monroe added a comment - Juju CI will run over the weekend to test the linked PR.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user kwmonroe opened a pull request:

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

          BIGTOP-2747: new charm revs for bigtop-1.2

          Bring charm/bundle source up to date with bigtop-1.2. This is mostly a README/metadata refactor with sprinkles of a more consistent UX for charm actions.

          There is one functional difference that comes with the move to support bigtop-1.2. The `hadoop-spark` bundle previously deployed 3 units of zookeeper, but with spark in yarn-client mode, this makes no sense. If users want Spark HA with Zookeeper, they should use the `spark-processing` bundle. Therefore, while rev'ing `hadoop-spark` for this PR, I've removed zookeeper references from the bundle.

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

          $ git pull https://github.com/juju-solutions/bigtop bug/BIGTOP-2747/charms-for-1.2

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

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


          commit 44a3968c3c6e07acc625de5b69e2ecef167022f2
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-19T17:31:52Z

          add version info; remove juju < 2 notes; add section for reporting issues; update reference links

          commit e221bb70bbc76b558895086a760a256b2baa10ee
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-19T18:53:25Z

          more guidance in issue section

          commit 6a3ea3a2bee3bf06948948b4e5572626a069d31d
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-19T19:13:46Z

          sparkbench does not yet work with spark 2.1, so remove refs from the benchmarking section of the readme

          commit eaa628982b08fd32eaf22c28c9ae0ed51e291bd6
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-20T23:25:45Z

          consistent UX for actions

          commit a7192cd506b124c711028bfee2aaaa8bfc1518ee
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-21T15:35:42Z

          update charm instructions for 2.0

          commit 0775fbe3d1495e35c6b098e9e0d93b67bc48e5f5
          Author: Kevin W Monroe <kevin.monroe@canonical.com>
          Date: 2017-04-21T21:14:46Z

          update charm revs; update readmes to remove juju-1 syntax and include app versions and issue tracking section


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user kwmonroe opened a pull request: https://github.com/apache/bigtop/pull/197 BIGTOP-2747 : new charm revs for bigtop-1.2 Bring charm/bundle source up to date with bigtop-1.2. This is mostly a README/metadata refactor with sprinkles of a more consistent UX for charm actions. There is one functional difference that comes with the move to support bigtop-1.2. The `hadoop-spark` bundle previously deployed 3 units of zookeeper, but with spark in yarn-client mode, this makes no sense. If users want Spark HA with Zookeeper, they should use the `spark-processing` bundle. Therefore, while rev'ing `hadoop-spark` for this PR, I've removed zookeeper references from the bundle. You can merge this pull request into a Git repository by running: $ git pull https://github.com/juju-solutions/bigtop bug/ BIGTOP-2747 /charms-for-1.2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/bigtop/pull/197.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 #197 commit 44a3968c3c6e07acc625de5b69e2ecef167022f2 Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-19T17:31:52Z add version info; remove juju < 2 notes; add section for reporting issues; update reference links commit e221bb70bbc76b558895086a760a256b2baa10ee Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-19T18:53:25Z more guidance in issue section commit 6a3ea3a2bee3bf06948948b4e5572626a069d31d Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-19T19:13:46Z sparkbench does not yet work with spark 2.1, so remove refs from the benchmarking section of the readme commit eaa628982b08fd32eaf22c28c9ae0ed51e291bd6 Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-20T23:25:45Z consistent UX for actions commit a7192cd506b124c711028bfee2aaaa8bfc1518ee Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-21T15:35:42Z update charm instructions for 2.0 commit 0775fbe3d1495e35c6b098e9e0d93b67bc48e5f5 Author: Kevin W Monroe <kevin.monroe@canonical.com> Date: 2017-04-21T21:14:46Z update charm revs; update readmes to remove juju-1 syntax and include app versions and issue tracking section

            People

            • Assignee:
              kwmonroe Kevin W Monroe
              Reporter:
              kwmonroe Kevin W Monroe
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development