[IMP] clouder: Path and Environment Handling

[lasley]

[IMP] clouder: Centralize and improve path/str handling …

• Centralize key and clouder tmp dirs via ClouderModel method
• Centralize backup directory definitions in ClouderModel
• Harden path handling by using os.path.join
• Prefer string interpolation vs concat
• Fix use of self.raise_error for models that aren't inherited from clouder.model
• Prefer ClouderError in models not inherited from clouder.model
• Update model attribute declaration orders
• Use lambdas for defaults in clouder.container
• Environment prefix is required
• Centralize commit methods and use isolated environment
• Change version to 9.0.2.0.0 to match OCA style for 9, while keeping w/ semantic increase

[lasley]

Alright this seems to work in local & is ready to review assuming CI goes 🍏

I updated the changelog in the PR description to contain a complete list of changes made (also documented in my commit msg)

Couple notes to bring out of the changelog:

• Directory handling is improved overall, with most string concatenation removed. Lots centralized
• Commits have been centralized into a clouder.model method providing a new env & commit in a context manager. This is a more transaction-safe approach while still maintaining original intent IMO
• The version should be 9.0.x.x.x to indicate we're using v9. Connector isn't out for 10 yet, so probably not good to make that jump quite yet (also removes a lint error).

cc @t3ddftw

[lasley]

@YannickB - I deployed a few containers, but yeah good call with the One Click deploy.

I'm going to wait for @t3ddftw to give us a review as well - lots of changes here that I'd like more eyes on.

[YannickB]

Ok. During the oneclick process, note that the last step (the duplication of Odoo instance into a test install) was already broken before your changes. Everything else was working.

In general oneclick is the best way to check all the code, we can't use travis here because it's obviously kinda difficult to test an infrastructure installation with unit test (but I still hope we'll find a solution one day to automatise this part).

[lasley]

I've got us covered on the testing of external services via unit tests, no problem (connector-carepoint coverage, connector-dns coverage).

Big undertaking though so I want to make sure we've got major changes done before tackling that.

[YannickB]

That's good to know, agree we have other priorities but that's something I have in mind since quite a long time, good to see we have some clues.

[lasley]

You're spammy @t3ddftw - please use the "review" feature instead of "single comment"

[lasley]

Thanks for the reviews @YannickB & @t3ddftw - comments attended to. Haven't functionally reviewed fully yet, so still WIP.

Description of latest changes:

• Add dynamic values to Registry X509 cert
• Make generate_random_password somewhat more secure
• Remove execution bit from key files
• Reverse container sql constraint for domain/name for efficiency
• Improve container sql constraint verbiage & names
• Change version to 9.0.10.0.0
• Prefix is not required on environment
• Remove more string concats

In hindsight, the top two I probably should have done in separate PRs.

[YannickB]

Ok looks good enough for me, let me know when I can merge.

[lasley]

FYI there's an issue with this that I have been resolving. Probably shouldn't have made so many changes at once - debugging has been a real pain. Too late now though 😆

[lasley]

Alright this seems to work now.

I had to remove the transaction handling for the moment in order to minimize the scope of this ticket troubleshooting. I left the centralization of it in though, which will make for an easier refactor once I study the environment a bit more. I'm going to make an issue for that.

[YannickB]

Ok, should I merge it now ?

[lasley]

Yup we're good to merge 👍

[YannickB]

And done, thanks for everything