Rework `zypper_lr` to actually check repositories #1495

mnowaksuse · 2016-06-29T10:05:36Z

PR#11708

Before zypper_lr test case just listed zypper repositories. Now
the test case makes sure repositories which should be present are
actually there.

SLES/SLED base products are supported, so are addons (those defined by
ADDONS, ADDONURL, and SCC_ADDONS variables) with Pool nad Updates SCC
channels if defined by SCC_ADDONS.

Verification runs:

Tumbleweed: http://assam.suse.cz/tests/1841#step/zypper_lr/37
Leap 42.1: http://assam.suse.cz/tests/1839#step/zypper_lr/51
Leap 42.2: http://assam.suse.cz/tests/1843#step/zypper_lr/47
SLES JeOS: http://assam.suse.cz/tests/1845#step/zypper_lr/27

Server-DVD-HA + GEO with SCC channels:
http://assam.suse.cz/tests/1830#step/zypper_lr/55

PR#11460 (FATE#320494)

This test case also verifies following test cases from FATE#320494:

Test case #1: not registered system

Install but do not register the system.
After the installation, type
zypper lr and check that the source corresponding to the CD/DVD is
enabled.

Test case #2: registered system + addon

Register the system during the installation.
Enable the SDK add-on during the registration (in the "Available
Extensions and Modules" screen).
After the installation, type
zypper lr and check that the source corresponding to the CD/DVD is
disabled (but it's still there).

Test case #3: registered system + not registered addon

Register the system during the installation.
Add the SDK add-on using a CD/DVD/USB stick (through the "Add-on
Product" screen).
After the installation, type
zypper lr and check that the source corresponding to the CD/DVD is
disabled (but it's still there) and the source corresponding to the SDK
add-on is enabled.

coolo · 2016-06-29T10:08:55Z

I would be more interested in a staging and a TW scenario

okurz · 2016-07-05T05:38:13Z

tests/console/zypper_lr.pm

+        validatelr(
+            {
+                product         => $base_product,
+                product_channel => check_var('SCC_REGISTER', 'installation') ? "Pool" : undef,


You are already within check_var('SCC_REGISTER') so why?

okurz · 2016-07-05T05:41:51Z

inline + what coolo said

mnowaksuse · 2016-07-07T10:29:06Z

New verification run: http://assam.suse.cz/tests/1674#step/zypper_lr/29.

mnowaksuse · 2016-07-07T10:36:19Z

I would be more interested in a staging and a TW scenario

Is there some table similar to this one https://wiki.microfocus.net/index.php?title=SLE12_SP2_Channels_Checking_Table but for openSUSE?

coolo · 2016-07-07T10:56:53Z

neither staging nor TW register the installation. The URLs are coming from control.xml - so zypper lr should be pretty static and you don't need that huge table.

aaannz · 2016-07-07T11:27:51Z

tests/console/zypper_lr.pm

+    my (%h_addons, %h_addonurl, %h_scc_addons);
+    my @addons_keys     = split(/,/, get_var('ADDONS'));
+    my @addonurl_keys   = split(/,/, get_var('ADDONURL'));
+    my @scc_addons_keys = split(/,/, get_var('SCC_ADDONS'));


These three will produce undef warnings when those vars are not set. Either use get_required_var if job should fail or get_var('ADDONS', '').

mnowaksuse · 2016-07-25T08:03:04Z

I believe the requested changes are implemented, please review.

mnowaksuse · 2016-07-26T09:12:24Z

Updated to correctly handle SLE SCC addons "streams" ("Pool", "Updates", ...).

okurz · 2016-07-26T09:49:56Z

cpanfile

@@ -3,3 +3,5 @@ requires 'File::Basename';
 requires 'Data::Dumper';
 requires 'XML::Writer';
 requires 'IO::File';
+requires 'XML::LibXML';
+requires 'XML::LibXML::XPathContext';


looks like you are not actually using these in the context of the driver, only within the SUT so this shouldn't be required, right?

okurz · 2016-07-26T09:52:03Z

I wouldn't have that the test would need to be that complicated 😟

okurz · 2016-07-26T15:30:31Z

LGTM, @coolo?

coolo · 2016-07-26T15:44:58Z

that's a hell of a test. I don't even know how I would review what it does on maintenance. So the chances that this breaks somewhere if we merge it are pretty high, no?

coolo · 2016-07-26T15:46:32Z

perhaps it makes sense to leave the zypper_lr for documentation purposes as is and only cherry-pick when to run this one?

coolo · 2016-07-26T15:47:59Z

tests/console/zypper_lr.pm

+    elsif (check_var('DISTRI', 'opensuse')) {
+        if ($product eq "openSUSE-Leap-42.1-Source-Non-Oss") {
+            script_run "zypper lr -d | grep \"$alias.*$product *| $enabled_repo.*$uri\"";
+            record_soft_failure 'boo#988736';


there is little point in record_soft_failure here. The bug is on the installation medium of 42.1 and won't be fixed there - so having this as soft_fail now only makes sure we will never have a green 42.1 update test

okurz · 2016-07-26T19:35:05Z

what about not overwriting the original test but put it into an extra branch which we call in the test development job group and gather some experience with it first?

mnowaksuse · 2016-07-27T15:17:46Z

Updated to avoid some odd corner cases. I believe it covers PR#11460 (FATE#320494) now.

Yes, the test will likely break until it's proven on real setups we use. Would be great if there's a way how to see it running with a wide setup and not breaking much stuff.

okurz · 2016-07-28T07:30:52Z

Would be great if there's a way how to see it running with a wide setup and not breaking much stuff.

Well, as I told. This is called feature switch and it is what we also kinda use with the uppercase variables in the tests, e.g. you could add "EXTENDED_ZYPPER_LR_VALIDATION" and we add one test_suite to openQA which we include as part of test development. Of course, as always the challenge with feature toggles is that we need to keep track of its use, assign a feature toggle responsible (that would be you in this case) or at best avoid it by introducing changes incrementally, as Martin Fowler said: "feature toggle is the second-best solution. The best solution is to find a way to gradually integrate without feature toggles"

PR#11708 Before `zypper_lr` test case just listed zypper repositories. Now the test case makes sure repositories which should be present are actually there. SLES/SLED base products are supported, so are addons (those defined by ADDONS, ADDONURL, and SCC_ADDONS variables) with Pool nad Updates SCC channels if defined by SCC_ADDONS. Verification runs: - Tumbleweed: http://assam.suse.cz/tests/1841#step/zypper_lr/37 - Leap 42.1: http://assam.suse.cz/tests/1839#step/zypper_lr/51 - Leap 42.2: http://assam.suse.cz/tests/1843#step/zypper_lr/47 - SLES JeOS: http://assam.suse.cz/tests/1845#step/zypper_lr/27 Server-DVD-HA + GEO with SCC channels: http://assam.suse.cz/tests/1830#step/zypper_lr/55 PR#11460 (FATE#320494) This test case also verifies following test cases from FATE#320494: Test case os-autoinst#1: not registered system 1. Install but do not register the system. 2. After the installation, type 3. zypper lr and check that the source corresponding to the CD/DVD is enabled. Test case os-autoinst#2: registered system + addon 1. Register the system during the installation. 2. Enable the SDK add-on during the registration (in the "Available Extensions and Modules" screen). 3. After the installation, type zypper lr and check that the source corresponding to the CD/DVD is disabled (but it's still there). Test case os-autoinst#3: registered system + not registered addon 1. Register the system during the installation. 2. Add the SDK add-on using a CD/DVD/USB stick (through the "Add-on Product" screen). 3. After the installation, type zypper lr and check that the source corresponding to the CD/DVD is disabled (but it's still there) and the source corresponding to the SDK add-on is enabled.

mnowaksuse · 2016-08-02T07:47:49Z

Originally I imagined "extra branch which we call in the test development job group" as a separate openQA instance running production scenarios without affecting production results. But, as I understand it, since it would be just a test suite (one, or couple of them), I believe it does not help to identify potential problem with the zypper_lr test. What I need is plethora of test cases we actually use to see if the test behaves, or misbehaves.

So, I cloned dozens of jobs from production instances, and they all passed, or failed, where they should fail (e.g. Leap 42.2's https://bugzilla.suse.com/show_bug.cgi?id=988736):

Anyways, the test affects SLE, and openSUSE staging jobs only. For the rest it just lists available repos.

okurz · 2016-08-02T09:19:59Z

ok. You invested a lot of time in confirming your test, we can see that :-) I will merge now but be prepared that probably we will need to adjust the test some times according to how the product will change and I assume not many are willing (or able) to understand what the test does so they will come back to you to fix it :-)

mnowaksuse · 2016-08-02T09:44:47Z

I am fine with that. Thanks! :)

nilxam · 2016-08-04T07:11:56Z

@mnowaksuse this PR won't 100% works on Factory/Leap staging, for TW staging, it's depend on when do snapshot online repo is published and what version number we are using in the staging, eg. while testing https://openqa.opensuse.org/tests/235834 the 0728 snpashot repo was published, so the local repository is disabled after insallation; but see https://openqa.opensuse.org/tests/235680#step/zypper_lr/3, when testing it 0730 repo was not published, so the local repo still enabled. Therefore, you have to improve the logic here, or we will get the same problem depend on timing. Note that, after introduced the change for fate#320494, we have to enable all local repo for staging anyway - 7e9e81f , but zypper_clear_repos is executed after zypper_lr.

After all it seems that openSUSE and staging is not that important.It brought problems which might not be fixed easily: os-autoinst#1495 (comment). After suggestion from Max Lin (os-autoinst#1636) this change removes openSUSE and staging testing; including control.xml validation code as it seems that SLE control.xml files do not include repository information at all.

okurz reviewed Jul 5, 2016
View reviewed changes

mnowaksuse force-pushed the zypper_lr_needle2 branch from 5095f8e to fa75a6f Compare July 7, 2016 10:28

aaannz reviewed Jul 7, 2016
View reviewed changes

mnowaksuse force-pushed the zypper_lr_needle2 branch from fa75a6f to a51eb3f Compare July 7, 2016 12:54

mnowaksuse force-pushed the zypper_lr_needle2 branch from a51eb3f to b882e9f Compare July 15, 2016 15:18

mnowaksuse added the WIP Work in progress label Jul 26, 2016

mnowaksuse force-pushed the zypper_lr_needle2 branch from b882e9f to 9cc8b36 Compare July 26, 2016 09:11

mnowaksuse removed the WIP Work in progress label Jul 26, 2016

okurz reviewed Jul 26, 2016
View reviewed changes

mnowaksuse force-pushed the zypper_lr_needle2 branch from 9cc8b36 to ff9de47 Compare July 26, 2016 10:10

coolo reviewed Jul 26, 2016
View reviewed changes

mnowaksuse force-pushed the zypper_lr_needle2 branch from ff9de47 to af6c086 Compare July 27, 2016 15:12

mnowaksuse added the WIP Work in progress label Jul 28, 2016

mnowaksuse force-pushed the zypper_lr_needle2 branch from af6c086 to d0ae7d5 Compare July 29, 2016 14:42

mnowaksuse force-pushed the zypper_lr_needle2 branch 2 times, most recently from 549dd5c to 6095cec Compare August 1, 2016 14:11

mnowaksuse force-pushed the zypper_lr_needle2 branch from 6095cec to 3cc024e Compare August 2, 2016 07:27

mnowaksuse removed the WIP Work in progress label Aug 2, 2016

okurz merged commit c764993 into os-autoinst:master Aug 2, 2016

nilxam mentioned this pull request Aug 4, 2016

Temporary disabled repos check in Staging #1636

Closed

mnowaksuse mentioned this pull request Aug 4, 2016

Remove openSUSE and staging portion from zypper_lr #1638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework `zypper_lr` to actually check repositories #1495

Rework `zypper_lr` to actually check repositories #1495

mnowaksuse commented Jun 29, 2016 •

edited

Loading

coolo commented Jun 29, 2016

okurz Jul 5, 2016

okurz commented Jul 5, 2016

mnowaksuse commented Jul 7, 2016

mnowaksuse commented Jul 7, 2016

coolo commented Jul 7, 2016

aaannz Jul 7, 2016

mnowaksuse commented Jul 25, 2016

mnowaksuse commented Jul 26, 2016

okurz Jul 26, 2016

okurz commented Jul 26, 2016

okurz commented Jul 26, 2016

coolo commented Jul 26, 2016

coolo commented Jul 26, 2016

coolo Jul 26, 2016

okurz commented Jul 26, 2016

mnowaksuse commented Jul 27, 2016

okurz commented Jul 28, 2016

mnowaksuse commented Aug 2, 2016

okurz commented Aug 2, 2016

mnowaksuse commented Aug 2, 2016

nilxam commented Aug 4, 2016

Rework zypper_lr to actually check repositories #1495

Rework zypper_lr to actually check repositories #1495

Conversation

mnowaksuse commented Jun 29, 2016 • edited Loading

coolo commented Jun 29, 2016

okurz Jul 5, 2016

Choose a reason for hiding this comment

okurz commented Jul 5, 2016

mnowaksuse commented Jul 7, 2016

mnowaksuse commented Jul 7, 2016

coolo commented Jul 7, 2016

aaannz Jul 7, 2016

Choose a reason for hiding this comment

mnowaksuse commented Jul 25, 2016

mnowaksuse commented Jul 26, 2016

okurz Jul 26, 2016

Choose a reason for hiding this comment

okurz commented Jul 26, 2016

okurz commented Jul 26, 2016

coolo commented Jul 26, 2016

coolo commented Jul 26, 2016

coolo Jul 26, 2016

Choose a reason for hiding this comment

okurz commented Jul 26, 2016

mnowaksuse commented Jul 27, 2016

okurz commented Jul 28, 2016

mnowaksuse commented Aug 2, 2016

okurz commented Aug 2, 2016

mnowaksuse commented Aug 2, 2016

nilxam commented Aug 4, 2016

Rework `zypper_lr` to actually check repositories #1495

Rework `zypper_lr` to actually check repositories #1495

mnowaksuse commented Jun 29, 2016 •

edited

Loading