Install packages that should be updated #4986
Conversation
|
It looks good to me! Everything's understandable and the output is easy to access and read from the openqa webui 👍 |
data/caasp/update.sh
Outdated
| [-r] Just reboot cluster test" 1>&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| while getopts "s:cr" opt; do | ||
| while getopts ":s:c:u:r:i:" opt; do |
There was a problem hiding this comment.
: marks options that expect an argument. (When you want getopts to expect an argument for an option, just place a : (colon) after the proper option flag). Please fix or point me to documentation saying otherwise.
There was a problem hiding this comment.
You are right. I misunderstood the documentation.
| @@ -34,6 +42,13 @@ case $opt in | |||
| esac | |||
| done | |||
|
|
|||
| # Make sure that Salt master container is running | |||
There was a problem hiding this comment.
This is already handled by set -exuo pipefail and script would fail on next line.
But if you want it, keep it - it might prevent some issues in future.
There was a problem hiding this comment.
I will prefer keeping it because it failed in one of my local tests.
| $runner "zypper lr UPDATE | grep Enabled" | ||
|
|
||
| # Reboot the cluster | ||
| where="-P roles:(admin|kube-(master|minion))" |
There was a problem hiding this comment.
I am not sure how salt handles cases when you reboot salt-master together with all minions.
Can it happen that admin reboots before (master|minion) and reboot command won't be processed on all nodes?
There was a problem hiding this comment.
Seems to work fine in my local testing.
tests/caasp/stack_update.pm
Outdated
| switch_to 'velum'; | ||
| } | ||
|
|
||
| # Check that update changed system as expected | ||
| sub check_update_changes { | ||
| my $admin = 'admin.openqa.test'; |
There was a problem hiding this comment.
You can define my $admin for whole module (~line 25) - but it's fine if you prefer this way
tests/caasp/stack_update.pm
Outdated
|
|
||
| # Install packages that should be updated - poo#114205 | ||
| if (is_caasp('qam')) { | ||
| assert_script_run "ssh $admin './update.sh -i $repo' | tee /dev/$serialdev | grep EXIT_OK", 600; |
There was a problem hiding this comment.
$repo parameter is not used by update.sh script. The same applies to -u $repo below, it should be just -u
|
Please fix parameter parsing (:s:c:u:r:i:) and double check if "$srun $where system.reboot" works - otherwise it looks good (you can ignore other comments) |
drpaneas
force-pushed
the
prepare_to_update
branch
2 times, most recently
from
4141e10
to
ae28919
Compare
Some maintenance incidents contain packages which are not pre-installed in MicroOS. In order to update them, we first have to install them.
drpaneas
force-pushed
the
prepare_to_update
branch
from
ae28919
to
dd4807c
Compare
|
Last verification run with the changes: http://skyrim.qam.suse.de/tests/2926# All seems to be fine. Let's merge it. |
Some maintenance incidents contain packages which are not
pre-installed in MicroOS. In order to update them, we first
have to install them.
QAM (with those packages): http://skyrim.qam.suse.de/tests/2854#step/stack_update/43
QAM (without those packages): http://skyrim.qam.suse.de/tests/2870#step/stack_update/37
QA: 3.0 does not have update repo yet, let's merge and see later