Skip to content

Added validation check for CSCwt14573 - #403

Open
dhaselva wants to merge 2 commits into
datacenter:v4.3.0-devfrom
dhaselva:dhaselva/servicegraph
Open

dhaselva wants to merge 2 commits into
datacenter:v4.3.0-devfrom
dhaselva:dhaselva/servicegraph

Conversation

@dhaselva

@dhaselva dhaselva commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@lovkeshsharma702
lovkeshsharma702 deleted the branch datacenter:v4.3.0-dev September 9, 2026 09:19
@lovkeshsharma702
lovkeshsharma702 changed the base branch from v4.2.0-dev to v4.3.0-dev September 16, 2026 02:00

@monrog2 monrog2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes requested

1. Restrict detection to PBR service graphs

File: aci-preupgrade-validation-script.py, around line 7340

CSCwt14573 and CSCwn95571 require locally configured PBR. The current implementation processes every applied service graph, so a non-PBR graph without translation entries is incorrectly reported as FAIL_O.

Please filter graph instances using vnsNodeInst.routingMode == "Redirect", consistent with the existing PBR detection in consumer_vzany_shared_services_check.

graph_query = (
    'vnsGraphInst.json?'
    'query-target-filter=eq(vnsGraphInst.configSt,"applied")'
    '&rsp-subtree=full'
    '&rsp-subtree-class='
    'vnsNodeInst,vnsTermNodeInst,vnsConnectionInst,'
    'vnsRsConnectionInstConns'
)
graph_insts = icurl('class', graph_query)

for graph_inst_mo in graph_insts:
    graph_inst = graph_inst_mo.get('vnsGraphInst', {})
    children = graph_inst.get('children', [])

    is_pbr = any(
        child.get('vnsNodeInst', {})
        .get('attributes', {})
        .get('routingMode') == 'Redirect'
        for child in children
    )
    if not is_pbr:
        continue

Also update docs/docs/validations.md to remove the statements that PBR is not required.

Please add tests proving:

  • routingMode="unspecified" returns PASS.
  • routingMode="Redirect" with a missing translation returns FAIL_O.

2. Preserve every graph instance and stretched VRF

File: aci-preupgrade-validation-script.py, around lines 7383 and 7433

sg_by_contract and vzany_on_stretched are keyed only by contract DN. A contract can be used by multiple VRFs, and each scoped graph instance requires its own translation check. Later entries currently overwrite earlier ones.

I reproduced two stretched VRFs sharing one contract where the first translation was missing and the second existed. The check queried only the second instance and returned PASS.

Please retain all graph instances and identify relationships by both contract and VRF scope.

sg_by_contract = defaultdict(list)

# While processing graph instances:
sg_by_contract[contract_dn].append({
    'gi_dn': gi_dn,
    'graph_name': graph_name,
    'scope_dn': scope_dn,
    'first_node': first_node_name,
})

vzany_on_stretched = set()

# While processing vzAny relationships:
vzany_on_stretched.add((contract_dn, vrf_dn))

# Evaluate every graph instance matching the relationship's VRF:
for contract_dn, vrf_dn in vzany_on_stretched:
    for sg_info in sg_by_contract.get(contract_dn, []):
        if sg_info['scope_dn'] != vrf_dn:
            continue

        # Check the translation for this specific graph instance.

Please add a regression test with two stretched VRFs using the same contract where only one graph instance has a translation. The result must identify the missing instance rather than return PASS.

3. Do not silently skip unparseable graph instances

File: aci-preupgrade-validation-script.py, around line 7440

When the first consumer node cannot be parsed, the check executes continue. If no other rows are generated, this produces a false PASS. A malformed contract DN is handled similarly.

Required parsing failures should produce ERROR and preserve the available graph evidence.

unformatted_headers = ['Graph Instance DN', 'Issue']
unformatted_data = []

# ...

if not first_node_name:
    has_error = True
    unformatted_data.append([
        sg_info.get('gi_dn') or contract_dn,
        'Unable to determine the first consumer node',
    ])
    continue

contract_match = re.match(r'uni/tn-([^/]+)/brc-([^/]+)', contract_dn)
if not contract_match:
    has_error = True
    unformatted_data.append([
        sg_info.get('gi_dn') or contract_dn,
        'Unable to parse the contract DN',
    ])
    continue

Return the accumulated evidence:

return Result(
    result=result,
    headers=headers,
    data=data,
    unformatted_headers=unformatted_headers,
    unformatted_data=unformatted_data,
    recommended_action=recommended_action,
    doc_url=doc_url,
)

Please add malformed/missing-node and malformed-contract tests asserting ERROR and the retained unformatted evidence.

4. Avoid retrieving every full service-graph subtree

File: aci-preupgrade-validation-script.py, around line 7340

The current request retrieves the complete subtree of every applied vnsGraphInst before establishing whether any relevant stretched VRF or vzAny relationship exists. This can create substantial APIC load on large fabrics.

Please collect stretched VRFs and relevant vzAny relationships first, then retrieve only the graph classes required for PBR and node discovery.

graph_query = (
    'vnsGraphInst.json?'
    'query-target-filter=eq(vnsGraphInst.configSt,"applied")'
    '&rsp-subtree=full'
    '&rsp-subtree-class='
    'vnsNodeInst,vnsTermNodeInst,vnsConnectionInst,'
    'vnsRsConnectionInstConns'
)

The test query key must exactly match the optimized production query.

@lovkeshsharma702 Please review these requested changes, particularly the PBR trigger and multi-VRF handling, before the next revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants