Ubuntu Repository Cache Charm

Merge ~hloeung/ubuntu-repository-cache:sync-host into ubuntu-repository-cache:master

  1. Git
  2. lp:~hloeung/ubuntu-repository-cache
  3. sync-host
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Ivan
Approved revision: da53ffc24b53247593ffc83ef0a9145441f3ea77
Merged at revision: 7ee8d86b75b4347df94ff1875d740568073ac291
Proposed branch: ~hloeung/ubuntu-repository-cache:sync-host
Merge into: ubuntu-repository-cache:master
Diff against target: 144 lines (+35/-19)
2 files modified
files/health_check.py (+15/-17)
tests/unit/test_health_check.py (+20/-2)
Reviewer Review Type Date Requested Status
Ivan Approve
Canonical IS Reviewers Pending
Review via email: mp+462828@code.launchpad.net

Commit message

Always include the upstream_host label, even for local content

Description of the change

That way, grafana panels look nicer and not something like this:

| SUCCESS /ubuntu/pool/main/ (ubuntu-repository-cache/0) (ipv4.us.archive.ubuntu.com)
| SUCCESS /ubuntu/dists/focal/InRelease (ubuntu-repository-cache/0) ()
| SUCCESS /ubuntu/pool/main/ (ubuntu-repository-cache/0) ()

See https://private-fileshare.canonical.com/~hloeung/tmp/aNDBKigUgV.png

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Ivan (ivaneloff) wrote :

