fiaas-deploy-daemon
fiaas-deploy-daemon copied to clipboard
proposal for multiple configmaps #112
WIP for issue #112
860 tests passed with tox -e test
Wasnt able to run the integration-test module it kept crashing so if there is more to be done here let me know.
The integration tests seem flaky on my machine if anyone could try to run them and give me feedback I would appreciate it!
Results (893.49s):
140 passed
________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________ integration_test: commands succeeded
congratulations :)
I like the idea, and it mostly looks good with just a couple of things I think should be clarified.
I also think Morten's comment from the issue, about also exposing them via volume-mounts makes sense - is there a reason not to do that?
I was not aware of the volume-mounts, but yeah i should add those aswell!
@mortenlj pushed some changes now, what do you think?
Starting to look good. The only thing I'd like to change is expanding the documentation with detailing of how precedence works for the added configmaps as we have discussed.
@skogie : Any chance you could sort out the documentation and maybe this can be merged? @gregjones : What do you think, good?
I will try to get the remaining changes out tomorrow. Missing some tests and documentation.
What's the state of this PR, any movement?
@mortenlj I have been working on getting the tests to work locally but rn i have a round-trip of around 20 minutes per integration_test-run and its still not passing here, I think everything should be correct but could you try to run it on your end?
I have been working on getting the tests to work locally but rn i have a round-trip of around 20 minutes per integration_test-run and its still not passing here, I think everything should be correct but could you try to run it on your end?
@skogie : The last run seems to fail because of codestyle issues. Should be enough to fix those. To just run the codestyle tests, use tox -e codestyle
(which is significantly quicker than waiting for the full set of tests).
@mortenlj I also have failing tests with tox -e integration_test
, not sure if the CI server is running those?
Results (16.41s):00:33
866 passed00:33
___________________________________ summary ____________________________________00:33
codestyle: commands succeeded00:33
test: commands succeeded00:33
congratulations :)
tests seems to be running but the tox -e integration_test
does not seem to be a part of the testing
tests seems to be running but the
tox -e integration_test
does not seem to be a part of the testing
Yes, we've hit https://github.com/fiaas/fiaas-deploy-daemon/issues/131
@oyvindio : Did you do any more research on that issue?
@mortenlj is it possible for you to run it locally to see if they actually pass, or if there is anything else i need to fix? I don't think they are working correctly on my machine
Fixed most of the tests now, but I still don't understand why these 2 are failing if someone could help me out: https://gist.github.com/skogie/4a786ff9ceec17a26466a3221819dbab and https://gist.github.com/skogie/0b480f01f46fd438e033134ef7a86de5
Yes, we've hit #131
@oyvindio : Did you do any more research on that issue?
It is on my list, but haven't been able to look into it further yet unfortunately. The only "workaround" I'm aware of to get the e2e tests to run before that issue is fixed, is to push the branch from the fork to a new branch on the source repository, which admittedly isn't a great solution
Would be nice to try to run it on the CI-server now if possible.
/sem-approve
I triggered a build directly in Semaphore after changing some settings which I hope doesn't expose our docker secrets to all PRs... 🙂 Also, I think perhaps, maybe, the above comment didn't work then, but should work now...
@gregjones : This looks good now, doesn't it?
We still haven't figured out how to get Semaphore to build from forks automatically I guess, but I'll see if I can trigger something...
/sem-approve
@mortenlj looks like the build is failing now because some of the tasks are not running, can we try to trigger a new build and see if we can merge it?
/sem-approve
/sem-approve
I have a feeling this PR can be closed ... ?