Cleanup Python and Ansible Code (#340)

* Format Python code using Black (https://github.com/psf/black)
* Normalize Python string literals to use double quotes
* Use specific Python imports where possible
* Address number of issues flagged by ansible-lint
* Refactor wipe.yml to avoid rm -rf commands
* Run flake8 and ansible-lint as part of Travis CI
* Add a script to detect files without EOF newline
* Call the EOF newline checker from Travis CI build script
* Add a CONTRIBUTING.md file with instructions for developers
* Move info about nose tests into the CONTRIBUTING.md file
* Link to the CONTRIBUTING.md file from README.md
* Fix minor typos in Ansible task names

Co-Authored-By: Christopher Tubbs <ctubbsii@apache.org>

.gitignore

@@ -1,3 +1,4 @@
 *.pyc
 /.idea/
 /pom.xml
+.vscode/

.travis.yml

@@ -15,4 +15,8 @@
 language: python
 python:
   - "3.5"
-script: nosetests -w lib/
+install:
+  - pip install flake8
+  - pip install ansible-lint
+script:
+  - ./scripts/cibuild

CONTRIBUTING.md

@@ -0,0 +1,49 @@
+<!--
+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.
+-->
+
+# Contributors Guide
+
+If you believe that you have found a bug, please search for an existing [issue](https://github.com/apache/fluo-muchos/issues) to see if it has already been reported. For simple changes, its ok to just submit a pull request without an issue.
+
+## Muchos Testing
+
+Muchos has unit tests.  To run them, first install nose using pip:
+```
+    pip install nose
+```
+The following command runs the unit tests:
+```
+    nosetests -w lib/
+```
+
+## Before you submit a PR
+
+If you are modifying any of the Python code in this project, please use [Black](https://github.com/psf/black) to enforce that Python code found under the [lib](https://github.com/apache/fluo-muchos/tree/master/lib) folder is formatted correctly. Before submitting a PR, please ensure that you have used Black to format the code with max line length set to 79 as below (it is to be run from the repo root):
+```
+black lib --line-length 79
+```
+
+The [CI](https://github.com/apache/fluo-muchos/tree/master/.travis.yml) for this project runs tools to detect common coding issues with Python and Ansible files. Rather than wait for the CI to flag any issues with your work, please run the [cibuild](https://github.com/apache/fluo-muchos/tree/master/scripts/cibuild.sh) script on your dev machine, which in turn runs the following tools:
+- [flake8](https://github.com/pycqa/flake8) to validate that the Python code in the project conforms to known good practices.
+- [Ansible-lint](https://github.com/ansible/ansible-lint/) prior to submitting a PR. This will ensure that you align with known good practices. Please also review the guidance on [false positives](https://docs.ansible.com/ansible-lint/rules/rules.html#false-positives-skipping-rules) from Ansible-lint.
+
+Please ensure that you address issues flagged by the above CI build script before creating a PR.
+
+## Review
+
+- We welcome reviews from anyone. Any committer can approve and merge the changes.
+- Reviewers will likely have questions and comments. They may use terms such as those in [RFC2119](https://tools.ietf.org/html/rfc2119).

README.md

@@ -297,15 +297,15 @@ managed in their own git repository (see [mikewalch/muchos-custom][mc] for an ex
 ## High-Availability (optional)
 
 Additionally, Muchos can be configured to provide High-Availability for HDFS & Accumulo components. By default,
-this feature is off, however it can be turned on by editing the following settings in [muchos.props] 
+this feature is off, however it can be turned on by editing the following settings in [muchos.props]
 under the `general` section as shown below:
 
 ```ini
 hdfs_ha = True                        # default is False
 nameservice_id = muchoshacluster      # Logical name for the cluster, no special characters
 ```
 
-Before enabling HA, it is strongly recommended you read the Apache doc for [HDFS HA] & [Accumulo HA] 
+Before enabling HA, it is strongly recommended you read the Apache doc for [HDFS HA] & [Accumulo HA]
 
 Also in the `[nodes]` section of [muchos.props] ensure the `journalnode` and `zkfc` service are configured to run.
 
@@ -340,6 +340,10 @@ $ ./bin/muchos config -p leader.public.ip
 10.10.10.10
 ```
 
+## Contributions
+
+We welcome contributions to the project. [These notes](./CONTRIBUTING.md) should be helpful.
+
 ## Powered by
 
 Muchos is powered by the following projects:
@@ -350,16 +354,6 @@ Muchos is powered by the following projects:
  * [azure-cli] - The Azure CLI is a command-line tool for managing Azure resources.
  * [ansible-azure] - Ansible includes a suite of modules for interacting with Azure Resource Manager.
 
-## Muchos Testing
-
-Muchos has unit tests.  To run them, first install nose using pip:
-
-    pip install nose
-
-The following command runs the unit tests:
-
-    nosetests -w lib/
-
 [centos7]: https://aws.amazon.com/marketplace/pp/B00O7WM7QW
 [aws-config]: http://docs.aws.amazon.com/cli/latest/userguide/cli-config-files.html
 [awscli]: https://docs.aws.amazon.com/cli/latest/userguide/installing.html

ansible/accumulo.yml

@@ -43,14 +43,14 @@
       command: "{{ accumulo_home }}/bin/start-here.sh"
       register: start_result
       changed_when: "'Starting' in start_result.stdout"
-      when: accumulo_major_version == '1' and use_systemd == False
+      when: accumulo_major_version == '1' and not use_systemd
 - hosts: workers
   tasks:
     - name: "start accumulo 2.0 tablet servers"
       command: "nohup {{ accumulo_home }}/bin/accumulo-service tserver start"
       register: start_result
       changed_when: "'Starting' in start_result.stdout"
-      when: accumulo_major_version == '2' and use_systemd == False
+      when: accumulo_major_version == '2' and not use_systemd
 - hosts: accumulomaster
   tasks:
     - name: "start accumulo 2.0 master, monitor, gc & tracer"
@@ -62,7 +62,7 @@
         - monitor
         - gc
         - tracer
-      when: accumulo_major_version == '2' and use_systemd == False
+      when: accumulo_major_version == '2' and not use_systemd
 - hosts: accumulomaster
   tasks:
     - name: "install and start all the accumulo services using systemd"
@@ -71,12 +71,12 @@
         - import_tasks: roles/accumulo/tasks/start-gc.yml
         - import_tasks: roles/accumulo/tasks/start-monitor.yml
         - import_tasks: roles/accumulo/tasks/start-tracer.yml
-      when: use_systemd == True
+      when: use_systemd
       become: yes
-      
+
 - hosts: workers
   gather_facts: false
   tasks:
     - import_tasks: roles/accumulo/tasks/start-tserver.yml
-      when: use_systemd == True
+      when: use_systemd
       become: yes

ansible/roles/azure/tasks/terminate_cluster.yml → ansible/azure_terminate.yml

@@ -16,11 +16,10 @@
 #
 
 ---
-
 - hosts: localhost
   tasks:
     - block:
-      - import_tasks: log_analytics_ws_common.yml
+      - import_tasks: roles/azure/tasks/log_analytics_ws_common.yml
 
       - name: Delete workbook
         azure_rm_resource:
@@ -56,7 +55,7 @@
          resource_group: "{{ resource_group }}"
          provider: ManagedIdentity
          resource_type: userAssignedIdentities
-         resource_name:  "{{ user_assigned_identity if user_assigned_identity !='' else vmss_name + '-ua-msi' }}"
+         resource_name:  "{{ user_assigned_identity if user_assigned_identity else vmss_name + '-ua-msi' }}"
          api_version: '2018-11-30'
          state: absent
       when: use_adlsg2

ansible/roles/azure/tasks/wipe_adlsg2.yml → ansible/azure_wipe.yml

@@ -36,7 +36,7 @@
     retries: 20
     delay: 30
     register: result
-    until: result is succeeded and (result.changed == False or (result.changed == True and result.container|length > 0))
+    until: result is succeeded and (not result.changed or (result.changed and result.container|length > 0))
     loop:
       "{{ instance_volumes_preferred.split(',')  }}"
     when: cluster_type == 'azure' and use_adlsg2

ansible/cancel_shutdown.yml

@@ -19,4 +19,4 @@
   become: yes
   tasks:
     - name: "cancelling node shutdown"
-      shell: shutdown -c
+      command: shutdown -c

ansible/docker.yml

@@ -24,15 +24,17 @@
   tasks:
     - name: get swarm status
       shell: >
-        docker info | egrep '^Swarm: ' | cut -d ' ' -f 2
+        set -o pipefail && docker info | egrep '^Swarm: ' | cut -d ' ' -f 2
+      args:
+        executable: bash
       register: swarm_status
       changed_when: "'active' not in swarm_status.stdout_lines"
     - name: initialize swarm
       shell: >
         docker swarm init --advertise-addr={{ ansible_default_ipv4.address }}:2377
       when: "'active' not in swarm_status.stdout_lines"
     - name: get swarm token
-      shell: docker swarm join-token -q worker
+      command: docker swarm join-token -q worker
       register: swarm_token
       changed_when: "'active' not in swarm_status.stdout_lines"
 - hosts: workers:!swarmmanager
@@ -43,7 +45,9 @@
   tasks:
     - name: get swarm status
       shell: >
-        docker info | egrep '^Swarm: ' | cut -d ' ' -f 2
+        set -o pipefail && docker info | egrep '^Swarm: ' | cut -d ' ' -f 2
+      args:
+        executable: bash
       register: swarm_status
       changed_when: "'active' not in swarm_status.stdout_lines"
     - name: join worker to swarm

ansible/hadoop.yml

@@ -21,25 +21,25 @@
 - hosts: journalnode
   tasks:
     - import_tasks: roles/hadoop/tasks/start-journal.yml
-      when: hdfs_ha == True
+      when: hdfs_ha
 - hosts: namenode[0]
   tasks:
     - import_tasks: roles/hadoop/tasks/format-nn.yml
 - hosts: namenode[0]
   tasks:
     - import_tasks: roles/hadoop/tasks/format-zk.yml
-      when: hdfs_ha == True    
+      when: hdfs_ha
 - hosts: namenode
   tasks:
     - import_tasks: roles/hadoop/tasks/start-zkfc.yml
-      when: hdfs_ha == True
+      when: hdfs_ha
 - hosts: namenode[0]
   tasks:
     - import_tasks: roles/hadoop/tasks/start-nn1.yml
 - hosts: namenode:!namenode[0]
   tasks:
     - import_tasks: roles/hadoop/tasks/start-nn2.yml
-      when: hdfs_ha == True
+      when: hdfs_ha
 - hosts: namenode
   tasks:
     - import_tasks: roles/hadoop/tasks/start-hdfs.yml

ansible/roles/accumulo/files/accumulo-cluster-systemd

@@ -379,4 +379,4 @@ function main() {
   esac
 }
 
-main "$@"
+main "$@"

ansible/roles/accumulo/tasks/download.yml

@@ -26,4 +26,4 @@
   retries: 3
   with_items:
     - { urlp: "{{ apache_mirror.stdout }}/accumulo/{{ accumulo_version }}", fn: "{{ accumulo_tarball }}", sum: "{{ accumulo_checksum }}" }
-  when: accumulo.stat.exists == False
+  when: not accumulo.stat.exists

ansible/roles/accumulo/tasks/main.yml

@@ -56,20 +56,20 @@
 - name: "configure tservers using managed templates"
   template: src=tservers dest={{ accumulo_home }}/conf/{{ accumulo_tservers_fn[accumulo_major_version] }}
 - name: "build accumulo native libraries"
-  shell: "{{ accumulo_build_native_cmd[accumulo_major_version] }}"
+  command: "{{ accumulo_build_native_cmd[accumulo_major_version] }}"
   args:
     creates: "{{ accumulo_home }}/lib/native/libaccumulo.so"
 - name: "Create accumulo log dir"
   file: path={{ worker_data_dirs[0] }}/logs/accumulo state=directory
 - name: "Copy the modified accumulo-cluster script that supports systemd to bin"
   copy: src=roles/accumulo/files/accumulo-cluster-systemd dest={{ accumulo_home }}/bin/accumulo-cluster-systemd mode=0755
-  when: use_systemd == True
+  when: use_systemd
 - name: "Remove the exisiting accumulo-service and accumulo-cluster scripts"
   file: path={{ accumulo_home }}/bin/{{ item }} state=absent
   with_items:
     - accumulo-service
     - accumulo-cluster
-  when: use_systemd == True
+  when: use_systemd
 - name: "Create a symlink for accumulo-cluster-systemd"
   file: src={{ accumulo_home }}/bin/accumulo-cluster-systemd dest={{ accumulo_home }}/bin/accumulo-cluster mode=0755 state=link
-  when: use_systemd == True
+  when: use_systemd

ansible/roles/accumulo/tasks/start-gc.yml

@@ -17,13 +17,13 @@
 ##
 - name: install accumulo gc systemd unit file
   template:
-    src: roles/accumulo/templates/gc.j2 
+    src: roles/accumulo/templates/gc.j2
     dest: "/etc/systemd/system/accumulo-gc.service"
     mode: 0644
 
 - name: start accumulo-gc using systemd
-  systemd: 
-    state: started 
+  systemd:
+    state: started
     name: "accumulo-gc"
     daemon_reload: yes
     enabled: yes

ansible/roles/accumulo/tasks/start-master.yml

@@ -17,13 +17,13 @@
 ##
 - name: install accumulo master systemd unit file
   template:
-    src: roles/accumulo/templates/master.j2 
+    src: roles/accumulo/templates/master.j2
     dest: "/etc/systemd/system/accumulo-master.service"
     mode: 0644
 
 - name: start accumulo-master using systemd
-  systemd: 
-    state: started 
+  systemd:
+    state: started
     name: "accumulo-master"
     daemon_reload: yes
     enabled: yes

ansible/roles/accumulo/tasks/start-monitor.yml

@@ -17,13 +17,13 @@
 ##
 - name: install accumulo monitor systemd unit file
   template:
-    src: roles/accumulo/templates/monitor.j2 
+    src: roles/accumulo/templates/monitor.j2
     dest: "/etc/systemd/system/accumulo-monitor.service"
     mode: 0644
 
 - name: start accumulo-monitor using systemd
-  systemd: 
-    state: started 
+  systemd:
+    state: started
     name: "accumulo-monitor"
     daemon_reload: yes
     enabled: yes

ansible/roles/accumulo/tasks/start-tracer.yml

@@ -17,13 +17,13 @@
 ##
 - name: install accumulo tracer systemd unit file
   template:
-    src: roles/accumulo/templates/tracer.j2 
+    src: roles/accumulo/templates/tracer.j2
     dest: "/etc/systemd/system/accumulo-tracer.service"
     mode: 0644
 
 - name: start accumulo-tracer using systemd
-  systemd: 
-    state: started 
+  systemd:
+    state: started
     name: "accumulo-tracer"
     daemon_reload: yes
     enabled: yes

ansible/roles/accumulo/tasks/start-tserver.yml

@@ -17,13 +17,13 @@
 ##
 - name: install accumulo tserver systemd unit file
   template:
-    src: roles/accumulo/templates/tserver.j2 
+    src: roles/accumulo/templates/tserver.j2
     dest: "/etc/systemd/system/accumulo-tserver@.service"
     mode: 0644
 
 - name: start accumulo-tserver(s) using systemd
-  systemd: 
-    state: started 
+  systemd:
+    state: started
     name: "accumulo-tserver@{{ item }}.service"
     daemon_reload: yes
     enabled: yes

ansible/roles/accumulo/templates/gc.j2

@@ -1,6 +1,6 @@
 [Unit]
 Description=GC Service for Accumulo
-Requires=network.target 
+Requires=network.target
 After=network.target
 
 [Service]

ansible/roles/accumulo/templates/master.j2

@@ -1,6 +1,6 @@
 [Unit]
 Description=Master Service for Accumulo
-Requires=network.target 
+Requires=network.target
 After=network.target
 
 [Service]

ansible/roles/accumulo/templates/monitor.j2

@@ -1,6 +1,6 @@
 [Unit]
 Description=Monitor Service for Accumulo
-Requires=network.target 
+Requires=network.target
 After=network.target
 
 [Service]