Skip to content
This repository has been archived by the owner on Mar 23, 2019. It is now read-only.

Build cache not hit #694

Closed
re-mscho opened this issue Aug 11, 2017 · 5 comments · Fixed by #695
Closed

Build cache not hit #694

re-mscho opened this issue Aug 11, 2017 · 5 comments · Fixed by #695
Assignees
Labels

Comments

@re-mscho
Copy link

ISSUE TYPE
  • Bug Report
container.yml
...
services:
  service_a:
    roles:
      - role_a
      - role: role_b
        variable_for_role_b: any_value

  service_b:
    roles:
      - role_a
      - role: role_b
        variable_for_role_b: different_value
OS / ENVIRONMENT
Ansible Container, version 0.9.2rc0
Linux, ac-controller, 4.4.0-38-generic, #57-Ubuntu SMP Tue Sep 6 15:42:33 UTC 2016, x86_64
2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] /usr/bin/python
{
  "ContainersPaused": 0, 
  "Labels": null, 
  "CgroupDriver": "cgroupfs", 
  "ContainersRunning": 6, 
  "ContainerdCommit": {
    "Expected": "cfb82a876ecc11b5ca0977d1733adbe58599088a", 
    "ID": "cfb82a876ecc11b5ca0977d1733adbe58599088a"
  }, 
  "InitBinary": "docker-init", 
  "NGoroutines": 63, 
  "Swarm": {
    "ControlAvailable": false, 
    "NodeID": "", 
    "Error": "", 
    "RemoteManagers": null, 
    "LocalNodeState": "inactive", 
    "NodeAddr": ""
  }, 
  "LoggingDriver": "json-file", 
  "OSType": "linux", 
  "HttpProxy": "", 
  "Runtimes": {
    "runc": {
      "path": "docker-runc"
    }
  }, 
  "DriverStatus": [
    [
      "Root Dir", 
      "/var/lib/docker/aufs"
    ], 
    [
      "Backing Filesystem", 
      "extfs"
    ], 
    [
      "Dirs", 
      "55"
    ], 
    [
      "Dirperm1 Supported", 
      "true"
    ]
  ], 
  "OperatingSystem": "Ubuntu 16.04.1 LTS", 
  "Containers": 7, 
  "HttpsProxy": "", 
  "BridgeNfIp6tables": true, 
  "MemTotal": 4143484928, 
  "SecurityOptions": [
    "name=apparmor", 
    "name=seccomp,profile=default"
  ], 
  "Driver": "aufs", 
  "IndexServerAddress": "https://index.docker.io/v1/", 
  "ClusterStore": "", 
  "InitCommit": {
    "Expected": "949e6fa", 
    "ID": "949e6fa"
  }, 
  "Isolation": "", 
  "SystemStatus": null, 
  "OomKillDisable": true, 
  "ClusterAdvertise": "", 
  "SystemTime": "2017-08-11T15:10:26.107662501+02:00", 
  "Name": "ac-controller", 
  "CPUSet": true, 
  "RegistryConfig": {
    "AllowNondistributableArtifactsCIDRs": [], 
    "Mirrors": [], 
    "IndexConfigs": {
      "docker.io": {
        "Official": true, 
        "Name": "docker.io", 
        "Secure": true, 
        "Mirrors": []
      }
    }, 
    "AllowNondistributableArtifactsHostnames": [], 
    "InsecureRegistryCIDRs": [
      "127.0.0.0/8"
    ]
  }, 
  "DefaultRuntime": "runc", 
  "ContainersStopped": 1, 
  "NCPU": 4, 
  "NFd": 66, 
  "Architecture": "x86_64", 
  "KernelMemory": true, 
  "CpuCfsQuota": true, 
  "Debug": false, 
  "ID": "JCF6:ZEHF:ZPSE:C3W2:IUPE:IED3:2CDZ:MGLY:G4EA:ZTK6:N7E3:ROKY", 
  "IPv4Forwarding": true, 
  "KernelVersion": "4.4.0-38-generic", 
  "BridgeNfIptables": true, 
  "NoProxy": "", 
  "LiveRestoreEnabled": false, 
  "ServerVersion": "17.06.0-ce", 
  "CpuCfsPeriod": true, 
  "ExperimentalBuild": false, 
  "MemoryLimit": true, 
  "SwapLimit": false, 
  "Plugins": {
    "Volume": [
      "local"
    ], 
    "Network": [
      "bridge", 
      "host", 
      "macvlan", 
      "null", 
      "overlay"
    ], 
    "Authorization": null, 
    "Log": [
      "awslogs", 
      "fluentd", 
      "gcplogs", 
      "gelf", 
      "journald", 
      "json-file", 
      "logentries", 
      "splunk", 
      "syslog"
    ]
  }, 
  "Images": 40, 
  "DockerRootDir": "/var/lib/docker", 
  "NEventsListener": 0, 
  "CPUShares": true, 
  "RuncCommit": {
    "Expected": "2d41c047c83e09a6d61d464906feb2a2f3c52aa4", 
    "ID": "2d41c047c83e09a6d61d464906feb2a2f3c52aa4"
  }
}
{
  "KernelVersion": "4.4.0-38-generic", 
  "Arch": "amd64", 
  "BuildTime": "2017-06-23T21:19:04.990631145+00:00", 
  "ApiVersion": "1.30", 
  "Version": "17.06.0-ce", 
  "MinAPIVersion": "1.12", 
  "GitCommit": "02c1d87", 
  "Os": "linux", 
  "GoVersion": "go1.8.3"
}
SUMMARY

When using the same role in two services, the cached role should be used by both services. This doesn't work if the services are using additional roles with different default variables.
I think this is a side effect of #678 because it worked as expected before this change:
In container/core.py in line 665 the service default variables are now used to generate the hash for
caching:

...
      service_defaults = json.dumps(service['defaults']) if service.get('defaults') else ''
      fingerprint_hash = hashlib.sha256('%s::%s' % (cur_image_id, service_defaults))
...

The default variables of a service contain the default variables from all roles of a service.
see container/config.py line 332:

...
   def _process_services(self):
        services = yaml.compat.ordereddict()
        for service, service_data in self._config.get('services', yaml.compat.ordereddict()).items():
            logger.debug('Processing service...', service=service, service_data=service_data)
            processed = yaml.compat.ordereddict()
            service_defaults = self.defaults.copy()
            for idx in range(len(service_data.get('volumes', []))):
                # To mount the project directory, let users specify `$PWD` and
                # have that filled in with the project path
                service_data['volumes'][idx] = re.sub(r'\$(PWD|\{PWD\})', self._config['settings'].get('pwd'),
                                                      service_data['volumes'][idx])
            for role_spec in service_data.get('roles', []):
                if isinstance(role_spec, dict):
                    # A role with parameters to run it with
                    role_spec_copy = copy.deepcopy(role_spec)
                    role_name = role_spec_copy.pop('role')
                    role_args = role_spec_copy
                else:
                    role_name = role_spec
                    role_args = {}
                role_metadata = get_metadata_from_role(role_name)
                processed.update(role_metadata, relax=True)
                service_defaults.update(get_defaults_from_role(role_name),
                                        relax=True)
                service_defaults.update(role_args, relax=True)
            processed.update(service_data, relax=True)
            logger.debug('Rendering service keys from defaults', service=service, defaults=service_defaults)
            services[service] = self._process_section(
                processed,
                templar=Templar(loader=None, variables=service_defaults)
            )
            services[service]['defaults'] = service_defaults
        self.services = services
...

So when there are different variables in two services coming form additional roles, than no cache of any role will be used in this service anymore.

STEPS TO REPRODUCE

Define two services which are sharing the same role role_a.
Add at least a second role role_b with different variables in both services.

EXPECTED RESULTS

The first role role_a should be cached and used for both services.
The second role role_b shouldn't be cached because the variables are different.

ACTUAL RESULTS

The cache for role_a isn't used.

@chouseknecht chouseknecht self-assigned this Aug 11, 2017
@chouseknecht
Copy link
Contributor

@re-mscho

Thanks for catching and reporting this. Attempting to reproduce it now.

@chouseknecht
Copy link
Contributor

chouseknecht commented Aug 11, 2017

@re-mscho

OK. I'm able to reproduce. I think the issue is the change to line #683 in core.py, where I include the service level defaults in the fingerprint.

When I remove that change, and run the build for the first time, it runs role_a and role_b to build the service_a. As it moves to service_b, it recognizes that it already has a layer that can be used, and skips role_a.

@re-mscho
Copy link
Author

re-mscho commented Aug 11, 2017

@chouseknecht
I think so too. Maybe only variables defined directly for a role inside the service definition should be incorporated into the cache key.
When no variables are incorporated, than also role_b would be cached and service_b would be wrong because the variable variable_for_role_b: different_value will be ignored.

@chouseknecht
Copy link
Contributor

@re-mscho

I think we're OK there, because variables that are part of the role declaration at the service level are included in the fingerprint in get_role_fingerprint

@re-mscho
Copy link
Author

Ok, got it. Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants