Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run inventory updates using containerized execution #7061

Closed
ryanpetrello opened this issue May 18, 2020 · 11 comments
Closed

Run inventory updates using containerized execution #7061

ryanpetrello opened this issue May 18, 2020 · 11 comments

Comments

@ryanpetrello
Copy link
Contributor

This is a subtask of a larger enhancement/initiative surrounding execution environments: #5157

Specifically, it is a component of the work described in #7060

Today, inventory updates operate by spawning an awx-manage command that:

  1. Uses bubblewrap and subprocess.Popen to fork an ansible-inventory invocation.
  2. Parses the JSON result of the ansible-inventory process.
  3. Translates that JSON data into a series of ORM calls in Python to create/update/delete appropriate inventory/group/host records.

Under a containerized model (as described in #7060), we'd need to rethink how we accomplish this.

@AlanCoding
Copy link
Member

I can see this having several pre-requisites.

See #6911. Because in the containerized model, we don't have the inventory scripts folder, but we do have collections. While collections do have inventory scripts, several don't work right now, and we would rather not make adjustments to get them working.

@AlanCoding
Copy link
Member

AlanCoding commented Jun 11, 2020

Trying to break this up into sub-tasks, @blomquisg @jladdjr. My preferred approach would be to start with some general refactoring. Not all may be required, depending on the tech debt we accept.

General refactoring to pave the way:

  • Remove scripts Remove vendored inventory scripts #6911
  • Remove the MemInventory objects https://github.com/ansible/awx/blob/devel/awx/main/utils/mem_inventory.py, these are old and unnecessary, the pattern doesn't reflect the ansible-inventory pattern (main reason IMO was to process group_vars type folders), and if not refactored, this complicates the latter logging solution
  • Replace the AnsibleInventoryLoader with an interface that is swappable with tasks.py code. Again, due to ansible-inventory we know what the interface should be, and the current code is not it. We should have some shared code (preferably not in the management command) that accepts a python dictionary and saves it to the inventory. The management command can call that.

Direct feature work:

  • Create a solution so that logging entries can be created outside the context of ansible-runner, an old POC is at [WIP] Proof of principle of directly calling task management commands #451. Does this feature entail a change to the UX of inventory updates? Easiest answer is "no", and if that's the case, then something resembling that approach will be needed.
  • Actually change the call stack to short circuit awx-manage in tasks.py, and call code separately to save inventory changes. This is likely to be combined with the prior item.
  • Actually integrate with EE containerized runs and receptor

The only thing blocked by other development is the final item. Not all of these may be needed, I'm just saying out loud what seems like the best approach.

Some of these can be worked on in parallel, and it doesn't have to be me who does it. Because I have some context, I want to help point out old gunk that we don't need to keep as this new system is developed. There's also a lot of exciting testing possibilities as parts of this become a reality.

@AlanCoding
Copy link
Member

The first part of this is started in #8323

@wenottingham
Copy link
Contributor

There is a request in #8494 to essentially delegate the inventory update to a node in the execution plane. Hanging off here in case it could be a potential use case.

@jladdjr
Copy link
Contributor

jladdjr commented Oct 30, 2020

Just to give a general update on things here:

With Alan out, I've picked up #8323. We've been testing those changes and I just had a chance to review the results with @shanemcd. We came up with a few remaining todo items:

  • need to pull out code that removed proot behavior (don't want there to be a gap period where proot disappear without having containers place to provide isolation)
  • alan mentioned a couple of tests that he'd like to see in the PR. I'm going to work on writing those up.

Alan's PR, once merged, should take us one step closer to being able to run inventory updates in a container. His work basically helps separate database updates (won't be able to run inside a container) from the actual work of updating an inventory with ansible-inventory (the part that we do want to happen inside the container).

cc @ryanpetrello

@jladdjr
Copy link
Contributor

jladdjr commented Nov 2, 2020

Restored the proot code on Alan's refactor PR. Confirmed that inv. update jobs create a separate folder for isolation and that sensitive files aren't present. Details here.

@jladdjr
Copy link
Contributor

jladdjr commented Nov 3, 2020

Restored venv's for project updates as well:
0428d2c

@AlanCoding
Copy link
Member

^ yes, that was necessary and I believe you fully finished it.

To my knowledge, we're stuck on just the 1 blocker for #8323, where, for some reason, it leaves around stale settings (for LICENSE and TOWER_URL_BASE). Both of us have now beaten our heads against that, and it remains a stubborn mystery.

Recap of the plan - assuming that gets resolved, then we merge PR 8323, and then rebase the execution-environments branch after that. However, there are 2 significant chunks of unmerged work waiting to get into that branch:

So we merge the 3rd one there, then the 2nd one, then rebase the 1st one, then merge it into devel.

Then after that, we start on the actual containerization of inventory updates. Hopefully, that task shouldn't be too bad.

@AlanCoding
Copy link
Member

We have merged things for the first part of this. Next part is that execution-environments branch gets rebased, and after that, we can start the actual containerization of inventory updates.

@AlanCoding
Copy link
Member

^ that was done a while ago, this is finished and working rather well in the execution-environments branch

@kdelee
Copy link
Member

kdelee commented Jan 25, 2021

This is done from QE perspective because all regression tests are passing.

There are a couple items that were not done on alan's checklist:

 Remove the MemInventory objects https://github.com/ansible/awx/blob/devel/awx/main/utils/mem_inventory.py, these are old and unnecessary, the pattern doesn't reflect the ansible-inventory pattern (main reason IMO was to process group_vars type folders), and if not refactored, this complicates the latter logging solution
 Replace the AnsibleInventoryLoader with an interface that is swappable with tasks.py code. Again, due to ansible-inventory we know what the interface should be, and the current code is not it. We should have some shared code (preferably not in the management command) that accepts a python dictionary and saves it to the inventory. The management command can call that.

but those appear to be internal refactors that would have no user facing component, so I'm going to leave it up to devs to address them later at their own pace.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants