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

Update azure_rm_aks.py #58423

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@iRootkit
Copy link

commented Jun 26, 2019

SUMMARY

Add a vnet_subnet_id parameter to the module docs, a description and note.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

agent profile changes docs

Update azure_rm_aks.py
##### SUMMARY
Add a vnet_subnet_id parameter to the module docs, a description and note.
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/azure/azure_rm_aks.py:0:0: E305 DOCUMENTATION.options.agent_pool_profiles.suboptions.vnet_subnet_id.description.1: expected str @ data['options']['agent_pool_profiles']['suboptions']['vnet_subnet_id']['description'][1]. Got {'Note': 'Does not overlap 169.254.0.0/16, 172.30.0.0/16, 172.31.0.0/16, 192.0.2.0/24'}

click here for bot help

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

@iRootkit, just so you are aware we have a dedicated Working Group for azure.
You can find other people interested in this in #ansible-azure on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@MyronFanQiu

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

@iRootkit Thanks for the PR. Please fix the CI issue. Based on my understanding, the error may be caused by Note:. It will be treated as a deeper dict in the yml file.

@MyronFanQiu

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

@zikalino @yungezz Please review this PR when you are free. It's a Docs Pull Request.

vnet_subnet_id=dict(type='str'),

@acozine
Copy link
Contributor

left a comment

Thanks @iRootkit for adding documentation for a missing sub-option.

Can you provide a bit more context or information? Will this module create a new subnet if the vnet_subnet_id does not exist? If the user specifies a value that does overlap, what happens?

@@ -77,6 +77,10 @@
os_disk_size_gb:
description:
- Size of the OS disk.
vnet_subnet_id:
description:
- The Subnet ID for a existing Subnet or if you use a different CIDR for the Nodes IP (Kubenet or Azure CNI)

This comment has been minimized.

Copy link
@acozine

acozine Jul 1, 2019

Contributor

I find this confusing. I've made a suggestion as a way to start the conversation - let me know if it's accurate.

Suggested change
- The Subnet ID for a existing Subnet or if you use a different CIDR for the Nodes IP (Kubenet or Azure CNI)
- The subnet for the agent pool profile. For existing subnet, use subnet ID. For new subnet, use CIDR notation.
vnet_subnet_id:
description:
- The Subnet ID for a existing Subnet or if you use a different CIDR for the Nodes IP (Kubenet or Azure CNI)
- Note: Does not overlap 169.254.0.0/16, 172.30.0.0/16, 172.31.0.0/16, 192.0.2.0/24

This comment has been minimized.

Copy link
@acozine

acozine Jul 1, 2019

Contributor

I think removing the : will fix the CI failure. However, I find this line confusing too. Are the ranges shown here examples of what not to do? What to do? Could we remove them altogether?

Suggested change
- Note: Does not overlap 169.254.0.0/16, 172.30.0.0/16, 172.31.0.0/16, 192.0.2.0/24
- Cannot overlap with other address ranges within the same virtual network.

@acozine acozine removed the needs_triage label Jul 1, 2019

@ansibot ansibot added the stale_ci label Jul 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.