LGTM

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 7ee8d86b75b4347df94ff1875d740568073ac291

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/files/health_check.py b/files/health_check.py
2index 5c5e5a9..7c05f4b 100755
3--- a/files/health_check.py
4+++ b/files/health_check.py
5@@ -38,7 +38,6 @@ METADATA_AGE_PATH = os.path.join(
6 os.path.dirname(os.path.dirname(os.path.realpath(__file__))), 'data/ubuntu_active/ls-lR.gz'
7 )
8
9-UPSTREAM_HOST = os.environ.get('UPSTREAM_HOST')
10 UPSTREAM_CHECK_URL = '/ubuntu/pool/main/'
11 CHECK_URLS.append(UPSTREAM_CHECK_URL)
12
13@@ -73,9 +72,9 @@ def send_to_influx(metrics, send_to='127.0.0.1:8094'):
14 def check_url(url, headers=CHECK_HEADERS):
15 start_time = time.time()
16
17- upstream = ''
18+ upstream = 'local'
19 if url == UPSTREAM_CHECK_URL:
20- upstream = ' ({})'.format(os.environ.get('UPSTREAM_HOST'))
21+ upstream = os.environ.get('UPSTREAM_HOST')
22
23 count = 0
24 while count < CHECK_RETRIES:
25@@ -90,13 +89,13 @@ def check_url(url, headers=CHECK_HEADERS):
26 status_code = resp.status_code
27 if resp.ok:
28 duration = time.time() - start_time_url
29- logging.info('OK: #%s: %s (%.3f)%s', count, url, duration, upstream)
30+ logging.info('OK: #%s: %s (%.3f) (%s)', count, url, duration, upstream)
31 break
32 else:
33 resp.raise_for_status()
34 except requests.exceptions.HTTPError as err:
35 duration = time.time() - start_time_url
36- logging.error('ERROR: #%s: %s: %s (%.3f)%s', count, url, err, duration, upstream)
37+ logging.error('ERROR: #%s: %s: %s (%.3f) (%s)', count, url, err, duration, upstream)
38 msg = err
39 except (
40 requests.exceptions.ConnectTimeout,
41@@ -104,13 +103,13 @@ def check_url(url, headers=CHECK_HEADERS):
42 requests.exceptions.Timeout,
43 ) as err:
44 duration = time.time() - start_time_url
45- logging.error('ERROR: #%s: %s: %s (%.3f)%s', count, url, err, duration, upstream)
46+ logging.error('ERROR: #%s: %s: %s (%.3f) (%s)', count, url, err, duration, upstream)
47 msg = err
48 except Exception as err:
49 duration = time.time() - start_time_url
50 # We don't want to show exactly what the error is, but we
51 # do want to log it.
52- logging.critical('ERROR: #%s: %s: %s (%.3f)%s', count, url, err, duration, upstream)
53+ logging.critical('ERROR: #%s: %s: %s (%.3f) (%s)', count, url, err, duration, upstream)
54 msg = 'Unknown error'
55
56 return status_code, time.time() - start_time, msg
57@@ -134,6 +133,7 @@ def main(disabled_flag=DISABLE_FLAG): # NOQA: C901
58 # replacing '+' with 'p' and '/' with 'a'.
59 uid = generate_uuid()
60 remote_addr = os.environ.get('REMOTE_ADDR', '-')
61+ upstream_host = os.environ.get('UPSTREAM_HOST')
62 logging.basicConfig(
63 filename=LOG_FILE,
64 format='%(asctime)s {} [{}]: %(message)s'.format(remote_addr, uid),
65@@ -193,12 +193,11 @@ def main(disabled_flag=DISABLE_FLAG): # NOQA: C901
66 status_code = val[0]
67 duration = 1 + val[1]
68 err = val[2]
69- labels = 'url={},status=FAIL,status_code={}'.format(url, status_code)
70- upstream = ''
71+ upstream = 'local'
72 if url == UPSTREAM_CHECK_URL:
73- labels += ',upstream_host={}'.format(UPSTREAM_HOST)
74- upstream = ' ({})'.format(UPSTREAM_HOST)
75- print('ERROR: {}: {} ({:.3f}s) {}'.format(url, err, duration, upstream))
76+ upstream = upstream_host
77+ print('ERROR: {}: {} ({:.3f}s) ({})'.format(url, err, duration, upstream))
78+ labels = 'url={},status=FAIL,status_code={},upstream_host={}'.format(url, status_code, upstream)
79 metrics.append(render_influx(HEALTH_STATUS_METRIC_NAME, labels, duration))
80
81 send_to_influx(render_influx(HEALTH_METRIC_NAME, None, 0))
82@@ -213,12 +212,11 @@ def main(disabled_flag=DISABLE_FLAG): # NOQA: C901
83 for url, val in successes.items():
84 status_code = val[0]
85 duration = val[1]
86- labels = 'url={},status=SUCCESS,status_code={}'.format(url, status_code)
87- upstream = ''
88+ upstream = 'local'
89 if url == UPSTREAM_CHECK_URL:
90- labels += ',upstream_host={}'.format(UPSTREAM_HOST)
91- upstream = ' ({})'.format(UPSTREAM_HOST)
92- print('OK: {} ({:.3f}s) {}'.format(url, duration, upstream))
93+ upstream = upstream_host
94+ print('OK: {} ({:.3f}s) ({})'.format(url, duration, upstream))
95+ labels = 'url={},status=SUCCESS,status_code={},upstream_host={}'.format(url, status_code, upstream)
96 metrics.append(render_influx(HEALTH_STATUS_METRIC_NAME, labels, duration))
97
98 # Flush the buffer to ensure we're not holding up
99diff --git a/tests/unit/test_health_check.py b/tests/unit/test_health_check.py
100index 103f098..f94f729 100644
101--- a/tests/unit/test_health_check.py
102+++ b/tests/unit/test_health_check.py
103@@ -87,6 +87,24 @@ class TestCharm(unittest.TestCase):
104
105 @mock.patch('files.health_check.send_to_influx')
106 @mock.patch('logging.info')
107+ @mock.patch('os.path.realpath')
108+ @mock.patch('requests.head', side_effect=mocked_requests_head)
109+ @mock.patch('time.time')
110+ def test_health_check(self, time_time, requests_head, realpath, log_info, send_to_influx):
111+ realpath.return_value = os.path.join(self.tmpdir, 'one', 'two')
112+ apache_root = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
113+ os.mkdir(os.path.join(apache_root, 'data'))
114+ os.mkdir(os.path.join(apache_root, 'data', 'ubuntu_active'))
115+ time_time.return_value = 1710912908.1487546
116+
117+ with mock.patch.dict(os.environ, {"UPSTREAM_HOST": "archive.ubuntu.com"}):
118+ health_check.main()
119+ log_info.assert_called_with('%s (%.3f)', '200 OK', 0.0)
120+ want = 'ubuntu_repository_cache_health_status,url=/ubuntu/pool/main/,status=SUCCESS,status_code=200,upstream_host=archive.ubuntu.com value=0.0' # NOQA: E501
121+ send_to_influx.assert_called_with(want)
122+
123+ @mock.patch('files.health_check.send_to_influx')
124+ @mock.patch('logging.info')
125 def test_health_check_disabled(self, log_info, send_to_influx):
126 disabled_flag = os.path.join(self.tmpdir, 'health-check-disabled.flag')
127 os.mknod(disabled_flag)
128@@ -129,7 +147,7 @@ class TestCharm(unittest.TestCase):
129
130 health_check.check_url('/ubuntu/dists/focal/InRelease')
131 requests_head.assert_called_with('http://localhost/ubuntu/dists/focal/InRelease', headers=headers, timeout=5)
132- log_info.assert_called_with('OK: #%s: %s (%.3f)%s', 1, '/ubuntu/dists/focal/InRelease', 0.0, '')
133+ log_info.assert_called_with('OK: #%s: %s (%.3f) (%s)', 1, '/ubuntu/dists/focal/InRelease', 0.0, 'local')
134 log_error.assert_not_called()
135 log_crit.assert_not_called()
136
137@@ -138,7 +156,7 @@ class TestCharm(unittest.TestCase):
138 with mock.patch.dict(os.environ, {"UPSTREAM_HOST": "archive.ubuntu.com"}):
139 health_check.check_url('/ubuntu/pool/main/')
140 requests_head.assert_called_with('http://localhost/ubuntu/pool/main/', headers=headers, timeout=5)
141- log_info.assert_called_with('OK: #%s: %s (%.3f)%s', 1, '/ubuntu/pool/main/', 0.0, ' (archive.ubuntu.com)')
142+ log_info.assert_called_with('OK: #%s: %s (%.3f) (%s)', 1, '/ubuntu/pool/main/', 0.0, 'archive.ubuntu.com')
143 log_error.assert_not_called()
144 log_crit.assert_not_called()
145

Subscribers

People subscribed via source and target branches