search

SUSE / caasp-salt

A collection of salt states used to provision a kubernetes cluster
Apache License 2.0
64 stars 29 forks source link

Use a temporary file in /tmp for file.managed #714

Closed inercia closed 5 years ago

inercia commented 5 years ago

When using file.managed, create a temporary file that is in /tmp instead of using the same directory the target file is. This fixes some problems with programs/daemons that could be monitoring that directory (like the kubelet with /etc/kubernetes/manifests)

bsc#1123716

ereslibre commented 5 years ago

There are some style check warnings here, can you please address them @inercia?

inercia commented 5 years ago

There are some style check warnings here, can you please address them @inercia?

From the output, the error does not really seem related to my code:

[ke8_PR-714-72TB2RPYD7J52P43BXJMJ] Running shell script
+ tox -e flake8 -- --format=junit-xml --output-file junit.xml
flake8 create: /home/jenkins/workspace/ke8_PR-714-72TB2RPYD7J52P43BXJMJ/.tox/flake8
flake8 installdeps: flake8==3.5.0, flake8_formatter_junit_xml==0.0.6
flake8 runtests: PYTHONHASHSEED='2717468562'
flake8 runtests: commands[0] | flake8 --format=junit-xml --output-file junit.xml
ERROR: InvocationError: '/home/jenkins/workspace/ke8_PR-714-72TB2RPYD7J52P43BXJMJ/.tox/flake8/bin/flake8 --format=junit-xml --output-file junit.xml'
___________________________________ summary ____________________________________
ERROR:   flake8: commands failed
script returned exit code 1
ereslibre commented 5 years ago

Hmm, jenkins show this trailing space errors: http://jenkins.caasp.suse.net/blue/organizations/jenkins/salt.flake8/detail/PR-714/2/tests

Edit: it should also be rebased on top of master for the tox3 error to go away.

ereslibre commented 5 years ago

@MalloZup Please when adding a commit to the PR make sure that it is meaningful and has a proper description. We should make commit messages more detailed.

MalloZup commented 5 years ago

let me reammend this thx @ereslibre

MalloZup commented 5 years ago

or @inercia feel free to rebase my commit to your commit into only 1

inercia commented 5 years ago

Please do not merge this. It is not ready.

inercia commented 5 years ago

Current solution still creates a temporary file in the same dir as the final file, but with the same contents of the final file, so the kubelet should be happy with that...

hwoarang commented 5 years ago

@inercia could you please rebase your branch on top of current master? Thank you

inercia commented 5 years ago

Blocked on Jenkins failing for reasons not related to this PR.

MalloZup commented 5 years ago

Can you retrigger?

hwoarang commented 5 years ago

@inercia Please rebase your branch against otherwise tests will not pass

hwoarang commented 5 years ago

@MalloZup the CI failures are legitimate. The branch needs rebasing to use the latest Jenkinsfiles