Skip to content

New validation for CSCwr51759 - #394

Open
Harinadh-Saladi wants to merge 1 commit into
datacenter:v4.2.0-devfrom
Harinadh-Saladi:feature/vns-rscifatt-cleanup-check
Open

New validation for CSCwr51759#394
Harinadh-Saladi wants to merge 1 commit into
datacenter:v4.2.0-devfrom
Harinadh-Saladi:feature/vns-rscifatt-cleanup-check

Conversation

@Harinadh-Saladi

@Harinadh-Saladi Harinadh-Saladi commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Added a new pre-upgrade validation for Verifying Cluster Interface Configuration For Deprecated RsCIfAtt Object.

Enclosed comprehensive logs for the same after testing in the lab with new scenarios addition and after addressing all final review comments made by Gabe.

Comprehensive Test Logs.txt
Full_Script_Run_Logs.txt
Pytest_Full_Logs.txt
Pytest_Logs.txt
Scale_Setup_Logs.txt

Fixes #371

Comment thread aci-preupgrade-validation-script.py Outdated
relation_dn = relation_attributes["dn"].strip()
if not relation_dn:
continue
target_dn = get_target_dn(relation_attributes)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dn is the full path of the relationship object, and tDn is the path of the target interface object it is attached to.

No need of check for tDN.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i agree. its not resolved. at all.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comparison now uses full relation DN instead of tDn and handles rscIfAtt vs rscIfAttN differences before matching.

Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated


@check_wrapper(check_title="Cleanup vnsRsCIfAtt usage in services")
def vns_rscifatt_cleanup_check(tversion, **kwargs):

@muthu-ku muthu-ku Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

change the proper function name and title

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated name and title

Comment thread docs/docs/validations.md Outdated

Impact:

If any `vnsRsCIfAtt` relation exists without a matching `vnsRsCIfAttN` for the same concrete interface target (`tDn`), the upgrade is outage-risky and should be treated as affected.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please update the document as per impact and requirement.

Comment thread aci-preupgrade-validation-script.py Outdated
continue
missing_dn = old_relation_dn_by_key[relation_key]
tenant_name, device_name, logical_interface, concrete_interface = parse_relation_context(missing_dn)
data.append([tenant_name, device_name, logical_interface, concrete_interface, missing_dn])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

where is that fault case handled for post upgrade case..

fault : F1690
uni/tn-CSCwj49418/lDevVip-test/lIf-intf-cons/vnsConfIssue-lif-invalid-CIf
Configuration is invalid due to LIf has an invalid CIf

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per below comment added by Lovkesh,

"After doing upgarde from 605h to 615e.

fab3-apic# moquery -c vnsRsCIfAtt | grep dn
fab3-apic# moquery -c vnsRsCIfAttN | grep dn
dn : uni/tn-CSCwj49418/lDevVip-test/lIf-intf-prov/rscIfAttN-[uni/tn-CSCwj49418/lDevVip-test/cDev-cdev/cIf-[prov]]
fab3-apic#

Gui shows cIf-[prov] deleted from concrete device interface.
and fault raised
F1690
Minor
2026-05-29T13:37:55.525+00:00
Raised
uni/tn-CSCwj49418/lDevVip-test/lIf-intf-cons/vnsConfIssue-lif-invalid-CIf
Configuration is invalid due to LIf has an invalid CIf
Config
configuration-failed
ignorable:no, issue:lif-invalid-CIf. this Fault removed asap i attached the cluster interface."

It'll be seen after upgrade to 6.1(5e). But if older mo config is not there in the new mo config, we are recommending cu to add the missing cluster interface in the lower version itself before doing upgrade. Then we will not see the fault post upgradation and that's the reason current check exists what I feel.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do not consider any fault in this case. You can ignore that. please do repro and test the pre-post upgrade and script.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, have recreated the issue in 5.2(4d) and 5.3(2f) and upgraded to 6.0(5h) and validated the script. I can do the validation one more time by upgrading to 6.1(5e)

Comment thread docs/docs/validations.md Outdated
[N9K-C9408 with more than 5 N9K-X9400-16W LEMs][d31] | CSCws82819 | :white_check_mark: | :no_entry_sign:
[Multi-Pod Modular Spine Bootscript File][d32] | CSCwr66848 | :white_check_mark: | :no_entry_sign:
[Inband Management Policy Misconfiguration][d33]| CSCwd40071 | :white_check_mark: | :no_entry_sign:
[Cleanup vnsRsCIfAtt usage in services][d34] | CSCwr51759 | :white_check_mark: | :no_entry_sign:

