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

FortiAnalyzer Network Manager Hotfix #62919

Merged
merged 12 commits into from Feb 14, 2020
Merged

Conversation

ftntcorecse
Copy link
Contributor

SUMMARY

Hotfix for connection manager persistent connection initialization.

The issue is that the login() function is no longer being automatically called when playbooks start. So our module_utils would assume that connection had been made and try to send data and fail. We've added some code to the send_request() portion to double check if the network manager is connected, and if it isn't, we run the _connect() function from the connection object.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/httpapi/fortianalyzer.py

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Sep 27, 2019

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 27, 2019
@ftntcorecse
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 5, 2019
@justjais justjais removed the needs_triage Needs a first human triage before being processed. label Oct 16, 2019
@justjais justjais requested a review from Qalthos October 16, 2019 13:49
Copy link
Contributor

@Qalthos Qalthos left a comment

Choose a reason for hiding this comment

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

So I see what the issue is... I don't really like the solution, but I don't have a better option for you right now.

lib/ansible/plugins/httpapi/fortianalyzer.py Show resolved Hide resolved
lib/ansible/plugins/httpapi/fortianalyzer.py Outdated Show resolved Hide resolved
lib/ansible/plugins/httpapi/fortianalyzer.py Outdated Show resolved Hide resolved
lib/ansible/plugins/httpapi/fortianalyzer.py Outdated Show resolved Hide resolved
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Oct 23, 2019
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 21, 2019
Copy link
Contributor Author

@ftntcorecse ftntcorecse left a comment

Choose a reason for hiding this comment

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

Sorry for the late feedback. Let me know if I missed anything.

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Dec 1, 2019
@ftntcorecse
Copy link
Contributor Author

@Qalthos -- Please come back to this. We need to get it merged. We have multiple issues and reports that are waiting on this hotfix and people are starting to manually install it from this PR. We need this merged as soon as possible. Please let me know how I can help you approve this, if anything else needs to happen.

@ansibot ansibot removed stale_review Updates were made after the last review and the last review is more than 7 days old. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 5, 2020
lib/ansible/plugins/httpapi/fortianalyzer.py Outdated Show resolved Hide resolved
lib/ansible/plugins/httpapi/fortianalyzer.py Outdated Show resolved Hide resolved
@ftntcorecse
Copy link
Contributor Author

My apologies, you're absolutely right. This was added in 2.9. We wrote them at the same time but it just took this long to get it. Change is coming...

@ftntcorecse
Copy link
Contributor Author

Thanks. Ship it.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 13, 2020
@Qalthos Qalthos merged commit c1d8bdb into ansible:devel Feb 14, 2020
@ansible ansible locked and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. has_issue networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants