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

Ansible request for Common tasks & vars - make section names more sensible #1757

Open
1 of 4 tasks
sej-jackson opened this issue Dec 9, 2020 · 6 comments
Open
1 of 4 tasks
Assignees
Labels

Comments

@sej-jackson
Copy link
Contributor

  • Missing install
  • Bug in ansible playbook
  • Request for new playbook addition
  • Improvement

Details:
As discussed with @sxa in PR #1743 , some of the labels to the section lists could be misleading in future when further releases are available. Raising this as an issue to get some agreement on what the names ought to be.

In particular, he raised that in roles/Common/vars/Redhat.yml the label Additional_Build_Tools_NOT_RHEL8 will likely apply to future releases also, by which time the name will no longer be entirely accurate. The same can be said for some of the other sections, and the other distros also.

If the labels in the vars files are changed, then the corresponding tasks also need to be changed to match.

In addition, it would be helpful if each task had its own individual tag as well as the broader build_tools to help with re-running smaller parts of the playbook when debugging.

@sej-jackson
Copy link
Contributor Author

sej-jackson commented Dec 10, 2020

Going through the roles/Common/vars/*.yml files to find section names that might be misleading, if not now, then in the future perhaps. For example, if a section is named for a specific release only - is it still correct once the next release is available?

CentOS.yml:

  • Additional_Build_Tools_CentOS7
    • I'm including this one because it contains a package that is not in the list for CentOS8. Is libstdc++-static only required for CentOS7 and no other release?
  • Additional_Build_Tools_CentOS8
  • Additional_Build_Tools_NOT_CentOS8

Debian.yml:

  • Additional_Packages_Debian8

MacOSX.yml:

  • Build_Tool_Packages_NOT_10_12

RedHat.yml:

  • Additional_Build_Tools_NOT_RHEL8
  • Additional_Build_Tools_RHEL8
  • Additional_Build_Tools_RHEL7
  • Additional_Build_Tools_RHEL7_PPC64LE
  • Java_RHEL8
  • Java_NOT_RHEL6_PPC64
    • this one has a comment saying "Not RHEL8 either".... does this perhaps mean "RHEL7 only"?
  • Java_RHEL6_PPC64

SLES.yml:

  • Additional_Build_Tools_SLES15
  • Additional_Build_Tools_SLES12
  • Additional_Build_Tools_SLES11

Ubuntu.yml:

  • Additional_Packages_Ubuntu16:
  • Additional_Packages_Ubuntu18

OpenSUSE.yml:

  • Additional_Build_Tools_SUSE12

The other yml files don't appear to have anything confusing.

@sej-jackson
Copy link
Contributor Author

So, how best to name these sections so that they're still correct once a subsequent release is available?

Also we need to be careful to ensure that the limits coded in the corresponding task do actually match the meaning.

I'm inclined to suggest that if a section genuinely applies to just one specific release (and not the older or newer ones) then it should be named: <description>_<release>_ONLY (so Additional_Build_Tools_CentOS7 would become Additional_Build_Tools_CentOS7_ONLY, if that's really what it means), and similarly <description>_NOT_<release> if that's the only release that should be excluded. This is likely to be the exception I think.

How best to name (and code) something that comes into play at a certain release?
For example, suppose something changes at RHEL8:

If the task applies to RHEL6, & 7 but not later - maybe call it <description>_BELOW_RHEL8, and code as:

ansible_distribution_major_version < "8"

While a task for RHEL8 (and subsequent releases) might be <description>_RHEL8_ONWARD, and code as:

ansible_distribution_major_version > "7", or
ansible_distribution_major_version >= "8"

Thoughts? Preferences? Alternatives?

@Willsparker
Copy link
Contributor

I like the idea of <description_BELOW_<release> & <description>_<release>_ONWARDS , however my concern is that the order becomes a bit jumbled ... Would it not be easier to have something like <description>_>=RHEL8 or <description>_<RHEL8 ...?

@sxa
Copy link
Member

sxa commented Dec 10, 2020

I'm definitely not going to advocate trying <= in the names of variables :-)
However perhaps <description>_LT8 (less than 8) or just <description>_before_rhel8 or description_after_rhel7 would work. the LT8 versions would likely be ok in this scenario because the distribution is usually implicit in those files due to them being named after the distribution. If you wanted to explicitly match the conditional statements you could use GE8 instead of GT7.

The other option would be to just not use the vars file and put those small amount of packages directly in the tasks file alongside the conditional, but that's not as clean and makes the "where do I add this?" question worse.

We also need to go back and figure out what the Additional... ones are for - perhaps things that were for building other dependencies (i.e. things that aren't the JDK?)

@Willsparker
Copy link
Contributor

+1 on the GT or GE or LT (or presumably, LE).

@sej-jackson
Copy link
Contributor Author

I'm definitely not going to advocate trying <= in the names of variables :-)

Goodness no - me neither!

I was looking for something shorter than (but equivalent to) "less than", "greater than", etc. and having trouble making them consistent. "GT", "GE", "LT", "LE" plus the release number would work perfectly - dunno why I didn't think of that.

Would you want to go the whole hog and use "EQ" and "NE" as well, or are "ONLY" and "NOT" still acceptable?

Some of the tasks files do already have tasks for odd packages here and there... personally, I quite like the variables lists as being easier to see the wood from the trees.

All of those sections do need further examination.... why are they split out, what are they required for, and do they only apply to the release they state or future ones too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants