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

document where to save cloud config files #79412

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

mdavis-xyz
Copy link
Contributor

SUMMARY

This fixes part of #79411.
This fixes the comment part.
And the part about the optional security_token field.
This does not modify the error message returned by ansible-test.

This should help with: ansible-collections/amazon.aws#924

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

config

ADDITIONAL INFORMATION

I don't know what most of those files I modified are. I've only tested AWS stuff previously. So I need someone to confirm that the changes to those other files are applicable to those.

I did not modify the cloudscale one, because that's very barebones. I don't know if it's the same situation as the others.

I did not modify the openshift one, because it already had some mention of putting another file next to this one. That sounds possibly not right. Should both files go into tests/integration instead?

@ansibot ansibot added affects_2.15 docs This issue/PR relates to or includes documentation. needs_triage Needs a first human triage before being processed. test This PR relates to tests. labels Nov 18, 2022
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 26, 2022
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Nov 29, 2022
Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

All of the cloud- prefixed templates are handled the same way, so the boilerplate should be updated on all of them.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 16, 2023
@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Jan 17, 2023

What should the boilerplate say for the openshift one?

It currently says:

place your kubeconfig file next to this file, with the same name, but without the .template extension.

The point of this change is that you're not supposed to modify these files or put files into the same folders.
So is "next to this file" incorrect?

I don't understand the point of that file. There's nothing in it. Is it just supposed to be some instructions, which are located in the same place as instructions for other modules?

Should it say:

place your kubeconfig file
into the tests/integration directory of the collection you're testing,
with the same name as this file, but without the .template extension.

@mattclay
Copy link
Member

Should it say:

place your kubeconfig file
into the tests/integration directory of the collection you're testing,
with the same name as this file, but without the .template extension.

Yes, that should work. The instructions in all of these template files were created when they originally resided in the test/integration/ directory (before we even had support for collections). The instructions were never updated when the files were moved.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 17, 2023
@mdavis-xyz
Copy link
Contributor Author

Ok, now I've updated all the cloud- files.

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 18, 2023
Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

Looks good. Just one minor cosmetic change requested.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 18, 2023
Co-authored-by: Matt Clay <matt@mystile.com>
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 19, 2023
@mattclay mattclay merged commit 8e80b03 into ansible:devel Jan 19, 2023
@mattclay
Copy link
Member

@mdavis-xyz Thanks for fixing this.

@ansible ansible locked and limited conversation to collaborators Jan 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 docs This issue/PR relates to or includes documentation. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants