Based on chat last week, we are considering autogenerating an issue to every collection repo included in the Ansible package. The goal here is to ensure they are aware of the data tagging feature and will (hopefully) engage in testing ahead of time to ensure their collections/playbooks/roles are compatible.
So the goal of this post is to draft out what would be in that autogenerated issue. I added the next meeting tag, but I think we need to resolve this before the meeting so we can engage all those collection owners as soon as possible.
I’m thinking of something like the following (note I’m hoping the PR merges into devel soon so that’s how I wrote the issue text. Can change to fallable if needed)
issue title: Validate compatibility with ansible-core data tagging feature
Issue body:
ansible-core has implemented an extensive rewrite of templating and error handling as part of the data tagging feature coming in core-2.19. You can find information about this change in that PR description, as well as in the draft porting guide.
We need the owners of collections that are part of the Ansible community package to now test against ansible-core devel and stable-2.19 branches to ensure they continue to work with the updated core. This should be an ongoing verification/check until the release of core 2.19 to ensure compatibility with any bugfixes or changes that happen until then.
Note: You are not required to implement data tagging as part of this request, though we do recommend you consider where it can improve the quality of your collection. This issue is focused on ensuring you have verified your collection works with ansible-core devel in order to remain included in Ansible 12 community package.
We ask that you resolve any problems and link those PRs to this issue. If you do not find any problems with your integration/unit testing, please provide that comment and a pointer to your test results before closing this issue as done.
You can reach out on the Ansible forum for more help or discussion.
You want to open the issue only in repositories that are part of the Ansible community package, right? So maybe make this clear:
We need the owners of collections that are part of the Ansible community package to now test against ansible-core devel branch to ensure they continue to work with the updated core.
Otherwise, people who maintain a collection that is and another one that isn’t might wonder why they get this issue for the one but not the other.
Should we include some practical advice? For example:
If your unit tests start failing because you mock module arguments like this:
from ansible.module_utils import basic
args = json.dumps({'ANSIBLE_MODULE_ARGS': args})
basic._ANSIBLE_ARGS = to_bytes(args)
... execute the module ...
This no longer works, you need to use the ansible.module_utils.testing.patch_module_args() context manager instead:
from ansible.module_utils.testing import patch_module_args
with patch_module_args(args):
... execute the module ...
One way to handle this for both older and newer ansible-core versions is to create your own context manager:
@_contextlib.contextmanager
def set_module_args(args):
"""
Context manager that sets module arguments for AnsibleModule
"""
if '_ansible_remote_tmp' not in args:
args['_ansible_remote_tmp'] = '/tmp'
if '_ansible_keep_remote_files' not in args:
args['_ansible_keep_remote_files'] = False
try:
from ansible.module_utils.testing import patch_module_args
except ImportError:
# Before data tagging support was merged, this was the way to go:
from ansible.module_utils import basic
serialized_args = to_bytes(json.dumps({'ANSIBLE_MODULE_ARGS': args}))
with patch.object(basic, '_ANSIBLE_ARGS', serialized_args):
yield
else:
# With data tagging support, we have a new helper for this:
with patch_module_args(args):
yield
Then you can always run:
with set_module_args({...}):
... execute the module ...
Some practical advice might be helpful and sounds like a good idea to me. But I wonder if this wouldn’t make the issue text very long. And it would be also hard to add more practical advice in case it would be useful or needed.
How about having the advice from @felixfontein somewhere else and just add a link to the issue? Maybe on the forum. We could re-use Data tagging playground for this or Data Tagging preview and testing or create a new thread like Making a collection compatible with data tagging or something.
Then we would have a central place to add more advice when needed. And this would also mean that we can answer general questions in one place once instead of repeatedly in several issues.
Yeah, it would be better to have a place here in the forum (or on GH) to collect such advice. I’m not sure what’s the best place for it… A new thread “Making a collection compatible with Data Tagging” sounds good to me!
Additionally, we might also suggest to start testing against stable-2.19.
Out of curiosity, what about the milestone branch? Is it dead, or will it be updated when the data tagging PR is merged / ansible-core beta 1 is released? It looks quite old to me.
Good idea about a new forum post to cover the hints and tips. @felixfontein are you up for creating it since you had a good batch of ideas to start with?
I can also go into the other two existing data tagging topic and add some cross linking between these three posts so folks can find it all.
@felixfontein - I’m going to creat the post now and you can add your stuff as the first comment. I want to craft a special bullhorn edition today that will be data tagging/core-2.19 focused so I need that link to be live in the next few minutes
Thanks for creating the topic! I’ve copied my unit test advice over from above.
I’m not sure whether I’ll have time for creating a newsbot announcement, as I’ll probably be busy with rebasing/merging PRs and potentially creating new ones / updating existing ones, and I’ll be gone for a few hours this evening as well to an external event…
no rush on the newsbot. I’ve manually added it to this week’s bullhorn. But would be good to have it there as a reminder for next week’s bullhorn readers… and thanks!!
I think that the advise is to not test against devel and use milestone instead. The reason is that devel might be broken temporarily, but milestone shouldn’t.
The good news is that it looks like the DT stuff has been added to the milestone branch.
Testing against devel is fine, as long as you’re prepared to handle the possibility that it’s broken. If you’re not, then testing against milestone is advised instead. If you’re up for it, testing against both will help catch issues earlier – but doing so comes with the cost of more effort on the part of collection maintainers.
I prefer testing against devel over testing against milestone since the CI breakages that a milestone bump carries are often larger than the more frequent CI breakages that devel gives me. I can’t just take 1-2 days off from work to fix all CIs once milestone gets bumped, but I can invest a couple of hours to fix all CIs when something happens in devel.
For someone who works full-time on this, or is getting paid to do this, the trade-off might be different.
Ideally, we could have this merged before creating these issues now that the Core change has been merged so we don’t need to link to draft PRs in the announcement,
Maybe “fellow maintainers” instead of “peers”? “Peers” seems a bit formal to me.
This issue is focused on ensuring you have verified your collection works with ansible-core devel in order to remain included in Ansible 12 community package.
This (and other text) implies that this is a requirement to remain included in the package, but I think we need to be more up-front and explicit (but obviously not overly harsh or rude), as it’s one of the most fundamental inclusion requirements that the collections included in an Ansible version are actually functional and compatible with its associated Core version.
I think I know what you mean, but I’m not able to come up with a good wording ATM.
Anyway, reading this a second time the original text is a little bit misleading. The collections included in ACP have to be compatible with the associated Core version, and that’s 2.19. Testing against the devel branch isn’t really a requirement, just a way to find any issues early. But now that there are the ansible-core 2.19.0b1 PyPI package and the stable-2.19 branch in the core repo available, they are also good ways to test.
Ultimately, the collections should work with ansible-core 2.19.0 and not devel.
