Clean up comment style and identify bugs and workarounds

Cleans up the comment style to remove author names and clarify
the comment as it relates to the code. Using the NOTE (NAME):
format is redundant and takes away attention from the purpose
of documenting why an action is being taken.

Also updates status of TODO and FIXME items, including removing
code was a workaround fixed by a recent patch.

Change-Id: I2e087be1e204c618d1dbe499b3f69eae34ce656f

dockerfiles/debian/Dockerfile

@@ -19,6 +19,5 @@ RUN sed -i \
         -e "s|%%DEBIAN_SECURITY_DISTRIBUTION%%|${DEBIAN_SECURITY_DISTRIBUTION}|g" \
         -e "s|%%CEPH_URL%%|${CEPH_URL}|g" \
         /etc/apt/sources.list
-
-# NOTE(SamYaple): Remove this when infra starts signing thier mirrors
-RUN echo "APT::Get::AllowUnauthenticated \"${ALLOW_UNAUTHENTICATED}\";" > /etc/apt/apt.conf.d/allow-unathenticated
+RUN echo "APT::Get::AllowUnauthenticated \"${ALLOW_UNAUTHENTICATED}\";" \
+         > /etc/apt/apt.conf.d/allow-unathenticated

dockerfiles/ubuntu/Dockerfile

@@ -17,6 +17,5 @@ RUN sed -i \
         -e "s|%%CLOUD_ARCHIVE_URL%%|${CLOUD_ARCHIVE_URL}|g" \
         -e "s|%%CEPH_URL%%|${CEPH_URL}|g" \
         /etc/apt/sources.list
-
-# NOTE(SamYaple): Remove this when infra starts signing thier mirrors
-RUN echo "APT::Get::AllowUnauthenticated \"${ALLOW_UNAUTHENTICATED}\";" > /etc/apt/apt.conf.d/allow-unathenticated
+RUN echo "APT::Get::AllowUnauthenticated \"${ALLOW_UNAUTHENTICATED}\";" \
+         > /etc/apt/apt.conf.d/allow-unathenticated

playbooks/files/apache.conf

@@ -10,7 +10,7 @@ LoadModule authn_core_module /usr/lib/apache2/modules/mod_authn_core.so
 LoadModule authz_core_module /usr/lib/apache2/modules/mod_authz_core.so
 LoadModule cgi_module /usr/lib/apache2/modules/mod_cgi.so
 
-# NOTE(SamYaple): 172.17.0.1 is the network we use for Docker so it will be in
+# 172.17.0.1 is the address we use for Docker so it will be in
 # the same subnet as the internal addesses in the build containers
 Listen 172.17.0.1:80
 <VirtualHost 172.17.0.1:80>

playbooks/loci-builder.yaml

@@ -9,17 +9,6 @@
           - "pydep.txt"
       environment:
           LC_ALL: C
-    # NOTE(evrardjp): While reuse_requirements is very nice and optimises
-    # checks and gating, there is a race condition here, because
-    # we are consuming prebuild wheels (see vars.yaml) by default:
-    # The jobs in zuul can be building a new "requirements" image, working
-    # And the new "nova" image would still build on previous "requirements"
-    # image that was last published. This could cause an issue. Instead
-    # in gating we should build directly what will be consumed.
-    # NOTE(SamYaple): This process is so we can take advantage of the infra
-    # DockerHub mirroring as configured through the Docker daemon. We do this
-    # instead of calling fetch_wheels initially. All-in-all this saves
-    # bandwidth and time.
     - name: Gather wheels to local registry
       block:
         - docker_image:

playbooks/post.yaml

@@ -2,9 +2,11 @@
   tasks:
     - name: Collect logs
       block:
-        # NOTE(SamYaple): https://github.com/ansible/ansible/issues/14131
+        # FIXME: https://github.com/ansible/ansible/issues/14131
+        # This issue closed on October 11, 2018. Patch will be released
+        # with Ansible 2.8 release.
         - command: cp -r /home/zuul/.ansible_async /logs/async_logs
-        # FIXME(SamYaple): running this is causing the gate to hang
+        # FIXME: running this is causing the gate to hang
         #- command: journalctl -xb -u docker.service
         #  register: docker_daemon_log
         #  no_log: True

playbooks/setup-gate.yaml

@@ -51,7 +51,7 @@
             state: started
             published_ports:
               - 172.17.0.1:5000:5000
-        # NOTE(SamYaple): Allow all connections from containers to host so the
+        # Allow all connections from containers to host so the
         # containers can access the http server for git and wheels
         - iptables:
             action: insert