@lovkeshsharma702 lovkeshsharma702 Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"Check missing vnsRsCIfAttN" ---> change the name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread docs/docs/validations.md Outdated
[d31]: #n9k-c9408-with-more-than-5-n9k-x9400-16w-lems
[d32]: #multi-pod-modular-spine-bootscript-file
[d33]: #inband-management-policy-misconfiguration
[d34]: #cleanup-vnsrscifatt-usage-in-services

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"check-missing-vnsRsCIfAttN"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread docs/docs/validations.md Outdated

This check will verify the count of the `svccoreCtrlr` Managed Object and raise and alarm with the bug if object count found more than 240. Remove the content or objects of `svccoreCtrlr` or `svccoreNode`. Contact Cisco TAC or upgrade to a release containing the fix for CSCws84232 before proceeding with an upgrade.

### Cleanup vnsRsCIfAtt usage in services

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

change the name accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread docs/docs/validations.md Outdated

### Cleanup vnsRsCIfAtt usage in services

Due to [CSCwr51759][70], when targeting 6.0(3)+, having only `vnsRsCIfAtt` without the corresponding `vnsRsCIfAttN` under the same `vnsLIf` can leave service graph interface attachment in an inconsistent state.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wrong buug mentioned. Please do the cleanup for bogus info.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

when upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted and without creating the corresponding 'vnsRsCIfAttN' under the same vnsLIf. this will leave service graph interface attachment in an inconsistent state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread docs/docs/validations.md Outdated

Impact:

If any `vnsRsCIfAtt` relation exists without a matching `vnsRsCIfAttN` for the same concrete interface target (`tDn`), the upgrade is outage-risky and should be treated as affected.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need for this ---> If any vnsRsCIfAtt relation exists without a matching vnsRsCIfAttN for the same concrete interface target (tDn), the upgrade is outage-risky and should be treated as affected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment thread docs/docs/validations.md Outdated

Suggestion:

