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

New modules incur weird looking new section 'New Modules' #18

Closed
geerlingguy opened this issue Jun 16, 2020 · 15 comments · Fixed by #22
Closed

New modules incur weird looking new section 'New Modules' #18

geerlingguy opened this issue Jun 16, 2020 · 15 comments · Fixed by #22

Comments

@geerlingguy
Copy link

In one release, our collection added two new modules, and the generated RST looked a bit weird to me...

New Modules
-----------

ansible.collections.ansible_collections.community.kubernetes.plugins.modules
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- .ansible.collections.ansible_collections.community.kubernetes.plugins.modules.k8s_exec - Execute command in Pod
- .ansible.collections.ansible_collections.community.kubernetes.plugins.modules.k8s_log - Fetch logs from Kubernetes resources

What is this supposed to look like? And why oh why is the listing for each module so long?

@geerlingguy
Copy link
Author

Also, I don't know how it detected the new modules to put them in its own section...?

I had a fragment like:

---
major_changes:
  - k8s_exec - New module for executing commands on pods via Kubernetes API.

Does it scan the text for "New module" and then aggregate any modules that are mentioned, or is there some other way the tool finds new modules and puts them into the 'New Modules' section?

@felixfontein
Copy link
Collaborator

It uses ansible-doc to get information on all modules. It's mostly interested in short_description and version_added, and constructs namespace from the path where ansible-doc found the module (compared to the collection's path).

Are you running the generator in a checkout of the collection that's also found by ansible-doc?

@felixfontein
Copy link
Collaborator

You can fix these paths by editing changelogs/changelog.yaml manually and adjusting the namespace for these plugins. Since the collection you're working with is flat (i.e. no subdirectories in plugins/modules/), simply set it to an empty string. Then run antsibull-changelog generate and the .rst file will get updated.

@geerlingguy
Copy link
Author

@felixfontein - Yes, I have the repository checked out into my ~/.ansible/collections path, and when I use ansible-doc it finds the module like so:

$ ansible-doc community.kubernetes.k8s
> K8S    (/Users/jgeerling/.ansible/collections/ansible_collections/community/kubernetes/plugins/modules/k8s.py)
...

Setting the namespace: '' in each of the entries in the config file worked—but that doesn't seem like a great idea for sustaining this long term... it would require someone to do that manually and then regenerate the CHANGELOG again every time we add a new module.

@jborean93
Copy link

jborean93 commented Jun 18, 2020

Curious as to why we add the fully qualified collection path under the New Modules section. An example when testing it out on the ansible.windows collection is

ansible.collections.ansible_collections.ansible.windows.plugins.modules
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- .ansible.collections.ansible_collections.ansible.windows.plugins.modules.win_ping2 - A windows version of the classic ping module v2

Is the ansible.collections.ansible_collections.ansible.windows.plugins.modules part required? Seems like unnecessary information that the user doesn't need to know about.

I also agree that the modules and action plugins should use the same format as what you put into the task, i.e. namespace.collection.name and not the actual python path to the file. Having to manually edit the file to get this behaviour is not really ideal and the defaults should favour what the end user of a collection will want to use.

@felixfontein
Copy link
Collaborator

@jborean93 it's not required and it shouldn't be there. I'll try to debug this.

@felixfontein
Copy link
Collaborator

@geerlingguy I didn't mean that it is a long-term solution, but a short-term fix until I fixed the code generating them :)

@felixfontein
Copy link
Collaborator

Hmm, I've tried to reproduce this with stable-2.9, stable-2.10 and devel, and with both community.crypto and community.kubernetes (PR 131's branch), and I always ended up with the correct value for namespace. If you can give me any hints how I can reproduce this, I'll be happy!

I've also tried the following (next to run it locally in different variants on my machine):

docker run --rm -it python:3 /bin/bash

And inside that:

pip install ansible==2.9.9
# pip install ansible-base==2.10.0b1
pip install antsibull-changelog
mkdir -p ~/.ansible/collections/ansible_collections/community/
cd ~/.ansible/collections/ansible_collections/community/
git clone https://github.com/ansible-collections/community.crypto.git crypto
cd crypto
antsibull-changelog release
cat changelogs/.plugin-cache.yaml 

Alternatively:

# pip install ansible==2.9.9
pip install ansible-base==2.10.0b1
pip install antsibull-changelog
mkdir -p ~/.ansible/collections/ansible_collections/community/
cd ~/.ansible/collections/ansible_collections/community/
git clone https://github.com/ansible-collections/community.kubernetes.git kubernetes
cd kubernetes
antsibull-changelog init .
antsibull-changelog release
cat changelogs/.plugin-cache.yaml 

Same result, namespace is '' for all modules.

@felixfontein
Copy link
Collaborator

pip install ansible-base==2.10.0b1
pip install antsibull-changelog
mkdir -p ~/.ansible/collections/ansible_collections/ansible/
cd ~/.ansible/collections/ansible_collections/ansible/
git clone https://github.com/ansible-collections/ansible.windows.git windows
cd windows
antsibull-changelog init .
antsibull-changelog release
cat changelogs/.plugin-cache.yaml 

also works (i.e. namespace is '' for modules).

@jborean93
Copy link

I just tried it on a new container and I'm not able to replicate the error anymore. Will try again on my desktop tomorrow morning and see if I can figure out what is happening there.

@felixfontein
Copy link
Collaborator

@jborean93 thanks a lot for trying to reproduce this!

@jborean93
Copy link

jborean93 commented Jun 18, 2020

Figured it out, I had a symlink in ~/.ansible/collections/ansible_collections/ansible/windows to ~/dev/ansible_collections/ansible/windows and because the former was higher in the ANSIBLE_COLLECTIONS_PATHS it was the one reported by ansible-doc. I'm not sure why I had the symlink there, must have been for testing but in any case when removing it, ansible-doc reported the correct path. I'm not sure if when generating the changelog we want to ensure the collections collection path is always first so it takes a higher precedence against the same collection that could already be installed elsewhere.

You can use this to test it out

pip install ansible-base==2.10.0b1
pip install antsibull-changelog
export ANSIBLE_COLLECTIONS_PATHS=~/.ansible2/collections:~/.ansible/collections
git clone https://github.com/ansible-collections/ansible.windows.git ~/.ansible/collections/ansible_collections/ansible/windows

mkdir -p ~/.ansible2/collections/ansible_collections/ansible
ln -s ~/.ansible/collections/ansible_collections/ansible/windows ~/.ansible2/collections/ansible_collections/ansible/windows

cd ~/.ansible/collections/ansible_collections/ansible/windows
antsibull-changelog release

cp plugins/modules/win_ping.py plugins/modules/win_ping2.py
cp plugins/modules/win_ping.ps1 plugins/modules/win_ping2.ps1
sed -i 's/module: win_ping/module: win_ping2\nversion_added: 1.0.0/g' plugins/modules/win_ping2.py

antsibull-changelog release --version 1.0.0

Edit: Just wanted to clarify the issue isn't solely the use of a symlink but rather having 2 collections installed under the same name with the other one having a higher precedence in ANSIBLE_COLLECTIONS_PATHS.

@felixfontein
Copy link
Collaborator

@jborean93 thanks for figuring this out!

As discussed on IRC, I'll implement the following:

  1. Check whether collection is in ansible_collections/<namespace>/<name> structure;
  2. If it is not, copy to /tmp/xxx/ansible_collections/<namespace>/<name>;
  3. Run ansible-doc with --playbook-dir, and/or set ANSIBLE_COLLECTIONS_PATH(S) before calling ansible-doc.

@felixfontein
Copy link
Collaborator

I think I fixed this in #22. It will always copy the collection to /tmp and use --playbook-dir (that one needs collections/ansible_collections/<namespace>/<name>, and I guess the additional collections usually isn't there, so I didn't add logic to detect whether this is already the case and not copy in that case; would be easy to add later though).

@geerlingguy @jborean93 I'd be happy if you could test whether it solves the problem for you!

felixfontein added a commit to felixfontein/antsibull-build that referenced this issue Oct 13, 2020
abadger pushed a commit to ansible-community/antsibull-build that referenced this issue Oct 13, 2020
@SHxKM
Copy link

SHxKM commented Dec 5, 2021

I'm having what I think is the same problem here: #68. I actually think #22 is what's causing my issue. What's a good workaround?

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

Successfully merging a pull request may close this issue.

4 participants