Salt: Feature request: cleanup old mine data on master

[marek-obuchowicz]

Whenever I remove a node, master still keeps it's mine information cached and sends it to all minions. Calling salt '*' mine.flush doesn't help, as master still has old mine data from not existing nodes. Calling salt-run cache.clear_mine on master also leaves all the cached data on master unchanged.

As a manual fix, I had to go to /var/cache/salt/master/minons and manually remove directories for minions, which don't exist anymore.

[gtmanfred]

Are you using salt-cloud to build and destroy servers?

https://github.com/saltstack/salt/blob/develop/salt/cloud/libcloudfuncs.py#L360

This is what I use (still needs to be fully documented) but if you have flush_mine_on_destroy: True in the cloud profile. Other than that, I would like to have something to just remove old mines as well.

[marek-obuchowicz]

flush_mine_on_destroy: True works fine, thanks for the tip, didn't find it in the docs.
I'm leaving issue open, as removing old mine data still seems helpful.

[basepi]

This would be very useful, thanks!

[donnlee]

Ran into this and I thought it was strange that the master wouldn't expire mine data from non-existent minions.
It was rather puzzling until I found this and saw that others encountered the same thing.

[wt]

Workaround that I use from within a state that sets up my salt-masters:

remove_stale_mine_data:
  cmd.run:
    - name: >
        find /var/cache/salt/master/minions/ -type f -name mine.p
        ! -newermt '-90 min' -exec bash '-c' 'rm -rf "$(dirname "{}")"' ';'

[basepi]

Nice, thanks for the workaround!

[wt]

FWIW, I set my mine_interval at 60. So this means that files are only purged after 1.5x the interval. Salt should probably have similar logic in the master.

[dansoap]

Ran into the same issue. I didn't destroy some servers using salt-cloud, so flush_mine_on_destroy didn't work of course.

@marek-obuchowicz: Thanks for the workaround, that did the trick for me...

[wt]

Is there a reason not to just default that option to True? Seems like it'd be an strange default to keep the mine data.

[basepi]

@wt You mean flush_mine_on_destroy? Yes, it does seem like True is a saner default there. I'll have to ask around for potential pitfalls there.

[wt]

@basepi ack

[tkent-xetus]

For what it's worth, mine data being removed when salt-cloud destroyed a minion is what I expected. Ended up at this issue trying to figure out why that wasn't happening.

[rikonor]

@wt thanks for your example, I created a runner based on it over at salt-mine-refresh that can be called from events, scheduled, etc

[jgmchan]

:+1: for this feature, it can cause some weird issues for formulas which relies on this information like HDFS

[BretFisher]

Yea I can't use salt-cloud to destroy and just assumed when salt-key -d was used it would cleanup mine. Thought it was a bug. Workaround from @wt works for me, but wished it was automatic.

[timcharper]

I used salt-cloud to destroy an instance with flush_mine_on_destroy: True in salt 2015.5.3 and the data was left behind.

༼ ༎ຶ ෴ ༎ຶ༽

[imchairmanm]

I'd like to add my vote for this feature. It seems more like a bug fix to me though. I'm not the most advanced user, but I can't think of a scenario where keeping the mine data for deleted nodes by default would be expected behavior.

Can we at least get some doc updates about the current behavior? I'm finding myself having to look at GitHub issues and source code instead of the docs very frequently to get a handle on the actual way that SaltStack functions, which is unfortunate.

[basepi]

I'm going to look into possibly adding this for Boron. No promises, though, I've assigned myself too much work for Boron already. ;)

[Dravu]

Thanks @basepi! I ran into this earlier today and while it was diagnosed fairly quickly, it certainly didn't seem like intended/expected functionality. I personally expected a minion's cached data to get deleted once their key is revoked. Even some sort of sync or refresh functionality for the master's cache would work I suppose.

[danlsgiga]

+1
I also thought this would be the default behaviour. Looking forward to get this feature in the next release.

[wt]

Honestly, I think that the master should be smart enough not to respond with data from nodes that should be gone. I'm am a little worried about the case where the master crashes between removing the key and removing the mine data. I feel like the master should do something that is atomic so that failures have less chance of causing issues.

[basepi]

You might be right. In addition to cleanup, we should probably filter mine results against accepted keys as well.

[anlutro]

FWIW, I ran salt-run cache.clear_all name-of-minion after shutting down a minion today, and that cleared the mine data. Not sure if that's an intentional fix since this issue seems to indicate that it's still a problem?

[danlsgiga]

Hey @anlutro, thanks for the trick. I can confirm that using salt-run cache.clear_all name-of-minion the cache is properly cleared.
I've also tested using a reactor to clear the minion cache once the minion is destroyed and it works seamlessly. Here is the reactor formula:

_/srv/reactor/clear_minion_cache.sls_

clear_minion_cache:
  runner.cache.clear_all:
    - tgt: {{ data.id }}   

[wt]

This is a cool workaround.

However, there is still (presumably small) risk that the salt master crashes while doing that and doesn't remove everything. I'd still like to see the proactive filtering to make sure stray files don't cause problems.

[Reiner030]

Hi, last error comments for this behavior are coming from still open bug https://github.com/saltstack/salt/issues/21986 which didn't clear cache since saltstack versions 2015.5.x (https://github.com/saltstack/salt/issues/21986#issuecomment-123132269).

Thx to @danlsgiga for the reactor state definition. I was searching for it (and therefore found this issue) but there is yet no official documentation howto use runner states in reactor definitions ...

EDIT: as suggestion: I added the state to destroyed reactor states. (I created for each env own _reactor structure; in this case in file base/_reactor/salt/cloud/destroyed/clear_caches.sls):

clear_minion_cache:
  runner.cache.clear_all:
    - tgt: '{{ data['name'] }}'
    - queue: True

[ealphonse]

Hello, I tested this feature,as I am evaluating Mine, and salt-key -d does delete the minion's mine data on 2016.11.2.

Did something change? What is the issue status?

Thanks

[gtmanfred]

This was implemented, but it appears that it might be broken for some cases?

I am closing this in favor of https://github.com/saltstack/salt/issues/21986

[ealphonse]

@gtmanfred thank you for the info