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

chore: move is_docker_rootless from tutor to tutor discovery #93

Merged
Merged
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
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
.DEFAULT_GOAL := help
.PHONY: docs
SRC_DIRS = ./tutordiscovery
SRC_DIRS = ./tutordiscovery ./tests
BLACK_OPTS = --exclude templates ${SRC_DIRS}

# Warning: These checks are run on every PR.
test: test-lint test-types test-format # Run some static checks.
test: test-lint test-types test-format test-unit # Run some static checks.

test-format: ## Run code formatting tests.
black --check --diff $(BLACK_OPTS)
Expand All @@ -15,6 +15,9 @@ test-lint: ## Run code linting tests
test-types: ## Run type checks.
mypy --exclude=templates --ignore-missing-imports --implicit-reexport --strict ${SRC_DIRS}

test-unit: ## Run unit tests
python -m unittest discover tests

format: ## Format code automatically.
black $(BLACK_OPTS)

Expand Down
1 change: 1 addition & 0 deletions changelog.d/20241122_121506_faraz.maqsood_sumac.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- [Improvement] Move is_docker_rootless method related to elasticsearch from tutor core to tutor-discovery. (by @Faraz32123)
Empty file added tests/__init__.py
Empty file.
26 changes: 26 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import subprocess
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice job moving the unit tests here!

import unittest
from unittest.mock import MagicMock, patch

from tutordiscovery import utils


class UtilsTests(unittest.TestCase):
@patch("subprocess.run")
def test_is_docker_rootless(self, mock_run: MagicMock) -> None:
# Mock rootless `docker info` output
utils.is_docker_rootless.cache_clear()
mock_run.return_value.stdout = "some prefix\n rootless foo bar".encode("utf-8")
self.assertTrue(utils.is_docker_rootless())

# Mock regular `docker info` output
utils.is_docker_rootless.cache_clear()
mock_run.return_value.stdout = "some prefix, regular docker".encode("utf-8")
self.assertFalse(utils.is_docker_rootless())

@patch("subprocess.run")
def test_is_docker_rootless_podman(self, mock_run: MagicMock) -> None:
"""Test the `is_docker_rootless` when podman is used or any other error with `docker info`"""
utils.is_docker_rootless.cache_clear()
mock_run.side_effect = subprocess.CalledProcessError(1, "docker info")
self.assertFalse(utils.is_docker_rootless())
5 changes: 5 additions & 0 deletions tutordiscovery/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from tutor.__about__ import __version_suffix__

from .__about__ import __version__
from .utils import is_docker_rootless

# Handle version suffix in nightly mode, just like tutor core
if __version_suffix__:
Expand Down Expand Up @@ -127,6 +128,10 @@ def _mount_course_discovery_on_build(
("discovery/apps", "plugins"),
],
)
# Template variables
tutor_hooks.Filters.ENV_TEMPLATE_VARIABLES.add_item(
("is_docker_rootless", is_docker_rootless),
)
# Load patches from files
for path in glob(
os.path.join(
Expand Down
16 changes: 16 additions & 0 deletions tutordiscovery/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import subprocess
from functools import lru_cache


@lru_cache(maxsize=None)
def is_docker_rootless() -> bool:
"""
A helper function to determine if Docker is running in rootless mode.

- https://docs.docker.com/engine/security/rootless/
"""
try:
results = subprocess.run(["docker", "info"], capture_output=True, check=True)
return "rootless" in results.stdout.decode()
except subprocess.CalledProcessError:
return False