-
Notifications
You must be signed in to change notification settings - Fork 43
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
SNO generate discovery iso should use the boot_iso host var #489
SNO generate discovery iso should use the boot_iso host var #489
Conversation
|
||
- name: Symlink discovery ISO into iso directory | ||
ansible.builtin.file: | ||
src: ../{{ item }}.iso | ||
dest: "{{ http_store_path }}/data/iso/{{ item }}.iso" | ||
dest: "{{ http_store_path }}/data/iso/{{ hostvars[item]['boot_iso'] }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. Yeah, boot-iso
seems to assume this 'boot_iso'
path, however generate_discovery_iso
has dest: "{{ http_store_path }}/data/iso/discovery.iso"
, which I think means that if the inventory ever has boot_iso
set to anything different, nothing's going to work anyway. Is this correct? If so, why even have it?
In fact, the inventory-bm.j2
, inventory-bm.j2
, and inventory-ibmcloud-bm.j2
templates hardcode discovery.iso
while inventory-sno.j2
and inventory-ibmcloud-sno.j2
don't define boot_iso
at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SNO's need a specific iso file generated for each SNO, where as a MNO/BM clusters uses the same ISO file for every node. So you can think of the iso files as specific to the cluster. The SNO inventory files do define boot_iso, it's just a host var instead of a host group var applied to all hosts in a group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Yeah, I see it now: I missed it in that enormous line in the SNO template. 😆
But the boot_iso
settings in the non-SNO cases still seem like "lies" as generate-discovery-iso
will always create discovery.iso
but if a BYOL inventory sets boot_iso
to something else, the boot-iso
role will try to use it and won't find it. Should this be an SNO-only setting, or should the generate roles be fixed to use the var for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, we should fix the generate roles so they use the vars for consistency, though I would really appreciate folks not mucking with generated inventory vars without researching what that does. But users will always be users...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, mucking with generated inventory files should come with a red blinking danger warning. I think BYOL is the bigger concern here, because we either should use boot_iso
consistently or document clearly that it must be the "mandatory default" 😆
|
||
- name: Symlink discovery ISO into iso directory | ||
ansible.builtin.file: | ||
src: ../{{ item }}.iso | ||
dest: "{{ http_store_path }}/data/iso/{{ item }}.iso" | ||
dest: "{{ http_store_path }}/data/iso/{{ hostvars[item]['boot_iso'] }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Yeah, I see it now: I missed it in that enormous line in the SNO template. 😆
But the boot_iso
settings in the non-SNO cases still seem like "lies" as generate-discovery-iso
will always create discovery.iso
but if a BYOL inventory sets boot_iso
to something else, the boot-iso
role will try to use it and won't find it. Should this be an SNO-only setting, or should the generate roles be fixed to use the var for consistency?
585dfd4
to
1ccfa5b
Compare
@Noreen21 or @dbutenhof Any chance you can test this with SNOs in your environment so I can merge this? |
No description provided.