Before the upgrade, add the missing `vnsRsCIfAttN` relation under the same cluster interface (`vnsLIf`) with the same concrete interface target (`tDn`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

specify the GUI path.. as discussed earlier

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread aci-preupgrade-validation-script.py Outdated
return Result(result=ERROR, msg="Error occurred while fetching svccore object counts: {}".format(str(e)), doc_url=doc_url)


@check_wrapper(check_title="Cleanup vnsRsCIfAtt usage in services")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

change the name accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread aci-preupgrade-validation-script.py Outdated
headers = ["Tenant", "Device Name", "Cluster Interface", "Missing Concrete Interface", "vnsRsCIfAtt DN"]
data = []
recommended_action = (
"Mo vnsRsCIfAtt is deprecated >=6.0(3d). Before upgrade, under Services, add the missing concrete interface as vnsRsCIfAttN under the same cluster interface"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is not explainatery enough. please work on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated recommended action with additional details.

Comment thread aci-preupgrade-validation-script.py Outdated

vnsRsCIfAttNs = icurl("class", "vnsRsCIfAttN.json?rsp-prop-include=config-only")

def get_target_dn(relation_attributes):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

look for whole DN, not just Tdn. . this seems wrong to me.
you will see one small diff there for RsCifAtt/RsCifAttN --> make sure you cover this while comparing DN.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by considering full DN.

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed all the comments

Comment thread docs/docs/validations.md Outdated
[N9K-C9408 with more than 5 N9K-X9400-16W LEMs][d31] | CSCws82819 | :white_check_mark: | :no_entry_sign:
[Multi-Pod Modular Spine Bootscript File][d32] | CSCwr66848 | :white_check_mark: | :no_entry_sign:
[Inband Management Policy Misconfiguration][d33]| CSCwd40071 | :white_check_mark: | :no_entry_sign:
[Cleanup vnsRsCIfAtt usage in services][d34] | CSCwr51759 | :white_check_mark: | :no_entry_sign:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread docs/docs/validations.md Outdated
[d31]: #n9k-c9408-with-more-than-5-n9k-x9400-16w-lems
[d32]: #multi-pod-modular-spine-bootscript-file
[d33]: #inband-management-policy-misconfiguration
[d34]: #cleanup-vnsrscifatt-usage-in-services

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread docs/docs/validations.md Outdated

This check will verify the count of the `svccoreCtrlr` Managed Object and raise and alarm with the bug if object count found more than 240. Remove the content or objects of `svccoreCtrlr` or `svccoreNode`. Contact Cisco TAC or upgrade to a release containing the fix for CSCws84232 before proceeding with an upgrade.

### Cleanup vnsRsCIfAtt usage in services

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment thread docs/docs/validations.md Outdated

### Cleanup vnsRsCIfAtt usage in services

Due to [CSCwr51759][70], when targeting 6.0(3)+, having only `vnsRsCIfAtt` without the corresponding `vnsRsCIfAttN` under the same `vnsLIf` can leave service graph interface attachment in an inconsistent state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread docs/docs/validations.md Outdated

Impact:

If any `vnsRsCIfAtt` relation exists without a matching `vnsRsCIfAttN` for the same concrete interface target (`tDn`), the upgrade is outage-risky and should be treated as affected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment thread aci-preupgrade-validation-script.py Outdated


@check_wrapper(check_title="Cleanup vnsRsCIfAtt usage in services")
def vns_rscifatt_cleanup_check(tversion, **kwargs):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated name and title

Comment thread aci-preupgrade-validation-script.py Outdated
headers = ["Tenant", "Device Name", "Cluster Interface", "Missing Concrete Interface", "vnsRsCIfAtt DN"]
data = []
recommended_action = (
"Mo vnsRsCIfAtt is deprecated >=6.0(3d). Before upgrade, under Services, add the missing concrete interface as vnsRsCIfAttN under the same cluster interface"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated recommended action with additional details.

Comment thread aci-preupgrade-validation-script.py Outdated

vnsRsCIfAttNs = icurl("class", "vnsRsCIfAttN.json?rsp-prop-include=config-only")

def get_target_dn(relation_attributes):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by considering full DN.

Comment thread aci-preupgrade-validation-script.py Outdated
relation_dn = relation_attributes["dn"].strip()
if not relation_dn:
continue
target_dn = get_target_dn(relation_attributes)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comparison now uses full relation DN instead of tDn and handles rscIfAtt vs rscIfAttN differences before matching.

Comment thread aci-preupgrade-validation-script.py Outdated
continue
missing_dn = old_relation_dn_by_key[relation_key]
tenant_name, device_name, logical_interface, concrete_interface = parse_relation_context(missing_dn)
data.append([tenant_name, device_name, logical_interface, concrete_interface, missing_dn])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per below comment added by Lovkesh,

"After doing upgarde from 605h to 615e.

fab3-apic# moquery -c vnsRsCIfAtt | grep dn
fab3-apic# moquery -c vnsRsCIfAttN | grep dn
dn : uni/tn-CSCwj49418/lDevVip-test/lIf-intf-prov/rscIfAttN-[uni/tn-CSCwj49418/lDevVip-test/cDev-cdev/cIf-[prov]]
fab3-apic#

Gui shows cIf-[prov] deleted from concrete device interface.
and fault raised
F1690
Minor
2026-05-29T13:37:55.525+00:00
Raised
uni/tn-CSCwj49418/lDevVip-test/lIf-intf-cons/vnsConfIssue-lif-invalid-CIf
Configuration is invalid due to LIf has an invalid CIf
Config
configuration-failed
ignorable:no, issue:lif-invalid-CIf. this Fault removed asap i attached the cluster interface."

It'll be seen after upgrade to 6.1(5e). But if older mo config is not there in the new mo config, we are recommending cu to add the missing cluster interface in the lower version itself before doing upgrade. Then we will not see the fault post upgradation and that's the reason current check exists what I feel.

Comment thread aci-preupgrade-validation-script.py Outdated
fabric_link_redundancy_check,
apic_downgrade_compat_warning_check,
svccore_excessive_data_check,
vns_rscifattn_missing_check,

@lovkeshsharma702 lovkeshsharma702 Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

vns_rscifattn_missing_check --> should be vnsrscifattn_missing_check correct it everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread aci-preupgrade-validation-script.py Outdated
continue
missing_dn = old_relation_dn_by_key[relation_key]
tenant_name, device_name, logical_interface, concrete_interface = parse_relation_context(missing_dn)
data.append([tenant_name, device_name, logical_interface, concrete_interface, missing_dn])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do not consider any fault in this case. You can ignore that. please do repro and test the pre-post upgrade and script.

Comment thread docs/docs/validations.md Outdated

### Check missing vnsRsCIfAttN

When upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted and without creating the corresponding 'vnsRsCIfAttN' under the same vnsLIf will leave the service graph interface attachment in an inconsistent state

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted without creating 'vnsRsCIfAttN' under 'vnsLIf' . this leave the service graph interface attachment in an inconsistent state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by correcting a few grammatical errors.

Comment thread docs/docs/validations.md Outdated

When upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted and without creating the corresponding 'vnsRsCIfAttN' under the same vnsLIf will leave the service graph interface attachment in an inconsistent state

Before the upgrade, in APIC GUI navigate to Tenant > Services > L4-L7 > Device and open cluster interface `intf-prov`. If concrete interface `cons` is missing, re-add concrete interface `cons` under the same cluster interface. so the corresponding `vnsRsCIfAttN` relation exists.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For all impacted Dn in this check, re-attach the Concrete interfaces associated to cluster interface under Devices in Services L4-L7 tab.

Tenant --> Services --> L4-L7 --> Devices (Device_name) --> cluster interface --> Concrete interfaces

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by correcting a few grammatical errors.

@lovkeshsharma702 lovkeshsharma702 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please work on given logic.,. validate this in lab again.

@Harinadh-Saladi Harinadh-Saladi changed the title New validation for CSCwr51759 Cleanup vnsRsCIfAtt usage in service New validation for missing vnsRsCIfAttN Jun 5, 2026

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yesterday have recreated the issue in 5.3(2f) and validated the script. Enclosed updated script validation logs, pytest and full script run logs

Comment thread docs/docs/validations.md Outdated

### Check missing vnsRsCIfAttN

When upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted and without creating the corresponding 'vnsRsCIfAttN' under the same vnsLIf will leave the service graph interface attachment in an inconsistent state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by correcting a few grammatical errors.

Comment thread docs/docs/validations.md Outdated

When upgrading to 6.0(3) and above, 'vnsRsCIfAtt' get deleted and without creating the corresponding 'vnsRsCIfAttN' under the same vnsLIf will leave the service graph interface attachment in an inconsistent state

Before the upgrade, in APIC GUI navigate to Tenant > Services > L4-L7 > Device and open cluster interface `intf-prov`. If concrete interface `cons` is missing, re-add concrete interface `cons` under the same cluster interface. so the corresponding `vnsRsCIfAttN` relation exists.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated by correcting a few grammatical errors.

Comment thread aci-preupgrade-validation-script.py Outdated
continue
missing_dn = old_relation_dn_by_key[relation_key]
tenant_name, device_name, logical_interface, concrete_interface = parse_relation_context(missing_dn)
data.append([tenant_name, device_name, logical_interface, concrete_interface, missing_dn])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, have recreated the issue in 5.2(4d) and 5.3(2f) and upgraded to 6.0(5h) and validated the script. I can do the validation one more time by upgrading to 6.1(5e)

Comment thread aci-preupgrade-validation-script.py Outdated
fabric_link_redundancy_check,
apic_downgrade_compat_warning_check,
svccore_excessive_data_check,
vns_rscifattn_missing_check,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated !

Comment thread docs/docs/validations.md
Comment thread tests/checks/vnsrscifatt_deprecation_check/test_vnsrscifatt_deprecation_check.py Outdated
Comment thread aci-preupgrade-validation-script.py

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the comments and tested in the lab with code changes and enclosed the latest logs

Comment thread docs/docs/validations.md
Comment thread tests/checks/vnsrscifatt_deprecation_check/test_vnsrscifatt_deprecation_check.py Outdated
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated
old_dn,
])

data.sort(key=itemgetter(-1))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what is the need of this sort and new library imported?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The above suggested optimized loop logic works functionally, but without sorting it can return rows in different orders based on API/input order. Our Pytest case compares as an ordered list, so it fails intermittently on order only. I added sorting to make pytest stable and output consistent.

Have removed new library imported and replaced with lambda instead.

Comment thread docs/docs/validations.md
[d31]: #n9k-c9408-with-more-than-5-n9k-x9400-16w-lems
[d32]: #multi-pod-modular-spine-bootscript-file
[d33]: #inband-management-policy-misconfiguration

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

resolve this all removed lines diff

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the comments and tested the code with changes in lab and enclosed comprehensive logs.

Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated
old_dn,
])

