Including dellemc.openmanage collections into Ansible 3.0.0

[rajeevarakkal]

Collection Link: https://galaxy.ansible.com/dellemc/openmanage
Repo: https://github.com/dell/dellemc-openmanage-ansible-modules/tree/collections
Issue Tracker: https://github.com/dell/dellemc-openmanage-ansible-modules/issues
GitId : rajeevarakkal

[Andersson007]

NOTE 1: Many places in modules documentation / examples / return to fix. Among others:

short_description: line 1 ends with a dot
opt idrac_user: line 1 does not start with a capital letter
opt share_name: explicit "require: false" declaration
opt baseuri: line 1 does not end with a dot
no "notes" section, it should, at least, contain info about check_mode support
examples: #1 no FQCN
return: no RETURN section, there must be, at least, RETURN = r"""#"""
examples: "- name" of #2 ends with a dot
ANSIBLE_METADATA is no longer needed
return msg: no sample
abbreviations/latin phrases found 'doesn't', use 'does not' instead

All modules must comply with Ansible documentation standard.

[rajeevarakkal]

Yup, we are in a process to fix all the sanity errors. Will take care of abbreviations/latin phrases as well

[Andersson007]

@rajeevarakkal cool:) some of the points are not covered by sanity tests though are very important, like the examples format

[rajeevarakkal]

Sure .. Will have to check all these now. will do it

[rajeevarakkal]

Hi Team,

All sanity errors are fixed now in our collections. Sanity against both Ansible Stable-2.10 and devel.

Also, removed the ANSIBLE_METADATA section. Added notes section for all the modules along with few other minor fixes.

https://github.com/dell/dellemc-openmanage-ansible-modules/tree/collections

Lets us know your further inputs

[felixfontein]

There are severe versioning problems (it neither adheres to semantic versioning nor is version_added used correctly in all cases).

Public availability and communication:

• published on Ansible Galaxy Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
  • Comment: I cannot find any information on this Contributor Covenant Code of Conduct v1.4 (D&I group needs to check whether it satisfies their standards - I know too little about CoCs to judge that :) )
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues

Standards and documentation:

• adheres to semantic versioning
  • Comment: Bugfix releases contain new features (2.1.5, 2.1.2, 2.1.1), new modules (2.1.4, 2.1.3, 2.1.2, 2.1.1), deprecations (2.1.2, 2.1.1).
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  • Comment: It would be better to use C(...) for examples, for example for the share_user and baseuri options (doc fragments).
  • Comment: modules should mention when they support check mode.
  • Comment: return value docs of dellemc_configure_idrac_eventing module is strange: Configures the iDRAC eventing attributes. sounds like an option description, not like a return value description. Also, the module does not seem to actually return something called dest. (Similar for more modules.)
• follows development conventions
  • Comment: dellemc_get_firmware_inventory and dellemc_get_system_inventory return ansible_facts, but do not even document that.
• has README.md
• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections
• modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix
  • Comment: file lookup in ome_template should be lookup('ansible.builtin.file', ....
  • Comment: copy module in ome_template should be ansible.builtin.copy:
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
  • Comment: There are multiple version_added: "2.10.0" entries.
• FQCNs is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml

Tests:

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it does not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
• all CI tests run against every pull request
  • Comment: PRs seem to be merged without passing tests, for example Sanity and unit fix dell/dellemc-openmanage-ansible-modules#207 (unit tests fail)
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
  • Comment: not clear whether this is the case
• all CI tests run regularly (nightly, or at least once per week)
  • Comment: there seems to be no regular

[rajeevarakkal]

@felixfontein ,

We will go through all these, in case any clarification we will get back to you. It will be great if you can share link of reference repo.

Is non tick mark check box points are the one we are missing?

[felixfontein]

@rajeevarakkal what reference repo do you mean? About the check marks: I've left the ones open which definitely need attention. Some checked check marks also have comments you should also look at though.

[rajeevarakkal]

@felixfontein - Any reference of already contributed/included collections upstream repo link , will be useful for team, to cross check .

We are actively looking into addressing the comments you have given, Status so far

1. have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)

Comment: I cannot find any information on this -- Addressed

2. modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix

