salt: states.file.managed returns empty changes dict in test mode

Description of Issue/Question

The file.managed state function often returns an empty changes dict when run with test=true. In fact, the only time a non-empty changes dict is returned is when there will be a change to the file content. All other managed changes (e.g. owner, group, mode) will return an empty changes dict, which causes several requisites to misbehave, including wait, listen, prereq, and onchanges

This error was introduced with PR #26134 when the changes dict was renamed to pchanges, resulting in and empty changes dict in all cases. It was partially fixed with PR #26982, which populated the changes dict for file content differences, which is probably why this has gone unnoticed for so long.

The real problem seems to be the original change from changes to pchanges introduced by @thatch45. No other state modules use pchanges, and I don’t see any other code that makes use of a pchanges key in the returned dictionary. It seems to me that the changes made in PR #26134 should be reverted entirely.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 32 (28 by maintainers)

Most upvoted comments

I’d like to add my voice to this. We wasted two sysadmin days and created a 60 second website outage across our customers because of this breakage.

salt developers need to be far more aware that making significant breaking changes in stable releases creates an overall impression that salt is unreliable. In this case, it was known that prereq was completely broken for file.managed and yet nothing appears in the documentation or release notes to state that the prereq feature has been dropped from salt 2018.3. The problem has been known since February, two months before the release.

I’m unclear what problem pchanges tried to solve. But breaking dependency management in salt should be considered a really big deal.

We can cope with changes being inaccurate from time to time, as long as it is consistently inaccurate. And I’m happy to submit PR requests to make file.archive give us changes in test mode when used with a source hash, or for other modules we come across.