data.sort(key=itemgetter(-1))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The above suggested optimized loop logic works functionally, but without sorting it can return rows in different orders based on API/input order. Our Pytest case compares as an ordered list, so it fails intermittently on order only. I added sorting to make pytest stable and output consistent.

Have removed new library imported and replaced with lambda instead.

Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread docs/docs/validations.md
[d33]: #inband-management-policy-misconfiguration
[d34]: #bgpProto-timer-policy-already-existing
[d35]: #wred-with-affected-fm-models
[d36]: #n9k-c93180yc-fx3-switch-memory-less-than-32gb

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why this links removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think during conflicts resolving, mistakenly got removed. Added back.

Comment thread aci-preupgrade-validation-script.py Outdated
@check_wrapper(check_title="Verifying Cluster Interface Configuration For Deprecated RsCIfAtt Object")
def verify_cluster_interface_config_for_rscifatt_use_check(tversion, cversion, **kwargs):
result = PASS
doc_url = "https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#check-missing-vnsrscifattn"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

update the correct doc url.. check-missing-vnsrscifattn is not present in validation.md

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the URL after changing the function name.

Comment thread aci-preupgrade-validation-script.py

@lovkeshsharma702 lovkeshsharma702 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good to me.

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the comments

Comment thread docs/docs/validations.md
[d33]: #inband-management-policy-misconfiguration
[d34]: #bgpProto-timer-policy-already-existing
[d35]: #wred-with-affected-fm-models
[d36]: #n9k-c93180yc-fx3-switch-memory-less-than-32gb

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think during conflicts resolving, mistakenly got removed. Added back.

