From 4efb0d23bd5392db2f4c7e264a986c12218387ca Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Jun 27 2019 11:10:39 +0000
Subject: Fix the deduplication code when `conf.lightblue_released_dependencies_only = True`.


This is follow-up of https://pagure.io/freshmaker/pull-request/388.

In that PR, we fixed deduplication code for case when image changed
its parent image completely within the same version but different
release.

The issue is that this works only when lightblue_released_dependencies_only
is set to False. In case it is set to True, the latest_released_nvr_index
is set to -1 indicating that the deduplication code should replace all
the images with latest release version (so in case the image is released
but based on the unreleased image, we move it back to released release
of that parent image).

When latest_released_nvr_index is -1, the current code to set latest_image
is simply broken, because it does `latest_image = nvr_to_image[nvrs[latest_released_nvr_index]]`
and therefore effectively sets the latest_image to the oldest image (`nvrs[-1]`).

This commit fixes this issue.

It also removes `n_to_nvs` which is not used anymore and one try/except
block which has been forgotten there from times when `n_to_nvs` was used.

---

diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index bfbc8d8..1258452 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -1359,9 +1359,6 @@ class LightBlue(object):
             nvr_to_image = {}
             # Temporary dict mapping NV to latest released NVR for that NV.
             nv_to_latest_released_nvr = {}
-            # Temporary list containing names of all images to rebuild. This is
-            # later used to replace "foo-docker" with "foo-container".
-            n_to_nvs = {}
 
             # Constructs the temporary dicts as desribed above.
             for image_id, images in enumerate(to_rebuild):
@@ -1375,10 +1372,6 @@ class LightBlue(object):
                         nv_to_nvrs[nv].append(nvr)
                     if nvr not in nvr_to_coordinates:
                         nvr_to_coordinates[nvr] = []
-                    if parsed_nvr["name"] not in n_to_nvs:
-                        n_to_nvs[parsed_nvr["name"]] = []
-                    if nv not in n_to_nvs[parsed_nvr["name"]]:
-                        n_to_nvs[parsed_nvr["name"]].append(nv)
                     nvr_to_coordinates[nvr].append([image_id, parent_id])
                     nvr_to_image[nvr] = image
                     if "latest_released" in image and image["latest_released"]:
@@ -1423,25 +1416,20 @@ class LightBlue(object):
                     latest_released_nvr = nv_to_latest_released_nvr[nv]
                 else:
                     latest_released_nvr = nvrs[0]
+
                 # The latest_released_nvr_index points to the latest released NVR
                 # in the `nvrs` list. Because `nvrs` list is desc sorted, every NVR
                 # with higher index is lower and therefore we need to replace it.
-                try:
-                    if not conf.lightblue_released_dependencies_only:
-                        latest_released_nvr_index = nvrs.index(latest_released_nvr)
-                    else:
-                        # In case we want to use only released versions of images,
-                        # replace all the images with the latest released one.
-                        latest_released_nvr_index = -1
-                except ValueError:
-                    # In case the latest_released_nvr is not found in the nvrs,
-                    # it means the all nvrs should be replaced by new one from
-                    # nvs_to_replace and therefore set index to -1 indicating we
-                    # want to replace everything.
+                if not conf.lightblue_released_dependencies_only:
+                    latest_released_nvr_index = nvrs.index(latest_released_nvr)
+                else:
+                    # In case we want to use only released versions of images,
+                    # replace all the images with the latest released one.
                     latest_released_nvr_index = -1
+
                 if phase == "handle_parent_change":
                     # Find out the name of parent image of latest release image.
-                    latest_image = nvr_to_image[nvrs[latest_released_nvr_index]]
+                    latest_image = nvr_to_image[latest_released_nvr]
                     if "parent" not in latest_image or not latest_image["parent"]:
                         continue
                     latest_parent_name = koji.parse_NVR(
@@ -1449,7 +1437,7 @@ class LightBlue(object):
 
                     # Go through the older images and in case the parent image differs,
                     # update its parents according to latest image parents.
-                    for nvr in nvrs[latest_released_nvr_index:]:
+                    for nvr in nvrs[latest_released_nvr_index + 1:]:
                         image = nvr_to_image[nvr]
                         if "parent" not in image or not image["parent"]:
                             continue
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index eadb3d3..4682906 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -1946,8 +1946,10 @@ class TestDeduplicateImagesToRebuild(helpers.FreshmakerTestCase):
             ])
         ]
 
-        ret = self.lb._deduplicate_images_to_rebuild([httpd, perl])
-        self.assertEqual(ret, expected_images)
+        for val in [True, False]:
+            with patch.object(freshmaker.conf, 'lightblue_released_dependencies_only', new=val):
+                ret = self.lb._deduplicate_images_to_rebuild([httpd, perl])
+                self.assertEqual(ret, expected_images)
 
 
 class TestArchitecturesFromRegistry(helpers.FreshmakerTestCase):