Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading Tron to py3.8 + patching it with the fix #934

Merged
merged 19 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 5 additions & 7 deletions Makefile
nemacysts marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ endif

NOOP = true
ifeq ($(PAASTA_ENV),YELP)
export PIP_INDEX_URL ?= https://pypi.yelpcorp.com/simple
export PIP_INDEX_URL ?= http://169.254.255.254:20641/$*/simple/
Copy link
Member

Choose a reason for hiding this comment

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

oh nice, this works! I was thinking that we'd need to do the $* bit inside the targets themselves :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yupp, same I thought I would need to add it at first to the targets themselves :p

export NPM_CONFIG_REGISTRY ?= https://npm.yelpcorp.com/
ADD_MISSING_DEPS_MAYBE:=-diff --unchanged-line-format= --old-line-format= --new-line-format='%L' ./requirements.txt ./yelp_package/extra_requirements_yelp.txt >> ./requirements.txt
else
Expand Down Expand Up @@ -47,8 +47,7 @@ deb_%: clean docker_% coffee_%
$(DOCKER_RUN) -e PIP_INDEX_URL=${PIP_INDEX_URL} tron-builder-$* /bin/bash -c ' \
dpkg-buildpackage -d && \
mv ../*.deb dist/ && \
rm -rf debian/tron && \
chown -R $(UID):$(GID) dist debian \
rm -rf debian/tron \
'
# restore the backed up files
mv requirements.txt.old requirements.txt
Expand All @@ -58,8 +57,7 @@ coffee_%: docker_%
$(DOCKER_RUN) tron-builder-$* /bin/bash -c ' \
rm -rf tronweb/js/cs && \
mkdir -p tronweb/js/cs && \
coffee -o tronweb/js/cs/ -c tronweb/coffee/ && \
chown -R $(UID):$(GID) tronweb/js/cs/ \
coffee -o tronweb/js/cs/ -c tronweb/coffee/ \
'

test:
Expand All @@ -81,13 +79,13 @@ itest_%: debitest_%
@echo "itest $* OK"

dev:
SSH_AUTH_SOCK=$(SSH_AUTH_SOCK) .tox/py38/bin/trond --debug --working-dir=dev -l logging.conf --host=0.0.0.0 --web-path=/nail/home/emanelsabban/pg/Tron/tronweb
SSH_AUTH_SOCK=$(SSH_AUTH_SOCK) .tox/py38/bin/trond --debug --working-dir=dev -l logging.conf --host=0.0.0.0

example_cluster:
tox -e example-cluster

yelpy:
.tox/py38/bin/pip install -i https://pypi.yelpcorp.com/simple -r yelp_package/extra_requirements_yelp.txt
.tox/py38/bin/pip install -r yelp_package/extra_requirements_yelp.txt

LAST_COMMIT_MSG = $(shell git log -1 --pretty=%B | sed -e 's/[\x27\x22]/\\\x27/g')
release:
Expand Down
9 changes: 3 additions & 6 deletions debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@ override_dh_auto_test:
override_dh_virtualenv:
echo $(PIP_INDEX_URL)
dh_virtualenv --index-url $(PIP_INDEX_URL) \
--extra-pip-arg --trusted-host=169.254.255.254 \
--python=/usr/bin/python3.8 \
--preinstall no-manylinux1 \
--preinstall cython \
--preinstall pip-custom-platform \
--preinstall cython==0.29.36 \
--preinstall pip==18.1 \
--preinstall setuptools==46.1.3 \
--pip-tool pip-custom-platform \
--extra-pip-arg "-vvv"
--preinstall setuptools==46.1.3
@echo patching k8s client lib for configuration class
patch debian/tron/opt/venvs/tron/lib/python3.8/site-packages/kubernetes/client/configuration.py contrib/patch-config-loggers.diff
override_dh_installinit:
Expand Down
142 changes: 53 additions & 89 deletions dev/config/MASTER.yaml
nemacysts marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Please visit <wiki link> for a guide on how to setup Tron for local development
state_persistence:
name: "tron_state"
table_name: "tmp-tron-state"
store_type: "shelve"
store_type: "dynamodb"
buffer_size: 1
dynamodb_region: us-west-1

Expand All @@ -10,101 +11,64 @@ ssh_options:
agent: True

nodes:
- hostname: localhost
name: localhost
username: batch
- hostname: localhost
name: paasta
username: batch
- hostname: localhost

k8s_options:
enabled: true
kubeconfig_path: /nail/home/emanelsabban/pg/Tron/tron.conf
# Replace this with the path relative to your home dir to use
# action_runner:
# runner_type: "subprocess"
# remote_status_path: "pg/tron/status"
# remote_exec_path: "pg/tron/.tox/py38/bin"

jobs:
k8s:
testjob0:
enabled: true
node: localhost
schedule: "cron * * * * *"
run_limit: 5
actions:
zeroth:
command: env
trigger_downstreams:
minutely: "{ymdhm}"
cpus: 1
mem: 100