Comment thread aci-preupgrade-validation-script.py Outdated
@check_wrapper(check_title="Verifying Cluster Interface Configuration For Deprecated RsCIfAtt Object")
def verify_cluster_interface_config_for_rscifatt_use_check(tversion, cversion, **kwargs):
result = PASS
doc_url = "https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#check-missing-vnsrscifattn"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the URL after changing the function name.

Comment thread aci-preupgrade-validation-script.py
Comment thread docs/docs/validations.md Outdated

@lovkeshsharma702 lovkeshsharma702 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All test passed.

@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.

please triple check that the number of API requests being asked for here are the smallest dataset that we need to perform this check.

if they are indeed switch objects, we will not be able to requests them on the intersight side.

if there is a scale fabric with these obejcts, how long does this take to run? the old VLAN validation check had to be removed due to time it took to complete, so i'm worried this one will have similar results.

Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread docs/docs/validations.md Outdated
Comment thread docs/docs/validations.md
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated

@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.

Additional findings beyond my previous review:

Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
@monrog2

monrog2 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Tracking issue: #371. This PR is the active implementation, but the issue remains open pending resolution of the outstanding correctness and scale review blockers.

@Harinadh-Saladi Harinadh-Saladi changed the title New validation for Verifying Cluster Interface Configuration For Deprecated RsCIfAtt Object New validation for CSCwr51759 Jul 31, 2026

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are trying to get the scale setup to test. Once we get the setup, will test and share an update.

Comment thread docs/docs/validations.md Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py

@lovkeshsharma702 lovkeshsharma702 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

replied to comments and hari wil address code changes

Comment thread docs/docs/validations.md Outdated
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py
Comment thread docs/docs/validations.md Outdated
Comment thread docs/docs/validations.md Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread docs/docs/validations.md
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py

@Harinadh-Saladi Harinadh-Saladi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed review comments

Comment thread docs/docs/validations.md Outdated
Comment thread docs/docs/validations.md Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py Outdated
Comment thread aci-preupgrade-validation-script.py
Comment thread aci-preupgrade-validation-script.py
@Harinadh-Saladi
Harinadh-Saladi force-pushed the feature/vns-rscifatt-cleanup-check branch from 062214d to 8bde82b Compare August 10, 2026 05:04
Rebased on top of latest upstream/v4.2.0-dev.
@Harinadh-Saladi
Harinadh-Saladi force-pushed the feature/vns-rscifatt-cleanup-check branch from 8bde82b to 428ebdb Compare August 10, 2026 05:08
@Harinadh-Saladi

Copy link
Copy Markdown
Contributor Author

Have tested the script in scale setup and response time took 4sec.

date; python aci-preupgrade-validation-script.py -d vnsRsCIfAtt_deprecation_check -t "6.2(2e)"; date
Mon Aug 10 11:31:25 UTC 2026
==== 2026-08-10T11-31-26+0000, Script Version v4.2.0-dev ====

!!!! Check https://github.com/datacenter/ACI-Pre-Upgrade-Validation-Script for Latest Release !!!!

To use a non-default Login Domain, enter apic#DOMAIN\USERNAME
Enter username for APIC login : admin
Enter password for corresponding User :

