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

Record arch #3520

Merged
merged 6 commits into from
Apr 1, 2023
Merged

Record arch #3520

merged 6 commits into from
Apr 1, 2023

Conversation

jvonau
Copy link
Contributor

@jvonau jvonau commented Mar 31, 2023

Avoid expanding scripts/local_facts.fact which should shrink in coming years, rather than expanding — to reduce the number of global variables (in favor of modular-not-monolithic code, whenever possible).

Should meet the above requirement, just record the value so it can be easily displayed in the admin-console

@holta
Copy link
Member

holta commented Mar 31, 2023

Both dpkg --print-architecture and dpkg --print-foreign-architectures are already recorded in iiab-summary and iiab-diagnostics output.

Are they also needing to be recorded during IIAB's initial Ansible install, is that the idea?

If so, that seems reasonable.

(0-init/tasks/create_iiab_ini.yml might then spell that out clarifying a bit in /etc/iiab/iiab.ini ...maybe something like option: original_dpkg_arch or option: initial_dpkg_arch or some such?)

@holta holta added this to the 8.1 milestone Mar 31, 2023
@holta
Copy link
Member

holta commented Mar 31, 2023

(0-init/tasks/create_iiab_ini.yml might then spell that out clarifying a bit in /etc/iiab/iiab.ini ...maybe something like option: original_dpkg_arch or option: initial_dpkg_arch or some such?)

Or maybe the section: version and section: location sections within /etc/iiab/iiab.ini are both sufficiently static already (implying these 2 sections created by create_iiab_ini.yml are not supposed to ever change ?)

@jvonau jvonau changed the title Rec arch Record arch Apr 1, 2023
@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

(0-init/tasks/create_iiab_ini.yml might then spell that out clarifying a bit in /etc/iiab/iiab.ini ...maybe something like option: original_dpkg_arch or option: initial_dpkg_arch or some such?)

Or maybe the section: version and section: location sections within /etc/iiab/iiab.ini are both sufficiently static already (implying these 2 sections created by create_iiab_ini.yml are not supposed to ever change ?)

You should know the purpose and reasons for the layout that was 'borrowed' from xsce but to spell it out for you the reason is to capture the install time variables that were in play at install time. Remember 'runtime' is updated every time ansible runs, it looks like a duplication but as you now realize create_iiab_ini.yml only runs once. Using 'original' or 'initial' really doesn't gain anything other that being a nit picker, should never change without willful alteration and that is the purpose of recording the value. (Thinking towards apt repo additions that are not correct for the arch in use and how that came into being) Now with this 'Avoid expanding scripts/local_facts.fact', all #3518 was doing was the same procedure that was used with #3439 as I felt this variable would of come in handy for other roles that might need attention going forward. What I find amusing is this 'to reduce the number of global variables' cope out, default_vars.yml and local_vars.yml are full of 'global variables'.

@holta
Copy link
Member

holta commented Apr 1, 2023

Sure.

But those 2 section titles should really be...something like section: initial-location and section: initial-version if we want other people to understand our work.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

Now if you want to add --print-foreign-architectures also that would be cool too. While partially a duplication of iiab-summary but that is a post-install tool that would not capture the 'out of the box' pre-iiab environment that might be altered in some unknown way. Just recording the known starting point, just like any other values that are recorded in the same stanza that should not change without willful intervention.

Sure.

But those 2 section titles should really be...something like section: initial-location and section: initial-version if we want other people to understand our work.

Just a header change feel free.

@holta
Copy link
Member

holta commented Apr 1, 2023

Now if you want to add --print-foreign-architectures also that would be cool too. While partially a duplication of iiab-summary but that is a post-install tool that would not capture the 'out of the box'

Ok

@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

'ansible_distribution' might be better with 'os_ver' ?

@holta
Copy link
Member

holta commented Apr 1, 2023

'ansible_distribution' might be better with 'os_ver' ?

FWIW after Ansible 2.6+ they ask us not to use ansible_distribution — for whatever reason, one is supposed to use ansible_facts['distribution'] instead:

https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_conditionals.html#ansible-facts-distribution

@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

os_ver matches the form of other entries in create_iiab_ini.yml, this is internal information, should match the detection in use and helps explain OS.yml usage.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

Grouping: Move to be under dpkg_foreign_arch?

@jvonau
Copy link
Contributor Author

jvonau commented Apr 1, 2023

Looks fine to me, run with it.

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

Successfully merging this pull request may close these issues.

None yet

2 participants