fiaas-deploy-daemon icon indicating copy to clipboard operation
fiaas-deploy-daemon copied to clipboard

proposal for multiple configmaps #112

Open skogie opened this issue 3 years ago • 24 comments

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.

skogie avatar Aug 28 '20 12:08 skogie

The integration tests seem flaky on my machine if anyone could try to run them and give me feedback I would appreciate it!

skogie avatar Aug 28 '20 15:08 skogie

Results (893.49s):
     140 passed
________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________  integration_test: commands succeeded
  congratulations :)

tg90nor avatar Aug 28 '20 16:08 tg90nor

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!

skogie avatar Sep 01 '20 10:09 skogie

@mortenlj pushed some changes now, what do you think?

skogie avatar Sep 07 '20 11:09 skogie

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.

mortenlj avatar Sep 07 '20 18:09 mortenlj

@skogie : Any chance you could sort out the documentation and maybe this can be merged? @gregjones : What do you think, good?

mortenlj avatar Oct 03 '20 17:10 mortenlj

I will try to get the remaining changes out tomorrow. Missing some tests and documentation.

skogie avatar Oct 05 '20 17:10 skogie

What's the state of this PR, any movement?

mortenlj avatar Nov 11 '20 11:11 mortenlj

@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?

skogie avatar Nov 16 '20 14:11 skogie

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 avatar Nov 17 '20 10:11 mortenlj

@mortenlj I also have failing tests with tox -e integration_test, not sure if the CI server is running those?

skogie avatar Nov 17 '20 11:11 skogie

Results (16.41s):00:33
     866 passed​00: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

skogie avatar Nov 17 '20 11:11 skogie

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 avatar Nov 17 '20 12:11 mortenlj

@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

skogie avatar Nov 17 '20 12:11 skogie

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

skogie avatar Nov 17 '20 13:11 skogie

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

oyvindio avatar Nov 24 '20 08:11 oyvindio

Would be nice to try to run it on the CI-server now if possible.

skogie avatar Nov 25 '20 13:11 skogie

/sem-approve

mortenlj avatar Nov 26 '20 10:11 mortenlj

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...

mortenlj avatar Nov 26 '20 10:11 mortenlj

@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...

mortenlj avatar Jan 15 '21 08:01 mortenlj

/sem-approve

mortenlj avatar Jan 15 '21 09:01 mortenlj

@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?

skogie avatar Mar 01 '21 09:03 skogie

/sem-approve

mortenlj avatar Mar 01 '21 10:03 mortenlj

/sem-approve

oyvindio avatar Mar 03 '21 09:03 oyvindio

I have a feeling this PR can be closed ... ?

mortenlj avatar Jan 12 '23 09:01 mortenlj