diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 055e9f3..6a2eea8 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -12,6 +12,8 @@ jobs: lint-unit: name: Lint Unit uses: charmed-kubernetes/workflows/.github/workflows/lint-unit.yaml@main + with: + python: "['3.8', '3.9', '3.10', '3.11']" needs: - call-inclusive-naming-check @@ -22,14 +24,15 @@ jobs: - lint-unit steps: - name: Check out code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Setup Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: 3.9 + python-version: "3.10" - name: Setup operator environment uses: charmed-kubernetes/actions-operator@main with: provider: lxd + juju-channel: 3/stable - name: Run integration test run: tox -e integration diff --git a/config.yaml b/config.yaml index 6539d13..745a8f2 100644 --- a/config.yaml +++ b/config.yaml @@ -84,6 +84,12 @@ options: default: description: | The HTTPS proxy the registry server should use to access the upstream registry. + storage-cache: + type: string + default: "inmemory" + description: | + Cache provider for image layer metadata. Valid options are "inmemory" or + "disabled". storage-delete: type: boolean default: false diff --git a/lib/charms/layer/docker_registry.py b/lib/charms/layer/docker_registry.py index 145fde2..0a996a6 100644 --- a/lib/charms/layer/docker_registry.py +++ b/lib/charms/layer/docker_registry.py @@ -15,6 +15,22 @@ from charms.reactive.helpers import any_file_changed, data_changed +def has_invalid_config(): + """Checks charm config for invalid values. + + If invalid values are found, return a list of the offending key(s). Otherwise, + return an empty list. + """ + charm_config = hookenv.config() + bad_config = [] + + storage_cache = charm_config.get("storage-cache", "") + if storage_cache not in ["inmemory", "disabled"]: + bad_config.append("storage-cache") + + return bad_config + + def configure_registry(): '''Recreate the docker registry config.yml.''' charm_config = hookenv.config() @@ -108,7 +124,7 @@ def configure_registry(): 'password': charm_config.get('cache-password', ''), } - # storage (https://docs.docker.com/registry/configuration/#storage) + # storage (https://distribution.github.io/distribution/about/configuration/#storage) # we must have 1 (and only 1) storage driver storage = {} if charm_config.get('storage-swift-authurl'): @@ -132,7 +148,6 @@ def configure_registry(): # If we're not swift, we're local. container_registry_path = '/var/lib/registry' storage['filesystem'] = {'rootdirectory': container_registry_path} - storage['cache'] = {'blobdescriptor': 'inmemory'} # Local storage is mounted from the host so images persist across # registry container restarts. @@ -143,6 +158,8 @@ def configure_registry(): storage['delete'] = {'enabled': True} if charm_config.get('storage-read-only'): storage['maintenance'] = {'readonly': {'enabled': True}} + if charm_config.get('storage-cache', 'inmemory') == 'inmemory': + storage['cache'] = {'blobdescriptor': 'inmemory'} registry_config['storage'] = storage os.makedirs(os.path.dirname(registry_config_file), exist_ok=True) diff --git a/reactive/docker_registry.py b/reactive/docker_registry.py index a77ebf9..099f34e 100644 --- a/reactive/docker_registry.py +++ b/reactive/docker_registry.py @@ -26,11 +26,17 @@ def start(): layer.status.maint('Configuring the registry.') - layer.docker_registry.configure_registry() - layer.docker_registry.start_registry() + bad_config = layer.docker_registry.has_invalid_config() + if bad_config: + layer.status.blocked(f"Invalid charm config: {', '.join(bad_config)}") + # NB: clear_flag just in case we were called from a handler where it is set. + clear_flag('charm.docker-registry.configured') + else: + layer.docker_registry.configure_registry() + layer.docker_registry.start_registry() - set_flag('charm.docker-registry.configured') - report_status() + set_flag('charm.docker-registry.configured') + report_status() @when('charm.docker-registry.configured') @@ -65,12 +71,21 @@ def config_changed(): charm_config = hookenv.config() name = charm_config.get('registry-name') - # If a provider gave us certs and http-host changed, make sure SANs are accurate + # If we have certs and http-host changed, ensure we'll refire the + # request_certificates handler. if ( is_flag_set('cert-provider.certs.available') and charm_config.changed('http-host') ): - request_certificates() + clear_flag('cert-provider.certs.available') + + # If we have connected clients and relevant config has changed, ensure we'll + # refire the configure_client handler. + if (is_flag_set('charm.docker-registry.client-configured') and + any((charm_config.changed('auth-basic-password'), + charm_config.changed('auth-basic-user'), + charm_config.changed('http-host')))): + clear_flag('charm.docker-registry.client-configured') # If our name changed, make sure we stop the old one if ( @@ -80,18 +95,7 @@ def config_changed(): name = charm_config.previous('registry-name') layer.docker_registry.stop_registry(name=name) - layer.docker_registry.configure_registry() - layer.docker_registry.start_registry() - - # Now that we reconfigured the registry, inform connected clients if - # anything changed that they should know about. - if (is_flag_set('charm.docker-registry.client-configured') and - any((charm_config.changed('auth-basic-password'), - charm_config.changed('auth-basic-user'), - charm_config.changed('http-host')))): - configure_client() - - report_status() + start() @when_not("endpoint.docker-registry.joined") diff --git a/tests/integration/test_docker_registry_integration.py b/tests/integration/test_docker_registry_integration.py index 1e37329..7461d86 100644 --- a/tests/integration/test_docker_registry_integration.py +++ b/tests/integration/test_docker_registry_integration.py @@ -46,9 +46,8 @@ async def test_push_image(ops_test): for unit in registry_units: action = await unit.run_action("push", image="python:3.9-slim", pull=True) output = await action.wait() # wait for result - assert output.data.get("status") == "completed" - assert output.data.get("results", {}).get("outcome") == "success" - assert output.data.get("results", {}).get("raw") == \ + assert output.status == "completed" + assert output.results.get("raw") == \ f"pushed {unit.public_address}:5000/python:3.9-slim" @@ -63,6 +62,5 @@ async def test_image_list(ops_test): for unit in registry_units: action = await unit.run_action("images", repository="python:3.9-slim") output = await action.wait() # wait for result - assert output.data.get("status") == "completed" - assert "python" in output.data.get("results", {}).get("output") - assert "3.9-slim" in output.data.get("results", {}).get("output") + assert output.status == "completed" + assert "python" in output.results.get("output") diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 9b736fc..f25a605 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,4 +1,4 @@ import charms.unit_test charms.unit_test.patch_reactive() -charms.unit_test.patch_module('charms.leadership') +charms.unit_test.patch_module("charms.leadership") diff --git a/tests/unit/test_docker_registry.py b/tests/unit/test_docker_registry.py index 6a651e0..243a836 100644 --- a/tests/unit/test_docker_registry.py +++ b/tests/unit/test_docker_registry.py @@ -1,4 +1,5 @@ from charms.unit_test import patch_fixture +from unittest import mock from charmhelpers.core import host # patched from charms import layer @@ -6,8 +7,8 @@ from reactive import docker_registry as handlers -start_registry = patch_fixture('charms.layer.docker_registry.start_registry') -stop_registry = patch_fixture('charms.layer.docker_registry.stop_registry') +start_registry = patch_fixture("charms.layer.docker_registry.start_registry") +stop_registry = patch_fixture("charms.layer.docker_registry.stop_registry") def test_series_upgrade(start_registry, stop_registry): @@ -28,3 +29,50 @@ def test_series_upgrade(start_registry, stop_registry): assert host.service_pause.call_count == 1 assert host.service_resume.call_count == 1 assert layer.status.blocked.call_count == 1 + + +@mock.patch("charms.layer.docker_registry._configure_local_client") +@mock.patch("charms.layer.docker_registry.host") +@mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") +@mock.patch("charms.layer.docker_registry.unitdata") +@mock.patch("charmhelpers.core.hookenv.config") +@mock.patch("os.makedirs", mock.Mock(return_value=0)) +def test_configure_registry(config, mock_kv, mock_write, mock_host, mock_lc): + # storage-cache: invalid value should not result in any cache structure + config.return_value = { + "log-level": "bananas", + "storage-cache": "bananas", + } + with mock.patch("charms.layer.docker_registry.yaml") as mock_yaml: + layer.docker_registry.configure_registry() + args, _ = mock_yaml.safe_dump.call_args_list[0] + assert "cache" not in args[0]["storage"] + + # storage-cache: valid value should result in a populated cache structure + config.return_value = { + "log-level": "bananas", + "storage-cache": "inmemory", + } + expected = { + "log": {"level": "bananas"}, + "storage": {"cache": {"blobdescriptor": "inmemory"}}, + } + with mock.patch("charms.layer.docker_registry.yaml") as mock_yaml: + layer.docker_registry.configure_registry() + args, _ = mock_yaml.safe_dump.call_args_list[0] + assert expected["storage"].items() <= args[0]["storage"].items() + + +@mock.patch("charmhelpers.core.hookenv.config") +def test_has_invalid_config(config): + # check valid config is valid + config.return_value = { + "storage-cache": "disabled", + } + assert not layer.docker_registry.has_invalid_config() + + # check for bad apples + config.return_value = { + "storage-cache": "bananas", + } + assert "storage-cache" in layer.docker_registry.has_invalid_config() diff --git a/tox.ini b/tox.ini index bc64fad..c028705 100644 --- a/tox.ini +++ b/tox.ini @@ -28,5 +28,5 @@ deps = pytest pytest-operator ipdb - juju < 3.0 # https://github.com/juju/python-libjuju/issues/719 + juju commands = pytest --tb native --show-capture=no --log-cli-level=INFO -s {posargs} {toxinidir}/tests/integration