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

ACI L4-L7 Services (DCNE-155) #186

Open
wants to merge 127 commits into
base: master
Choose a base branch
from

Conversation

timcragg
Copy link
Contributor

@timcragg timcragg commented Sep 1, 2021

This PR includes modules and integration tests for L4-L7 services. The modules allow configuration of the following ACI classes:

  • vnsCDev
  • vnsCIf
  • vnsRsCIfPathAtt
  • vnsRsCIfAttN
  • vnsLDevVip
  • vnsRsALDevToPhysDomP
  • vnsRsLDevCtxToLDev
  • vnsLIfCtx
  • vnsRsLIfCtxToBD
  • vnsRsLIfCtxToLIf
  • vnsRsLIfCtxToSvcRedirectPol
  • vnsLDevCtx
  • vnsLIf
  • svcRedirectPol
  • vnsRedirectDest
  • vnsRsRedirectHealthGroup
  • vnsRsIPSLAMonitoringPol
  • vnsRedirectHealthGroup
  • vnsRsIPSLAMonitoringPol
  • vnsRedirectHealthGroup
  • vnsAbsGraph
  • vnsAbsConnection
  • vnsRsAbsConnectionConns
  • vnsAbsFuncConn
  • vnsAbsNode
  • vnsAbsTermNodeCon
  • vnsAbsTermNodeProv
  • vnsAbsTermConn
  • fvIPSLAMonitoringPol

@timcragg timcragg removed their assignment Nov 18, 2021
else:
path_type = 'paths'

path_dn = ('topology/pod-{0}/{1}-{2}/pathep-[{3}]'.format(pod_id,
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we include some form of validation of input to ensure valid path_dn is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how much validation we can add, since the pathep for PCs and vPCs is just an IPG name (so just a generic string). The pod ID is already typed as int, and I've added a regex check to make sure the node ID is either an integer or a hyphen separated pair of integers.

Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

review part 1
-> needs changes
-> Might need to add another module for object vnsSvcVip (aci_l4l7_device_selection_if_context_vip) -> inside aci_l4l7_device_selection_if_context

threshold=dict(type="int"),
traffic_class=dict(type="int"),
http_version=dict(type="str", choices=["1.0", "1.1"]),
http_uri=dict(type="str"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Module :

  • Missing destination_port and detect_multiplier arguments (available when sla_type = TCP).
  • missing description

Tests
-> in test cases
-> missing test cases with traffic_class, type_of_service arguments
-> missing test case when sla_type = L2ping

Comment on lines +210 to +211
health_group=dict(type="str"),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing description argument in module

Comment on lines +253 to +254
device = module.params.get("device")

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Missing context_name argument.
  • While testing device (aci_l4l7_device) is created and available on UI, but it is not attached to aci_l4l7_device_selection_policy. (required parameter)

argument_spec.update(aci_annotation_spec())
argument_spec.update(
tenant=dict(type="str", aliases=["tenant_name"]),
contract=dict(type="str", aliases=["contract_name"]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think contract is the right name.

logical_device=dict(type="str"),
logical_interface=dict(type="str"),
redirect_policy=dict(type="str"),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

-> contract might not be the right name.

  • In module custom_qos_policy, permit_logging and l3out arguments are missing

  • l3_dest and permit_log arguments missing in test cases

  • In Test :
    - Modify L4-L7 Device Selection Logical Interface Context - fails
    - Verify context query - fails
    - Azure cloud fails at - Add a Bridge Domain in ansible_tenant

module_object=context,
target_filter={"connNameOrLbl": context},
),
child_classes=["vnsRsLIfCtxToBD", "vnsRsLIfCtxToLIf", "vnsRsLIfCtxToSvcRedirectPol"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

child_classes=["vnsRsLIfCtxToBD", "vnsRsLIfCtxToLIf", "vnsRsLIfCtxToSvcRedirectPol"] -> should be modified by adding one more classes -> vnsRsLIfCtxToCustQosPol

Comment on lines +210 to +214
tenant=dict(type="str", aliases=["tenant_name"]),
service_graph=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
ui_template_type=dict(type="str"),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

test case for ui_template_type missing

Comment on lines +234 to +241
tenant=dict(type="str", aliases=["tenant_name"]),
service_graph=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
connection_name=dict(type="str"),
direct_connect=dict(type="bool"),
unicast_route=dict(type="bool"),
adjacency_type=dict(type="str", choices=["l2", "l3"]),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing connType and connDir from modules.

DOCUMENTATION = r"""
---
module: aci_l4l7_service_graph_template_abs_connection_conns
short_description: Manage L4-L7 Service Graph Template Connections (vns:RsAbsConnectionConns)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a separate module for relationship object?

Comment on lines +255 to +264
tenant=dict(type="str", aliases=["tenant_name"]),
service_graph=dict(type="str"),
node=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
func_template_type=dict(type="str", choices=["fw_trans", "fw_routed", "adc_one_arm", "adc_two_arm", "other"]),
func_type=dict(type="str", choices=["go_to", "go_through", "l1", "l2"]),
device=dict(type="str"),
device_tenant=dict(type="str"),
managed=dict(type="bool"),
routing_mode=dict(type="str"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing isCopy argument in the module.

tenant=dict(type="str", aliases=["tenant_name"]),
service_graph=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
node_name=dict(type="str", choices=["T1", "T2"]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this module is built for 2 nodes, no example of what happens if there are more than 2 node, or when copy node is present.

svc_type=dict(type="str", choices=["adc", "fw", "others"]),
trunking=dict(type="bool"),
domain=dict(type="str"),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

fails on 6.2 v and azure cloud

prom_mode=dict(type="bool"),
svc_type=dict(type="str", choices=["adc", "fw", "others"]),
trunking=dict(type="bool"),
domain=dict(type="str"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename arguments, no example for copy device

Comment on lines +221 to +224
device=dict(type="str"),
logical_interface=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
encap=dict(type="str"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing configIssues (configuration issues) parameter.

Comment on lines +227 to +231
device=dict(type="str"),
logical_interface=dict(type="str"),
state=dict(type="str", default="present", choices=["absent", "present", "query"]),
concrete_device=dict(type="str"),
concrete_interface=dict(type="str"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a relationship object.

@samiib samiib added the jira-sync Sync this issue to Jira label Aug 15, 2024
@github-actions github-actions bot changed the title ACI L4-L7 Services ACI L4-L7 Services (DCNE-155) Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira-sync Sync this issue to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants