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

Unable to "watch" a state which is a "prereq" #6057

Closed
jacksontj opened this issue Jul 9, 2013 · 13 comments
Closed

Unable to "watch" a state which is a "prereq" #6057

jacksontj opened this issue Jul 9, 2013 · 13 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@jacksontj
Copy link
Contributor

If i have a state (file.managed below) which is a prereq_in it cannot be watched by another state. The below SLS file causes infinite recursion-- which doesn't make a ton of sense to me. If i am watching a state it shouldn't matter if that state has a prereq-- just if it changed or not, unless i'm missing something.

remove_from_lb:
  cmd.run:
    - name: /home/thjackso/tmp/rotation/oor.sh

# Command to add host to LB
add_to_lb:
    cmd.wait:
        - name: /home/thjackso/tmp/rotation/ir.sh

/home/thjackso/workspace/salt/atsdeploy/test/haproxy.cfg:
  file.managed:
    - source: {{ salt['pillar.get']('haproxy:source', 'http://localhost/cfg/default') }}
    - user: thjackso
    - group: eng
    - mode: 644
    - source_hash: {{ salt['pillar.get']('haproxy:hash', 'md5=7822bba98f950fc9e12e1211548fd730') }}
    - prereq_in:
      - cmd: remove_from_lb

haproxy:
  service:
    - running
    # remove from LB if you have a change
    - prereq_in:
      - cmd: remove_from_lb
    # add it back when you are done
    #- watch_in:
    #  - cmd: add_to_lb
    - watch:
      - file: /home/thjackso/workspace/salt/atsdeploy/test/haproxy.cfg
@jacksontj
Copy link
Contributor Author

You get the same outcome with the direct approach--

remove_from_lb:
  cmd.run:
    - name: /home/thjackso/tmp/rotation/oor.sh
    - prereq:
      - file: /home/thjackso/workspace/salt/atsdeploy/test/haproxy.cfg
      - service: haproxy

/home/thjackso/workspace/salt/atsdeploy/test/haproxy.cfg:
  file.managed:
    - source: {{ salt['pillar.get']('haproxy:source', 'http://localhost/cfg/default') }}
    - user: thjackso
    - group: eng
    - mode: 644
    - source_hash: {{ salt['pillar.get']('haproxy:hash', 'md5=7822bba98f950fc9e12e1211548fd730') }}

haproxy:
  service:
    - running
    - watch:
      - file: /home/thjackso/workspace/salt/atsdeploy/test/haproxy.cfg

@basepi
Copy link
Contributor

basepi commented Jul 9, 2013

Thanks for the report -- I'll have @thatch45 look into it.

@ghost ghost assigned thatch45 Jul 9, 2013
@jacksontj
Copy link
Contributor Author

Any update?

@basepi
Copy link
Contributor

basepi commented Aug 6, 2013

I'll bug @thatch45 for an update tomorrow.

@thatch45
Copy link
Member

thatch45 commented Aug 6, 2013

I am glad this was so simple, but this was happening not as an artifact of watch but of all requisites! This patch fixes it for me, please double check. (I finally had a solid 4 hours to track this down)

@jacksontj
Copy link
Contributor Author

I just tested it an I now get an error saying that one of the states is recursive (http://pastebin.com/Fxzi3BSR), so definetly better ;)

thatch45 added a commit that referenced this issue Aug 7, 2013
a recursive requisite populate the pre dict with the test=True data
instead of populating the running dict too early, this should fix the
last of #6057
@thatch45
Copy link
Member

thatch45 commented Aug 7, 2013

This was surprisingly easier then I thought once I realized that all of the data is already available for the cross check and that I just needed to put the right data in the pre dict! On my end this looks like it is working the way it is intended to, can you take it for a spin @jacksontj ?

@jacksontj
Copy link
Contributor Author

Beautiful! Yea, the prereq state is the only "unidirectonal" relationship in the requisite system-- so it gets handled differently ;) That works for me-- i'll have to do some more testing around the in-rotation stuff afterwards-- but the state above does what it is supposed to. Thanks again @thatch45 !

thatch45 added a commit that referenced this issue Aug 7, 2013
thatch45 added a commit that referenced this issue Aug 7, 2013
a recursive requisite populate the pre dict with the test=True data
instead of populating the running dict too early, this should fix the
last of #6057
@jacksontj
Copy link
Contributor Author

I imagine this'll make it into 0.16.3?

@basepi
Copy link
Contributor

basepi commented Aug 8, 2013

Yes, already cherry-picked. =)

@jacksontj
Copy link
Contributor Author

When is that release scheduled to go out? A week or so? (curious)

@basepi
Copy link
Contributor

basepi commented Aug 9, 2013

We're planning on cutting the release in the next hour or so, then we'll give the packagers a couple of days to actually package it. Expect the announcement early Monday (though it will be available on PyPI later today)

@jacksontj
Copy link
Contributor Author

Awesome, I'll hold of on my internal release for that ;)
On Aug 9, 2013 9:59 AM, "Colton Myers" notifications@github.com wrote:

We're planning on cutting the release in the next hour or so, then we'll
give the packagers a couple of days to actually package it. Expect the
announcement early Monday (though it will be available on PyPI later today)


Reply to this email directly or view it on GitHubhttps://github.com//issues/6057#issuecomment-22408474
.

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 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

3 participants