Jeremiah Bonney pushed to branch master at BuildGrid / buildgrid
Commits:
- 
3fd9add5
by Jeremiah Bonney at 2019-02-07T17:01:49Z
- 
5c10c0ad
by Jeremiah Bonney at 2019-02-07T17:01:52Z
4 changed files:
- buildgrid/_app/settings/parser.py
- buildgrid/_app/settings/reference.yml
- buildgrid/server/actioncache/storage.py
- tests/integration/action_cache_service.py
Changes:
| ... | ... | @@ -247,12 +247,13 @@ class Action(YamlFactory): | 
| 247 | 247 |        storage(:class:`buildgrid.server.cas.storage.storage_abc.StorageABC`): Instance of storage to use.
 | 
| 248 | 248 |        max_cached_refs(int): Max number of cached actions.
 | 
| 249 | 249 |        allow_updates(bool): Allow updates pushed to CAS. Defaults to ``True``.
 | 
| 250 | +      cache_failed_actions(bool): Whether to store failed (non-zero exit code) actions. Default to ``True``.
 | |
| 250 | 251 |      """
 | 
| 251 | 252 |  | 
| 252 | 253 |      yaml_tag = u'!action-cache'
 | 
| 253 | 254 |  | 
| 254 | -    def __new__(cls, storage, max_cached_refs, allow_updates=True):
 | |
| 255 | -        return ActionCache(storage, max_cached_refs, allow_updates)
 | |
| 255 | +    def __new__(cls, storage, max_cached_refs, allow_updates=True, cache_failed_actions=True):
 | |
| 256 | +        return ActionCache(storage, max_cached_refs, allow_updates, cache_failed_actions)
 | |
| 256 | 257 |  | 
| 257 | 258 |  | 
| 258 | 259 |  class Reference(YamlFactory):
 | 
| ... | ... | @@ -74,6 +74,9 @@ instances: | 
| 74 | 74 |          ##
 | 
| 75 | 75 |          # Whether or not writing to the cache is allowed.
 | 
| 76 | 76 |          allow-updates: true
 | 
| 77 | +        ##
 | |
| 78 | +        # Whether failed actions (non-zero exit code) are stored
 | |
| 79 | +        cache-failed-actions: true
 | |
| 77 | 80 |  | 
| 78 | 81 |        - !execution
 | 
| 79 | 82 |          ##
 | 
| ... | ... | @@ -26,6 +26,18 @@ from ..referencestorage.storage import ReferenceCache | 
| 26 | 26 |  | 
| 27 | 27 |  class ActionCache(ReferenceCache):
 | 
| 28 | 28 |  | 
| 29 | +    def __init__(self, storage, max_cached_refs, allow_updates=True, cache_failed_actions=True):
 | |
| 30 | +        """ Initialises a new ActionCache instance.
 | |
| 31 | + | |
| 32 | +        Args:
 | |
| 33 | +            storage (StorageABC): storage backend instance to be used. Passed to ReferenceCache
 | |
| 34 | +            max_cached_refs (int): maximum number of entries to be stored. Passed to ReferenceCache
 | |
| 35 | +            allow_updates (bool): allow the client to write to storage. Passed to ReferenceCache
 | |
| 36 | +            cache_failed_actions (bool): cache actions with non-zero exit codes.
 | |
| 37 | +        """
 | |
| 38 | +        super().__init__(storage, max_cached_refs, allow_updates)
 | |
| 39 | +        self._cache_failed_actions = cache_failed_actions
 | |
| 40 | + | |
| 29 | 41 |      def register_instance_with_server(self, instance_name, server):
 | 
| 30 | 42 |          server.add_action_cache_instance(self, instance_name)
 | 
| 31 | 43 |  | 
| ... | ... | @@ -34,8 +46,9 @@ class ActionCache(ReferenceCache): | 
| 34 | 46 |          return self.get_action_reference(key)
 | 
| 35 | 47 |  | 
| 36 | 48 |      def update_action_result(self, action_digest, action_result):
 | 
| 37 | -        key = self._get_key(action_digest)
 | |
| 38 | -        self.update_reference(key, action_result)
 | |
| 49 | +        if self._cache_failed_actions or action_result.exit_code == 0:
 | |
| 50 | +            key = self._get_key(action_digest)
 | |
| 51 | +            self.update_reference(key, action_result)
 | |
| 39 | 52 |  | 
| 40 | 53 |      def _get_key(self, action_digest):
 | 
| 41 | 54 |          return (action_digest.hash, action_digest.size_bytes) | 
| ... | ... | @@ -85,3 +85,37 @@ def test_disabled_update_action_result(context): | 
| 85 | 85 |      ac_service.UpdateActionResult(request, context)
 | 
| 86 | 86 |  | 
| 87 | 87 |      context.set_code.assert_called_once_with(grpc.StatusCode.UNIMPLEMENTED)
 | 
| 88 | + | |
| 89 | + | |
| 90 | +def test_disabled_cache_failed_actions(cas, context):
 | |
| 91 | +    disabled_failed_actions = ActionCache(cas, 50, True, False)
 | |
| 92 | +    with mock.patch.object(service, 'remote_execution_pb2_grpc'):
 | |
| 93 | +        ac_service = ActionCacheService(server)
 | |
| 94 | +        ac_service.add_instance("", disabled_failed_actions)
 | |
| 95 | + | |
| 96 | +    failure_action_digest = remote_execution_pb2.Digest(hash='failure', size_bytes=4)
 | |
| 97 | + | |
| 98 | +    # Add a non-zero exit code ActionResult to the cache
 | |
| 99 | +    action_result = remote_execution_pb2.ActionResult(stdout_raw=b'Failed', exit_code=1)
 | |
| 100 | +    request = remote_execution_pb2.UpdateActionResultRequest(action_digest=failure_action_digest,
 | |
| 101 | +                                                             action_result=action_result)
 | |
| 102 | +    ac_service.UpdateActionResult(request, context)
 | |
| 103 | + | |
| 104 | +    # Check that before adding the ActionResult, attempting to fetch it fails
 | |
| 105 | +    request = remote_execution_pb2.GetActionResultRequest(instance_name="",
 | |
| 106 | +                                                          action_digest=failure_action_digest)
 | |
| 107 | +    ac_service.GetActionResult(request, context)
 | |
| 108 | +    context.set_code.assert_called_once_with(grpc.StatusCode.NOT_FOUND)
 | |
| 109 | + | |
| 110 | +    success_action_digest = remote_execution_pb2.Digest(hash='success', size_bytes=4)
 | |
| 111 | + | |
| 112 | +    # Now add a zero exit code Action result to the cache, and check that fetching
 | |
| 113 | +    # it is successful
 | |
| 114 | +    success_action_result = remote_execution_pb2.ActionResult(stdout_raw=b'Successful')
 | |
| 115 | +    request = remote_execution_pb2.UpdateActionResultRequest(action_digest=success_action_digest,
 | |
| 116 | +                                                             action_result=success_action_result)
 | |
| 117 | +    ac_service.UpdateActionResult(request, context)
 | |
| 118 | +    request = remote_execution_pb2.GetActionResultRequest(instance_name="",
 | |
| 119 | +                                                          action_digest=success_action_digest)
 | |
| 120 | +    fetched_result = ac_service.GetActionResult(request, context)
 | |
| 121 | +    assert fetched_result.stdout_raw == success_action_result.stdout_raw | 
