[AmrNJ]

Ensure featureprofiles tests restore persistent DUT AAA users, gNSI credentialz/acctz policies, system logging selectors, and gRIBI state via t.Cleanup / defer so failed or interrupted test runs do not leave testbeds in an unhealthy state:

• gNSI Credentialz (feature/gnsi/credentialz, internal/security/credz):
  • internal/security/credz/credz.go: Register t.Cleanup in credz.SetupUser to delete created OpenConfig AAA test users on exit.
  • feature/gnsi/credentialz/tests/hiba_authentication/hiba_authentication_test.go: Register t.Cleanup before RotateAuthenticationTypes and RotateTrustedUserCA so SSH authentication policies and authorized principal checks are restored even if subtests fail early.
  • feature/gnsi/credentialz/tests/host_certificates/host_certificates_test.go: Register t.Cleanup to restore authentication artifacts on exit.
  • feature/gnsi/credentialz/tests/ssh_password_login_disallowed/ssh_password_login_disallowed_test.go: Register t.Cleanup before RotateAuthenticationTypes so password/pubkey/keyboard-interactive SSH authentication types are restored even if subtests fail early.
• gNSI Acctz (feature/gnsi/acctz):
  • feature/gnsi/acctz/tests/AccountingAuthenErrorMulti/accounting_authen_error_multi_test.go: Fix setupTestUser (which previously queued a gnmi.BatchDelete onto an unapplied batch) and configureTacacsAAAWithLocalFallback to delete the test user and TACACS server group on the DUT via gnmi.Delete in t.Cleanup.
  • feature/gnsi/acctz/tests/record_payload_truncation/record_payload_truncation_test.go: Batch acctz-test-ni-* network-instance deletions in t.Cleanup using gnmi.SetBatch.
• System Logging (feature/system/logging):
  • feature/system/logging/otg_tests/console_vty_file/console_vty_file_test.go: Snapshot /system/logging/config before test modifications and restore or delete it inside fptest.NonFatal in t.Cleanup.
• gRIBI (feature/gribi):
  • feature/gribi/mpls_in_udp/otg_tests/mpls_in_udp/mpls_in_udp_test.go: Register defer c.FlushAll(t) alongside defer c.Close(t) so gRIBI AFT entries are flushed before closing the client even if the test exits early.
  • feature/gribi/otg_tests/hashing_test/hashing_test.go: Register t.Cleanup to revert Nokia per-complex-randomization load-balancing hash option to false.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request standardizes the cleanup process across various feature profiles tests by migrating from 'defer' to 't.Cleanup'. This change ensures that DUT configurations, including AAA settings, network instances, and vendor-specific CLI overrides, are consistently reverted regardless of test outcomes, thereby improving testbed stability and reducing cross-test interference.

Highlights

• Standardized Cleanup: Converted numerous 'defer' statements and manual cleanup routines to 't.Cleanup' to ensure testbed resources are reliably restored even when tests fail or are interrupted.
• DUT State Management: Implemented comprehensive cleanup for AAA users, gNSI policies, NTP servers, system logging, and vendor-specific CLI overrides across multiple test suites.
• Improved Reliability: Ensured that persistent configurations, such as network instances and hardware profiles, are properly reverted to prevent testbed contamination between runs.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[OpenConfigBot]

Pull Request Functional Test Report for #6260 / e90f1b1

Virtual Devices

Device	Test	Status	Test Documentation	Job	Raw Log
Arista cEOS		Hashing: failure TR-6.2: failure ACCTZ-10.1: failure ACCTZ-4.2: failure Credentialz-5: failure Credentialz-3: failure Credentialz-2: failure TE-18.1: success	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	5fe270b2	Log
Cisco 8000E		Hashing: failure TR-6.2: failure ACCTZ-10.1: failure ACCTZ-4.2: failure Credentialz-5: failure Credentialz-3: failure Credentialz-2: failure TE-18.1: failure	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	ad67c8a6	Log
Cisco XRd		Hashing: pending execution TR-6.2: pending execution ACCTZ-10.1: failure ACCTZ-4.2: running Credentialz-5: pending execution Credentialz-3: pending execution Credentialz-2: pending execution TE-18.1: pending execution	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	bc7d9ad0	Log
Juniper ncPTX		Hashing: failure TR-6.2: failure ACCTZ-10.1: failure ACCTZ-4.2: success Credentialz-5: failure Credentialz-3: failure Credentialz-2: failure TE-18.1: failure	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	77a58749	Log
Nokia SR Linux		Hashing: failure TR-6.2: failure ACCTZ-10.1: failure ACCTZ-4.2: failure Credentialz-5: failure Credentialz-3: failure Credentialz-2: failure TE-18.1: failure	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	c6f25298	Log
Openconfig Lemming		Hashing: failure TR-6.2: failure ACCTZ-10.1: failure ACCTZ-4.2: failure Credentialz-5: failure Credentialz-3: failure Credentialz-2: failure TE-18.1: failure	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation	3f6e578e	Log

