Conversation
mtarking
added
enhancement
| """ | ||
|
|
||
| fabric_name: str = Field(alias="fabricName") | ||
| allowed_vlans: Optional[List[str]] = Field(default=None, alias="allowedVlans") |
There was a problem hiding this comment.
Should this be aligned with the pattern matching from @allenrobel https://github.com/CiscoDevNet/ansible-nd/pull/270/changes#diff-d2ab524cc8cd4727b8449727f59773a79668d8ac08e5617de6638ef13c4c74a4R58? Is there currently some shared pattern or shared field setup?
wider alignment might abe required also for doc fragments in this case
| from ansible_collections.cisco.nd.plugins.module_utils.types import IdentifierKey | ||
|
|
||
|
|
||
| class TenantDomainNameMixin(BaseModel): |
There was a problem hiding this comment.
why is this not part of the mixins.py file?
| from ansible_collections.cisco.nd.plugins.module_utils.types import IdentifierKey | ||
|
|
||
|
|
||
| class TenantNameMixin(BaseModel): |
There was a problem hiding this comment.
why is this not part of the mixins.py file?
| from ansible_collections.cisco.nd.plugins.module_utils.models.nested import NDNestedModel | ||
|
|
||
|
|
||
| class InfraTenantFabricAssociationModel(NDNestedModel): |
There was a problem hiding this comment.
why is this part of infra_tenant folder when the endpoint appears in manage?
| of dicts with snake_case keys. | ||
| """ | ||
|
|
||
| # --- Identifier Configuration --- |
There was a problem hiding this comment.
i notice that there are multiple styles maintained across PRs where you introduce these comments, is this a standard that should be enforced throughout the repository?
| query_one_endpoint: Type[NDEndpointBaseModel] = EpInfraTenantsGet | ||
| query_all_endpoint: Type[NDEndpointBaseModel] = EpInfraTenantsGet | ||
|
|
||
| # --- Helpers for fabric associations (manage API) --- |
There was a problem hiding this comment.
why are all these functions not a isolated orchestrator that is specific for the InfraTenantFabricAssociationModel?
| description: | ||
| - Manage tenants on Cisco Nexus Dashboard (ND). | ||
| - It supports creating, updating, and deleting tenants. | ||
| - Optionally manage fabric associations for each tenant, linking tenants to NDFC-managed fabrics |
There was a problem hiding this comment.
is NDFC still correct here?
| fabric_associations: | ||
| description: | ||
| - The list of fabric associations for the tenant. | ||
| - Each entry associates the tenant with a fabric managed by NDFC and defines the allowed VLANs. |
There was a problem hiding this comment.
is NDFC still correct here? likely in more locations
| description: | ||
| description: | ||
| - The description of the tenant. | ||
| - The description can be up to 128 characters. |
There was a problem hiding this comment.
This is not wrong but I notice different modules use different description notation for similar things like max amount of characters allowed. Do we want to align the description sentence for ranges and maximum characters, etc. As example it deviates from: https://github.com/CiscoDevNet/ansible-nd/pull/270/changes#diff-1bc3ea994025d9c3d17dfc41a4b4e62aba250b851cd388081f17ada991dc838bR120.
| - fabric_name: my_fabric | ||
| allowed_vlans: | ||
| - "10-50" | ||
| state: replaced |
There was a problem hiding this comment.
this is more a generic example question, is it on purpose to only show examples for merged, replaced and deleted states? This also deviates from what I have seen for instance in comment made here: https://github.com/CiscoDevNet/ansible-nd/pull/270/changes#r3186791791
2 participants
Related Issue(s)
Resolves #257
Proposed Changes
Add two new Ansible modules for managing Nexus Dashboard multi-tenancy resources using the new pydantic-based architecture:
nd_infra_tenant— Manage tenants on Cisco Nexus Dashboard/api/v1/infra/tenants)/api/v1/manage/tenantFabricAssociations)merged,replaced,overridden, anddeletedstatesnd_infra_tenant_domain— Manage tenant domains on Cisco Nexus Dashboard/api/v1/infra/tenantDomains)merged,replaced,overridden, anddeletedstatesNew files:
plugins/modules/nd_infra_tenant.pyplugins/modules/nd_infra_tenant_domain.pyplugins/module_utils/models/infra_tenant/infra_tenant.pyplugins/module_utils/models/infra_tenant_domain/infra_tenant_domain.pyplugins/module_utils/orchestrators/infra_tenant.pyplugins/module_utils/orchestrators/infra_tenant_domain.pyplugins/module_utils/endpoints/v1/infra/tenants.pyplugins/module_utils/endpoints/v1/infra/tenant_domains.pyplugins/module_utils/endpoints/v1/manage/tenant_fabric_associations.pytests/unit/module_utils/endpoints/test_endpoints_api_v1_infra_tenants.pytests/unit/module_utils/endpoints/test_endpoints_api_v1_infra_tenant_domains.pytests/unit/module_utils/endpoints/test_endpoints_api_v1_manage_tenant_fabric_associations.pytests/integration/targets/nd_infra_tenant/tasks/main.ymltests/integration/targets/nd_infra_tenant_domain/tasks/main.ymlTest Notes
pytest tests/unit/ -v), including 38 new endpoint tests covering infra tenants (16), infra tenant domains (16), and manage tenant fabric associations (6)merged,replaced,overridden,deleted) with check mode, normal mode, and idempotency assertions for both modulesnd_infra_tenantintegration tests include fabric association scenarios: create with associations, update associations, replace with associations, override with associations, and delete with association cleanupCisco Nexus Dashboard Version
4.2.1
Related ND API Resource Category
Checklist