candidate: network.restore

[rohitthakur2590]

Candidate Name

network.restore

Link to Github Repo

https://github.com/redhat-cop/network.restore

Review Due By

CHECKLIST

Business Checks:

Content Viability

• [x] must support a “business-viable” use case
• [X] isn’t just a demo
• [x] is production-ready, as in able to run in Ansible Controller “as-is”
• [x] provides a solution an end-to-end automation use case and brings value to our customers

Content Duplication

• [X] use case addressed by the content isn’t covered by another validated content collection already - to the best of our knowledge

Technical Checks:

Content Conformity

• [x] is packaged as a content collection - see the Collection Requirements Checklist
• [X] is YAML only (Exception: if plugin(s) included are necessary for this specific use-case and are not generic enough for a certified collection)
• [X] if there is a plugin(s), it is not a duplicate of something from a certified or supported collection - to the best of our knowledge
• [X] supported plugin/module used in a validated role is noted as a dependency

Content Compliance

• [X] no known policy or legal reason for why the content shouldn’t be published
• [x] README contains information about the purpose of the collection and how to install it
• [X] README contains a link to the collection license file
• [X] README contains general usage and requirements information such as required versions of ansible-core and Python, and other required libraries or SDKs
• [X] Repo is self-certified with the OpenSSF Best Practices and has the OpenSSF Best Practices Badge
• [x] Has 2 or more active maintainers listed in a CODEOWNERS file.
• [x] Has an approved OSI license
• [X] Content falls logically into one of these domains - infra, security, network, cloud, edge
• [x] (If internal content) Content has been or is being migrated into the redhat-cop Github org

Content Testing/Validation

• [x] runs ansible-lint “Production” profile in an active Github workflow on the repository
• [x] passes the ansible-lint “Production” profile
• [x] ansible-lint.yml file (if present) only contains justified ignores - with a comment explaining the need for each ignore

[alisonlhart]

Version needs to be changed to a semver prod version: 1.0.0+

[alisonlhart]

@arnav3000

[alisonlhart]

@rohitthakur2590 Initial pass of items that need resolution (subject to change)

• Not passing ansible-lint production profile
• Remove the network.base collection content from the roles/ dir
• Bump version to a prod release of 1.0.0+
• Add an "Install" section to the README with how to install the collection from Automation Hub
• Migrate content into the redhat-cop Github Org. Open a repository management issue here. Reach out to @Andrew Block in RH slack with any migration questions.

[arnav3000]

• Only 1 Maintainer.
• Ansible-lint not running.
• Please verify the OpenSSF tests as it is passing only 10%

[alisonlhart]

@rohitthakur2590 Checking in on this issue! Any updates on the final checklist items ?

[alisonlhart]

This is passing all checklist items! Posting a message in the cop Slack channel for immediate sign-off.

[djdanielsson]

so first off it looks like this role just does one thing which makes me wonder if it is worth creating a whole collection over this instead of putting it inside an already created collection. next the layout of the role does not match the standard layout of a role. another issue is the name of the role itself called run which does not explain what it does, but this goes back to the issue of it only having a single purpose. there might be more but I stopped looking after seeing these issues

[ericzolf]

The use case seems sound but I'm struggling with the structure of the one role:

1. one single run role with methods which seem to be actually different actions, would probably be better split into multiple roles
2. the includes directory next to the tasks one is very uncommon, and I don't grasp the added value compared to the more standard approach of having everything under tasks
3. the task name are not prefixed by the name of the task file (which helps debugging)

So, basically the same as @djdanielsson but with different suggestions (and wording).

I also noticed that the inventory in the "Testing" section is probably wrong, the first line should probably read [network_hosts:children] as it seems to contain other groups.

[rohitthakur2590]

@djdanielsson Thanks for your insightful feedback. In addition to the platform-agnostic restore operation capability, our network.backup and network.restore collections also incorporate SCM support, currently including Github/tagging, with plans for future enhancements such as diff-based restore and support for GitLab and S3.

The decision to maintain separate network.backup and network.restore collections stemmed from our aim for better organization and scalability. By segregating these functionalities, we can ensure that as the network backup and restore processes evolve and become more intricate, each collection can independently scale and adapt to changes.

@ericzolf Regarding the role of actions, it aligns with the approach/pattern we've adopted for networking-validated content. Your point about organizing and naming task files is well taken, and we'll definitely consider improvements in an upcoming release.

[rohitthakur2590]

cc @cidrblock any thoughts on this ?

[ericzolf]

I still don't grok the need to split backup from restore, where I'd expect that they're tightly related and need to stay aligned to work properly. Collections are generally aligned along technologies, not actions, that's what roles are meant for. And I can't imagine why/how the two would need different lifecycles, especially as they're maintained by the same persons. Validated content is aimed at providing content for our customers to reuse in their automation, we can't swallow all our good practices or we become inconsistent.

[rohitthakur2590]

Closing this as we have consolidated the backup and restore with the updated file structures.