Hardware Devices

Device	Test	Status	Test Documentation	Raw Log
Arista		Hashing: pending authorization TR-6.2: pending authorization ACCTZ-10.1: pending authorization ACCTZ-4.2: pending authorization Credentialz-5: pending authorization Credentialz-3: pending authorization Credentialz-2: pending authorization TE-18.1: pending authorization	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation
Cisco		Hashing: pending authorization TR-6.2: pending authorization ACCTZ-10.1: pending authorization ACCTZ-4.2: pending authorization Credentialz-5: pending authorization Credentialz-3: pending authorization Credentialz-2: pending authorization TE-18.1: pending authorization	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation
Juniper		Hashing: pending authorization TR-6.2: pending authorization ACCTZ-10.1: pending authorization ACCTZ-4.2: pending authorization Credentialz-5: pending authorization Credentialz-3: pending authorization Credentialz-2: pending authorization TE-18.1: pending authorization	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation
Nokia		Hashing: pending authorization TR-6.2: pending authorization ACCTZ-10.1: pending authorization ACCTZ-4.2: pending authorization Credentialz-5: pending authorization Credentialz-3: pending authorization Credentialz-2: pending authorization TE-18.1: pending authorization	Hashing: Dataplane Hashing with Physical Loopbacks TR-6.2: Local logging destinations ACCTZ-10.1: gNSI.acctz.v1 (Accounting) Test Accounting Authentication Error - Multi-transaction ACCTZ-4.2: Record Payload Truncation Credentialz-5: Hiba Authentication Credentialz-3: Host Certificates Credentialz-2: SSH Password Login Disallowed TE-18.1: gRIBI MPLS-in-UDP Encapsulation

Help

[gemini-code-assist]

Code Review

This pull request introduces comprehensive cleanup handlers (t.Cleanup) across various test suites (including AAA, credentialz, gRIBI, policy forwarding, and system logging/NTP) to ensure that modified configurations are reverted and the shared testbed is restored to its original state. While these additions align with the repository's mandatory state reversion guidelines, the reviewer noted that unconditionally deleting the entire NTP configuration in system_ntp_test.go can wipe out pre-existing NTP servers on shared devices. It is recommended to capture and restore the original NTP configuration instead of performing a flat deletion.

[AmrNJ]

/gemini review

[gemini-code-assist]

Code Review

This pull request systematically introduces robust cleanup routines using t.Cleanup across various test suites (including AAA, NTP, logging, and gRIBI tests) to ensure that any modified configurations or added users are reverted upon test completion. This aligns with the repository's mandatory state reversion guidelines. One critical feedback item is provided regarding the NTP test cleanup, where unconditionally deleting the entire loopback interface config (lo0 on Juniper) can disrupt shared testbeds and lock out management access; it is recommended to target only the specific subinterface instead.

[AmrNJ]

/gemini review

[gemini-code-assist]

Code Review

This pull request systematically introduces t.Cleanup handlers across multiple test suites (including acctz, credentialz, gribi, policy_forwarding, security, and system logging tests) to ensure proper cleanup and state reversion of DUT configurations (both gNMI and CLI) upon test completion. This directly aligns with the repository's mandatory state reversion guidelines. There are no review comments provided, and I have no additional feedback to offer as the changes are correct and follow the style guide.

[AmrNJ]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves test reliability and state cleanup across various feature tests (gNSI, gRIBI, and system logging) by registering cleanup operations using t.Cleanup() to ensure the DUT is restored to its original state. The changes include batching delete operations, moving cleanup registrations earlier in tests, and reverting specific device configurations. The review feedback suggests replacing defer with t.Cleanup() for flushing and closing the gRIBI client in the MPLS-in-UDP test to align with the repository's style guide.

[gemini-code-assist]

In alignment with the repository style guide and the goal of this PR, persistent state cleanup on the DUT (such as flushing and closing the gRIBI client) should be registered using t.Cleanup() rather than defer. This ensures that the cleanup is guaranteed to run even if the test fails or panics early, and avoids potential LIFO ordering issues with multiple defers.

Suggested change

	defer c.Close(t)
	defer c.FlushAll(t)
	t.Cleanup(func() {
	c.FlushAll(t)
	c.Close(t)
	})

References

1. All cleanup operations, whether for gNMI configurations or raw CLI commands, must be registered using t.Cleanup() to guarantee they are executed even if the test fails or panics early. ^(link)
2. Always register cleanup operations using 't.Cleanup()' before executing configuration pushes or destructive operations to ensure that the cleanup routine is safely queued and will execute to restore the system state even if the operation fails.