On Tue, Nov 26, 2019 at 2:21 AM Jim Fehlig <jfehlig(a)suse.com> wrote:
On 11/19/19 6:52 AM, Fabiano FidĂȘncio wrote:
> On Tue, Nov 19, 2019 at 11:33 AM Andrea Bolognani <abologna(a)redhat.com>
wrote:
>>
>> On Tue, 2019-11-19 at 00:21 +0000, Jim Fehlig wrote:
>>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
>>> ---
>>> guests/configs/autoinst.xml | 86 +++++++++++++++++++
>>> .../libvirt-opensuse-15.1/docker.yml | 2 +
>>> .../libvirt-opensuse-15.1/install.yml | 2 +
>>> .../host_vars/libvirt-opensuse-15.1/main.yml | 22 +++++
>>> guests/inventory | 1 +
>>> guests/lcitool | 2 +
>>> guests/vars/mappings.yml | 38 +++++++-
>>> 7 files changed, 151 insertions(+), 2 deletions(-)
>>
>> First of all, thank you for following through with your promise of
>> looking into this! I'm looking forward to being able to merge your
>> changes and finally have proper openSUSE support in our CI :)
>>
>> [...]
>>> +++ b/guests/configs/autoinst.xml
>>> + <partitioning config:type="list">
>>> + <drive>
>>> + <device>/dev/vda</device>
>>> + <use>all</use>
>>> + <partitions config:type="list">
>>> + <partition>
>>> + <filesystem
config:type="symbol">swap</filesystem>
>>> + <size>500M</size>
>>> + <mount>swap</mount>
>>
>> We give other guests only 256 MiB of swap, so do the same here to
>> be consistent. Building libvirt and friends is CPU-bound rather than
>> memory-bound anyway, so more than that leeway is not necessary.
>>
>> [...]
>>> + <add-on>
>>> + <add_on_products config:type="list">
>>> + <listentry>
>>> +
<
media_url>http://download.opensuse.org/distribution/leap/15.1/repo/oss...
>>> + <name>repo-oss</name>
>>> + </listentry>
>>> + <listentry>
>>> +
<
media_url>http://download.opensuse.org/update/leap/15.1/oss</media_...
>>> + <name>repo-update</name>
>>> + </listentry>
>>> + <listentry>
>>> +
<
media_url>http://download.opensuse.org/distribution/leap/15.1/repo/non...
>>> + <name>repo-non-oss</name>
>>> + </listentry>
>>> + <listentry>
>>> +
<
media_url>http://download.opensuse.org/update/leap/15.1/non-oss/</m...
>>> + <name>repo-update-non-oss</name>
>>> + </listentry>
>>
>> Do we actually need the non-OSS repositories to be updated? I would
>> hope not! But I'm not familiar with how openSUSE organizes its
>> repositories, so I'm going by name only :)
>>
>> [...]
>>> + <firewall>
>>> + <enable_firewall>true</enable_firewall>
>>> + </firewall>
>>
>> As you mention somewhere else, we probably don't need this.
>>
>> [...]
>>> +++ b/guests/host_vars/libvirt-opensuse-15.1/main.yml
>>> +package_format: 'rpm'
>>> +package_manager: 'zypper'
>>> +os_name: 'openSUSE'
>>> +os_version: '15.1'
>>
>> So, about the naming.
>>
>> What I would have done here is
>>
>> os_name: 'OpenSUSE'
>> os_version: '15'
>>
>> The intial capital letter in os_name goes against the actual branding
>> for openSUSE so I'm not perfectly happy with it, but on the other
>> hand it's very useful when defining mappings because package formats
>> all start with a lowercase letter and all OS names start with an
>> uppercase letter. So I would try to stick with that convention.
>>
>> As for os_version, if you look at all existing entries we use the
>> major version number only: eg. we have CentOS7 instead of CentOS7.7
>> and FreeBSD12 instead of FreeBSD12.1: this makes sense because, as
>> the guest gets updated over time, it will naturally pick up the
>> latest minor release. Will this work for openSUSE too?
>>
>> (Ubuntu is a slight exception in that the major version itself
>> contains a dot, so we just shortened 18.04 to 18 because we know
>> that there's never going to be two LTS releases per year.)
>>
>>> +++ b/guests/host_vars/libvirt-opensuse-15.1/docker.yml
>>> @@ -0,0 +1,2 @@
>>> +---
>>> +docker_base: openSUSE:15.1
>>
>> I believe these images are now deprecated, and opensuse/leap
>> should be used instead.
>>
>> Looking at
>>
>>
https://hub.docker.com/r/opensuse/leap/tags
>>
>> I see that the '15.1', '15' and 'latest' tags point to
the same set
>> of digests, so that seems to confirm that we can use just 15 as the
>> version number and have
>>
>> docker_base: opensuse/leap:15
>>
>> in this file.
>>
>> [...]
>>> +++ b/guests/inventory
>>> @@ -8,5 +8,6 @@ libvirt-fedora-rawhide
>>> libvirt-freebsd-11
>>> libvirt-freebsd-12
>>> libvirt-freebsd-current
>>> +libvirt-opensuse-15.1
>>
>> Based on the points above, I think this could and should be
>>
>> libvirt-opensuse-15
>>
>> [...]
>>> @@ -127,6 +129,7 @@ mappings:
>>> dbus-daemon:
>>> default: dbus
>>> Fedora: dbus-daemon
>>> + openSUSE: dbus-1
>>
>> You see how weird this looks, due to the first letter being
>> lower case? :)
>>
>> I'm not going to review the mappings in detail right now because I
>> simply lack the time. Once 'lcitool update' works for you without
>> errors, I'll look into it.
>>
>>
>> Fabiano already pointed out where you need to look to sort out the
>> issues you've been experiencing, so I'll leave you to it now :)
>
> Apart from what I pointed out, I've also faced a few issues when
> trying out your patch:
> - grub stays in a loop, forever;
> - I've worked this around by not setting "--graphics none --console
> pty" and removing "console=ttyS0" from the extra_arg line;
> - It really has to be solved in a better way ;-)
I've fixed this in V2 [0] via grub configuration in the autoyast file. I've also
updated the autoyast file to work with any current version of openSUSE.
Cool. I'll give it a try later Today!
> - Update installed packages:
> - I've added:
> ```
> +- name: Update installed packages
> + command: '{{ package_manager }} update -y'
> + args:
> + warn: no
> + when:
> + - os_name == 'openSUSE'
> ```
>
> - Clean up packages after update:
> - I've added:
> ```
> - name: Clean up packages after update
> shell: '{{ package_manager }} clean packages -y && {{
> package_manager }} autoremove -y'
> args:
> warn: no
> when:
> - package_format == 'rpm'
> + - not os_name == "openSUSE"
> +
> +- name: Clean up packages after update
> + shell: '{{ package_manager }} clean -y'
> + args:
> + warn: no
> + when:
> + - os_name = "openSUSE"
> +
> ```
I've added these with minor tweaks.
> - Missing mappings:
> - qemu-img:
> - Seems that the package doesn't exist in openSUSE. So, please,
> just do something like:
> ```
> qemu-img:
> default: qemu-utils
> rpm: qemu-img
> + openSUSE:
> ```
> - zfs:
> - Same as above
> - yajl:
> - It seems to be called libyajl-devel for openSUSE
Mappings are fixed.
> - Look for files
> - There's a "Look for files" rule (in
> guests/playbooks/update/tasks/paths.yml) which will just bail out as
> /usr/local/etc doesn't exist in openSUSE. It has to be tweaked as
> well;
It is not clear to me what can be done in the yml file. How far can I bend the
rules? :-) E.g. would something like the following work?
shell: 'find /usr/local/etc -name {{ item }} 2>/dev/null || find /etc -name {{
item }} 2>/dev/null'
I do believe that's the way to go.
> - Configure ccache:
> - This is a rule in guests/playbooks/update/tasks/users.yml, which
> takes into consideration that when the test user is created, we'll
> have a test group created as well, which doesn't happen on openSUSE.
> So, it also need some tweak;
I can add the test group via the autoyast file. Does the group require any
specific properties? gid? System group?
AFAIU we just need the group to be there. So, no, no specific properties.