Browse Source

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
Chris Hoge 5 months ago
parent
commit
d0ef425ef6

+ 2
- 3
dockerfiles/debian/Dockerfile View File

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

+ 2
- 3
dockerfiles/ubuntu/Dockerfile View File

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

+ 1
- 1
playbooks/files/apache.conf View File

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

+ 0
- 11
playbooks/loci-builder.yaml View File

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

+ 4
- 2
playbooks/post.yaml View File

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

+ 1
- 1
playbooks/setup-gate.yaml View File

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

+ 7
- 3
playbooks/vars.yaml View File

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

+ 5
- 6
scripts/cleanup.sh View File

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

+ 2
- 3
scripts/fetch_wheels.sh View File

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

+ 0
- 4
scripts/install.sh View File

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

+ 7
- 7
scripts/install_nova_console.sh View File

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

+ 19
- 15
scripts/requirements.sh View File

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

+ 2
- 3
scripts/setup_pip.sh View File

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

Loading…
Cancel
Save