Gathering Node Information...

Current APIC Version...6.2(2.215)
Lowest Switch Version...6.2(2.200)

Target APIC version is overridden to 6.2(2e)

Collecting VPC Node IDs...101, 102, 3000, 3001, ... (and 416 more)

Progress: |████████████████████████████████████████████████████████████████████████████████████████████████████| 1/1 checks completed

=== Check Result (failed only) ===

=== Summary Result ===

PASS : 1
FAIL - OUTAGE WARNING!! : 0
FAIL - UPGRADE FAILURE!! : 0
MANUAL CHECK REQUIRED : 0
POST UPGRADE CHECK REQUIRED : 0
N/A : 0
ERROR !! : 0
TOTAL : 1

Pre-Upgrade Check Complete.
Next Steps: Address all checks flagged as FAIL, ERROR or MANUAL CHECK REQUIRED

Result output and debug info saved to below bundle for later reference.
Attach this bundle to Cisco TAC SRs opened to address the flagged checks.

  Result Bundle: /data/techsupport/preupgrade/preupgrade_validator_2026-08-10T11-31-26+0000.tgz

==== Script Version v4.2.0-dev FIN ====
Mon Aug 10 11:31:29 UTC 2026

@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.

Requesting changes for three result-integrity issues in the new CSCwr51759 validation. The first can produce a false PASS for the documented defect condition; the other two can classify inactive service-graph contexts as deployed and produce false outage warnings. These findings are based on focused tests against the exact PR head and read-only APIC query-shape validation.

new_lif_dns = set(lifs_with_new_relation) # LIF DNs covered by vnsRsCIfAttN (for coverage check)
new_dn_keys = set() # New DNs rewritten as old-style keys (for consistency check)
for relation_mo in vnsRsCIfAttNs:
relation_dn = safe_extract_attrs(lambda: relation_mo["vnsRsCIfAttN"]["attributes"]["dn"].strip())

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.

[Blocking] Compare old/new relations by tDn, not relation dn. The exact vnsLIf query above uses rsp-prop-include=config-only; on a live APIC its vnsRsCIfAttN children returned annotation, tDn, and userdom, but no dn. Consequently this extraction returns None, and the corresponding old-relation extraction below is skipped too. A deployed LIF with vnsRsCIfAtt.tDn=A and vnsRsCIfAttN.tDn=B, using that live response shape, returned PASS with no rows. That misses the CSCwr51759 condition, which requires the replacement relation under the same LIF with the same target DN. Please compare (parent_lif_dn, tDn) keys directly. Also update the stale-target regression fixture to omit relation dn and assert matching and mismatching tDn values; its current synthetic dn attributes hide this false PASS.

# ── Step 1: Collect deployed service-graph LIF DNs ──────────────────────
# Build (contract_name, graph_name) keys from applied vnsGraphInst objects
graph_keys = set()
for entry in icurl("class", "vnsGraphInst.json?rsp-prop-include=config-only") or []:

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.

[High] This does not collect applied graph instances as the comment states. On a live APIC, this exact rsp-prop-include=config-only query returned 0 objects, while the unrestricted class returned 83 and query-target-filter=eq(vnsGraphInst.configSt,"applied") returned 13. Empty graph_keys then activates the fallback at lines 6892-6899 and treats every complete vnsLDevCtx as deployed: 193 contexts/48 LIFs instead of 13 applied contexts/13 LIFs on that fabric. Although all 48 happened to have vnsRsCIfAttN, an inactive or stale context missing it would produce a false outage warning. Please use the explicit applied-state filter and do not reinterpret a successful empty applied inventory as all contexts being active. Add coverage for mixed applied/non-applied instances and no applied instances with lingering contexts.

contract_match = re.search(r"/brc-([^/]+)$", contract_dn)
graph_match = re.search(r"/AbsGraph-([^/]+)$", graph_dn)
if contract_match and graph_match:
graph_keys.add((contract_match.group(1), graph_match.group(1)))

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.

[Medium] Preserve tenant identity in this graph key. Both source DNs contain the tenant, but the key retains only (contract_name, graph_name). If tenant A has an applied web/fw graph and tenant B has an inactive context with the same labels, tenant B is treated as deployed and may receive a false FAIL_O. This is a valid multi-tenant configuration even though the inspected fabric had no duplicate name pairs. Please correlate tenant, contract, and graph identity, with explicit handling for imported/common-tenant graphs, and add a duplicate-name-across-tenants regression test where only one graph is applied.

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.

6 participants