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

Touch deploy.sh before use #38867

Closed
wants to merge 1 commit into from
Closed

Conversation

mchugh19
Copy link
Contributor

http.query output will only write to an existing file, so touch it before using it.

What does this PR do?

Adds a file.touch before the http.query attempts to write to the file.

What issues does this PR fix or reference?

Adding hosts into /etc/salt/roster, then running

salt-run manage.bootstrap hosts='*' script_args='-A <salt master ip>'

fails after the http.query with .../deploy.sh no such file. Touching the file before the http.query allows the contents to be updated, and the runner proceeds as expected.

Previous Behavior

/tmp/uuid directory structure created. Then error is thrown about missing deploy.sh file.

New Behavior

/tmp/uuid directory structure created, deploy.sh touched, the deploy.sh is then updated from the http.query and the runner continues.

Tests written?

No

@techhat
Copy link
Contributor

techhat commented Jan 22, 2017

@mchugh19 you should be aware of #32026.

@mchugh19
Copy link
Contributor Author

Thanks @techhat. Created this PR since we've seen this in our environment when trying to use the manage.bootstrap runner as mentioned above. We saw that behavior from a master running 2016.11.1 and previously 2016.3. Seems sort of weird since the fix you mentioned seems to have gone in a previous release. If there's a guarantee that salt-ssh should be okay, this could be dropped.

@techhat
Copy link
Contributor

techhat commented Jan 23, 2017

@mchugh19 it doesn't look like that fix was merged forward. I've left a note in that PR to try and get that fixed.

@techhat
Copy link
Contributor

techhat commented Jan 23, 2017

@mchugh19 we just merged #38883, which will resolve this as soon as it gets merged all the way up to develop. In the meantime, as helpful as this PR was in letting us know about the problem, it only addresses a symptom, and will be rendered obsolete in short order, so I would recommend closing it, unless you have reservations in doing so.

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jan 23, 2017
@mchugh19
Copy link
Contributor Author

Agreed. Thanks much!

@mchugh19 mchugh19 closed this Jan 23, 2017
@mchugh19 mchugh19 deleted the bootstrap_fix branch February 18, 2017 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants