Harmonize updating handling SUSE/openSUSE #5175

jknphy · 2018-06-06T13:54:49Z

Harmonize updating handling SUSE/openSUSE:

Related ticket: https://progress.opensuse.org/issues/31954

okurz · 2018-06-08T08:58:40Z

products/sle/main.pm

+        loadtest "update/zypper_up";
+    }
+}
+


I am not sure why you want to have this method in the sle-main but still check for is_sle which should always be set in here. Did you copy-paste this from opensuse/main.pm? Then it should be rather moved to lib/main_common.pm . The idea was to not introduce duplicate but to reduce

This is what was extracted from subs load_consoletests and load_x11tests. I will try to move it to a common place.

okurz · 2018-06-08T09:00:27Z

products/sle/main.pm

@@ -919,6 +928,7 @@ elsif (get_var("QAM_MINIMAL")) {
    loadtest "qam-minimal/check_logs";
    if (check_var("QAM_MINIMAL", 'full')) {
        loadtest "qam-minimal/install_patterns";
+        load_system_update_tests();


why do you want to schedule this additionally now?

okurz · 2018-06-08T09:06:46Z

lib/main_common.pm

@@ -1155,8 +1154,6 @@ sub load_x11tests {
    loadtest "x11/xterm";
    loadtest "x11/sshxterm" unless get_var("LIVETEST");
    if (gnomestep_is_applicable()) {
-        # openSUSE has an explicit update check elsewhere
-        loadtest "update/updates_packagekit_gpk" if is_sle && !is_staging;


yes, it's good if you want to remove it from here but I think you are doing it the wrong way around. Do not define a SLE specific function elsewhere but find out why SLE calls it here and openSUSE does not. And then try to find a way which is common for both openSUSE and SLE

jknphy · 2018-06-08T12:23:16Z

What I tried now is to call the update subroutine, one call from load_x11tests and another one from load_consoletests and I moved the required code to main_common to only have one subroutine load_system_update_tests that try to work with both products.

jknphy · 2018-06-08T12:49:57Z

VRs: gnome and minimal_x

okurz

Your changes seem to become more clear now so I think we are moving in the right direction.

For the difference we still have between SLE and openSUSE I assume there could be mainly two reasons:

Simply historical reasons, the both main.pm files deviated because we had the duplication and there is no better reason so we can just select one of both approaches and align the other to it
Actual different product behaviour which we need to preserve but properly describe in comments why it must differ

okurz · 2018-06-09T05:56:34Z

lib/main_common.pm

+            loadtest "update/zypper_up";
+        }
+    }
+}


ok, this looks like it's going in the right direction but you still have the whole update function still in products/opensuse/main.pm which I think you can remove and fully replace by this one.

okurz · 2018-06-09T05:57:38Z

products/opensuse/main.pm

@@ -480,7 +467,6 @@ else {
    {
        load_fixup_network();
        load_fixup_firewall();
-        load_system_update_tests();


can we actually remove the call here? Why should this be ok?

jknphy · 2018-06-11T07:05:43Z

Using an internal variable to not schedule updates tests in openSUSE three times in the same tests (1 for the removed call that I added back again, 1 for the console tests and 1 for the x11 tests) and still keep the same behavior.
VRs:

okurz · 2018-06-12T09:31:48Z

lib/main_common.pm

+        }
+    }
+    set_var('SYSTEM_UPDATED', 1);
+}


Already a big step forward. Now we should make sure that the part loadtest "update/updates_packagekit_gpk"; as well as loadtest "update/zypper_up"; only appears once.

ok, could you please still try to extract the duplicate call of the two mentioned modules?

I missed that comment, now that part is refactored.

okurz · 2018-06-12T09:32:45Z

lib/main_common.pm

+        }
+        if (get_var('ZYPPER_ADD_REPOS')) {
+            loadtest "console/zypper_add_repos";
+        }


@rwx788 do you think we can just extract the above two loadtest outside of the is_sle/opensuse branch? I think both should be applicable just based on the variables, e.g. ZYPPER_ADD_REPOS, and not need to be additionally depending on is_sle/opensuse

This part was there long time ago, so I haven't touched it. As of now it's used only for openSUSE. Of course we can move control to variables instead of using variables we already have. But it won't help much as main.pms are spaghetti code which contains complex logic which cannot be always generalized.

