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

Add --ident option to process command #939

Merged
merged 1 commit into from Dec 16, 2021

Conversation

AlanCoding
Copy link
Member

Fixes #821

Below is my bash scripts where I emulate AWX behavior on hybrid nodes. I might document these in a repository sometime... maybe they would work for docs? Maybe not because I actually run them.

echo 'Initial cleanup tasks'
rm -rf overwrite
rm -rf demo/artifacts
mkdir overwrite
cp -Ra demo overwrite/1

ident='b20e75dd2a4049708f3ed10a73cad3cc'
echo "The ident for this run is: $ident"

echo 'Transmission phase - fills the first buffer'
ansible-runner transmit ./overwrite/1 -p test.yml --ident=$ident > ./overwrite/output1

echo 'Worker phase - fills second buffer'
ansible-runner worker --private-data-dir=./overwrite/1 < ./overwrite/output1 > ./overwrite/output2

echo 'Process phase - runs callbacks and pretty much nothing else'
ansible-runner process overwrite/1 --ident=$ident < ./overwrite/output2

tree overwrite

Long story short, this is how AWX works with a slight modification for how I think it should work, which is the added --ident option.

Without this, you have 2 files for overwrite/1/artifacts/rc and overwrite/1/artifacts/b20e75dd2a4049708f3ed10a73cad3cc/rc/. So which one is the right one? I want to avoid that confusion. Ping @jbradberry

@AlanCoding AlanCoding requested a review from a team as a code owner December 10, 2021 14:19
@github-actions github-actions bot added docs Changes to documentation needs_triage New item that needs to be triaged labels Dec 10, 2021
Copy link
Member

@jbradberry jbradberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this worry early on but then nothing ever seemed to come of it. Ultimately, I think this is a good idea.

@AlanCoding
Copy link
Member Author

We could add more things to the ansible-runner process CLI, but the issue is that I frankly just don't need them. It's confusing from the code, but we don't have --artifact-dir. That's the one other notable absence I saw. In terms of all the other config parameters that affect job execution, we don't want them because the job is already off, and we can't change it.

@AlanCoding
Copy link
Member Author

If you wanted, we could actually add the --artifact-dir, not add the --ident, and then not change anything about the streaming.py module. AWX could make due with that, no problem. If you consider ansible-runner run --help, both of these are options, so I figure they're both equally valid here.

@Shrews Shrews added needs_triage New item that needs to be triaged and removed needs_triage New item that needs to be triaged labels Dec 13, 2021
Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is most likely ok. I see ident is already documented in the interface.run() method(s). I've been trying to get this to work via the Python API to make sure it works that way, but I'm having some difficulty getting that working properly (probably usage error on my part, not a bug here).

@samdoran samdoran removed the needs_triage New item that needs to be triaged label Dec 14, 2021
@Shrews Shrews added the gate label Dec 15, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ansible-zuul
Copy link
Contributor

ansible-zuul bot commented Dec 15, 2021

Pull request merge failed: Resource not accessible by integration

@Shrews
Copy link
Contributor

Shrews commented Dec 15, 2021

recheck

4 similar comments
@goneri
Copy link

goneri commented Dec 15, 2021

recheck

@goneri
Copy link

goneri commented Dec 15, 2021

recheck

@goneri
Copy link

goneri commented Dec 15, 2021

recheck

@Shrews
Copy link
Contributor

Shrews commented Dec 15, 2021

recheck

@Shrews
Copy link
Contributor

Shrews commented Dec 16, 2021

recheck

@Shrews Shrews added gate and removed gate labels Dec 16, 2021
@ansible-zuul
Copy link
Contributor

ansible-zuul bot commented Dec 16, 2021

Pull request merge failed: Resource not accessible by integration

This will allow users to write artifacts in a way that
matches normal non-remote ansible-runner commands

It makes the most sense to have the process --ident match
the --ident value passed to transmit, but you do not have to.

The intention is that this will be used by AWX so that
jobs ran and processed on the same node will not write
artifacts twice.
@ansible-zuul ansible-zuul bot merged commit d013044 into ansible:devel Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Changes to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ansible-runner process puts job event artifacts in the wrong folder
5 participants