playbooks/vars.yaml

@@ -7,11 +7,15 @@ docker_daemon:
   insecure-registries:
     - 172.17.0.1:5000
 
+# Setting reuse_requirements to True will use the most recent
+# requirements build from the gate registry. This can save bandwidth
+# and time. However, it introduces a gate race condition if a change
+# is posted that updates requirments. We set to false to prefer
+# correctness to speed.
 reuse_requirements: False
 
-# NOTE(SamYaple): When running in the loci repo, the project is "loci", but
-# when running as a post job for cinder, the project is "cinder". We statically
-# declare the path rather than using zuul variables so thats not an issue
+# Override Zuul inferrence of source directory from project name to always
+# use "loci".
 loci_src_dir: "src/git.openstack.org/openstack/loci"
 branch: "{{ zuul_execution_branch.split('/')[-1] }}"
 

scripts/cleanup.sh

@@ -12,7 +12,7 @@ case ${distro} in
         rm -rf /var/lib/apt/lists/*
         ;;
     centos)
-        # NOTE(SamYaple): We should be removing 'patch' here, but that breaks
+        # We should be removing 'patch' here, but that breaks
         # centos as it tries to rip out systemd for some reason
         yum -y autoremove \
             git \
@@ -21,7 +21,6 @@ case ${distro} in
         yum clean all
         ;;
     opensuse|opensuse-leap|sles)
-        # NOTE(evrardjp): Remove all them packages!
         if [[ "${PYTHON3}" == "no" ]]; then
             remove_packages=("python-virtualenv")
         else
@@ -39,10 +38,10 @@ case ${distro} in
         ;;
 esac
 
-# NOTE(SamYaple): Removing this file allows python to use libraries outside of
-# the virtualenv if they do not exist inside the venv. This is a requirement
-# for using python-rbd which is not pip installable and is only available in
-# packaged form.
+# Removing this file allows python to use libraries outside of the
+# virtualenv if they do not exist inside the venv. This is a requirement
+# for using python-rbd which is not pip installable and is only available
+# in packaged form.
 rm /var/lib/openstack/lib/python*/no-global-site-packages.txt
 rm -rf /tmp/* /root/.cache /etc/machine-id
 find /usr/ /var/ \( -name "*.pyc" -o -name "__pycache__" \) -delete

scripts/fetch_wheels.sh

@@ -11,7 +11,6 @@ fi
 ${python} $(dirname $0)/fetch_wheels.py
 
 mkdir -p /tmp/wheels/
-# NOTE(SamYaple): We exclude all files starting with '.' as these can be
-# control files for AUFS which have special meaning on AUFS backed file
-# stores.
+# Exclude all files starting with '.' as these can be control files for
+# AUFS which have special meaning on AUFS backed file stores.
 tar xf /tmp/wheels.tar.gz --exclude='.*' -C /tmp/wheels/

scripts/install.sh

@@ -50,10 +50,6 @@ case ${distro} in
             sudo \
             tar \
             ${rpm_python_packages[@]}
-        #NOTE(evrardjp) Temporary workaround until bindep is fixed
-        # for leap 15: https://review.openstack.org/#/c/586038/
-        # should be merged and released.
-        sed -i 's/ID="opensuse-leap"/ID="opensuse"/g' /etc/os-release
         ;;
     *)
         echo "Unknown distro: ${distro}"

scripts/install_nova_console.sh

@@ -2,17 +2,17 @@
 
 set -ex
 
-# NOTE(SamYaple): Nova console is a special case. The html files needed to make
-# this work exist only upstream. The "packaged" versions of these come only
-# from openstack specific repos and they have hard requirements to a massive
-# amount of packages. Installing from "source" is the only way to get these
-# html files into the container. In total this adds less than a MB to the image
-# size
+# Nova console is a special case. The html files needed to make this work
+# exist only upstream. The "packaged" versions of these come only from
+# OpenStack specific repos and they have hard requirements to a massive
+# amount of packages. Installing from "source" is the only way to get
+# these html files into the container. In total this adds less than a MB
+# to the image size
 
 mkdir /usr/share/novnc
 git clone -b ${NOVNC_REF} --depth 1 ${NOVNC_REPO} /usr/share/novnc
 if [[ ! -f /usr/share/novnc/vnc_auto.html ]]; then
-    # NOTE(SamYaple): novnc >= 1.0.0 is installed
+    # novnc >= 1.0.0 is installed
     ln -s vnc_lite.html /usr/share/novnc/vnc_auto.html
 fi
 

scripts/requirements.sh

@@ -9,8 +9,9 @@ $(dirname $0)/install_packages.sh
 $(dirname $0)/clone_project.sh
 mv /tmp/requirements/{global-requirements.txt,upper-constraints.txt} /
 
-# NOTE(SamYaple): https://issues.apache.org/jira/browse/PROTON-1381
-# TODO(SamYaple): Make python-qpid-proton build here (possibly patch it)
+# TODO: Make python-qpid-proton build here (possibly patch it)
+# or remove when python-qpid-proton is updated with build fix.
+#   https://issues.apache.org/jira/browse/PROTON-1381
 if (( $(openssl version | awk -F'[ .]' '{print $3}') >= 1 )); then
     sed -i '/python-qpid-proton/d' /upper-constraints.txt
 fi
@@ -23,19 +24,22 @@ fi
 
 pushd $(mktemp -d)
 
-# NOTE(SamYaple): Build all deps in parallel. This is safe because we are
+# Build all dependencies in parallel. This is safe because we are
 # constrained on the version and we are building with --no-deps
 export CASS_DRIVER_BUILD_CONCURRENCY=8
-# NOTE(hrw): Drop python packages requested by monasca_analytics. Their
+
+# Drop python packages requested by monasca_analytics. Their
 # build time is huge and on !x86 we do not get binaries from Pypi.
 egrep -v "(scipy|scikit-learn)" /upper-constraints.txt | split -l1
 
-# NOTE(aostapenko): When a package uses the variable 'setup_requires' in 'setup.py', 'pip wheel'
-# dependency management will be overridden, resulting in possible incompatibilities
-# between packages. Installing packages using upper-constraints.txt before building the wheels
-# ensures the correct package versions will be available and installed locally.
-# https://pip.readthedocs.io/en/stable/user_guide/#installation-bundles
-# This allows to work around such issues as https://github.com/lxc/pylxd/issues/308
+# When a package uses the variable 'setup_requires' in 'setup.py',
+# 'pip wheel' dependency management will be overridden, resulting in
+# possible incompatibilities between packages. Installing packages using
+# upper-constraints.txt before building the wheels ensures the correct
+# package versions will be available and installed locally.
+#   https://pip.readthedocs.io/en/stable/user_guide/#installation-bundles
+# This allows to work around such issues as
+#   https://github.com/lxc/pylxd/issues/308
 if [ ! -z "${PIP_PACKAGES}" ]; then
   pip install -c /upper-constraints.txt ${PIP_PACKAGES}
 fi
@@ -43,18 +47,18 @@ fi
 echo uwsgi enum-compat ${PIP_PACKAGES} | xargs -n1 | split -l1 -a3
 ls -1 | xargs -n1 -P20 -t bash -c 'pip wheel --no-deps --wheel-dir / -c /upper-constraints.txt -r $1 || echo %1 >> /failure' _ | tee /tmp/wheels.txt
 
-# TODO(SamYaple): Improve the failure catching
+# TODO: Improve the failure catching
 if [[ -f /failure ]]; then
     echo Wheel failed to build
     cat /failure
     exit 1
 fi
 
-# NOTE(SamYaple) Remove native-binary wheels, we only want to keep wheels that
-# we compiled ourselves.
+# Remove native-binary wheels, we only want to keep wheels that we
+# compiled ourselves.
 awk -F'[ ,]+' '/^Skipping/ {gsub("-","_");print $2}' /tmp/wheels.txt | xargs -r -n1 bash -c 'ls /$1-*' _ | sort -u | xargs -t -r rm
 
-# NOTE(SamYaple): We want to purge all files that are not wheels or txt to
-# reduce the size of the layer to only what is needed
+# Purge all files that are not wheels or txt to reduce the size of the
+# layer to only what is needed
 shopt -s extglob
 rm -rf /!(*whl|*txt) > /dev/null 2>&1 || :

scripts/setup_pip.sh

@@ -8,9 +8,8 @@ else
     TMP_VIRTUALENV="python3 -m virtualenv --python=python3"
 fi
 
-# NOTE(SamYaple): This little dance allows us to install the latest pip and
-# setuptools without get_pip.py or the python-pip package (which is in epel on
-# centos)
+# This little dance allows us to install the latest pip and setuptools
+# without get_pip.py or the python-pip package (in epel on centos)
 if (( $(${TMP_VIRTUALENV} --version | cut -d. -f1) >= 14 )); then
     SETUPTOOLS="--no-setuptools"
 fi