testjob1:
enabled: false
node: localhost
schedule: "cron * * * * *"
actions:
first:
command: "sleep 5"
cpus: 1
mem: 100
second:
command: "echo 'hello world'"
requires: [first]
triggered_by:
- "MASTER.testjob0.zeroth.minutely.{ymdhm}"
trigger_downstreams:
minutely: "{ymdhm}"
cpus: 1
mem: 100

testjob2:
enabled: false
node: localhost
schedule: "cron * * * * *"
actions:
first:
command: "echo 'goodbye, world'"
cpus: 1
mem: 100
triggered_by:
- "MASTER.testjob1.second.minutely.{ymdhm}"

retrier:
node: localhost
schedule: "cron 0 0 1 1 *"
actions:
sleep:
annotations:
paasta.yelp.com/routable_ip: 'false'
cap_add: []
cap_drop:
- SETPCAP
- MKNOD
- AUDIT_WRITE
- CHOWN
- NET_RAW
- DAC_OVERRIDE
- FOWNER
- FSETID
- KILL
- SETGID
- SETUID
- NET_BIND_SERVICE
- SYS_CHROOT
- SETFCAP
command: sleep 60; echo "I'm up and running $PAASTA_DOCKER_IMAGE"
cpus: 0.1
disk: 1024
docker_image: docker-paasta.yelpcorp.com:443/services-paasta-contract-monitor:paasta-a0c12fd6fbce6707947ce1fafcfdf7a7c6aaff9d
env:
ENABLE_PER_INSTANCE_LOGSPOUT: '1'
PAASTA_CLUSTER: infrastage
PAASTA_DEPLOY_GROUP: everything
PAASTA_DOCKER_IMAGE: services-paasta-contract-monitor:paasta-a0c12fd6fbce6707947ce1fafcfdf7a7c6aaff9d
PAASTA_GIT_SHA: a0c12fd6
PAASTA_INSTANCE: k8s.sleep
PAASTA_INSTANCE_TYPE: tron
PAASTA_MONITORING_TEAM: compute_infra
PAASTA_RESOURCE_CPUS: '0.1'
PAASTA_RESOURCE_DISK: '1024'
PAASTA_RESOURCE_MEM: '50'
PAASTA_SERVICE: paasta-contract-monitor
executor: kubernetes
extra_volumes: []
field_selector_env:
PAASTA_POD_IP:
field_path: status.podIP
labels:
paasta.yelp.com/cluster: infrastage
paasta.yelp.com/instance: k8s.sleep
paasta.yelp.com/pool: default
paasta.yelp.com/service: paasta-contract-monitor
yelp.com/owner: compute_infra_platform_experience
mem: 50
node_selectors:
yelp.com/pool: default
failing:
command: exit 1
retries: 1
secret_env:
PAASTA_SECRET_TEST:
key: goss
secret_name: tron-secret-paasta-contract-monitor-goss
secret_volumes:
- container_path: /super-secret-data
items:
- key: goss
path: secret.py
secret_name: goss
secret_volume_name: tron-secret-paasta-contract-monitor-goss
service_account_name: paasta--arn-aws-iam-528741615426-role-paasta-contract-monitor
node: paasta
queueing: false
monitoring:
alert_after: 25m
description: k8s actions failing on Tron locally
page: false
runbook: http://y/rb-tron
slack_channels:
- emans-super-cool-channel
team: noop
ticket: false
tip: Check "paasta logs" and read y/rb-paasta-contract-monitor
schedule:
type: cron
value: '*/1 * * * *'
use_k8s: true
retries_delay: 5m
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"eslint-plugin-react-hooks": "^4.1.0"
},
"resolutions": {
"axe-core": "4.4.1"
"axe-core": "4.7.0"
},
"engines": {
"node-version-shim": "10.x",
Expand Down
7 changes: 1 addition & 6 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ certifi==2022.12.7
cffi==1.12.3
cfn-lint==0.24.4
chardet==3.0.4
clusterman-metrics==2.2.1 # used by tron for pre-scaling for Spark runs
constantly==15.1.0
cryptography==39.0.1
dataclasses==0.6
Expand Down Expand Up @@ -76,12 +75,11 @@ requests-oauthlib==1.2.0
responses==0.10.6
rsa==4.9
s3transfer==0.6.0
scribereader==0.14.1 # used by tron to get tronjob logs
setuptools==65.5.1
six==1.15.0
sshpubkeys==3.1.0
stack-data==0.6.2
task-processing==0.12.2
task-processing==0.13.0
traitlets==5.0.0
Twisted==22.10.0
typing-extensions==4.5.0
Expand All @@ -91,7 +89,4 @@ websocket-client==0.56.0
Werkzeug==2.2.3
wrapt==1.11.2
xmltodict==0.12.0
yelp-clog==7.0.1 # scribereader dependency
yelp-logging==4.17.0 # scribereader dependency
yelp-meteorite==2.1.1 # used by task-processing to emit metrics, clusterman-metrics dependency
zope.interface==5.1.0
23 changes: 23 additions & 0 deletions tests/commands/retry_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,26 @@ def test_wait_for_retry_deps_done(fake_retry_action, mock_client_request, event_
data=dict(command="retry", use_latest_command=1),
user_attribution=True,
)


@mock.patch.object(retry, "RetryAction", autospec=True)
def test_retry_actions(mock_retry_action, mock_client, event_loop):
mock_wait_and_retry = mock_retry_action.return_value.wait_and_retry
mock_wait_and_retry.return_value = _empty_coro()

r_actions = retry.retry_actions(
"http://localhost",
["a_job.0.an_action_0", "another_job.1.an_action_1"],
use_latest_command=True,
deps_timeout_s=4,
)

assert r_actions == [mock_retry_action.return_value] * 2
assert mock_retry_action.call_args_list == [
mock.call(mock_client.return_value, "a_job.0.an_action_0", use_latest_command=True),
mock.call(mock_client.return_value, "another_job.1.an_action_1", use_latest_command=True),
]
assert mock_wait_and_retry.call_args_list == [
mock.call(deps_timeout_s=4, jitter=False),
mock.call(deps_timeout_s=4),
]
8 changes: 4 additions & 4 deletions tests/utils/scribereader_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_read_log_stream_for_action_run_min_date_and_max_date_today():
"tron.utils.scribereader.get_scribereader_host_and_port",
autospec=True,
return_value=("host", 1234),
), mock.patch(
), mock.patch("tron.utils.scribereader.scribereader", autospec=True,), mock.patch(
"tron.utils.scribereader.scribereader.get_stream_reader",
autospec=True,
) as mock_stream_reader, mock.patch(
Expand Down Expand Up @@ -97,7 +97,7 @@ def test_read_log_stream_for_action_run_min_date_and_max_date_different_days():
"tron.utils.scribereader.get_scribereader_host_and_port",
autospec=True,
return_value=("host", 1234),
), mock.patch(
), mock.patch("tron.utils.scribereader.scribereader", autospec=True,), mock.patch(
"tron.utils.scribereader.scribereader.get_stream_reader",
autospec=True,
) as mock_stream_reader, mock.patch(
Expand Down Expand Up @@ -195,7 +195,7 @@ def test_read_log_stream_for_action_run_min_date_and_max_date_in_past():
"tron.utils.scribereader.get_scribereader_host_and_port",
autospec=True,
return_value=("host", 1234),
), mock.patch(
), mock.patch("tron.utils.scribereader.scribereader", autospec=True,), mock.patch(
"tron.utils.scribereader.scribereader.get_stream_reader",
autospec=True,
) as mock_stream_reader, mock.patch(
Expand Down Expand Up @@ -261,7 +261,7 @@ def test_read_log_stream_for_action_run_min_date_and_max_date_for_long_output():
"tron.utils.scribereader.get_scribereader_host_and_port",
autospec=True,
return_value=("host", 1234),
), mock.patch(
), mock.patch("tron.utils.scribereader.scribereader", autospec=True,), mock.patch(
"tron.utils.scribereader.scribereader.get_stream_reader",
autospec=True,
) as mock_stream_reader, mock.patch(
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ commands =
check-requirements
# optionally install yelpy requirements - this is after check-requirements since
# check-requirements doesn't understand these extra requirements
-pip-custom-platform install --index-url https://pypi.yelpcorp.com/simple -r yelp_package/extra_requirements_yelp.txt
-pip install -r yelp_package/extra_requirements_yelp.txt
# we then run tests at the very end so that we can run tests with yelpy requirements
py.test -s {posargs:tests}

Expand Down
5 changes: 2 additions & 3 deletions tron/commands/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import urllib.request
from collections import namedtuple
from typing import Dict
from typing import Mapping

import tron
from tron.config.schema import MASTER_NAMESPACE
Expand All @@ -18,7 +17,7 @@

assert simplejson # Pyflakes
except ImportError:
import json as simplejson
import json as simplejson # type: ignore
nemacysts marked this conversation as resolved.
Show resolved Hide resolved

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -99,7 +98,7 @@ def build_get_url(url, data=None):
return url


def ensure_user_attribution(headers: Mapping[str, str]) -> Dict[str, str]:
def ensure_user_attribution(headers: Dict[str, str]) -> Dict[str, str]:
headers = headers.copy()
if "User-Agent" not in headers:
headers["User-Agent"] = USER_AGENT
Expand Down
2 changes: 1 addition & 1 deletion tron/commands/display.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class DisplayJobRuns(TableDisplay):
colors = {
"id": partial(Color.set, "yellow"),
"state": add_color_for_state,
"manual": lambda value: Color.set("cyan" if value else None, value),
"manual": lambda value: Color.set("cyan" if value else None, value), # type: ignore # can't type a lambda
}

def format_value(self, field_idx, value):
Expand Down
8 changes: 4 additions & 4 deletions tron/config/config_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ def build_child_context(self, path):

class NullConfigContext:
path = ""
nodes = set()
command_context = {}
nodes = set() # type: ignore
command_context = {} # type: ignore
namespace = MASTER_NAMESPACE
partial = False

Expand All @@ -292,8 +292,8 @@ class Validator:
"""

config_class = None
defaults = {}
validators = {}
defaults = {} # type: ignore
validators = {} # type: ignore
optional = False

def validate(self, in_dict, config_context):
Expand Down
Loading
Loading