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

Enable non-openwrt devices as hosts #1028

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

FFHener
Copy link
Contributor

@FFHener FFHener commented Oct 24, 2024

This is a first step into implementing #751. It currently only supports hosts that doesnt need airos_dfs_reset. The changes wont break the current way of defining everything. This way we can make a smooth transition.

Im not to shure what could be the best way to deal with locations that doesnt use an own mgmt-network.

Im not super experienced in writing jinja templates. Feel free to suggest better ways.

Merging this PR will enable us to move forward with #1012

@FFHener FFHener force-pushed the host-include-all-devices branch 2 times, most recently from 677df4c to 3264784 Compare October 24, 2024 18:15
@Noki
Copy link
Member

Noki commented Oct 25, 2024

We should have one or two example of devices that can run more than one OS e.g. the stock OS and OpenWRT or SwOS and RouterOS. How do we name model files in this case?

Ideas:

/group_vars/models/[os]/[name].yml
/group_vars/model_[os]_[name].yml

Currently I can't think of a good way of doing this without also touching all location files, but maybe we can also implement a fallback.

@FFHener FFHener marked this pull request as draft October 25, 2024 19:46
@FFHener
Copy link
Contributor Author

FFHener commented Oct 25, 2024

If we choose the first option and set openwrt as default I think we could find a solution to not touch the current locations. I will have a look into it, but I'm not shure how to implement it in a simple way. We would need to add the os_flag to the hosts than instead.

Set all to openwrt for now.

This can be used in the future for proprietary devices
@FFHener FFHener force-pushed the host-include-all-devices branch from 3264784 to 34857fd Compare October 25, 2024 20:21
@FFHener FFHener force-pushed the host-include-all-devices branch from 34857fd to ba885ce Compare October 25, 2024 21:32
@Noki
Copy link
Member

Noki commented Oct 26, 2024

For the snmp_devices we can and should continue to use snmp_profile and simply set it as "os" in order to be able to use it in the j2 template, because if we start changing something in the snmp_devices, we can also migrate everything at the same time, but that makes the PR extensive and therefore difficult to handle.

Suggestion:

{# collect profiles from new way of writing hosts as well and append them #}

We insert a step here that runs through the profiles in a loop and ensures that os is set and filled with the value of snmp_profile so that the old notation is compatible. The changes to the locations are then optional.

Let me know if you have any questions regarding this proposal.

@@ -12,6 +12,7 @@
apply:
tags: always
tags: always
when: os == "openwrt" | default ("openwrt")
Copy link
Member

Choose a reason for hiding this comment

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

I think there are much better ways to archieve that. Lets have a workshop or chat on that.

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.

3 participants