Rely on env variable for manageiq-appliance repo directory

[jrafanie]

ManageIQ code shouldn't know where the old system/TEMPLATE is
Use the APPLIANCE_TEMPLATE_DIRECTORY environment variable provided by manageiq-appliance PR below.

Related PRs:

• Appliance repo shouldn't be in www root. manageiq-appliance-build#33 [ clone repo in new location ]
• Allow the appliance repo to live anywhere by exposing its location. manageiq-appliance#12 [set environment variable for new location]
• Rely on env variable for manageiq-appliance repo directory #4105 [use environment variable above]

[jrafanie]

@simaishi @carbonin @Fryguy please review.

@roliveri @jerryk55 @movitto @hsong-rh Note, if you clone your manage-appliance repo somewhere else on your dev/appliance setup, you'll need to set your APPLIANCE_REPO_DIRECTORY environment variable here when these related PRs are merged. manageiq repo knew too much about where other repos lived and what they contained.

[kbrock]

I can't imagine that we will re-arrange our directory structure more than once or twice over the next few years.
This seems like it should be hardcoded, or part of a build time script.

If this is a cross repo concern, then we have 2 choices:

1. put that variable in a known spot ENV
2. move this code into THAT repo, so it isn't a cross repo concern

[chessbyte]

@jrafanie @abellotti @kbrock This PR makes it seem that maybe it is time to extract appliance_console into its own repo?

[chessbyte]

@simaishi @carbonin @Fryguy any comments on this PR or is this good to merge?

[jrafanie]

If this is a cross repo concern, then we have 2 choices:

put that variable in a known spot ENV
move this code into THAT repo, so it isn't a cross repo concern

Good point @kbrock, it doesn't really belong in ManageIQ, maybe you or @abellotti have some ideas if the manageiq code from appliance_console really needs to do this work in the first place. It's really, if you've configured X for (apache/postgresql), set these things. I believe ManageIQ via appliance_console determines X and is also doing the setting of things. If instead, appliance_console called a manageiq-appliance script with some parameters, manageiq-appliance could house all of the logic and TEMPLATE configuration files.

This is probably outside the scope of this PR.

[carbonin]

I think you would also need to change https://github.com/ManageIQ/manageiq/blob/master/gems/pending/appliance_console/external_httpd_authentication.rb#L115 .

[jrafanie]

I think you would also need to change https://github.com/ManageIQ/manageiq/blob/master/gems/pending/appliance_console/external_httpd_authentication.rb#L115 .

Good catch, I'll fix that.

[abellotti]

I see them defined in the other PR, but wouldn't hurt to have code early on in appliance_console to check these ENV variables are indeed defined and bail out, just in case. Otherwise, we'll get stack traces or worse.

[miq-bot]

Checked commits jrafanie/manageiq@1bea799~...ec7b5e9 with rubocop 0.33.0 and haml-lint 0.13.0
6 files checked, 0 offenses detected
Everything looks good. 👍

[kbrock]

@chessbyte

Can we pull the database.yml creation/modification code out of rails/rake and into appliance console before extracting this?

[chessbyte]

@kbrock Can you clarify which code you want to extract into appliance console?

[jrafanie]

I see them defined in the other PR, but wouldn't hurt to have code early on in appliance_console to check these ENV variables are indeed defined and bail out, just in case. Otherwise, we'll get stack traces or worse.

@abellotti I don't know, we've never had checks for APPLIANCE and RAILS_ENV env variables which are used all over the place and are also set in similar places in manageiq-appliance repo. If we've had any problems with them, we'd be using the wrong Rails env or installing development gems on appliances.

Any issues we get now might be temporary until these env variables stabilize or any of my bugs are fixed. When we tag upstream, the different repos will have to be tagged together to work together. If I'm running on an appliance and get a KeyError: key not found: "APPLIANCE_*", I probably git pulled manageiq without also doing manageiq-appliance. This is a side effect of splitting repos.

I also added a appliance alias so we don't have to remember where the new manageiq-appliance repo is checked out, see: https://github.com/ManageIQ/manageiq-appliance/pull/12/files#diff-b9a3e1852620f16206bbe0f3e6736614R5. You can use vmdb and appliance to switch back and forth.

[jrafanie]

@kbrock @chessbyte Feel free to open an issue to extract appliance_console to a gem or to the manageiq-appliance repo itself (the latter would eliminate the need of the ENV variable here since it would reside in the same repo as the needed template files). While it sounds doable, it requires a lot of manual testing and is beyond this PR.

[kbrock]

@jrafanie @chessbyte I was thinking something like #4212

I like the idea of keeping the appliance code separate from the rails code. never thought of the ENV basically showing where we are coupled across repos.

I've always viewed tasks like database create / backup / restore / set region / database.yml creation and tasks that would basically create or destroy a rails app to be the job of the appliance (and appliance console). But I'm curious where others draw that line.

Case and point: I also consider the job of starting and stopping the rails apps / workers to be outside the role of the rails app as well. And we clearly have that inside the rails app.

[Fryguy]

Is this right? I feel like this should just be template_directory directly?

[jrafanie]

It's pretty complicated but the TEMPLATE directory has stuff laid out as they get applied to /, like a chroot. So, we need to the root of the TEMPLATE directory and the normal PG_DATA directory (postgres_dir) in this case.

[Fryguy]

Right, but why does PostgresAdmin know about template_directory? Seems like that class should not care about that.

[kbrock]

Agreed that it is too bad we have to rely upon template directory, but lets not revisit at this time.

Ok, so is the request to move PostgresAdmin.template_directory to InternalDatabaseConfiguration?

[jrafanie]

Yeah, it's "weird" that PostgresAdmin knows that the templates for postgresql are stored there.

[jrafanie]

It feels like we need a generic template directory on a more generic module/class that these other places can call.