and this is why we are improving the code here. I was asking to simplify by just moving that part out of the is_sle-branch, nothing else, no new variables, no change in variables.

jknphy · 2018-06-18T05:14:40Z

Moved outside those two subroutines for clearing and addding repos.

rwx788

Can we get schedule for 2 runs before changes and after to reduce risks of scheduling something differently? You can use _EXIT_AFTER_SCHEDULE test suite variable to just output scheduled test modules and exit. But looks good already.

jknphy · 2018-06-22T08:30:03Z

http://dhcp254.suse.cz/tests/1740
http://dhcp254.suse.cz/tests/1741

jknphy · 2018-06-25T07:10:45Z

Now looks similar comparing among different schedules:

opensuse-Staging:J-Staging2-DVD-x86_64-Build1945.2-gnome@64bit: O3 vs local
opensuse-Staging:J-Staging2-DVD-x86_64-Build1945.2-textmode@64bit: O3 vs local
sle-15-Installer-DVD-x86_64-Build668.1-textmode@64bit: OSD vs local
sle-15-Installer-DVD-x86_64-Build668.1-gnome@64bit: OSD vs local

rwx788

LGTM. Provided schedules are identical, so if @okurz you don't have other points, let's merge.

okurz · 2018-06-27T20:54:37Z

Yes, the PR looks very nice now. We should keep the ticket open though to answer why we have the remaining differences, e.g. why we skip some tests on SLE-staging but not openSUSE-staging.

okurz · 2018-06-27T20:55:50Z

@andreasstieger @lnussel @DimStar77 @nilxam @mimi1vx FYI this PR might have some influence on the test schedule and update handling even though it is intended to not cause any problems

DimStar77 · 2018-06-28T07:24:49Z

FYI this PR might have some influence on the test schedule and update handling even though it is intended to not cause any problems

Thanks for the heads-up

andreasstieger · 2018-06-28T09:20:06Z

@czerw for openSUSE QAM

czerw · 2018-06-28T10:04:36Z

Was this change verified on SLE12 ? Maybe it is just coincidence but today started to fail
updates_packagekit_gpk https://openqa.suse.de/tests/1790992#step/updates_packagekit_gpk/18.
What I see that order of the test is little bit different when comparing with last good result https://openqa.suse.de/tests/1789711. (progress is down at the moment, so there is no poo yet).

czerw · 2018-06-28T10:39:38Z

Did verification with code before the change and it works http://10.100.12.105/tests/2391. @okurz could you please check it?

rwx788 · 2018-06-28T10:56:24Z

@czerw, that's weird as schedules are identical for runs before the change and after, and there is no functional change. Have you reverted just this commit for your local run? Looks like test/product issue as window you get http://10.100.12.105/tests/2391#step/updates_packagekit_gpk/13 doesn't show up on osd.

czerw · 2018-06-28T11:30:45Z

@rwx788 , my local run was done without this PR, i just cloned failing job from osd and it works. So it is not product issue.
Schedules are not identical before and after:

before https://openqa.suse.de/tests/1789711
updates_packagekit_gpk is scheduled in x11 section
after https://openqa.suse.de/tests/1790992
updates_packagekit_gpk is scheduled in console section

Same behavior is also for SLE15 https://openqa.suse.de/tests/1791027, although it was failing even before, but for other reason.

czerw · 2018-06-28T11:33:43Z

@rwx788 probably for the reason that in consoletest_setup is packagekit stopped and is not running and is started in consoletest_finish

rwx788 · 2018-06-28T11:36:33Z

@czerw right, missed where test suit is scheduled. I'll revert this PR then, because @jknphy is off today. Thanks!

czerw · 2018-06-28T11:56:35Z

@rwx788 Thank you!

jknphy force-pushed the harmonize_updating_handling branch 8 times, most recently from e715250 to 165b904 Compare June 8, 2018 05:18

okurz reviewed Jun 8, 2018

View reviewed changes

jknphy force-pushed the harmonize_updating_handling branch 5 times, most recently from 20f157e to d736cf2 Compare June 8, 2018 12:17

jknphy force-pushed the harmonize_updating_handling branch from d736cf2 to 178fcb8 Compare June 8, 2018 12:44

