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 Huawei special_login_handler (#2711) #2719

Closed
wants to merge 1 commit into from

Conversation

fharbe
Copy link
Contributor

@fharbe fharbe commented Apr 4, 2022

This is a proposal to fix the special_login_handler() issue (#2711).

TL;DR: I removed the special_login_handler() completely and moved the handling for the password change prompt into session_preparation(), because I did not see a better way of fixing the issues altogether (read the whole story below).

After spending some time investigating the issue I found out that the pattern matching logic in read_until_pattern expects the pattern to split exactly in 3 parts (using re.split), see base_connection.py#L579 for reference. Seems this change was introduced during some fundamental changes to the channel reading logic in in 793100d.

At first I tried to simply fix the password_change_prompt regex (see huawei.py#L104) by replacing the inner groups (parenthesis) with (?:) like so:

        password_change_prompt = r"((?:Change now|Please choose))|(?:[\]>]\s*$)"

However, when looking more closely at the code, I stumbled upon the if re.search(password_change_prompt, output): condition, which seemed wrong (would always evaluate to True). So I modified the if-condition to actually search for the password_change_prompt only, like so:

password_change_prompt = r"(?:Change now|Please choose).+"
combined_pattern = r"({}|[>\]])".format(password_change_prompt)
output = self.read_until_pattern(combined_pattern, read_timeout=3.0)
if re.search(password_change_prompt, output):
    self.write_channel("N\n")

At first it seemed to work, but it turns out that connecting to the device now only works if a password change prompt is present. When no password change prompt is found, the following exception is raised:

netmiko.exceptions.ReadTimeout: 
Pattern not detected: '[>\\]]' in output.

Huh? Inspecting the debug logs, it turns out that self.write_channel("N\n") (which was always called before changing the if-condition) was responsible for the following debug logs:

DEBUG:netmiko:read_channel: N
DEBUG:netmiko:read_channel: 
        ^
Error: Unrecognized command found at '^' position.
<AC6508>

This means, the self.write_channel("N\n") did not only produce an error, but also a new prompt!

To me, it looks like it was only this new prompt (which was triggered by the "Unrecognized command" error) that actually made _test_channel_read in session_preparation work at all.

Therefore, I think it would only be possible to keep the handling logic for the password change prompt in special_login_handler if either _test_channel_read in session_preparation would be removed completely or self.write_channel(self.RETURN) would be needed to to trigger a new prompt for _test_channel_read to read it. Both of which are not really elegant IMHO. Maybe it would also be possible to read until the prompt by using a lookahead assertion, but I don't know if that's possible with the way read_until_pattern is currently implemented.

Let me know what you think.

Kind regards
Flo

@fharbe
Copy link
Contributor Author

fharbe commented Apr 5, 2022

@ktbyers Fixed the failing tests 😄 Did you already have time to look into my proposed fix? 🙂

Copy link

@nivaldoinacios nivaldoinacios left a comment

Choose a reason for hiding this comment

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

work here

@ktbyers
Copy link
Owner

ktbyers commented Apr 12, 2022

Superseded by:

#2728

@ktbyers ktbyers closed this Apr 12, 2022
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.

None yet

3 participants