Browse Source

Revert "Revert "feat(dev): warn on insufficient Docker.app memory (#33825)" (#33922)" (#33923)

This reverts commit 4957b19f4462be227f43c81883bb01b797859905.
asottile-sentry 2 years ago
parent
commit
12556c96ab
4 changed files with 183 additions and 3 deletions
  1. 2 0
      .envrc
  2. 3 3
      setup.cfg
  3. 106 0
      tests/tools/docker_memory_check_test.py
  4. 72 0
      tools/docker_memory_check.py

+ 2 - 0
.envrc

@@ -227,6 +227,8 @@ if ! require pre-commit; then
     commands_to_run+=("make setup-git")
 fi
 
+python3 -m tools.docker_memory_check
+
 ### Node ###
 
 debug "Checking node..."

+ 3 - 3
setup.cfg

@@ -12,6 +12,8 @@ extend-ignore = E203,E501,E402,E731,B007,B009,B010,B011
 per-file-ignores =
     # allow prints in tests
     tests/*: S002
+    # these scripts must have minimal dependencies so opt out of the usual sentry rules
+    tools/*: S
 
 [flake8:local-plugins]
 paths = .
@@ -24,6 +26,4 @@ python-tag = py38
 [coverage:run]
 omit =
     src/sentry/migrations/*
-source =
-    src
-    tests
+source = .

+ 106 - 0
tests/tools/docker_memory_check_test.py

@@ -0,0 +1,106 @@
+import os
+from unittest import mock
+
+import pytest
+
+from tools import docker_memory_check
+
+
+@pytest.mark.parametrize(
+    ("option", "expected"),
+    (
+        ("always", True),
+        ("never", False),
+    ),
+)
+def test_should_use_color_forced(option, expected):
+    assert docker_memory_check.should_use_color(option) is expected
+
+
+def test_should_use_color_determined_by_CI_variable_missing():
+    with mock.patch.dict(os.environ, clear=True):
+        assert docker_memory_check.should_use_color("auto") is True
+
+
+def test_should_use_color_determined_by_CI_variable_empty():
+    with mock.patch.dict(os.environ, {"CI": ""}):
+        assert docker_memory_check.should_use_color("auto") is True
+
+
+def test_should_use_color_determined_by_CI_variable_present():
+    with mock.patch.dict(os.environ, {"CI": ""}):
+        assert docker_memory_check.should_use_color("1") is False
+
+
+def test_color_using_color():
+    ret = docker_memory_check.color("hello hello", "\033[33m", use_color=True)
+    assert ret == "\033[33mhello hello\033[m"
+
+
+def test_color_not_using_color():
+    ret = docker_memory_check.color("hello hello", "\033[33m", use_color=False)
+    assert ret == "hello hello"
+
+
+def test_check_ignored_file_does_not_exist(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+
+    assert docker_memory_check.main(("--settings-file", str(json_file))) == 0
+    out, err = capsys.readouterr()
+    assert out == err == ""
+
+
+def test_check_ignored_file_is_not_json(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+    json_file.write_text("not json")
+
+    assert docker_memory_check.main(("--settings-file", str(json_file))) == 0
+    out, err = capsys.readouterr()
+    assert out == err == ""
+
+
+def test_check_ignored_file_missing_field(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+    json_file.write_text("{}")
+
+    assert docker_memory_check.main(("--settings-file", str(json_file))) == 0
+    out, err = capsys.readouterr()
+    assert out == err == ""
+
+
+def test_check_ignored_memory_limit_not_int(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+    json_file.write_text('{"memoryMiB": "lots"}')
+
+    assert docker_memory_check.main(("--settings-file", str(json_file))) == 0
+    out, err = capsys.readouterr()
+    assert out == err == ""
+
+
+def test_check_has_enough_configured_memory(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+    json_file.write_text('{"memoryMiB": 9001}')
+
+    args = ("--settings-file", str(json_file), "--memory-minimum", "8092")
+    assert docker_memory_check.main(args) == 0
+    out, err = capsys.readouterr()
+    assert out == err == ""
+
+
+def test_check_insufficient_configured_memory(capsys, tmp_path):
+    json_file = tmp_path.joinpath("settings.json")
+    json_file.write_text('{"memoryMiB": 7000}')
+
+    args = ("--settings-file", str(json_file), "--memory-minimum=8092", "--color=never")
+    assert docker_memory_check.main(args) == 0
+    out, err = capsys.readouterr()
+    assert out == ""
+    assert (
+        err
+        == """\
+WARNING: docker is configured with less than the recommended minimum memory!
+- open Docker.app and adjust the memory in Settings -> Resources
+- current memory (MiB): 7000
+- recommended minimum (MiB): 8092
+"""
+    )

+ 72 - 0
tools/docker_memory_check.py

@@ -0,0 +1,72 @@
+from __future__ import annotations
+
+import argparse
+import json
+import os.path
+import sys
+from typing import Sequence
+
+
+def should_use_color(setting: str) -> bool:
+    # normally I would use `sys.stdout.isatty()` however direnv always pipes this
+    return setting == "always" or (setting == "auto" and not os.environ.get("CI"))
+
+
+def color(s: str, color: str, *, use_color: bool) -> str:
+    if use_color:
+        return f"{color}{s}\033[m"
+    else:
+        return s
+
+
+def main(argv: Sequence[str] | None = None) -> int:
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "--settings-file",
+        default=os.path.expanduser("~/Library/Group Containers/group.com.docker/settings.json"),
+        help=argparse.SUPPRESS,
+    )
+    parser.add_argument(
+        "--memory-minimum",
+        default=8092,
+        type=int,
+        help="the minimum amount of allocated memory to warn for.  default: %(default)s (MiB)",
+    )
+    parser.add_argument(
+        "--color",
+        choices=("always", "never", "auto"),
+        default="auto",
+        help="whether to use color.  default: %(default)s (auto is determined by CI environment variable)",
+    )
+    args = parser.parse_args(argv)
+
+    use_color = should_use_color(args.color)
+
+    try:
+        with open(args.settings_file) as f:
+            contents = json.load(f)
+    except (json.JSONDecodeError, OSError):
+        return 0  # file didn't exist or was not json
+
+    try:
+        configured = contents["memoryMiB"]
+    except KeyError:
+        return 0  # configuration did not look like what we expected
+
+    if not isinstance(configured, int):
+        return 0  # configuration did not look like what we expected
+
+    if configured < args.memory_minimum:
+        msg = f"""\
+WARNING: docker is configured with less than the recommended minimum memory!
+- open Docker.app and adjust the memory in Settings -> Resources
+- current memory (MiB): {configured}
+- recommended minimum (MiB): {args.memory_minimum}
+"""
+        print(color(msg, "\033[33m", use_color=use_color), end="", file=sys.stderr)
+
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())