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

[dockerng] Disable notset check #31455

Merged
merged 2 commits into from Feb 25, 2016
Merged

Conversation

ticosax
Copy link
Contributor

@ticosax ticosax commented Feb 24, 2016

_api_mismatch() was a good idea.

But impractical since dockerd doesn't necessarily return all configuration items.

/cc @terminalmage

fixes #29727

But impractical since dockerd doesn't necessarily return all configuration items.
@oeuftete
Copy link
Contributor

@ticosax I tested this change against the 2015.8 branch (using my own fork) on my work's states. Getting some issues when I run a subsequent highstate after my initial containers are created. Docker 1.10.2, docker-py 1.5.0. I can try to set up a minimal state if necessary to reproduce:

[ERROR   ] Uncaught exception "'NoneType' object is not iterable" encountered while comparing existing container against desired configuration. Exception info follows:
  File "/usr/lib/python2.7/dist-packages/salt/states/dockerng.py", line 1561, in running
    defaults_from_image)
  File "/usr/lib/python2.7/dist-packages/salt/states/dockerng.py", line 191, in _compare
    actual_ports = sorted(actual_data)

@cachedout
Copy link
Contributor

Let's hold on this until we can work out the possible issue reported by @oeuftete

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 24, 2016
@rallytime
Copy link
Contributor

Some of the tests that were refactored/improved in #31326 are now failing in the merge-forward I am doing from 2016.3 to develop in #31462. I noticed that the test failures were similar to what this change is trying to solve (hitting the "Unable to compare configuration ..." message.) I just wanted to note that applying this change to my merge-forward branch fixes the test failures.

@ticosax
Copy link
Contributor Author

ticosax commented Feb 25, 2016

@rallytime thanks for the notification.

@oeuftete I could reproduce your use case with

sudo salt-call -l error state.single dockerng.running name=hello image=alpine:3.3 ports=[8000] cmd="/bin/echo 'Hello world'"

I pushed a new commit to that branch.

@rallytime rallytime removed the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 25, 2016
@cachedout
Copy link
Contributor

Go Go Jenkins!

rallytime pushed a commit that referenced this pull request Feb 25, 2016
@rallytime rallytime merged commit e85ae23 into saltstack:2015.8 Feb 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants