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

Fix the regression test failure for the aaa resource module. #80

Merged
merged 1 commit into from
May 11, 2022

Conversation

kerry-meyer
Copy link
Collaborator

SUMMARY
(Double commit: Also committed to the "main" branch)

This change modifies the testing of the "delete" test case for the
aaa resource module to enable testing of the functionality without
causing a failure in the "idempotent" test for the subsequent
merge/restore test case.

Problem analysis:

The "delete" test (Delete aaa properties; test_case_04) is
intended to verify functionality for deletion of an aaa configuration
attribute. Restoring the "fail_through" bool attribute after it has been
deleted, however, by changing it from the "null" (deleted) state back to a 
bool  value ("true", in this case) has the side effect in the SONiC functional
code of deleting the configuration for the other aaa configuration attributes
("group" and "local").
 
Because of this side effect when "restoring" the "fail_through"
attribute, the merge/restore test (aaa properties; test_case_05) fails
during the "idempotent" section of the test because the specified
values for the "group" and "local" attributes do not remain as
specified when simultaneously restoring the deleted value of the
of the "fail_through" attribute.

(See the "ADDITIONAL INFORMATION" section for specific information on the failure mechanism.)

Fix:

Test/verify the "delete" functionality using an attribute
that does not have side effects on the other attributes:
The "group" attribute is a suitable choice for this testing.
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

sonic_aaa

ADDITIONAL INFORMATION
The problem mechanism can be seen in the following log output from a failing run: (See "<<<" below.)

Although the first test section passes because the "target" configuration of setting
the "fail_through" value to "true", this causes a failure in the subsequent "idempotent"
test because the required "command" list, which should be empty, contains the
unexpected extra commands to restore the deleted "group" and "local" attributes.

Failure Case

                "test_case_05.1": {
                    "after": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": null,            <<< Unexpected consequence of the change
                                "local": null               <<< Unexpected consequence of the change
                            }
                        }
                    },
                    "before": {
                        "authentication": {
                            "data": {
                                "fail_through": null,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "commands": [
                        {
                            "authentication": {
                                "data": {
                                    "fail_through": true
                                }
                            },
                            "state": "merged"
                        }
                    ],
                    "configs": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "module_stderr": "No Error",
                    "status": "Passed"
                },

                "test_case_05.2": {
                    "after": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "before": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": null,            <<< Unexpected consequence of the previous change
                                "local": null               <<< Unexpected consequence of the previous change
                            }
                        }
                    },
                    "commands": [            <<< Not empty, causing the "idempotent" test to fail.
                        {
                            "authentication": {
                                "data": {
                                    "group": "radius",
                                    "local": true
                                }
                            },
                            "state": "merged"
                        }
                    ],
                    "configs": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "module_stderr": "No Error",
                    "msg": "Not defined",
                    "status": "Failed"
                }

Successful case with the fix

                "test_case_05.1": {
                    "after": {
                        "authentication": {
                            "data": {    <<< Okay for the idempotent test because all three attributes
                                             end up in the expected state,
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "before": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": null,
                                "local": null
                            }
                        }
                    },
                    "commands": [
                        {
                            "authentication": {
                                "data": {
                                    "group": "radius",
                                    "local": true
                                }
                            },
                            "state": "merged"
                        }
                    ],
                    "configs": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "module_stderr": "No Error",
                    "status": "Passed"
                },

                "test_case_05.2": {
                    "after": "Not defined",
                    "before": {
                        "authentication": {   <<<   All three attributes already have the desired values.
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "commands": [],    <<< Successful (empty) because all attributes are already
                                           in the "restored" state.
                    "configs": {
                        "authentication": {
                            "data": {
                                "fail_through": true,
                                "group": "radius",
                                "local": true
                            }
                        }
                    },
                    "module_stderr": "No Error",
                    "msg": "Not defined",
                    "status": "Passed"
                }
            }
        }
    }
},

Test Results

Regression test results with the fix:

aaa_regression_test_fix_1.x_regression-2022-05-10-16-30-50.pdf

regression_aaa_test_fix_1.x.log

Regression test results without the fix:

aaa_regression_test_fix_1.x_base.pdf

regression_aaa_test_fix_1.x_base.log


This change modifies the testing of the "delete" test case for the
aaa resource module to enable testing of the functionality without
causing a failure in the "idempotent" test for the subsequent
merge/restore test case.

Problem analysis:

The "delete" test (Delete aaa properties; test_case_04) is
intended to verify functionality for deletion of an aaa configuration
attribute. Deletion of the "fail_through" bool attribute, however,
has the side effect in the SONiC functional code of also deleting
the configuration for the other aaa configuration attributes
("group" and "local"). Changing it from the "null" (deleted)
state back to a bool value also deletes the other attributes.
Because of this side effect when "restoring" the "fail_through"
attribute, the merge/restore test (aaa properties; test_case_05) fails
during the "idempotent" section of the test because the specified
values for the "group" and "local" attributes do not remain as
specified when simultaneously restoring the deleted value of the
of the "fail_through" attribute.

Fix:

Test/verify the "delete" functionality using an attribute
that does not have side effects on the other attributes:
The "group" attribute is a suitable choice for this testing.
@kerry-meyer kerry-meyer requested a review from stalabi1 May 11, 2022 04:01
@kerry-meyer kerry-meyer merged commit 654c2ad into ansible-collections:1.x May 11, 2022
@kerry-meyer kerry-meyer deleted the aaa_test_fix_1.x branch June 2, 2022 16:51
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.

2 participants