diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index e30e03e3..2828d4ab 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -66,6 +66,44 @@ When tokens are received from an issuer during redemption, these are the only public keys which will satisfy the redeemer and cause the tokens to be made available to the client to be spent. Tokens received with any other public key will be sequestered and will *not* be spent until some further action is taken. +lease.crawl-interval.mean +~~~~~~~~~~~~~~~~~~~~~~~~~ + +This item controls the frequency at which the lease maintenance crawler runs. +The lease maintenance crawler visits all shares and renews their leases if necessary. +The crawler will run at random intervals. +The client will try to make the average (mean) interval between runs equal to this setting. +The value is an integer number of seconds. +For example to run on average every 26 days:: + + [storageclient.plugins.privatestorageio-zkapauthz-v1] + lease.crawl-interval.mean = 2246400 + + +lease.crawl-interval.range +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This item also controls the frequency of lease maintenance crawler runs. +The random intervals between runs have a uniform distribution with this item's value as its range. +The value is an integer number of seconds. +For example to make all intervals fall within a 7 day period:: + + [storageclient.plugins.privatestorageio-zkapauthz-v1] + lease.crawl-interval.range = 302400 + + +lease.min-time-remaining +~~~~~~~~~~~~~~~~~~~~~~~~ + +This item controls the lease renewal behavior of the lease maintenance crawler. +It specifies an amount of time left on a lease. +If the crawler encounters a lease with less time left than this then it will renew the lease. +The value is an integer number of seconds. +For example to renew leases on all shares which will expire in less than one week:: + + [storageclient.plugins.privatestorageio-zkapauthz-v1] + lease.min-time-remaining = 604800 + Server ------ diff --git a/src/_zkapauthorizer/_plugin.py b/src/_zkapauthorizer/_plugin.py index cf331c69..8ae7fbeb 100644 --- a/src/_zkapauthorizer/_plugin.py +++ b/src/_zkapauthorizer/_plugin.py @@ -18,7 +18,7 @@ """ import random -from datetime import datetime, timedelta +from datetime import datetime from functools import partial from weakref import WeakValueDictionary @@ -33,6 +33,7 @@ from zope.interface import implementer from .api import ZKAPAuthorizerStorageClient, ZKAPAuthorizerStorageServer +from .config import lease_maintenance_from_tahoe_config from .controller import get_redeemer from .lease_maintenance import ( SERVICE_NAME, @@ -220,26 +221,15 @@ def get_now(): store = storage_server._get_store(node_config) + maint_config = lease_maintenance_from_tahoe_config(node_config) + # Create the operation which performs the lease maintenance job when # called. maintain_leases = maintain_leases_from_root( get_root_nodes=partial(get_root_nodes, client_node, node_config), storage_broker=client_node.get_storage_broker(), secret_holder=client_node._secret_holder, - # The greater the min lease remaining time, the more of each lease - # period is "wasted" by renewing the lease before it has expired. The - # premise of ZKAPAuthorizer's use of leases is that if they expire, - # the storage server is free to reclaim the storage by forgetting - # about the share. However, since we do not know of any - # ZKAPAuthorizer-enabled storage grids which will garbage collect - # shares when leases expire, we have no reason not to use a zero - # duration here - for now. - # - # In the long run, storage servers must run with garbage collection - # enabled. Ideally, before that happens, we will have a system that - # doesn't involve trading of wasted lease time against reliability of - # leases being renewed before the shares are garbage collected. - min_lease_remaining=timedelta(seconds=0), + min_lease_remaining=maint_config.min_lease_remaining, progress=store.start_lease_maintenance, get_now=get_now, ) @@ -252,6 +242,7 @@ def get_now(): reactor, last_run_path, random, + lease_maint_config=maint_config, ) diff --git a/src/_zkapauthorizer/config.py b/src/_zkapauthorizer/config.py new file mode 100644 index 00000000..064ff522 --- /dev/null +++ b/src/_zkapauthorizer/config.py @@ -0,0 +1,105 @@ +# Copyright 2019 PrivateStorage.io, LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Helpers for reading values from the Tahoe-LAFS configuration. +""" + +from datetime import timedelta + +try: + from typing import Optional +except ImportError: + pass + +from allmydata.node import _Config + +from .lease_maintenance import LeaseMaintenanceConfig + + +class _EmptyConfig(object): + """ + Weakly pretend to be a Tahoe-LAFS configuration object with no + configuration. + """ + + def get_config(self, section, option, default=object(), boolean=False): + return default + + +empty_config = _EmptyConfig() + + +def lease_maintenance_from_tahoe_config(node_config): + # type: (_Config) -> LeaseMaintenanceConfig + """ + Return a ``LeaseMaintenanceConfig`` representing the values from the given + configuration object. + """ + return LeaseMaintenanceConfig( + crawl_interval_mean=_read_duration( + node_config, + u"lease.crawl-interval.mean", + timedelta(days=26), + ), + crawl_interval_range=_read_duration( + node_config, + u"lease.crawl-interval.range", + timedelta(days=4), + ), + # The greater the min lease remaining time, the more of each lease + # period is "wasted" by renewing the lease before it has expired. The + # premise of ZKAPAuthorizer's use of leases is that if they expire, + # the storage server is free to reclaim the storage by forgetting + # about the share. However, since we do not know of any + # ZKAPAuthorizer-enabled storage grids which will garbage collect + # shares when leases expire, we have no reason not to use a zero + # duration here - for now. + # + # In the long run, storage servers must run with garbage collection + # enabled. Ideally, before that happens, we will have a system that + # doesn't involve trading of wasted lease time against reliability of + # leases being renewed before the shares are garbage collected. + # + # Also, since this is configuration, you can set it to something else + # if you want. + min_lease_remaining=_read_duration( + node_config, + u"lease.min-time-remaining", + timedelta(days=0), + ), + ) + + +def _read_duration(cfg, option, default): + """ + Read an integer number of seconds from the ZKAPAuthorizer section of a + Tahoe-LAFS config. + + :param cfg: The Tahoe-LAFS config object to consult. + :param option: The name of the option to read. + + :return: ``None`` if the option is missing, otherwise the parsed duration + as a ``timedelta``. + """ + # type: (_Config, str) -> Optional[timedelta] + section_name = u"storageclient.plugins.privatestorageio-zkapauthz-v1" + value_str = cfg.get_config( + section=section_name, + option=option, + default=None, + ) + if value_str is None: + return default + return timedelta(seconds=int(value_str)) diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py index a5763b3d..49adebf5 100644 --- a/src/_zkapauthorizer/lease_maintenance.py +++ b/src/_zkapauthorizer/lease_maintenance.py @@ -21,6 +21,11 @@ from errno import ENOENT from functools import partial +try: + from typing import Any, Dict +except ImportError: + pass + import attr from allmydata.interfaces import IDirectoryNode, IFilesystemNode from allmydata.util.hashutil import ( @@ -315,12 +320,17 @@ class _FuzzyTimerService(Service): :ivar IReactorTime reactor: A Twisted reactor to use to schedule runs of the operation. + + :ivar get_config: A function to call to return the service's + configuration. The configuration is represented as a service-specific + object. """ name = attr.ib() operation = attr.ib() initial_interval = attr.ib() sample_interval_distribution = attr.ib() + get_config = attr.ib() # type: () -> Any reactor = attr.ib() def startService(self): @@ -358,8 +368,7 @@ def lease_maintenance_service( reactor, last_run_path, random, - interval_mean=None, - interval_range=None, + lease_maint_config, ): """ Get an ``IService`` which will maintain leases on ``root_node`` and any @@ -378,18 +387,14 @@ def lease_maintenance_service( :param random: An object like ``random.Random`` which can be used as a source of scheduling delay. - :param timedelta interval_mean: The mean time between lease renewal checks. - - :param timedelta interval_range: The range of the uniform distribution of - lease renewal checks (centered on ``interval_mean``). + :param lease_maint_config: Configuration for the tweakable lease + maintenance parameters. :param maintain_leases: A no-argument callable which performs a round of lease-maintenance. The resulting service calls this periodically. """ - if interval_mean is None: - interval_mean = timedelta(days=26) - if interval_range is None: - interval_range = timedelta(days=4) + interval_mean = lease_maint_config.crawl_interval_mean + interval_range = lease_maint_config.crawl_interval_range halfrange = interval_range / 2 def sample_interval_distribution(): @@ -420,6 +425,9 @@ def sample_interval_distribution(): timedelta(0), ) + def get_lease_maint_config(): + return lease_maint_config + return _FuzzyTimerService( SERVICE_NAME, lambda: bracket( @@ -432,10 +440,64 @@ def sample_interval_distribution(): ), initial_interval, sample_interval_distribution, + get_lease_maint_config, reactor, ) +@attr.s(frozen=True) +class LeaseMaintenanceConfig(object): + """ + Represent the configuration for a lease maintenance service. + + :ivar crawl_interval_mean: The mean time between lease renewal checks. + + :ivar crawl_interval_range: The range of the uniform distribution of lease + renewal checks (centered on ``interval_mean``). + + :ivar min_lease_remaining: The minimum amount of time remaining to allow + on a lease without renewing it. + """ + + crawl_interval_mean = attr.ib() # type: datetime.timedelta + crawl_interval_range = attr.ib() # type: datetime.timedelta + min_lease_remaining = attr.ib() # type: datetime.timedelta + + +def lease_maintenance_config_to_dict(lease_maint_config): + # type: (LeaseMaintenanceConfig) -> Dict[str, str] + return { + "lease.crawl-interval.mean": _format_duration( + lease_maint_config.crawl_interval_mean, + ), + "lease.crawl-interval.range": _format_duration( + lease_maint_config.crawl_interval_range, + ), + "lease.min-time-remaining": _format_duration( + lease_maint_config.min_lease_remaining, + ), + } + + +def _format_duration(td): + # type: (timedelta) -> str + return str(int(td.total_seconds())) + + +def _parse_duration(duration_str): + # type: (str) -> timedelta + return timedelta(seconds=int(duration_str)) + + +def lease_maintenance_config_from_dict(d): + # type: (Dict[str, str]) -> LeaseMaintenanceConfig + return LeaseMaintenanceConfig( + crawl_interval_mean=_parse_duration(d["lease.crawl-interval.mean"]), + crawl_interval_range=_parse_duration(d["lease.crawl-interval.range"]), + min_lease_remaining=_parse_duration(d["lease.min-time-remaining"]), + ) + + def write_time_to_path(path, when): """ Write an ISO8601 datetime string to a file. diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index d14a62df..6cf2806d 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -17,7 +17,7 @@ """ from base64 import b64encode, urlsafe_b64encode -from datetime import datetime +from datetime import datetime, timedelta from urllib import quote import attr @@ -47,6 +47,7 @@ from zope.interface import implementer from ..configutil import config_string_from_sections +from ..lease_maintenance import LeaseMaintenanceConfig, lease_maintenance_config_to_dict from ..model import ( DoubleSpend, Error, @@ -347,6 +348,55 @@ def client_errorredeemer_configurations(details): ) +def integer_seconds_timedeltas( + # Our intervals mostly want to be non-negative. + min_value=0, + # We can't make this value too large or it isn't convertable to a + # timedelta. Also, even values as large as this one are of + # questionable value for the durations we measure. + max_value=60 * 60 * 24 * 365, +): + """ + Build ``timedelta`` instances without a fractional seconds part. + """ + return integers( + min_value=min_value, + max_value=max_value, + ).map(lambda n: timedelta(seconds=n)) + + +def interval_means(): + """ + Build timedeltas representing the mean time between lease maintenance + runs. + """ + return integer_seconds_timedeltas() + + +def lease_maintenance_configurations(): + """ + Build LeaseMaintenanceConfig instances. + """ + return builds( + LeaseMaintenanceConfig, + interval_means(), + integer_seconds_timedeltas(), + integer_seconds_timedeltas(), + ) + + +def client_lease_maintenance_configurations(maint_configs=None): + """ + Build dictionaries representing the lease maintenance options that go into + the ZKAPAuthorizer client plugin section. + """ + if maint_configs is None: + maint_configs = lease_maintenance_configurations() + return maint_configs.map( + lambda lease_maint_config: lease_maintenance_config_to_dict(lease_maint_config), + ) + + def direct_tahoe_configs( zkapauthz_v1_configuration=client_dummyredeemer_configurations(), shares=just((None, None, None)), diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py index 156757ef..02e2230c 100644 --- a/src/_zkapauthorizer/tests/test_lease_maintenance.py +++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py @@ -31,7 +31,6 @@ builds, composite, dictionaries, - floats, integers, just, lists, @@ -55,10 +54,14 @@ from twisted.python.filepath import FilePath from zope.interface import implementer +from ..config import empty_config, lease_maintenance_from_tahoe_config from ..foolscap import ShareStat from ..lease_maintenance import ( + LeaseMaintenanceConfig, MemoryMaintenanceObserver, NoopMaintenanceObserver, + lease_maintenance_config_from_dict, + lease_maintenance_config_to_dict, lease_maintenance_service, maintain_leases_from_root, renew_leases, @@ -67,6 +70,8 @@ from .matchers import Provides, between, leases_current from .strategies import ( clocks, + interval_means, + lease_maintenance_configurations, node_hierarchies, posix_timestamps, sharenums, @@ -79,21 +84,7 @@ pass -def interval_means(): - return floats( - # It doesn't make sense to have a negative check interval mean. - min_value=0, - # We can't make this value too large or it isn't convertable to a - # timedelta. Also, even values as large as this one are of - # questionable value. - max_value=60 * 60 * 24 * 365, - ).map( - # By representing the result as a timedelta we avoid the cases where - # the lower precision of timedelta compared to float drops the whole - # value (anything between 0 and 1 microsecond). This is just one - # example of how working with timedeltas is nicer, in general. - lambda s: timedelta(seconds=s), - ) +default_lease_maint_config = lease_maintenance_from_tahoe_config(empty_config) def dummy_maintain_leases(): @@ -221,6 +212,23 @@ def storage_brokers(draw, clocks): ) +class LeaseMaintenanceConfigTests(TestCase): + """ + Tests related to ``LeaseMaintenanceConfig``. + """ + + @given(lease_maintenance_configurations()) + def test_config_roundtrip(self, config): + """ + ``LeaseMaintenanceConfig`` round-trips through + ``lease_maintenance_config_to_dict`` and + ``lease_maintenance_config_from_dict``. + """ + dumped = lease_maintenance_config_to_dict(config) + loaded = lease_maintenance_config_from_dict(dumped) + self.assertThat(loaded, Equals(config)) + + class LeaseMaintenanceServiceTests(TestCase): """ Tests for the service returned by ``lease_maintenance_service``. @@ -237,6 +245,7 @@ def test_interface(self, random): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) self.assertThat( service, @@ -265,8 +274,11 @@ def test_initial_interval(self, random, mean): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, - mean, - range_, + LeaseMaintenanceConfig( + mean, + range_, + timedelta(0), + ), ) service.startService() [maintenance_call] = clock.getDelayedCalls() @@ -308,8 +320,11 @@ def test_initial_interval_with_last_run(self, random, clock, mean, since_last_ru clock, last_run_path, random, - mean, - range_, + LeaseMaintenanceConfig( + mean, + range_, + timedelta(0), + ), ) service.startService() [maintenance_call] = clock.getDelayedCalls() @@ -355,6 +370,7 @@ def test_clean_up_when_stopped(self, random, clock): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) service.startService() self.assertThat( @@ -388,6 +404,7 @@ def maintain_leases(): clock, FilePath(self.useFixture(TempDir()).join("last-run")), random, + lease_maint_config=default_lease_maint_config, ) service.startService() [maintenance_call] = clock.getDelayedCalls() diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py index 78959a5d..0e87c89c 100644 --- a/src/_zkapauthorizer/tests/test_plugin.py +++ b/src/_zkapauthorizer/tests/test_plugin.py @@ -18,7 +18,6 @@ from __future__ import absolute_import -import tempfile from functools import partial from os import makedirs @@ -49,6 +48,7 @@ Equals, HasLength, IsInstance, + Matcher, MatchesAll, MatchesStructure, ) @@ -65,7 +65,7 @@ from .._storage_client import IncorrectStorageServerReference from ..controller import DummyRedeemer, IssuerConfigurationMismatch, PaymentController from ..foolscap import RIPrivacyPassAuthorizedStorageServer -from ..lease_maintenance import SERVICE_NAME +from ..lease_maintenance import SERVICE_NAME, LeaseMaintenanceConfig from ..model import NotEnoughTokens, VoucherStore from ..spending import GET_PASSES from .eliot import capture_logging @@ -74,8 +74,10 @@ from .strategies import ( announcements, client_dummyredeemer_configurations, + client_lease_maintenance_configurations, dummy_ristretto_keys, lease_cancel_secrets, + lease_maintenance_configurations, lease_renew_secrets, minimal_tahoe_configs, pass_counts, @@ -537,44 +539,38 @@ class LeaseMaintenanceServiceTests(TestCase): Tests for the plugin's initialization of the lease maintenance service. """ - def _created_test(self, get_config, servers_yaml, rootcap): - original_tempdir = tempfile.tempdir + def _create(self, get_config, servers_yaml, rootcap): + """ + Create a client node using ``create_client_from_config``. + + :param get_config: A function to call to get a Tahoe-LAFS config + object. + + :param servers_yaml: ``None`` or a string giving the contents for the + node's ``servers.yaml`` file. + :param rootcap: ``True`` to write some bytes to the node's ``rootcap`` + file, ``False`` otherwise. + """ tempdir = self.useFixture(TempDir()) nodedir = tempdir.join(b"node") privatedir = tempdir.join(b"node", b"private") makedirs(privatedir) config = get_config(nodedir, b"tub.port") - # Provide it a statically configured server to connect to. - config.write_private_config( - b"servers.yaml", - servers_yaml, - ) + if servers_yaml is not None: + # Provide it a statically configured server to connect to. + config.write_private_config( + b"servers.yaml", + servers_yaml, + ) if rootcap: config.write_private_config( b"rootcap", b"dddddddd", ) - try: - d = create_client_from_config(config) - self.assertThat( - d, - succeeded( - AfterPreprocessing( - lambda client: client.getServiceNamed(SERVICE_NAME), - Always(), - ), - ), - ) - finally: - # create_client_from_config (indirectly) rewrites tempfile.tempdir - # in a destructive manner that fails most of the rest of the test - # suite if we don't clean it up. We can't do this with a tearDown - # or a fixture or an addCleanup because hypothesis doesn't run any - # of those at the right time. :/ - tempfile.tempdir = original_tempdir + return create_client_from_config(config) @settings( deadline=None, @@ -589,7 +585,8 @@ def test_created(self, get_config, servers_yaml): maintenance service after it has at least one storage server to connect to. """ - return self._created_test(get_config, servers_yaml, rootcap=True) + d = self._create(get_config, servers_yaml, rootcap=True) + self.assertThat(d, succeeded(has_lease_maintenance_service())) @settings( deadline=None, @@ -603,7 +600,71 @@ def test_created_without_rootcap(self, get_config, servers_yaml): The lease maintenance service can be created even if no rootcap has yet been written to the client's configuration directory. """ - return self._created_test(get_config, servers_yaml, rootcap=False) + d = self._create(get_config, servers_yaml, rootcap=False) + self.assertThat(d, succeeded(has_lease_maintenance_service())) + + @given( + # First build the simple lease maintenance configuration object that + # represents the example to test. + lease_maintenance_configurations().flatmap( + # Then build a function that will get us a Tahoe configuration + # that includes at least that lease maintenance configuration. + lambda lease_maint_config: tahoe_configs( + zkapauthz_v1_configuration=client_lease_maintenance_configurations( + just(lease_maint_config), + ), + ).map( + # Then bundle up both pieces to pass to the function. By + # preserving the lease maintenance configuration model object + # and making it available to the test, the test logic is much + # simplified (eg, we don't have to read values out of the + # Tahoe configuration to figure out what example we're working + # on). + lambda get_config: (lease_maint_config, get_config), + ), + ), + ) + def test_values_from_configuration(self, config_objs): + """ + If values for lease maintenance parameters are supplied in the + configuration file then the lease maintenance service is created with + those values. + """ + lease_maint_config, get_config = config_objs + d = self._create(get_config, servers_yaml=None, rootcap=False) + self.assertThat( + d, + succeeded(has_lease_maintenance_configuration(lease_maint_config)), + ) + + +def has_lease_maintenance_service(): + """ + Return a matcher for a Tahoe-LAFS client object that has a lease + maintenance service. + """ + # type: () -> Matcher + return AfterPreprocessing( + lambda client: client.getServiceNamed(SERVICE_NAME), + Always(), + ) + + +def has_lease_maintenance_configuration(lease_maint_config): + """ + Return a matcher for a Tahoe-LAFS client object that has a lease + maintenance service with the given configuration. + """ + # type: (LeaseMaintenanceConfig) -> Matcher + def get_lease_maintenance_config(lease_maint_service): + return lease_maint_service.get_config() + + return AfterPreprocessing( + lambda client: get_lease_maintenance_config( + client.getServiceNamed(SERVICE_NAME), + ), + Equals(lease_maint_config), + ) class LoadSigningKeyTests(TestCase):