Comment: file lookup in ome_template should be lookup('ansible.builtin.file', ....
Comment: copy module in ome_template should be ansible.builtin.copy:

-- Addressed

Few of the clarification / response below

01 Our versioning strategy so far.

 adheres to semantic versioning

> Comment: Bugfix releases contain new features (2.1.5, 2.1.2, 2.1.1), new modules 
(2.1.4, 2.1.3, 2.1.2, 2.1.1), deprecations (2.1.2, 2.1.1).

We used to follow a monthly release (versions 2.1.x's) cadence with every month last week we used to update our github repo with new set of features/modules and bug fixes. Regarding version numbering, we were never following semantic versioning style officially, Once you provided us the input on CI PR last month, we decided to follow semantic versioning there on. After that, we are done with our 2.1.5 Patch release by 1/6 (delayed due Xmas holidays). Now we are in refreshing our content getting our Major release out by this month end around by 1/27. Hope this will address versioning issues.

all CI tests run against every pull request
Comment: PRs seem to be merged without passing tests, for example dell/dellemc-openmanage-ansible-modules#207 (unit tests fail)

During these days, we are even trying to increase UT Config coverages and stabilize our CI workflows along with addressing various other comments as part of this discussions. There were a series of code commits happening on all these sanity, standardization, comment addressing, UT improvement and so on. Though UT is optional, currently only one of our UT jobs is failing on Python 2.7 against Ansible devel with PyTest lib error. With this Major release, yes all PRs will be strictly go through this Sanity /UT pass.

 all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
Comment: not clear whether this is the case

Agreed. This is the case. We will follow this.

We are actively on addressing remaining comments as well.

[rajeevarakkal]

@felixfontein - All comments are addressed or responded , except below one

documentation and return sections use version_added: containing the collection version for which 
an option, module or plugin was added
Comment: There are multiple version_added: "2.10.0" entries.

Could really get this comment. Should version_added: "2.10.0" changed with version_added: "3.0.0"?

[felixfontein]

version_added should specify the version of your collection at which the feature / module / ... was added. Since it's already there, it should not be 3.0.0, but something before. I don't know that's the correct value, but 2.10.0 is most likely not (since you don't have a version 2.10.0 of your collection).

[rajeevarakkal]

Thanks @felixfontein , We will get this update with appropriate major/minor version of our collection releases.

Also, is it Ok, if we remove this version_added field from our collection modules? (I can not see it, this has been removed from community.general)

WE were so far refering to The version of Ansible when the module was added. to our collection repository. (https://docs.ansible.com/ansible/devel/dev_guide/developing_modules_documenting.html)

Is should use our own repositories (https://github.com/dell/dellemc-openmanage-ansible-modules/tree/collections) release version right?

[felixfontein]

I guess that documentation needs updating. See https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#documentation for more up-to-date information on version_added.

community.general changed version_added: 2.10 for modules that were planned to be added to Ansible 2.10 to version_added: 0.2.0 IIRC, since 0.2.0 was the first proper release of that collection.

[rajeevarakkal]

@felixfontein , we done with version_added correction as well

We are planning to freeze changes by 1/20 and release our collection 3.0.0 around 1/25 after (last week of this month), hope we are good with all the comments by this now.

If you any further comments, Is it possible share to us by tomorrow? it will be great help

[felixfontein]

I looked through the current state of the collection, there are still a few things that could be improved:

• Some modules use RETURNS instead of RETURN (dellemc_get_firmware_inventory, idrac_user, dellemc_idrac_lc_attributes, idrac_lifecycle_controller_logs, redfish_storage_volume, idrac_lifecycle_controller_status_info, idrac_syslog, idrac_lifecycle_controller_job_status_info, dellemc_idrac_storage_volume, idrac_redfish_storage_controller, redfish_powerstate, idrac_network, dellemc_configure_idrac_eventing, idrac_bios, idrac_reset, dellemc_configure_idrac_services, idrac_timezone_ntp, dellemc_system_lockdown_mode, dellemc_get_system_inventory). This results in the return value documentation not being shown in ansible-doc.
• dellemc_get_firmware_inventory and dellemc_get_system_inventory should be _facts modules to indicate that they return ansible_facts. Ansible users expect _facts modules to return ansible_facts, but don't expect that from modules named differently. (The only exception are modules like hostname, which update facts like ansible_hostname. But these modules clearly don't fall in that category.)
• Some modules use the return value msg for a dictionary. This is somewhat unexpected, even though documented; users usually expect a string here (see https://docs.ansible.com/ansible/latest/reference_appendices/common_return_values.html#msg). I guess it's better to use another name; if you want something generic, use maybe result.

The first one is simple to fix (just remove the S). The other two potentially break backwards compatibility. (Since you plan to release a new major version this is possible, but breaks should still not be made lightly.)

About the ansible_facts returning modules: you could rename them, and keep the old name (in deprecated form) for some time. That way, the rename won't break users, and users will be informed that they have to update their roles/playbooks.

[rajeevarakkal]

I looked through the current state of the collection, there are still a few things that could be improved:

• Some modules use RETURNS instead of RETURN (dellemc_get_firmware_inventory, idrac_user, dellemc_idrac_lc_attributes, idrac_lifecycle_controller_logs, redfish_storage_volume, idrac_lifecycle_controller_status_info, idrac_syslog, idrac_lifecycle_controller_job_status_info, dellemc_idrac_storage_volume, idrac_redfish_storage_controller, redfish_powerstate, idrac_network, dellemc_configure_idrac_eventing, idrac_bios, idrac_reset, dellemc_configure_idrac_services, idrac_timezone_ntp, dellemc_system_lockdown_mode, dellemc_get_system_inventory). This results in the return value documentation not being shown in ansible-doc.
• dellemc_get_firmware_inventory and dellemc_get_system_inventory should be _facts modules to indicate that they return ansible_facts. Ansible users expect _facts modules to return ansible_facts, but don't expect that from modules named differently. (The only exception are modules like hostname, which update facts like ansible_hostname. But these modules clearly don't fall in that category.)
• Some modules use the return value msg for a dictionary. This is somewhat unexpected, even though documented; users usually expect a string here (see https://docs.ansible.com/ansible/latest/reference_appendices/common_return_values.html#msg). I guess it's better to use another name; if you want something generic, use maybe result.

The first one is simple to fix (just remove the S). The other two potentially break backwards compatibility. (Since you plan to release a new major version this is possible, but breaks should still not be made lightly.)

About the ansible_facts returning modules: you could rename them, and keep the old name (in deprecated form) for some time. That way, the rename won't break users, and users will be informed that they have to update their roles/playbooks.

@felixfontein Status below

#1 - Fixed and refreshed the repository
#2 Fixed by deprecating these two modules and replaced with proper _info module
#3 taken asre in almost all the modules other than ~3-4 modules, which will completed by tomorrow.

[felixfontein]

Thanks! About 2:

• Since you added a redirect (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-18d6cff77d3f99e665fab5cd24dc4384256c8aa59f5300c2a72098a582e6a1d8R9), ansible-base 2.10+ users will directly get the new _info module, which has a different return value. So this will break their playbooks/roles. It's better to remove that redirect.
• The version_added for the new _info modules is 2.1.0 (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-1f819589f60f38d6a27838a7bb942ebcc64e44b62134eafcdf352617114e002fR20), though that version of your collection has alrady been released in the past. You need to put the next major/minor release version number there.

[rajeevarakkal]

Thanks! About 2:

• Since you added a redirect (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-18d6cff77d3f99e665fab5cd24dc4384256c8aa59f5300c2a72098a582e6a1d8R9), ansible-base 2.10+ users will directly get the new _info module, which has a different return value. So this will break their playbooks/roles. It's better to remove that redirect.
• The version_added for the new _info modules is 2.1.0 (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-1f819589f60f38d6a27838a7bb942ebcc64e44b62134eafcdf352617114e002fR20), though that version of your collection has alrady been released in the past. You need to put the next major/minor release version number there.

Completed

[rajeevarakkal]

Thanks! About 2:

• Since you added a redirect (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-18d6cff77d3f99e665fab5cd24dc4384256c8aa59f5300c2a72098a582e6a1d8R9), ansible-base 2.10+ users will directly get the new _info module, which has a different return value. So this will break their playbooks/roles. It's better to remove that redirect.
• The version_added for the new _info modules is 2.1.0 (https://github.com/dell/dellemc-openmanage-ansible-modules/pull/218/files#diff-1f819589f60f38d6a27838a7bb942ebcc64e44b62134eafcdf352617114e002fR20), though that version of your collection has alrady been released in the past. You need to put the next major/minor release version number there.

Completed #3 point msg correction as well for remaining modules

[felixfontein]

@rajeevarakkal thanks, I think it looks good now! :)

[Andersson007]

1. (necessary) I ran sanity tests against the collections, they didn't pass (in collections branch).

Running sanity test 'pylint' with Python 3.6
ERROR: Found 2 pylint issue(s) which need to be resolved:
ERROR: plugins/module_utils/idrac_redfish.py:202:12: no-else-break: Unnecessary "elif" after "break"
ERROR: plugins/modules/idrac_firmware.py:239:12: no-else-break: Unnecessary "elif" after "break"
See documentation for help: https://docs.ansible.com/ansible/devel/dev_guide/testing/sanity/pylint.html

2. I consider the following points not crucial, i.e. optional, but highly recommended to fix to make the collection's documentation consistent with Ansible documentation. If anybody disagree, please say.

You can use this tool to quickly find the issues below.

For example,

examples: "- name" of #1 ends with a dot
opt nvme_mode: line 1 does not start with a capital letter
return msg: line 1 does not start with a capital letter
opt device_service_tags: line 1 does not end with a dot
abbreviations/latin phrases found 'e.g', use 'for example' instead
examples: #10 without using "name"

Also would be good:

• Remove unnecessary quotes and dots at the end of examples: - name: "Create a template from a reference device."
• Also they must start with a capital letter: - name: "export a template" => - name: Export a template
• Get rid of unnecessary things (i.e. visual noise) like required: False which is the default

3. (necessary) In addition to what @felixfontein already mentioned (very thorough review BTW), I'm concerned about the following:

• "all CI tests run against every pull request". Now there are 4 open PRs now and I see tests only in one of them.
• "all CI tests run regularly (nightly, or at least once per week)". Do they? If yes, could you point out where we can see it?

[Andersson007]

I corrected p.1 above, sanity tests didn't pass (in collections branch)

[rajeevarakkal]

@Andersson007 please find the update below

1. (necessary) I ran sanity tests against the collections, they didn't pass (in collections branch).

Running sanity test 'pylint' with Python 3.6
ERROR: Found 2 pylint issue(s) which need to be resolved:
ERROR: plugins/module_utils/idrac_redfish.py:202:12: no-else-break: Unnecessary "elif" after "break"
ERROR: plugins/modules/idrac_firmware.py:239:12: no-else-break: Unnecessary "elif" after "break"
See documentation for help: https://docs.ansible.com/ansible/devel/dev_guide/testing/sanity/pylint.html

This is fixed

1. I consider the following points not crucial, i.e. **optional**, but highly recommended to fix to make the collection's documentation consistent with Ansible documentation. If anybody disagree, please say.

You can use this [tool](https://github.com/Andersson007/ansible_reviewer) to quickly find the issues below.

For example,

examples: "- name" of #1 ends with a dot Taken care in most of the places
opt nvme_mode: line 1 does not start with a capital letter this is a deprecated option, it will start with (deprecated)
return msg: line 1 does not start with a capital letter couldn't find this out
opt device_service_tags: line 1 does not end with a dot couldn't find this out
abbreviations/latin phrases found 'e.g', use 'for example' instead couldn't find this out
examples: #10 without using "name" will revert you back soon

Also would be good:

* Remove unnecessary quotes and dots at the end of examples: - name: "Create a template from a reference device."  

taken care

• Also they must start with a capital letter: - name: "export a template" => - name: Export a template
taken care
• Get rid of unnecessary things (i.e. visual noise) like required: False which is the default -
  won't be doing it currently, will take it up later

1. (necessary) In addition to what @felixfontein already mentioned (very thorough review BTW), I'm concerned about the following:

• "all CI tests run against every pull request". Now there are 4 open PRs now and I see tests only in one of them.
  CI is enabled only for collections branch recently
• "all CI tests run regularly (nightly, or at least once per week)". Do they? If yes, could you point out where we can see it? `Yup currently schduled for once in a week, you can find result in Actions tab``

[rajeevarakkal]

Thanks @Andersson007 , we have used your tool and solved almost all issues reported including required: False

[rajeevarakkal]

@felixfontein - we are done with all the changes for the comments so far. Hope we are good with this :)

[rajeevarakkal]

Thanks @felixfontein , is there any more things we should look at & Are we good to include dellemc.openmanage collections?

[jillr]

We voted on the Code of Conduct today in the D&I working group. We found that the Contributor Covenant CoC v1.4 has changed over time, without having its version incremented: (https://github.com/ContributorCovenant/contributor_covenant/commits/release/content/version/1/4/code-of-conduct.md) so the text in the collection's CoC file is slightly different from the latest "version" of 1.4. We recommend that the collection SHOULD either update the text to match what is currently on https://www.contributor-covenant.org/version/1/4/code-of-conduct/ or replace the text with contact info plus a link to the CoC website but this is not a blocker.
The CoC is approved.

[rajeevarakkal]

We voted on the Code of Conduct today in the D&I working group. We found that the Contributor Covenant CoC v1.4 has changed over time, without having its version incremented: (https://github.com/ContributorCovenant/contributor_covenant/commits/release/content/version/1/4/code-of-conduct.md) so the text in the collection's CoC file is slightly different from the latest "version" of 1.4. We recommend that the collection SHOULD either update the text to match what is currently on https://www.contributor-covenant.org/version/1/4/code-of-conduct/ or replace the text with contact info plus a link to the CoC website but this is not a blocker.
The CoC is approved.

@jillr - updated the text

[rajeevarakkal]

@felixfontein , Hope we are good with respect to getting dellemc.openmanage collections in ansible 3.0.0 release. it will be great if we can get some update.

Thanks
Rajeev

[dmsimard]

Hi @rajeevarakkal and thanks for your interest.

Considering the extent of the review so far and the changes that were made, I am offerring an additional/fresh review.

The community has agreed on a list of criteria that must be met to move forward with the inclusion of a collection in the Ansible package. For this discussion, we will use a shortened checklist to verify and record compliance, but you can find the full details in the Collection Requirements documentation.

This is a new and gradual process, and we will edit this comment as we progress along. The items in the checklist can be in one of the following states:

• Items with a checkmark and no attached comment are OK and need no intervention from your side.
• Items with a checkmark that contain a comment will not block the inclusion process, but by following the advice given in the comment, the collection's quality can be improved.
• Items with no checkmark and no extra details are waiting for evaluation.
• Items with no checkmark that contain at least one comment need your attention. Issues in comments marked as required need to be fixed before collection can be included in Ansible. Comments marked as optional are just friendly advice and will not block the inclusion.

Feel free to ask any questions and provide feedback. Thanks in advance for your understanding.

Public availability and communication:

• published on Ansible Galaxy Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues

Standards and documentation:

• adheres to semantic versioning
  • @felixfontein already picked this up in his review so I won't re-iterate here
  • 2.1.5 is currently the latest release of the collection, it seems to be preparing 3.0.0 and appropriately calling out deprecation removals in the changelog
• follows licensing rules
  • GPLv3 is fine though I think worth mentioning: requires DCO for contributions
• follows the Ansible documentation standards and the style guide
• follows development conventions
• has README.md
• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections
• modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module, or plugin was added
• FQCNs is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

Collection management:

• galaxy.yml having tags field set
  • nitpick: in the galaxy.yml both authors and tags are lists but formatted differently.
• collection dependencies must have a lower bound on the version which is at least 1.0.0 and are all part of the ansible package
  • no dependencies
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
  • Out of curiosity, why Ansible >=2.10.3 specifically ?
• has changelog, preferably with changelogs/changelog.yaml

Tests:

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it does not contain error codes listed here
  • All ignore files are empty
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
  • No tests against Ansible 2.9
• all CI tests run against every pull request
  • Picked up by @Andersson007 already: there is CI for PRs opened against the collections branch but not the devel branch (example).
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
  • The release process for the collection isn't immediately clear to me at first glance -- there are tags from both the devel and the collections branch
• all CI tests run regularly (nightly, or at least once per week)

[rajeevarakkal]

Hi @dmsimard ,

Please find the update inline

Hi @rajeevarakkal and thanks for your interest.

Considering the extent of the review so far and the changes that were made, I am offerring an additional/fresh review.

The community has agreed on a list of criteria that must be met to move forward with the inclusion of a collection in the Ansible package. For this discussion, we will use a shortened checklist to verify and record compliance, but you can find the full details in the Collection Requirements documentation.

Standards and documentation:

• adheres to semantic versioning

  • @felixfontein already picked this up in his review so I won't re-iterate here
  • 2.1.5 is currently the latest release of the collection, it seems to be preparing 3.0.0 and appropriately calling out deprecation removals in the changelog

Yup, we are preparing for our 3.0.0 release, once these minor changes done, we will be publishing the content in Galaxy, and tag the release in GitHub. Planning to release by tomorrow once we are almost clear here.

• follows licensing rules

  • GPLv3 is fine though I think worth mentioning: requires DCO for contributions

We have copied the CODING_GUIDELINES, CONTRIBUTOR_AGREEMENT to the collections branch as well. README has appropriately calling out these files in the in the contributing section.

Collection management:

• meta/runtime.yml defines the minimal version of Ansible which the collection works with

  • Out of curiosity, why Ansible >=2.10.3 specifically ?

Yes, updated to >=2.10

Tests:

• All ignore files are empty

Removed these empty files

• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports

  • No tests against Ansible 2.9

Included Ansible 2.9 in Sanity matrix

• all CI tests run against every pull request

  • Picked up by @Andersson007 already: there is CI for PRs opened against the collections branch but not the devel branch (example).

• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released

From this 3.0.0 release (tomorrow) onwards, active development and contributions will be happening on our new default branch collections.

• The release process for the collection isn't immediately clear to me at first glance -- there are tags from both the devel and the collections branch

Post this 3.0.0 release (tomorrow), there won't be any active development happening in our devel branch. We have marked collections as our main/default branch now. Only customer reported bug fixes/security patches will be taken up in the devel branch in case they are critical. devel README.md and CODING_GUIDELINES files updated appropriately to communicate the same to customers. Also, collections updated to call out, it is the active development branch here on.

[abadger]

Just a note: the published deadline for new collections to be accepted was today, 27-01-2021, but we voted at today's community irc meeting to extend that to Friday:

• dellemc.openmanage has until Friday to be approved.
• If it doesn't make it, it is not a blocker for ansible-3.0 (it will have to target ansible-4, tentatively scheduled for May, instead of ansible-3)

[rajeevarakkal]

@dmsimard , is there anything pending that we have to take care?

[rajeevarakkal]

@dmsimard , @felixfontein , @abadger - We had our 3.0.0 release officially dellemc.openmanage in Galaxy now and tagged the GitHub release as well. With this are we good for approval?

[felixfontein]

Thanks for the 3.0.0 release, I'll try to take a look today, but maybe I only manage over the weekend.

[rajeevarakkal]

Thanks @felixfontein

[dmsimard]

@rajeevarakkal I will be looking at this today and will let you know if we're ready to move forward. Thanks for your patience !

[dmsimard]

@rajeevarakkal Can you confirm if the collection support Ansible 2.9 or not ? The runtime.yml file and README claim >=2.10 but you added an ansible-test sanity job against 2.9 and it looks okay: https://github.com/dell/dellemc-openmanage-ansible-modules/pull/232/checks?check_run_id=1776615837

My comment about the lack of 2.9 tests in the review was meant to highlight it for the sake of completeness, not that it was necessary to add them if 2.9 isn't meant to be supported by the collection.

[rajeevarakkal]

@rajeevarakkal Can you confirm if the collection support Ansible 2.9 or not ? The runtime.yml file and README claim >=2.10 but you added an ansible-test sanity job against 2.9 and it looks okay: https://github.com/dell/dellemc-openmanage-ansible-modules/pull/232/checks?check_run_id=1776615837

My comment about the lack of 2.9 tests in the review was meant to highlight it for the sake of completeness, not that it was necessary to add them if 2.9 isn't meant to be supported by the collection.

@dmsimard; our collections functionalities validated against Ansible 2.10 and above.

We have added a ansible-test sanity job against stable-2.9; to double check and ensure that collections are sanitized against 2.9 checks as well as it is good to have, also keeping in mind that this clean slate against 2.9 may be required if we plan to submit our collection for ansible certification in the future. Also we thought probably this help us to ignore non-applicable 2.9 sanity errors.

Your comments were in fact really helpful :)

Yes, our collection supports from 2.10 onwards.

[dmsimard]

I believe all items have been addressed and proposed a PR for inclusion in Ansible 3.0: ansible-community/ansible-build-data#58

[rajeevarakkal]

Thanks a lot @dmsimard , @felixfontein , @abadger @Andersson007. It was really systematic, smooth and nice working with you guys !!

[felixfontein]

I've taken a quick look, I think everything's good now!

[abadger]

Thanks for submitting this and all the hard work you've put in! We look forward to working with you into the future!

[rajeevarakkal]

Thanks a lot!! Like I mentioned above, it was really nice working with you!!