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.

Sign up for GitHub

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

Jump to bottom

JeOS: Add Update tests for maintenance 15-SP3 #12657

Merged
merged 1 commit into from
Jun 7, 2021
Merged

JeOS: Add Update tests for maintenance 15-SP3 #12657

merged 1 commit into from
Jun 7, 2021

Conversation

jlausuch
Copy link
Contributor

@jlausuch jlausuch commented Jun 4, 2021

@jlausuch jlausuch added the WIP Work in progress label Jun 4, 2021
@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 4, 2021 •
edited
Loading

Discussion:
In SLE images, we install updates during DVD installation phase, and then we patch the system later on.

Since JeOS doesn't have installation per se, I wonder if I should add zypper up instead of zypper patch in patch_and_reboot.pm ...

Difference in JeOS between patch and up:

zypper -n patch --with-interactive -l
...
The following 4 NEW patches are going to be installed:
  SUSE-SLE-Module-Basesystem-15-SP3-2021-19625 SUSE-SLE-Module-Basesystem-15-SP3-2021-19739 SUSE-SLE-Module-Basesystem-15-SP3-2021-19809 SUSE-SLE-Module-Server-Applications-15-SP3-2021-19464

The following 8 packages are going to be upgraded:
  libpcre1 libxml2-2 libxml2-tools openssh openssh-clients openssh-common openssh-server qemu-guest-agent
 ...

zypper -n up
...
2 libxml2-tools openssh openssh-clients openssh-common openssh-server python3-distro python3-salt qemu-guest-agent salt salt-minion

12 packages to upgrade
...

So, I guess we need to go with zypper up for JeOS case..

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 4, 2021

This is a local VR but I'd like to run it on OSD once we have enabled JeOS in the openQA bot.

@jlausuch jlausuch removed the WIP Work in progress label Jun 4, 2021
@mloviska
Copy link
Contributor

mloviska commented Jun 4, 2021

Discussion:
In SLE images, we install updates during DVD installation phase, and then we patch the system later on.

Since JeOS doesn't have installation per se, I wonder if I should add zypper up instead of zypper patch in patch_and_reboot.pm ...

Difference in JeOS between patch and up:

zypper -n patch --with-interactive -l
...
The following 4 NEW patches are going to be installed:
  SUSE-SLE-Module-Basesystem-15-SP3-2021-19625 SUSE-SLE-Module-Basesystem-15-SP3-2021-19739 SUSE-SLE-Module-Basesystem-15-SP3-2021-19809 SUSE-SLE-Module-Server-Applications-15-SP3-2021-19464

The following 8 packages are going to be upgraded:
  libpcre1 libxml2-2 libxml2-tools openssh openssh-clients openssh-common openssh-server qemu-guest-agent
 ...

zypper -n up
...
2 libxml2-tools openssh openssh-clients openssh-common openssh-server python3-distro python3-salt qemu-guest-agent salt salt-minion

12 packages to upgrade
...

So, I guess we need to go with zypper up for JeOS case..

Yup, this seems reasonable, just wondering whether we should not at least list patches for further debug/bug report purposes.

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 4, 2021

Yup, this seems reasonable, just wondering whether we should not at least list patches for further debug/bug report purposes.

http://fromm.arch.suse.de/tests/1329#step/patch_and_reboot/167

grisu48
grisu48 reviewed Jun 7, 2021
View reviewed changes
tests/qa_automation/patch_and_reboot.pm Outdated
record_info('Updates', script_output('zypper lu'));
zypper_call('up', timeout => 300);
}

fully_patch_system;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need fully_patch_system if we are running a zypper up before? Perhaps putting this into a else block makes sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it intact "just in case" but after looking at the tests, this step really does nothing, so better to skip it.
http://fromm.arch.suse.de/tests/1314#step/patch_and_reboot/167

grisu48
grisu48 approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@grisu48 grisu48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment only, otherwise LGTM 🚀

mloviska
mloviska approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@mloviska mloviska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very nice, thanks!

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 7, 2021

b10n1k
b10n1k reviewed Jun 7, 2021
View reviewed changes
tests/qa_automation/patch_and_reboot.pm Outdated
Comment on lines 45 to 46
# JeOS is a bootable image and doesn't have installation where we can install
# install updates as for SLE DVD installation, so we need to update manually.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# JeOS is a bootable image and doesn't have installation where we can install
# install updates as for SLE DVD installation, so we need to update manually.
# JeOS is a bootable image and doesn't have installation where we can install
# updates as for SLE DVD installation, so we need to update manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

b10n1k
b10n1k reviewed Jun 7, 2021
View reviewed changes
tests/qa_automation/patch_and_reboot.pm Outdated

assert_script_run('rpm -ql --changelog kernel-default >/tmp/kernel_changelog.log');
my $suffix = is_jeos ? '-base' : '';
assert_script_run("rpm -ql --changelog kernel-default$suffix >/tmp/kernel_changelog.log");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking: add a space after redirection '>'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

b10n1k
b10n1k requested changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two small comments from my side

b10n1k
b10n1k approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@jlausuch jlausuch merged commit f47ec82 into os-autoinst:master Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@b10n1k b10n1k b10n1k approved these changes

@pdostal pdostal pdostal approved these changes

@grisu48 grisu48 grisu48 approved these changes

@mloviska mloviska mloviska approved these changes

@dzedro dzedro Awaiting requested review from dzedro

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jlausuch @mloviska @b10n1k @pdostal @grisu48