okurz reviewed Jun 9, 2018

View reviewed changes

jknphy force-pushed the harmonize_updating_handling branch 2 times, most recently from f2bd6fc to 7b43a76 Compare June 11, 2018 07:01

jknphy force-pushed the harmonize_updating_handling branch from 7b43a76 to 0ce28f4 Compare June 11, 2018 07:08

okurz reviewed Jun 12, 2018

View reviewed changes

jknphy force-pushed the harmonize_updating_handling branch from 0ce28f4 to 4562709 Compare June 13, 2018 11:49

jknphy changed the title ~~[WIP] Harmonize updating handling SUSE/openSUSE~~ Harmonize updating handling SUSE/openSUSE Jun 18, 2018

rwx788 reviewed Jun 20, 2018

View reviewed changes

jknphy force-pushed the harmonize_updating_handling branch from 4562709 to 860bb64 Compare June 25, 2018 06:58

Harmonize updating handling SUSE/openSUSE

758ac03

jknphy force-pushed the harmonize_updating_handling branch from 860bb64 to 758ac03 Compare June 25, 2018 07:12

rwx788 approved these changes Jun 27, 2018

View reviewed changes

okurz merged commit 5742ab6 into os-autoinst:master Jun 27, 2018

rwx788 mentioned this pull request Jun 28, 2018

Revert "Harmonize updating handling SUSE/openSUSE" #5308

Merged

jknphy mentioned this pull request Jul 4, 2018

Fix harmonization update #5351

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harmonize updating handling SUSE/openSUSE #5175

Harmonize updating handling SUSE/openSUSE #5175

jknphy commented Jun 6, 2018 •

edited

Loading

okurz Jun 8, 2018

jknphy Jun 8, 2018

okurz Jun 8, 2018

okurz Jun 8, 2018

jknphy commented Jun 8, 2018 •

edited

Loading

jknphy commented Jun 8, 2018 •

edited

Loading

okurz left a comment

okurz Jun 9, 2018

okurz Jun 9, 2018

jknphy commented Jun 11, 2018

okurz Jun 12, 2018

okurz Jun 18, 2018

okurz Jun 22, 2018

jknphy Jun 25, 2018

okurz Jun 12, 2018

rwx788 Jun 12, 2018

okurz Jun 12, 2018

jknphy commented Jun 18, 2018

rwx788 left a comment

jknphy commented Jun 22, 2018

jknphy commented Jun 25, 2018

rwx788 left a comment

okurz commented Jun 27, 2018

okurz commented Jun 27, 2018

DimStar77 commented Jun 28, 2018

andreasstieger commented Jun 28, 2018

czerw commented Jun 28, 2018

czerw commented Jun 28, 2018

rwx788 commented Jun 28, 2018 •

edited

Loading

czerw commented Jun 28, 2018

czerw commented Jun 28, 2018

rwx788 commented Jun 28, 2018 •

edited

Loading

czerw commented Jun 28, 2018

Harmonize updating handling SUSE/openSUSE #5175

Harmonize updating handling SUSE/openSUSE #5175

Conversation

jknphy commented Jun 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jknphy commented Jun 8, 2018 • edited Loading

jknphy commented Jun 8, 2018 • edited Loading

okurz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jknphy commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jknphy commented Jun 18, 2018

rwx788 left a comment

Choose a reason for hiding this comment

jknphy commented Jun 22, 2018

jknphy commented Jun 25, 2018

rwx788 left a comment

Choose a reason for hiding this comment

okurz commented Jun 27, 2018

okurz commented Jun 27, 2018

DimStar77 commented Jun 28, 2018

andreasstieger commented Jun 28, 2018

czerw commented Jun 28, 2018

czerw commented Jun 28, 2018

rwx788 commented Jun 28, 2018 • edited Loading

czerw commented Jun 28, 2018

czerw commented Jun 28, 2018

rwx788 commented Jun 28, 2018 • edited Loading

czerw commented Jun 28, 2018

jknphy commented Jun 6, 2018 •

edited

Loading

jknphy commented Jun 8, 2018 •

edited

Loading

jknphy commented Jun 8, 2018 •

edited

Loading

rwx788 commented Jun 28, 2018 •

edited

Loading

rwx788 commented Jun 28, 2018 •

edited

Loading