Salt: Feature: state module dockerng.image_present and salt.modules.dockerng.create module to support docker container log-driver options.

Created on 23 Jan 2016  路  26Comments  路  Source: saltstack/salt

Conainer logging support has been added to docker recently: https://docs.docker.com/engine/reference/logging/overview/

It'd be nice to have the following docker parameters supported in aforementioned salt modules:

--log-driver=[log driver]
--log-opt [foo=bar] --log-opt [baz=quux] ...
Feature P2 Platform State Module help-wanted

Most helpful comment

+1 for needing this feature, not having the ability to start docker containers with custom logging modules is painful ATM!

All 26 comments

@zerthimon, thanks for reporting.

A workaround to add support for log-config:

  1. overload default dockerng modules:
copy /usr/lib/python2.7/dist-packages/salt/modules/dockerng.py to your salt tree /srv/salt/_modules
copy/usr/lib/python2.7/dist-packages/salt/states/dockerng.py to your salt tree /srv/salt/_states

The paths may differ in your distry

  1. edit /srv/salt/_modules/dockerng.py and add under VALID_CREATE_OPTS = {
    'log_config': {
        'validator': 'dict',
        'path': 'HostConfig:LogConfig',
    },
  1. edit srv/salt/_states/dockerng.py and add AFTER the import from salt.modules.dockerng import ( ....)
VALID_CREATE_OPTS['log_config'] = {
   'validator': 'dict',
   'path': 'HostConfig:LogConfig',
}
  1. run salt '*' saltutil.sync_modules to sync your modules
  2. run salt '*' saltutil.sync_states to sync your states
  3. you can now add your logging config to dockerng.create in your sls like this:
    - log_config:
        Type: syslog
        Config:
          syslog-facility: local6
          syslog-tag: my_tag

P.S: Works 4 Me

@zerthimon, are you able to send in a pull request with that change?

@jfindlay sure, if you think that's a good change ...
Cos, I was thinking of a more friendly way of supporting these parameters, like specifying individual values like:

 - log-driver: syslog
 - log-facility: local6
 - ...

On the other hand, that's exactly how volume binds and restart policies are implemented.... (a user has to construct a valid dict the docker API expects)

@zerthimon, I'm not too familiar with the docker code, but that's a good point.

HI @zerthimon

I'm willing to help with this one. If I made some modifications based on what you referer in salt way. Are you able to test the code?

Thanks.

@abednarik, yep I can test the code.

Great, many thanks @zerthimon. I will let you known when PR is ready to test.

@abednarik @zerthimon
Hi! What about PR? This feature is really neeeded :)

HI @LawYard

I didn't get the time to work on this, sorry. Will see what I can do during this week.

Cheers

Found a bug - if log_config not used in sls we have an error:

[DEBUG   ] dockerng.running: Analysis of container 'test-container' 
after creation/replacement reveals the following changes still need 
to be made: {'log_config': {'new': None, 'old': {u'Config': {}, u'Type': u'json-file'}}}

Problem is in sending default state with 'new': None state, but default for docker container is {u'Config': {}, u'Type': u'json-file'}.

so, fix for this issue is setting default parameter:

   'validator': 'dict',
   'path': 'HostConfig:LogConfig',
   'default': {'Type': 'json-file', 'Config': {}}

@LawYard good catch!
Thanks for sharing.

+1 for needing this feature, not having the ability to start docker containers with custom logging modules is painful ATM!

Hello everyone,
Are there any workarounds for dockerio?
Besides just executing plane docker command.

I have something in the works, but ran into two problems that I need to work out:

  1. Docker remote API doesn't expose log driver/options. This is critical, since the default that people assume of json-file is not always true, since that can be changed as a docker daemon startup option. I will need to create a patch to the docker engine to allow querying of this information.
  2. Setting log options to a log driver as an empty dictionary does not get honored. Not sure if this is a bug in my dockerng code, docker-py or docker engine.

Hopefully, I can get something working by the end of this week.

Hi all, when this is going to be official released? I applied changes from this commit as a monkey patch and didnt' work.

app-container-running:
  dockerng.running:
    - name: anyname
    - image: anyimage:tag
    - force: True
    - restart_policy: always
    - log_conf:
       type: json-file
       config:
         max-file: '10'

Error Message from dockerng.running
Comment: The following arguments are invalid: config, log_conf, type

Master Report

Salt Version:
           Salt: 2016.3.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.2.2
       dateutil: 1.5
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-93-generic
         system: Linux
        version: Ubuntu 14.04 trusty

Minion Report

Salt Version:
           Salt: 2016.3.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-93-generic
         system: Linux
        version: Ubuntu 14.04 trusty

@danieloliveira079 Do you try to use this opts with proper case? I mean: Type, Config instead of type and config?

And u must use log_config, not log_conf

@LawYard yes same error. But the strange is the error massage that says the argument is invalid.

Using:

    - log_conf:
       Type: json-file
       Config:
         max-file: '10'

Comment: The following arguments are invalid: log_conf

Or:

    - log_config:
       Type: json-file
       Config:
         max-file: '10'

Comment: The following arguments are invalid: log_config

Even both options are generating errors I am not sure what would be the right one. In the dockerng module and state always reference to log_config but in the example provided in dockerng line 3053 says log_conf.

I put my modified files under /srv/salt/modules and /srv/salt/states and run:
salt '*' saltutil.sync_states
salt '*' saltutil.sync_modules

I could notice that output respectively:

minion1:
    - states.dockerng
minion1:
    - modules.dockerng

Thank you for you help. Any suggestion will be more than welcome. My use case using Salt Stack is really nice but that SysLog option is crucial to keep up using Salt for containers orchestration.

I used file.patch to apply this commit to the state and module dockerng.py files on each minion, followed by a minion restart.

@xbglowx can you post here please your final .sls file?

I'm in the same boat as daniel. I used the original 2 files (model and state) as provided in the PR. I monkey patched my existing dockerng module and state file. Nothing I do can get me past the

Comment: The following arguments are invalid: log_config

I have tried :

- log_config:
  Type: syslog
  Config:
    syslog-facility: local6
    syslog-tag: my_tag

and

  - log_config:
    Type: awslogs
    Config:
      awslogs-region: eu-west-1a
      awslogs-group: /cloud_dev/master/internal
      awslogs-stream: web

The config above requires 2 levels of indention (double tab). Is this normal for dictionaries as childs of lists in salt ? I 3-4 hours trying different things.

Support for this is allready in docker-py. Why is it not included in dockerng?

It seems like this feature will be included in the upcoming release:
https://github.com/saltstack/salt/blob/2016.11/salt/modules/dockerng.py#L543

Was there any update on this, I am testing this with saltstack 2016.11.1 and receive the same error as above:

Function: dockerng.running
      Result: False
     Comment: The following arguments are invalid: Type

with the following config:

dockerng.running:
...
    - log_config:
      Type: fluentd
Was this page helpful?
0 / 5 - 0 ratings

Related issues

golmaal picture golmaal  路  3Comments

twangboy picture twangboy  路  3Comments

erwindon picture erwindon  路  3Comments

qiushics picture qiushics  路  3Comments

Arguros picture Arguros  路  3Comments