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

salt.modules.match.search_by not working 2019.2.2 #55149

Closed
angeloudy opened this issue Oct 29, 2019 · 9 comments
Closed

salt.modules.match.search_by not working 2019.2.2 #55149

angeloudy opened this issue Oct 29, 2019 · 9 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Execution-Module P2 Priority 2 Regression The issue is a bug that breaks functionality known to work in previous releases. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around v2019.2.3 unsupported version ZRELEASED - Neon retired label
Milestone

Comments

@angeloudy
Copy link
Contributor

I have the following pillar setup following instructions
https://docs.saltstack.com/en/2018.3/ref/modules/all/salt.modules.match.html

It has been working well for two years until I upgrade salt to 2019.2.2 today.

{% set region = grains['id'].split('.')[0] %}
region: {{ region }}
{% load_yaml as role_defs %}
web:
  - region1.minion1
  - region1.minion2
zabbix-agent:
  - "G@os:FreeBSD"
{% endload %}
{% set roles = salt.match.search_by(role_defs) or [] %}
role: {{ roles | yaml() }}
rr: [{{ region  }}.{{ roles | join(', ' ~ region ~ '.' ) }}]

For example, if I run
salt region1.minion1 pillar.get role, it used to give a list

- web
- zabbix-agent

but now it gives only one

- zabbix-agent

This is happening to all my minions. And my highstate is based on role, which means everything is broken now.

Salt Version:
           Salt: 2019.2.2

Dependency Versions:
           cffi: 1.12.3
       cherrypy: unknown
       dateutil: 2.8.0
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.33.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.1
   mysql-python: 1.4.2.post1
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.9 (default, Aug 22 2019, 07:04:57)
   python-gnupg: Not Installed
         PyYAML: 5.1
          PyZMQ: 18.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist:
         locale: UTF-8
        machine: amd64
        release: 11.2-RELEASE-p10
         system: FreeBSD
        version: Not Installed
@angeloudy
Copy link
Contributor Author

By inspecting the code, I have finally made it work by changing
{% set roles = salt.match.search_by(role_defs) or [] %}
to
{% set roles = salt.match.search_by(role_defs, minion_id=grains['id']) or [] %}

What's been changed?

@angeloudy
Copy link
Contributor Author

I think the behaviour was changed by #50183

 salt/matchers/compound_match.py

+ if not opts:
+         opts = __opts__

This doesn't seem right because in this case opts was passed through from match.search_by, so it never hits the line opts = __opts__. Here, __opts__ contains the minion id while opts only have the master's minion id which is xxxx_master. As a result, all minions will get the exact same roles as xxxx_master.

@waynew
Copy link
Contributor

waynew commented Oct 30, 2019

Did you try removing those two lines to see if it works properly?

@frogunder frogunder added this to the Approved milestone Oct 30, 2019
@frogunder frogunder added the Bug broken, incorrect, or confusing behavior label Oct 30, 2019
@angeloudy
Copy link
Contributor Author

Removing the first line if not opts: worked.

@frogunder
Copy link
Contributor

@angeloudy Thank you for reporting this issue.

We will look into it. Thanks.

@frogunder frogunder added Execution-Module severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P2 Priority 2 Regression The issue is a bug that breaks functionality known to work in previous releases. labels Oct 31, 2019
@KChandrashekhar KChandrashekhar added the v2019.2.3 unsupported version label Oct 31, 2019
@Akm0d
Copy link
Contributor

Akm0d commented Dec 3, 2019

I tried to reproduce the issue with a unit test: #55497.
It doesn't seem like the issue is with search_by. Rather, the problem is that the opts['id'] is returning the master's id rather than the minion's id. This is what causes web to not match in the end. Will look deeper tomorrow.

@angeloudy
Copy link
Contributor Author

@Akm0d
That's exactly what I saw. It's returning the master's id because opts was pass through from the initial call instead of getting it from __opts__.

@Akm0d
Copy link
Contributor

Akm0d commented Dec 5, 2019

Verified that this is the PR that introduced this issue: #54001 (master port of #50183)

Akm0d added a commit to Akm0d/salt that referenced this issue Dec 6, 2019
Akm0d added a commit to Akm0d/salt that referenced this issue Dec 6, 2019
dwoz added a commit that referenced this issue Dec 12, 2019
@sagetherage sagetherage added the ZRELEASED - Neon retired label label Dec 19, 2019
@sagetherage
Copy link
Contributor

this is fixed by a merged PR closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module P2 Priority 2 Regression The issue is a bug that breaks functionality known to work in previous releases. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around v2019.2.3 unsupported version ZRELEASED - Neon retired label
Projects
None yet
Development

No branches or pull requests

6 participants