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

Doc: add example of using kubectl connection plugin #741

Merged

Conversation

yurnov
Copy link
Contributor

@yurnov yurnov commented Jun 1, 2024

SUMMARY

Currently documentation for collection don't include any examples of using kubenrenes.core.kubectl connection plugin and it's hard to start using that plugin.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

kubenrenes.core.kubectl connection plugin

ADDITIONAL INFORMATION

This PR was inspired by #288 and based on feedback on that PR and my own experience. Thanks @tpo for his try and @geerlingguy for his Ansible for DevOps book

Copy link

three hyphens as the berrinign of playbook in the example lead to "unparsable-with-libyaml: expected a single document in the stream - but found another document" error in yamllint, so three hyphens is removed in examples
Copy link

@tpo
Copy link

tpo commented Jun 2, 2024

Terrific improvement of the documentation. Thanks a lot @yurnov ! I have reviewed the changes and hope this can be merged.

In case there is a formal flag "reviewed by tpo" or such then I could add it if someone could point me to what the syntax is.

@yurnov
Copy link
Contributor Author

yurnov commented Jun 3, 2024

Hi @abikouo and @gravesm,

Can this PR be merged and backported to the stable-3... stable-5 branches? The request to backport to stable-3 and release some 3.2 or so is related to the fact that major version 4 is not included in Ansible 10 which means that documentation changes for collection version 4.x.x and 5.x.x. will not be part of official ansible documentation for another 6+ months.

Based on Ansible Project 10, the current stable version is an Ansible 9 (which include kubernetes.core version 2.4.2) and Ansible 10 should be released tomorrow or in the next week and that version will consist of collection version 3.1.0

Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

Just minor remarks regarding FQCN and naming playbooks

docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
@yurnov
Copy link
Contributor Author

yurnov commented Jun 3, 2024

Just minor remarks regarding FQCN and naming playbooks

Thanks, your comments was considered

comment intendation aligned
Copy link

Copy link

@samccann samccann left a comment

Choose a reason for hiding this comment

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

A few spelling fixes.

Also, I'm unsure where any of the docs in the /docs directory here ever show up for users. They don't show up on Galaxy or docs.ansible.com. That's a broader issue than this PR but something to investigate. My limited knowledge of Galaxy/AH says files directly in the /docs directory must be .md not rst. And docs for docs.ansible.com must be in docs/docsite/rst.

docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
plugins/connection/kubectl.py Outdated Show resolved Hide resolved
@gravesm
Copy link
Member

gravesm commented Jun 3, 2024

Also, I'm unsure where any of the docs in the /docs directory here ever show up for users. They don't show up on Galaxy or docs.ansible.com. That's a broader issue than this PR but something to investigate. My limited knowledge of Galaxy/AH says files directly in the /docs directory must be .md not rst. And docs for docs.ansible.com must be in docs/docsite/rst.

@samccann We use https://github.com/ansible-network/collection_prep to generate docs. I believe this is done solely so there are docs on GH. I assume the release pipeline has its own docs build step, but I'm not sure. The docs for this plugin do show up on https://docs.ansible.com/ansible/latest/collections/kubernetes/core/kubectl_connection.html#ansible-collections-kubernetes-core-kubectl-connection.

@samccann
Copy link

samccann commented Jun 3, 2024

@samccann We use https://github.com/ansible-network/collection_prep to generate docs. I believe this is done solely so there are docs on GH. I assume the release pipeline has its own docs build step, but I'm not sure. The docs for this plugin do show up on https://docs.ansible.com/ansible/latest/collections/kubernetes/core/kubectl_connection.html#ansible-collections-kubernetes-core-kubectl-connection.

Ah thanks for the clarification. I saw the files but didn't put 2+2 together and realize they were just a local copy (in GH) of the module docs.

@yurnov
Copy link
Contributor Author

yurnov commented Jun 3, 2024

Also, I'm unsure where any of the docs in the /docs directory here ever show up for users.

I'm not sure how a ansible site shown docs, but in any case, they generated from DOCUMENTATION and EXAMPLES in the Python code by the https://github.com/ansible-network/collection_prep, as mentioned in CONTRIBUTING.

It looks like they are combined from the rst fragments.

doc fragment (docs/kubernetes.core.kubectl_connection.rst) is generated by collection_prep_add_docs
Copy link

Copy link

@yurnov yurnov requested review from gravesm and abikouo June 4, 2024 09:38
Copy link

@purdzan purdzan left a comment

Choose a reason for hiding this comment

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

it seems typo

