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

Jump to bottom

docker_container.running recreates containers with multiple links #44258

Closed
oarmstrong opened this issue Oct 24, 2017 · 2 comments · Fixed by #44262
Closed

docker_container.running recreates containers with multiple links #44258

oarmstrong opened this issue Oct 24, 2017 · 2 comments · Fixed by #44262
Labels
Bug broken, incorrect, or confusing behavior P3 Priority 3 RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module
Milestone
Approved

Comments

@oarmstrong
Copy link
Contributor

oarmstrong commented Oct 24, 2017 •
edited

Description of Issue/Question

When using the docker_container.running state and using multiple container links, the container is recreated at the next run. This is because the new and old state comparisons include the links in a different order. The new order appears to be non-deterministic and will occasionally match the order of the old order.

I noticed this after #42743 and it may be relevant.

My suggested fix is to order both new and old sets of links alphabetically before the comparison.

Setup

run_container_1:
  docker_container.running:
    - name: container1
    - image: nginx
run_container_2:
  docker_container.running:
    - name: container2
    - image: nginx
run_container_3:
  docker_container.running:
    - name: container3
    - image: nginx
run_load_balancer:
  docker_container.running:
    - name: loadbalancer
    - image: nginx
    - port_bindings:
      - 80:80
      - 443:443
    - links:
      - container1:container1
      - container2:container2
      - container3:container3

Steps to Reproduce Issue

Run the above state once and see the containers created. The next run will see the loadbalancer container recreated.

----------
          ID: run_load_balancer
    Function: docker_container.running
        Name: loadbalancer
      Result: None
     Comment: Container 'loadbalancer' would be replaced
     Started: 14:53:58.545750
    Duration: 1618.833 ms
     Changes:   
              ----------
              container:
                  ----------
                  HostConfig:
                      ----------
                      Links:
                          ----------
                          new:
                              - /container1:/container1
                              - /container2:/container2
                              - /container3:/container3
                          old:
                              - /container2:/container2
                              - /container3:/container3
                              - /container1:/container1

Versions Report

Salt Version:
           Salt: 2017.7.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  4 2017, 00:39:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4
 
System Versions:
           dist: centos 7.4.1708 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-693.5.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.4.1708 Core

No differences between master and minion.
@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 24, 2017

Seems you have a pretty good grasp on a fix. Mind submitting a PR?

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P3 Priority 3 RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. State-Module labels Oct 24, 2017
@Ch3LL Ch3LL added this to the Approved milestone Oct 24, 2017
@oarmstrong
Copy link
Contributor Author

I'll give it a shot! Might have something tomorrow.

@oarmstrong oarmstrong mentioned this issue Oct 24, 2017
Merged
rallytime pushed a commit to rallytime/salt that referenced this issue Oct 27, 2017
@oarmstrong
docker_container.running sort list of links
ae34a15 
When the docker_container.running module is comparing the defined state
and the current state of a container, the list of linked containers in
the current state is is a non-deterministic order. This results in a
container recreation even though the links are the same (just in a
different order).

This patches the comparison function to lexically sort both the existing
and desired list of links, making the comparison deterministic and not
recreating a container when the links have not changed.

Fixes saltstack#44258.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug broken, incorrect, or confusing behavior P3 Priority 3 RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module
Projects
None yet
Milestone
Approved
Development

Successfully merging a pull request may close this issue.

docker_container.running sort list of links
2 participants
@oarmstrong @Ch3LL