Conversation
f17cd09 to
98fe327
Compare
|
I broke out the start/stop behaviour into it's own PR -- #132 |
f45f3f2 to
b7aa809
Compare
729afc2 to
7886741
Compare
ekohl
left a comment
There was a problem hiding this comment.
I took a stab at making it really shippable and ekohl@55664e8 is the result. Probably still incomplete and I didn't look at the README yet.
I had an idea to also use this as an input to an RPM and then add packit, but this is what I could do before heading back into a meeting.
d625463 to
4b3ed36
Compare
3420981 to
e95a933
Compare
| return path | ||
|
|
||
| @staticmethod | ||
| def inventory_path(): |
There was a problem hiding this comment.
@evgeni I think this concept get's kinda weird in this use case compared to Obal's. The inventory will be dynamic for this use case, and therefore shouldn't live in the Python package. For now, here in the git repository, the inventories/ top level path works. But if I were taking it a step further and installing the Python package on a fresh machine, I would need an inventory defined and specified for using this against remote targets. For running this on the same machine (local) then having a default inventory works just fine. I was thinking of:
- Default to localhost so that the default mode is installation on the machine the installer exists.
- A user parameter that allows specifying the location of an inventory file.
I wondered about defaulting to also checking /etc/rop for an inventory file for the standard packaged case.
There was a problem hiding this comment.
Yeah, I think for this case, /etc/rop/inventory.yml (or whatever) is the cleanest solution.
There was a problem hiding this comment.
To elaborate a bit what I wanted to say: Ansible implies localhost when the given intentory is empty or non-parse-able, so we can just point this to the "production" inventory (which the user will edit, IF they want to add the external DB) that we ship in the RPM, and the dev setup should still work?
% ansible -i /etc/lol/what -m ping localhost
[WARNING]: Unable to parse /etc/lol/what as an inventory source
[WARNING]: No inventory was parsed, only implicit localhost is available
localhost | SUCCESS => {
"changed": false,
"ping": "pong"
}
|
This is now rebased on latest, and converted to a Python package along with updates to testing and Github actions. This also introduces a script to setup the virtualenv easily for local development and use in CI. |
There was a problem hiding this comment.
This file also installs Ansible, but we now pull that in via obsah so doesn't have to mentioned either.
|
I opted for a change in strategy to drop the
|
2ff020b to
7a4883c
Compare
27b1125 to
c977ee6
Compare
ekohl
left a comment
There was a problem hiding this comment.
Random shower thought: can we use a simple shell script wrapper instead?
My idea is to configure obsah via environment variables. Something like:
#!/usr/bin/sh
OBSAH_NAME=rop
OBSAH_DATA=data
OBSAH_INVENTORY=
export OBSAH_NAME OBSAH_DATA OBSAH_INVENTORY
exec obsah "$@"Right now OBSAH_NAME doesn't exist but we can enhance it to. I had a thought to enhance obsah where you symlink /usr/bin/obsah and it looks at __name__ to derive it, but since you need to set OBSAH_DATA anyway I think you need a wrapper anyway.
How I started to think about this was development. Can we install production rop (or whatever), ship all data somewhere and set ROP_DATA / OBSAH_DATA to that location? Then if you take that to a more logical extreme: do we even need rop then?
If we go that route then it may be feasible to package it up as a true Ansible collection while in the RPM we only add the trivial wrapper and package dependencies.
Thinking more about that, perhaps we can still use the symlink /usr/bin/obsah idea but add a /usr/libexec/obsah-collection (or something) that will look at an ansible collection with the same name in /usr/share/ansible/collections/ansible_collections (though that may be too hard with namespaces). Or we can introduce /usr/share/obsah for that purpose.
| long_description_content_type='text/markdown', | ||
| url='https://github.com/theforeman/foreman-quadlet', | ||
| author='The Foreman Project', | ||
| author_email='foreman-dev@googlegroups.com', |
Most probably: yes. We originally went this route, as the current That said, this is an improvement we can do later. Or are you hinting at "then we don't need a python package after all"?
We should not put (or require! or even accept!) things in But yes, we should place things in |
| pip install -r requirements.txt | ||
| ansible-galaxy install -r requirements.yml | ||
|
|
||
| echo "\n\nSetup complete. Activate the virtualenv 'source .venv/bin/activate' and then run the cli 'rop'" |
There was a problem hiding this comment.
bash's echo does not interpret escape chars by default. you need to either use echo -e (non POSIX, afaik) or printf
I think it's an interesting idea, that we should capture but I'd like to see us get further and through more of the complex bits before we decide that it can be that simple. We also have some non-collection things in our tool like the metadata. I guess we could put it in there but would the collection validators complain? |
| name='cli', | ||
| version='0.0.1', | ||
| license='GPL-2.0-only', | ||
| description='packaging wrapper using ansible', |
There was a problem hiding this comment.
| description='packaging wrapper using ansible', | |
| description='foreman deployment using ansible', |
I was thinking about that. At least for the prototype it would keep things simple if we don't need a Python package. Then when we get closer to an actual installer (and perhaps tool to replace foreman-maintain) we can look at how to build it. For example, |
|
I like the "simple shell script" idea, as it not only reduces the amount of (boilerplate) code, but also simplifies the directory structure in the repo. OTOH the current state in the PR works fine and would allow us to continue iterating. Read this comment as: ACK, merge it, unless you want to rewrite it again right now :)
Collection validators don't care much about additional files |
|
that's merged, and the following works after a do whatever you want with that info :) |
Are you thinking this would be recreated in Ansible and follow the same CLI model? Or that it would require some real code in python that would be handled? |
That will depend on how much of the functionality we keep. I'd love to keep it minimal but at this point I'm unclear on where we'd end up and if it's realistic. |
Signed-off-by: Eric D. Helms <ericdhelms@gmail.com>
|
Closing in favor of #143 |
Simple as I could get CLI using Obsah based on the
data-typesbranch (theforeman/obsah#63). I'll follow up with adding some additional variables that make use of types to play around with the idea more.