Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ jobs:
fail-fast: false
matrix:
make_target:
- itest_bionic
- itest_jammy
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: 3.7
python-version: 3.12
- run: |
sudo apt-get -y install libzookeeper-mt-dev
pip install tox
Expand Down
27 changes: 21 additions & 6 deletions dockerfiles/itest/docker-compose-jammy.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
version: '2.4'
services:
itest:
build: ../../dockerfiles/itest/itest_jammy
Expand All @@ -8,26 +7,42 @@ services:
volumes:
- ../..:/work
- "${SSH_AUTH_SOCK}:${SSH_AUTH_SOCK}"
links:
- servicethree
- serviceone
- scribe
- zookeeper
depends_on:
- servicethree
- serviceone
- scribe
- zookeeper

servicethree:
# Create dummy service listening on port 1024 that returns /status
image: python:3-alpine
command: /bin/sh -c "echo OK > status && python3 -m http.server 1024"
networks:
default:
aliases:
- servicethree_1

serviceone:
# Create dummy service listening on port 1025
image: alpine/socat:latest
command: TCP4-LISTEN:1025,fork /dev/null
networks:
default:
aliases:
- serviceone_1

scribe:
# Create dummy scribe listening on port 1464
image: alpine/socat:latest
command: TCP4-LISTEN:1464,fork /dev/null
networks:
default:
aliases:
- scribe_1

zookeeper:
image: zookeeper:3.5
networks:
default:
aliases:
- zookeeper_1
20 changes: 15 additions & 5 deletions dockerfiles/itest/itest/Dockerfile.jammy
Original file line number Diff line number Diff line change
@@ -1,30 +1,40 @@
FROM ubuntu:jammy

ARG DEBIAN_FRONTEND=noninteractive
ENV DEBIAN_FRONTEND=${DEBIAN_FRONTEND}
Comment on lines +3 to +4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, we can just hardcode this :p

ENV TZ=Etc/UTC

ARG PIP_INDEX_URL=https://pypi.yelpcorp.com/simple
ENV PIP_INDEX_URL=$PIP_INDEX_URL

RUN apt-get update && \
apt-get install -y --no-install-recommends gnupg2 software-properties-common && \
DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends gnupg2 software-properties-common && \
add-apt-repository ppa:deadsnakes/ppa

RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get -y install \
build-essential \
python3-kazoo \
python3-pytest \
python3-setuptools \
socat \
ruby ruby-dev ruby-bundler zlib1g-dev \
libcurl4 \
python3.7 \
python3.7-dev \
python3.7-distutils \
python3-pip \
libffi-dev \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if only we could ensure that public pypi had wheels for everything - when we could get rid of all the -dev packages :p

python3.11 \
python3.11-dev \
python3.11-venv \
python3.12 \
python3.12-dev \
python3.12-venv \
python3-zope.interface \
git \
libyaml-dev \
libc6-dev \
libstdc++6 \
wget

RUN python3.11 -m ensurepip --upgrade

# Install nerve
ADD nerve_Gemfile Gemfile
RUN mkdir -p /opt/nerve
Expand Down
4 changes: 2 additions & 2 deletions dockerfiles/itest/itest/itest.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
]


@pytest.yield_fixture(scope='module')
@pytest.fixture(scope='module')
def setup():
# Forward healthchecks to the services
socat_procs = []
Expand Down Expand Up @@ -140,7 +140,7 @@ def test_nerve_service_config(setup):
],
"host": MY_IP_ADDRESS,
"port": 1024,
"weight": CPUS,
"weight": 10,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're not using CPUS anymore, we probably wanna remove it at the top of the file

