Potential fix for #36021

[mirceaulinic]

What does this PR do?

Hopefully will fix #36021 - works fine for proxy minions, but let's make sure it won't blow others. I would like to ask an admin to guide me and tell if data['fun_args'] should be a tuple or a list, or perhaps does not matter?

[mirceaulinic]

Before:

(virtualenv)root@36netops3:~# salt-run state.event pretty=True
/usr/local/salt/virtualenv/lib/python2.7/site-packages/salt/grains/core.py:1493: DeprecationWarning: The "osmajorrelease" will be a type of an integer.



salt/run/20160902143454211747/ret   {
    "_stamp": "2016-09-02T14:34:54.387877",
    "fun": "runner.dummy.test",
    "jid": "20160902143454211747",
    "return": "Exception occurred in runner dummy.test: Traceback (most recent call last):\n  File \"/usr/local/salt/virtualenv/lib/python2.7/site-packages/salt/client/mixins.py\", line 352, in low\n    data['fun_args'] = args + ([kwargs] if kwargs else [])\nTypeError: can only concatenate tuple (not \"list\") to tuple\n",
    "success": false,
    "user": "root"
}
salt/run/20160902143554515090/ret   {
    "_stamp": "2016-09-02T14:35:54.674344",
    "fun": "runner.dummy.test",
    "jid": "20160902143554515090",
    "return": "Exception occurred in runner dummy.test: Traceback (most recent call last):\n  File \"/usr/local/salt/virtualenv/lib/python2.7/site-packages/salt/client/mixins.py\", line 352, in low\n    data['fun_args'] = args + ([kwargs] if kwargs else [])\nTypeError: can only concatenate tuple (not \"list\") to tuple\n",
    "success": false,
    "user": "root"
}
salt/run/20160902143654774890/ret   {
    "_stamp": "2016-09-02T14:36:54.935636",
    "fun": "runner.dummy.test",
    "jid": "20160902143654774890",
    "return": "Exception occurred in runner dummy.test: Traceback (most recent call last):\n  File \"/usr/local/salt/virtualenv/lib/python2.7/site-packages/salt/client/mixins.py\", line 352, in low\n    data['fun_args'] = args + ([kwargs] if kwargs else [])\nTypeError: can only concatenate tuple (not \"list\") to tuple\n",
    "success": false,
    "user": "root"
}

After:

(virtualenv)root@36netops3:~# salt-run state.event pretty=True
/usr/local/salt/virtualenv/lib/python2.7/site-packages/salt/grains/core.py:1493: DeprecationWarning: The "osmajorrelease" will be a type of an integer.
salt/auth   {
    "_stamp": "2016-09-02T14:48:08.013579",
    "act": "accept",
    "id": "mine",
    "pub": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA6861fSbTHlhKMKuDWgNL\ncD8cp+cgN6zPk2K1CfSR63xxdWRMe1Aiy5xqpaX5+FhznZYLBj1gAwwFLs5XaH7A\n7jghEVR+cd31Jz+i2+VjYpNNrJ8if9ZZpaxBM3LCyVsXhFtcFX7IqdFSESuQfAuL\nFh6dMjpjwwRk8g3j3Dq4D7L1/NNDtQtTSFUI+oaFStbeanWN2QapP2dpYxopTwXT\nc02qS+U24DewUGGNVw3BNwj7jCetn7R/m92KoKjlDRMsrZYTNmDYsApwdU/+PX/P\n24774Du7SVXT9oHFlVilzJkJie0PnJT9aKKCKbSSFtYi3w+wNnNEsFncTmiYP1Er\niwIDAQAB\n-----END PUBLIC KEY-----",
    "result": true
}
salt/run/20160902144837714970/new   {
    "_stamp": "2016-09-02T14:48:37.880787",
    "fun": "runner.dummy.test",
    "fun_args": [],
    "jid": "20160902144837714970",
    "user": "root"
}
salt/run/20160902144837714970/ret   {
    "_stamp": "2016-09-02T14:48:37.881189",
    "fun": "runner.dummy.test",
    "fun_args": [],
    "jid": "20160902144837714970",
    "return": true,
    "success": true,
    "user": "root"
}

[rallytime]

Go Go Jenkins!

[rallytime]

Hrm. Looks like the tests never triggered on this! They're running now.

I'd like @cachedout to take a look at this. We want to tread lightly here. Depending on what @cachedout thinks, it might be best to move this to develop instead of 2016.3, but I'll wait to hear from him.

[mirceaulinic]

Hi @rallytime I agree with you, I looked in the PR that introduced this problem and they passed the tests. However, as one can see in #36021 the problem is there.

Please @rallytime @cachedout check this and confirm the datatype is dict or should cast to tuple - right now the schedulers are not usable and this seems to be happening not only for proxy minions, as per #36021 (comment)

[mirceaulinic]

After a closer look into the first PR that introduced this, #35059, I see that it definitely should be a list.

[cachedout]

I think this is right. Thanks, @mirceaulinic