SUSE / caasp-salt

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

Fix python3 iteration over dictionary. #733

Closed bergmannf closed 5 years ago

bergmannf commented 5 years ago

In python3 python prevents modifying the dictionary that is iterated over.

Instead of modifying the initial dictionary it will be copied first.

Also iteritems is no longer available in python3, so the code should use the six function instead to be cross-version compatible.

bergmannf commented 5 years ago

@MalloZup Thanks for the review - updated the PR

MalloZup commented 5 years ago

failure on CI was:

Error: Error applying plan:

1 error(s) occurred:

* libvirt_domain.admin: 1 error(s) occurred:

* libvirt_domain.admin: timeout while waiting for state to become 'all-addresses-obtained' (last state: 'waiting-addresses', timeout: 5m0s)

Terraform does not automatically rollback in the face of errors.

This is a terraform race condition afaik. retry

MalloZup commented 5 years ago

tox failure:

+ tox --notest -e tests-salt-2018.3.0-py34

tests-salt-2018.3.0-py34 create: /srv/jenkins/workspace/salt.tests_PR-733/.tox/tests-salt-2018.3.0-py34

tests-salt-2018.3.0-py34 installdeps: git+https://github.com/opensuse/salt@openSUSE-2018.3.0, mock==2.0.0, nose==1.3.7

wrapper script does not seem to be touching the log file in /srv/jenkins/workspace/salt.tests_PR-733@tmp/durable-729564b2

(JENKINS-48300: if on an extremely laggy filesystem, consider -Dorg.jenkinsci.plugins.durabletask.BourneShellScript.HEARTBEAT_CHECK_INTERVAL=86400)

script returned exit code -1

rescheduling

hwoarang commented 5 years ago

@MalloZup please document these random failures on the confluence page

bergmannf commented 5 years ago

Update: the v3 bug seems to be something else, so I guess this PR can go ahead: https://bugzilla.suse.com/show_bug.cgi?id=1125954

We might have to wait with merging this - i was told that this error actually also pops up on v3, so I'm investigating that.

I would be surprised if it does, but if so the code needs to be backwards compatible.

MalloZup commented 5 years ago

@bergmannf take a look at flake errors

bergmannf commented 5 years ago

@MalloZup Flake8 errors are fixed - little pointer: the CI does not tell me what's wrong so I have to run it locally - maybe it shouldn't just pipe the output to an unreachable xml file?

MalloZup commented 5 years ago

@bergmannf you need to checkout the result in Webui and you have the results