"zk_hosts": [ZOOKEEPER_CONNECT_STRING],
"zk_path": "/smartstack/global/service_three.main",
'labels': {
Expand Down
9 changes: 6 additions & 3 deletions dockerfiles/itest/itest/run_itest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ set -vx
set -e

echo "installing hacheck"
GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" git clone git@github.yelpcorp.com:packages/hacheck && cd /hacheck && python3.7 -m pip install . && cp /usr/local/bin/ha* /usr/bin/
GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" git clone --depth=1 git@github.yelpcorp.com:packages/hacheck /hacheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, i guess we haven't been expecting folks to run these itests outside of yelp :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, not sure what else we can do now we've taken hacheck internal

cd /hacheck
python3.11 -m pip install .
cp /usr/local/bin/ha* /usr/bin/

echo "installing paasta-tools (dependency of nerve-tools.)"
. /etc/lsb-release
PAASTA_VERSION=0.145.0
PAASTA_VERSION=1.47.0
PAASTA_DEB_NAME=paasta-tools_${PAASTA_VERSION}.${DISTRIB_CODENAME}1_amd64.deb
wget "https://github.com/Yelp/paasta/releases/download/v${PAASTA_VERSION}/${PAASTA_DEB_NAME}"

Expand All @@ -25,4 +28,4 @@ echo "Testing that pyyaml uses optimized cyaml parsers if present"
/opt/venvs/nerve-tools/bin/python -c 'import yaml; assert yaml.__with_libyaml__'

echo "Full integration test"
py.test-3 -vvv /itest.py
python3 -m pytest -vvv /itest.py
26 changes: 15 additions & 11 deletions dockerfiles/jammy/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
FROM ubuntu:jammy

ARG DEBIAN_FRONTEND=noninteractive
ENV DEBIAN_FRONTEND=${DEBIAN_FRONTEND}
ENV TZ=Etc/UTC

ARG PIP_INDEX_URL=https://pypi.yelpcorp.com/simple
ENV PIP_INDEX_URL=$PIP_INDEX_URL

# Need Python 3.7 for nerve-tools
# Need Python 3.12 for nerve-tools
RUN apt-get update && \
apt-get install -y --no-install-recommends gnupg2 software-properties-common && \
DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends gnupg2 software-properties-common && \
add-apt-repository ppa:deadsnakes/ppa

RUN apt-get update && apt-get -y install \
RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get -y install \
build-essential \
cargo \
curl \
Expand All @@ -22,17 +26,17 @@ RUN apt-get update && apt-get -y install \
libyaml-dev \
libzookeeper-mt-dev \
protobuf-compiler \
python3.7-dev \
python3.7-distutils \
python3.12 \
python3.12-dev \
python3.12-venv \
rustc \
wget

# Preferred method to install pip when not bundled with python - https://pip.pypa.io/en/stable/installing/
RUN curl https://bootstrap.pypa.io/get-pip.py -o get-pip.py
RUN python3.7 get-pip.py
RUN pip install -U tox pip virtualenv setuptools
RUN python3.12 -m ensurepip --upgrade
ADD src/requirements-bootstrap.txt /tmp/requirements-bootstrap.txt
RUN python3.12 -m pip install -U -r /tmp/requirements-bootstrap.txt tox virtualenv

ADD location_types.json /nail/etc/services/
ADD location_mapping.json /nail/etc/services/
ADD dockerfiles/jammy/location_types.json /nail/etc/services/
ADD dockerfiles/jammy/location_mapping.json /nail/etc/services/
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this because of context: ../.. in the docker-compose file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah - just copied how we do it elsewhere


WORKDIR /work
7 changes: 4 additions & 3 deletions dockerfiles/jammy/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
version: '2.4'
services:
jammy:
build: ../../dockerfiles/jammy
command: bash -c "cp -R /code/* /work && cd /work/src && tox -e py37-linux && dpkg-buildpackage -d -uc -us && mv ../*.deb /dist"
build:
context: ../..
dockerfile: ./dockerfiles/jammy/Dockerfile
command: bash -c "cp -R /code/* /work && cd /work/src && tox -e py312-linux && dpkg-buildpackage -d -uc -us && mv ../*.deb /dist"
volumes:
- ../..:/code:ro
- ../../dist:/dist
2 changes: 1 addition & 1 deletion src/debian/compat
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9
10
6 changes: 5 additions & 1 deletion src/debian/control
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
Source: nerve-tools
Section: admin
Priority: optional
Comment on lines +2 to +3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just leave these out? i was a little confused as to whether this was correct since this is duplicated below (but then i realized that there's two different "sections" here)

i don't think we have anything that really cares about these being set unless they're required by the debian/compat bump - which i also don't know much about what that implies/entails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, excluding them will trigger a warning.

Maintainer: John Billings <billings@yelp.com>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!8ball clean this up while we're here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can do - I was just going to leave it for the sake of a smaller diff

Build-Depends:
dh-virtualenv,

Depends: python3.7, paasta-tools (>= 0.145.0~), ${shlibs:Depends}
Package: nerve-tools
Section: admin
Priority: optional
Architecture: any
Depends: python3.12, paasta-tools (>= 1.47.0~), ${shlibs:Depends}
Description: Nerve-related tools for use on Yelp machines.
5 changes: 1 addition & 4 deletions src/debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ override_dh_auto_test:
# TravisCI can't handle the large logs that verbose output creates and it will just terminate the build :(
override_dh_virtualenv:
dh_virtualenv -i $(PIP_INDEX_URL) \
--python=/usr/bin/python3.7 \
--python=/usr/bin/python3.12 \
--preinstall no-manylinux1

# For some reason dh virtualenv can't find libcrypto bundled but this seems to work. Probably there is a real fix somewhere?
override_dh_shlibdeps:
dh_shlibdeps --dpkg-shlibdeps-params=-ldebian/nerve-tools/opt/venvs/nerve-tools/lib/python3.7/site-packages/cryptography.libs
2 changes: 1 addition & 1 deletion src/mypy.ini
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[mypy]
python_version = 3.6
python_version = 3.12
ignore_missing_imports = True
check_untyped_defs = True
disallow_untyped_defs = True
Expand Down
8 changes: 4 additions & 4 deletions src/nerve_tools/config.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from mypy_extensions import TypedDict
from typing import Mapping
from typing import Iterable
from typing import Dict
from typing import Tuple
from typing import Iterable
from typing import Mapping
from typing import Optional
from typing import Tuple
from typing import TypedDict


class CheckDict(TypedDict, total=False):
Expand Down
2 changes: 1 addition & 1 deletion src/nerve_tools/updown_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import requests
from requests.exceptions import RequestException

from paasta_tools.marathon_tools import load_service_namespace_config
from paasta_tools.long_running_service_tools import load_service_namespace_config
from service_configuration_lib import read_service_configuration


Expand Down
6 changes: 3 additions & 3 deletions src/requirements-bootstrap.txt
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
pip==21.0.1
setuptools==54.1.2
wheel==0.36.2
pip==25.2
setuptools==80.10.1
wheel==0.46.2
8 changes: 4 additions & 4 deletions src/requirements-dev.txt
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
coverage==4.5.4
flake8==3.7.9
mock==3.0.5
pytest==5.2.1
coverage==6.5.0
flake8==7.3.0
mypy==1.17.1
pytest==7.4.3
12 changes: 3 additions & 9 deletions src/requirements.txt
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
argparse==1.2.1
cryptography==35.0.0
dulwich==0.17.3
environment_tools==1.1.3
kazoo==2.8.0
ldap3==2.7
mypy-extensions==0.4.3
paasta-tools==0.105.0
paasta-tools==1.47.0
PyYAML==6.0.1
requests==2.23.0
service-configuration-lib==2.5.4
setuptools==42.0.2
requests==2.32.5
service-configuration-lib==3.3.8
19 changes: 12 additions & 7 deletions src/setup.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-

from pkg_resources import yield_lines
from setuptools import setup, find_packages
from pathlib import Path

from setuptools import find_packages
from setuptools import setup

def get_install_requires():
with open('requirements.txt', 'r') as f:
minimal_reqs = list(yield_lines(f.read()))

return minimal_reqs
HERE = Path(__file__).parent


def get_install_requires():
return [
line.strip()
for line in (HERE / 'requirements.txt').read_text().splitlines()
if line.strip() and not line.startswith('#')
]
Comment on lines +13 to +18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, we haven't really been doing the right thing with install_requires here since the beginning - but that's for later us to fix (maybe) :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I can break this out into a separate PR



setup(
Expand All @@ -20,7 +26,6 @@ def get_install_requires():
author_email='compute-infra@yelp.com',
description='Nerve-related tools for use on Yelp machines',
packages=find_packages(exclude=['tests']),
setup_requires=['setuptools'],
include_package_data=True,
install_requires=get_install_requires(),
entry_points={
Expand Down
2 changes: 1 addition & 1 deletion src/tests/clean_nerve_test.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from nerve_tools import clean_nerve

import mock
from unittest import mock


def test_parse_args():
Expand Down
12 changes: 6 additions & 6 deletions src/tests/configure_nerve_test.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import copy
import mock
from mock import call
from mock import patch
from mock import mock_open
from mock import MagicMock
from mock import Mock
from unittest import mock
from unittest.mock import call
from unittest.mock import MagicMock
from unittest.mock import mock_open
from unittest.mock import Mock
from unittest.mock import patch
import pytest
import sys
import multiprocessing
Expand Down
2 changes: 1 addition & 1 deletion src/tests/envoy_test.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from mock import patch
from unittest.mock import patch

from nerve_tools.envoy import get_envoy_ingress_listeners

Expand Down
2 changes: 1 addition & 1 deletion src/tests/updown_service_test.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import mock
from unittest import mock
import pytest
from requests.exceptions import RequestException
import yaml
Expand Down
Loading
Loading