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.

Sign up for GitHub

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

Jump to bottom

feat(edgeswitch): allow to differentiate between running and startup config #105

Merged
merged 10 commits into from
Sep 3, 2020
Merged

feat(edgeswitch): allow to differentiate between running and startup config #105

merged 10 commits into from
Sep 3, 2020

Conversation

mrwiora
Copy link
Contributor

@mrwiora mrwiora commented Aug 26, 2020

SUMMARY

Allow the collection of running and startup config to compare these to versions or to save one of them

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

edgeswitch

@mrwiora mrwiora changed the title feat(config): allow to differentiate between running and startup config feat(edgeswitch): allow to differentiate between running and startup config Aug 26, 2020
@felixfontein
Copy link
Collaborator

Your change breaks backwards compatibility. I don't think that's a good idea.

@mrwiora
Copy link
Contributor Author

mrwiora commented Aug 27, 2020

Your change breaks backwards compatibility. I don't think that's a good idea.

oh that's interesting - you mean an earlier version did not recognize the command show startup-config?

Just hoping that the most of the ubiquiti edgeswitch users will use the latest stable released and longtermin supported version - which version are we talking about? I didn't find a test for that.

In my case I needed a procedure to stop the processing of changes when there are changes not commited - as it might result in an unstable environment. I think I am not the only one having this requirement.

@felixfontein
Copy link
Collaborator

No, but you are returning results with a different name. Playbooks/roles expecting the data to be there as ansible_net_config will no longer work, since it is now called ansible_net_runningconfig. Also, if they use the gather_subset parameter, it will no longer work as expected since you renamed the value there.

@mrwiora
Copy link
Contributor Author

mrwiora commented Aug 27, 2020

oh nice. Thanks for that hint!
Agreeing that the runningconfig is config (as it was before) I"ll update my PR, so the only thing that changes is that startupconfig is available too.

@mrwiora
fix(edgeswitch): ensuring backward compatibility - runningconfig rema…
7a045c0 
…ins config and is completed by startupconfig
felixfontein
felixfontein reviewed Aug 27, 2020
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Looks good, though I have no idea about edgeswitch, so I cannot say whether this change actually works :)

plugins/modules/network/edgeswitch/edgeswitch_facts.py Show resolved Hide resolved
@felixfontein
Copy link
Collaborator

@f-bor as the author of this module, could you please take a look?

@mwiora you also need a changelog fragment.

@mrwiora
Copy link
Contributor Author

mrwiora commented Aug 27, 2020

thanks man! Next time I'll provide that - I have some more needs on this module, so there will come more most probably ;)

@f-bor
Copy link

f-bor commented Aug 28, 2020

@felixfontein it looks good to me
@mwiora you also need to update the unit tests file (test_edgeswitch_facts.py). test_edgeswitch_facts_default gathers all the facts, except config and interfaces. You also have to exclude startup-config in set_module_args (or it will fail because you did not provide a file with the "show startup-config" output).

@felixfontein
Copy link
Collaborator

Also check out the linting errors in CI: https://app.shippable.com/github/ansible-collections/community.network/runs/527/1/tests

@mrwiora
Copy link
Contributor Author

mrwiora commented Sep 2, 2020

@felixfontein I hope I didn't miss anything :) checks look good. Just tell me, if you need something from me :)

@felixfontein
Copy link
Collaborator

@mwiora you still need to add a changelog fragment. Besides that, looks good from my side.

@f-bor are you still happy with the change?

@mrwiora
Copy link
Contributor Author

mrwiora commented Sep 2, 2020

@felixfontein done :)

@f-bor
Copy link

f-bor commented Sep 2, 2020

@felixfontein still good to me

@felixfontein felixfontein merged commit 1aa8898 into ansible-collections:main Sep 3, 2020
@felixfontein
Copy link
Collaborator

@mwiora thanks a lot for implementing this!
@f-bor thanks a lot for reviewing!

@felixfontein felixfontein added the needs_backport_to_stable_1 PR needs to be backported to the stable-1 branch label Sep 3, 2020
@felixfontein felixfontein added backport-stable-1 and removed needs_backport_to_stable_1 PR needs to be backported to the stable-1 branch labels Sep 17, 2020
patchback bot pushed a commit that referenced this pull request Sep 17, 2020
@mrwiora
feat(edgeswitch): allow to differentiate between running and startup …
31994cb 
…config (#105)

* feat(config): allow to differentiate between running and startup config

* fix(edgeswitch): ensuring backward compatibility - runningconfig remains config and is completed by startupconfig

* feat(changelog): adding version change info for startupconfig

Co-authored-by: Felix Fontein <[email protected]>

* fix(tests): ignore startupconfig as parameter

* fix(lint): satify linter - blank lines missmatch

* feat(changelog): adding changelog fragment

* fix(changelog): changelog fragment as required

* trigger(): retrigger

* Update changelogs/fragments/105_edgeswitch_add-startupconfig.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 1aa8898)
@patchback patchback bot mentioned this pull request Sep 17, 2020
Merged
felixfontein pushed a commit that referenced this pull request Sep 17, 2020
@patchback @mrwiora
feat(edgeswitch): allow to differentiate between running and startup …
857aa7d 
…config (#105) (#115)

* feat(config): allow to differentiate between running and startup config

* fix(edgeswitch): ensuring backward compatibility - runningconfig remains config and is completed by startupconfig

* feat(changelog): adding version change info for startupconfig

Co-authored-by: Felix Fontein <[email protected]>

* fix(tests): ignore startupconfig as parameter

* fix(lint): satify linter - blank lines missmatch

* feat(changelog): adding changelog fragment

* fix(changelog): changelog fragment as required

* trigger(): retrigger

* Update changelogs/fragments/105_edgeswitch_add-startupconfig.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 1aa8898)

Co-authored-by: Matthias R. Wiora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mrwiora @felixfontein @f-bor