docs/kubernetes.core.kubectl_connection.rst Outdated Show resolved Hide resolved
Copy link

@yurnov yurnov requested a review from purdzan June 5, 2024 15:11
it is recomendation by @felixfontein to avoid changed_when: false as it may confuse users and they mayht assume that you have to use that parameter

most probably no-changed-when linter rule was enforced with ansible-lint v24.6.0 released yesterday https://github.com/ansible/ansible-lint/releases/tag/v24.6.0
Copy link

Copy link

@yurnov
Copy link
Contributor Author

yurnov commented Jun 5, 2024

Hi @gravesm @abikouo,

somehow linter started to fail on example. I considered adding changed_when: false (please find the commit 03be82d2a2d11fb358bc38cd991632e9bdec6645), but some users might assume that you have to use changed_when: false

So, I decided (actually, it was recommended by @felixfontein in community:ansible.com matrix channel to add rule no-changed-when to ignore list. Do you ok with that?

Copy link

@gravesm gravesm added the mergeit label Jun 6, 2024
Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/c6a2f2f6a7374c9d92cd11fcc7015ab7

✔️ ansible-galaxy-importer SUCCESS in 5m 23s
✔️ build-ansible-collection SUCCESS in 7m 56s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit fb80d97 into ansible-collections:main Jun 6, 2024
44 checks passed
Copy link

patchback bot commented Jun 6, 2024

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/fb80d973c446d6626aa00a0f55a8ae53e3e517c4/pr-741

Backported as #743

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jun 6, 2024
Doc: add example of using kubectl connection plugin

SUMMARY
Currently documentation for collection don't include any examples of using kubenrenes.core.kubectl connection plugin and it's hard to start using that plugin.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
kubenrenes.core.kubectl connection plugin
ADDITIONAL INFORMATION
This PR was inspired by #288 and based on feedback on that PR and my own experience. Thanks @tpo for his try and @geerlingguy for his Ansible for DevOps book

Reviewed-by: Bikouo Aubin
Reviewed-by: Sandra McCann <[email protected]>
Reviewed-by: Mike Graves <[email protected]>
Reviewed-by: Yuriy Novostavskiy
Reviewed-by: purdzan
(cherry picked from commit fb80d97)
Copy link

patchback bot commented Jun 6, 2024

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/fb80d973c446d6626aa00a0f55a8ae53e3e517c4/pr-741

Backported as #744

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jun 6, 2024
Doc: add example of using kubectl connection plugin

SUMMARY
Currently documentation for collection don't include any examples of using kubenrenes.core.kubectl connection plugin and it's hard to start using that plugin.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
kubenrenes.core.kubectl connection plugin
ADDITIONAL INFORMATION
This PR was inspired by #288 and based on feedback on that PR and my own experience. Thanks @tpo for his try and @geerlingguy for his Ansible for DevOps book

Reviewed-by: Bikouo Aubin
Reviewed-by: Sandra McCann <[email protected]>
Reviewed-by: Mike Graves <[email protected]>
Reviewed-by: Yuriy Novostavskiy
Reviewed-by: purdzan
(cherry picked from commit fb80d97)
@yurnov
Copy link
Contributor Author

yurnov commented Jun 6, 2024

Thanks @gravesm!

@yurnov yurnov deleted the kubectl_example branch June 6, 2024 14:06
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 6, 2024
[PR #741/fb80d973 backport][stable-3] Doc: add example of using kubectl connection plugin

This is a backport of PR #741 as merged into main (fb80d97).
SUMMARY
Currently documentation for collection don't include any examples of using kubenrenes.core.kubectl connection plugin and it's hard to start using that plugin.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
kubenrenes.core.kubectl connection plugin
ADDITIONAL INFORMATION
This PR was inspired by #288 and based on feedback on that PR and my own experience. Thanks @tpo for his try and @geerlingguy for his Ansible for DevOps book

Reviewed-by: Yuriy Novostavskiy
Reviewed-by: Mike Graves <[email protected]>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 6, 2024
[PR #741/fb80d973 backport][stable-5] Doc: add example of using kubectl connection plugin

This is a backport of PR #741 as merged into main (fb80d97).
SUMMARY
Currently documentation for collection don't include any examples of using kubenrenes.core.kubectl connection plugin and it's hard to start using that plugin.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
kubenrenes.core.kubectl connection plugin
ADDITIONAL INFORMATION
This PR was inspired by #288 and based on feedback on that PR and my own experience. Thanks @tpo for his try and @geerlingguy for his Ansible for DevOps book

Reviewed-by: Mike Graves <[email protected]>
This was referenced Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants