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

edgeos: Always exit configure mode #7

Closed
wants to merge 1 commit into from
Closed

edgeos: Always exit configure mode #7

wants to merge 1 commit into from

Conversation

jplitza
Copy link
Contributor

@jplitza jplitza commented Apr 8, 2020

SUMMARY

Previously, the configure mode was kept active when not committing the changes.
This made later invocations of show configuration commands invalid (because
that command must be executed outside of configure mode) which caused all later
edgeos_config module invocations in check_mode to erroneously return a changed
state.

I have tested that the behavior in check_mode is fixed and unchanged (working) without check_mode.

Fixes ansible/ansible#68350

Also fixes another bug that edgeos_facts doesn't work after an edgeos invocation in check mode.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

edgeos

ADDITIONAL INFORMATION

@felixfontein
Copy link
Collaborator

Please also add a changelog fragment here.

@jplitza
Copy link
Contributor Author

jplitza commented Apr 13, 2020

"edgeos module: Fix false-positive change reports in --check mode"
Like this?

@felixfontein
Copy link
Collaborator

bugfixes:
 - edgeos - Fix false-positive change reports in check mode

@felixfontein
Copy link
Collaborator

(in changelogs/fragments/7-edgeos-false-positive-check-mode.yml)

Previously, the configure mode was kept active when not committing the
changes. This made later invocations of `show configuration commands`
invalid (because that command must be executed outside of configure
mode) which caused all later edgeos_config module invocations in
check_mode to erroneously return a changed state.
@jplitza
Copy link
Contributor Author

jplitza commented Apr 14, 2020

Done.

And I can't help but say: The experience of this PR so far was the worst possible. I created it at the ansible repository, was told to recreate in the community.general repository (which didn't exist when I first created the PR), there I was told to recreate it in the yet-to-be-created community.network repository, where I am now told to add a changelog fragment. At no point did I receive any feedback about the content of the PR, whether it had any chances of being accepted or whatever.

@gundalow gundalow self-assigned this Apr 14, 2020
@felixfontein
Copy link
Collaborator

@mjbnz @ganeshrn since you created/reviewed a similar PR (#34), could you please check whether this PR is still needed, and if it is, whether it is ok?

@jplitza
Copy link
Contributor Author

jplitza commented Jun 7, 2020

@felixfontein Even though I am not @mjbnz, I'll take the liberty of saying "Yes, it does".

@jplitza jplitza closed this Jun 7, 2020
@felixfontein
Copy link
Collaborator

@jplitza also fine with me. I don't know who is looking after the edge module(s), I'm just trying to connect people who seem to be interested in it.

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.

edgeos_config in check_mode reports too many changes
3 participants