diff --git a/rel-eng/packages/subscription-manager b/rel-eng/packages/subscription-manager
index 1d1a8e1..f9393db 100644
--- a/rel-eng/packages/subscription-manager
+++ b/rel-eng/packages/subscription-manager
@@ -1 +1 @@
-1.10.14-7 ./
+1.10.14-8 ./
diff --git a/src/subscription_manager/cache.py b/src/subscription_manager/cache.py
index d773741..298f64e 100644
--- a/src/subscription_manager/cache.py
+++ b/src/subscription_manager/cache.py
@@ -508,3 +508,30 @@ class PoolTypeCache(object):
def clear(self):
self.pooltype_map = {}
+
+
+class WrittenOverrideCache(CacheManager):
+ '''
+ Cache to keep track of the overrides used last time the a redhat.repo
+ was written. Doesn't track server status, we've got another cache for
+ that.
+ '''
+
+ CACHE_FILE = "/var/lib/rhsm/cache/written_overrides.json"
+
+ def __init__(self, overrides=None):
+ self.overrides = overrides or {}
+
+ def to_dict(self):
+ return self.overrides
+
+ def _load_data(self, open_file):
+ try:
+ self.overrides = json.loads(open_file.read()) or {}
+ return self.overrides
+ except IOError:
+ log.error("Unable to read cache: %s" % self.CACHE_FILE)
+ except ValueError:
+ # ignore json file parse errors, we are going to generate
+ # a new as if it didn't exist
+ pass
diff --git a/src/subscription_manager/repolib.py b/src/subscription_manager/repolib.py
index ad0fc30..fa0913d 100644
--- a/src/subscription_manager/repolib.py
+++ b/src/subscription_manager/repolib.py
@@ -20,12 +20,11 @@ import logging
import os
import string
import subscription_manager.injection as inj
-from subscription_manager.cache import OverrideStatusCache
+from subscription_manager.cache import OverrideStatusCache, WrittenOverrideCache
from urllib import basejoin
from rhsm.config import initConfig
from rhsm.connection import RemoteServerException, RestlibException
-from rhsm.utils import UnsupportedOperationException
from certlib import ActionLock, DataLib
from certdirectory import Path, ProductDirectory, EntitlementDirectory
@@ -56,10 +55,7 @@ class RepoLib(DataLib):
action = UpdateAction(self.uep, cache_only=self.cache_only,
apply_overrides=apply_overrides, identity=self.identity)
repos = action.get_unique_content()
- if self.identity.is_valid() and action.override_supported:
- return repos
- # Otherwise we are in a disconnected case or dealing with an old server
current = set()
# Add the current repo data
repo_file = RepoFile()
@@ -83,6 +79,8 @@ class RepoLib(DataLib):
repo_file = RepoFile()
if os.path.exists(repo_file.path):
os.unlink(repo_file.path)
+ # When the repo is removed, also remove the override tracker
+ WrittenOverrideCache.delete_cache()
# WARNING: exact same name as another action in factlib and certlib.
@@ -111,8 +109,9 @@ class UpdateAction:
self.manage_repos = int(CFG.get('rhsm', 'manage_repos'))
self.release = None
- self.overrides = []
+ self.overrides = {}
self.override_supported = bool(self.uep and self.uep.supports_resource('content_overrides'))
+ self.written_overrides = WrittenOverrideCache()
# If we are not registered, skip trying to refresh the
# data from the server
@@ -122,6 +121,7 @@ class UpdateAction:
# Only attempt to update the overrides if they are supported
# by the server.
if self.override_supported:
+ self.written_overrides._read_cache()
try:
override_cache = inj.require(inj.OVERRIDE_STATUS_CACHE)
except KeyError:
@@ -131,8 +131,11 @@ class UpdateAction:
else:
status = override_cache.load_status(self.uep, self.identity.uuid)
- if status is not None:
- self.overrides = status
+ for item in status or []:
+ # Don't iterate through the list
+ if item['contentLabel'] not in self.overrides:
+ self.overrides[item['contentLabel']] = {}
+ self.overrides[item['contentLabel']][item['name']] = item['value']
message = "Release API is not supported by the server. Using default."
try:
@@ -174,14 +177,9 @@ class UpdateAction:
repo_file.add(cont)
updates += 1
else:
- # In the non-disconnected case, destroy the old repo and replace it with
- # what's in the entitlement cert plus any overrides.
- if self.identity.is_valid() and self.override_supported:
- repo_file.update(cont)
- updates += 1
- else:
- updates += self.update_repo(existing, cont)
- repo_file.update(existing)
+ # Updates the existing repo with new content
+ updates += self.update_repo(existing, cont)
+ repo_file.update(existing)
for section in repo_file.sections():
if section not in valid:
@@ -190,6 +188,12 @@ class UpdateAction:
# Write new RepoFile to disk:
repo_file.write()
+
+ if self.override_supported:
+ # Update with the values we just wrote
+ self.written_overrides.overrides = self.overrides
+ self.written_overrides.write_cache()
+
log.info("repos updated: %s" % updates)
return updates
@@ -295,9 +299,16 @@ class UpdateAction:
def _set_override_info(self, repo):
# In the disconnected case, self.overrides will be an empty list
- for entry in self.overrides:
- if entry['contentLabel'] == repo.id:
- repo[entry['name']] = entry['value']
+ for name, value in self.overrides.get(repo.id, {}).items():
+ repo[name] = value
+
+ def _is_overridden(self, repo, key):
+ return key in self.overrides.get(repo.id, {})
+
+ def _was_overridden(self, repo, key, value):
+ written_value = self.written_overrides.overrides.get(repo.id, {}).get(key)
+ # Compare values as strings to avoid casting problems from io
+ return written_value is not None and value is not None and str(written_value) == str(value)
def _set_proxy_info(self, repo):
proxy = ""
@@ -328,29 +339,33 @@ class UpdateAction:
url = url.lstrip('/')
return basejoin(base, url)
+ def _build_props(self, old_repo, new_repo):
+ result = {}
+ all_keys = old_repo.keys() + new_repo.keys()
+ for key in all_keys:
+ result[key] = Repo.PROPERTIES.get(key, (1, None))
+ return result
+
def update_repo(self, old_repo, new_repo):
"""
Checks an existing repo definition against a potentially updated
version created from most recent entitlement certificates and
configuration. Creates, updates, and removes properties as
appropriate and returns the number of changes made. (if any)
-
- This method should only be used in disconnected cases!
"""
- if self.identity.is_valid() and self.override_supported:
- log.error("Can not update repos when registered!")
- raise UnsupportedOperationException()
-
changes_made = 0
- for key, mutable, default in Repo.PROPERTIES:
+ for key, (mutable, default) in self._build_props(old_repo, new_repo).items():
new_val = new_repo.get(key)
# Mutable properties should be added if not currently defined,
- # otherwise left alone.
- if mutable:
- if (new_val is not None) and (not old_repo[key]):
- if old_repo[key] == new_val:
+ # otherwise left alone. However if we see that the property was overridden
+ # but that override has since been removed, we need to revert to the default
+ # value.
+ if mutable and not self._is_overridden(old_repo, key) \
+ and not self._was_overridden(old_repo, key, old_repo.get(key)):
+ if (new_val is not None) and (not old_repo.get(key)):
+ if old_repo.get(key) == new_val:
continue
old_repo[key] = new_val
changes_made += 1
@@ -358,7 +373,7 @@ class UpdateAction:
# Immutable properties should be always be added/updated,
# and removed if undefined in the new repo definition.
else:
- if new_val is None or (new_val.strip() == ""):
+ if new_val is None or (str(new_val).strip() == ""):
# Immutable property should be removed:
if key in old_repo.keys():
del old_repo[key]
@@ -366,7 +381,7 @@ class UpdateAction:
continue
# Unchanged:
- if old_repo[key] == new_val:
+ if old_repo.get(key) == new_val:
continue
old_repo[key] = new_val
@@ -377,22 +392,21 @@ class UpdateAction:
class Repo(dict):
# (name, mutable, default) - The mutability information is only used in disconnected cases
- PROPERTIES = (
- ('name', 0, None),
- ('baseurl', 0, None),
- ('enabled', 1, '1'),
- ('gpgcheck', 1, '1'),
- ('gpgkey', 0, None),
- ('sslverify', 1, '1'),
- ('sslcacert', 0, None),
- ('sslclientkey', 0, None),
- ('sslclientcert', 0, None),
- ('metadata_expire', 1, None),
- ('proxy', 0, None),
- ('proxy_username', 0, None),
- ('proxy_password', 0, None),
- ('ui_repoid_vars', 0, None),
- )
+ PROPERTIES = {
+ 'name': (0, None),
+ 'baseurl': (0, None),
+ 'enabled': (1, '1'),
+ 'gpgcheck': (1, '1'),
+ 'gpgkey': (0, None),
+ 'sslverify': (1, '1'),
+ 'sslcacert': (0, None),
+ 'sslclientkey': (0, None),
+ 'sslclientcert': (0, None),
+ 'metadata_expire': (1, None),
+ 'proxy': (0, None),
+ 'proxy_username': (0, None),
+ 'proxy_password': (0, None),
+ 'ui_repoid_vars': (0, None)}
def __init__(self, repo_id, existing_values=None):
# existing_values is a list of 2-tuples
@@ -412,7 +426,7 @@ class Repo(dict):
# NOTE: This sets the above properties to the default values even if
# they are not defined on disk. i.e. these properties will always
# appear in this dict, but their values may be None.
- for k, m, d in self.PROPERTIES:
+ for k, (m, d) in self.PROPERTIES.items():
if k not in self.keys():
self[k] = d
diff --git a/subscription-manager.spec b/subscription-manager.spec
index dea8402..04a660d 100644
--- a/subscription-manager.spec
+++ b/subscription-manager.spec
@@ -14,7 +14,7 @@
Name: subscription-manager
Version: 1.10.14
-Release: 7%{?dist}
+Release: 8%{?dist}
Summary: Tools and libraries for subscription and repository management
Group: System Environment/Base
License: GPLv2
@@ -419,6 +419,10 @@ fi
%endif
%changelog
+* Tue May 27 2014 ckozak <ckozak@redhat.com> 1.10.14-8
+- 1098891: Update repos, persisting local settings when possible
+ (ckozak@redhat.com)
+
* Tue Mar 25 2014 ckozak <ckozak@redhat.com> 1.10.14-7
- 1080531: Require newer python-rhsm to support branding (ckozak@redhat.com)
diff --git a/test/test_repolib.py b/test/test_repolib.py
index 57274e6..72505a3 100644
--- a/test/test_repolib.py
+++ b/test/test_repolib.py
@@ -18,8 +18,6 @@ import unittest
from mock import Mock, patch
from StringIO import StringIO
-from rhsm.utils import UnsupportedOperationException
-
from fixture import SubManFixture
from stubs import StubCertificateDirectory, StubProductCertificate, \
StubProduct, StubEntitlementCertificate, StubContent, \
@@ -111,24 +109,32 @@ class UpdateActionTests(SubManFixture):
self.assertFalse(override_cache_mock.load_status.called)
def test_overrides_trump_ent_cert(self):
- self.update_action.overrides = [{
- 'contentLabel': 'x',
- 'name': 'gpgcheck',
- 'value': 'blah'
- }]
+ self.update_action.overrides = {'x': {'gpgcheck': 'blah'}}
r = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')])
self.assertEquals('original', r['gpgcheck'])
self.update_action._set_override_info(r)
self.assertEquals('blah', r['gpgcheck'])
self.assertEquals('some_key', r['gpgkey'])
+ def test_overrides_trump_existing(self):
+ self.update_action.overrides = {'x': {'gpgcheck': 'blah'}}
+ values = [('gpgcheck', 'original'), ('gpgkey', 'some_key')]
+ old_repo = Repo('x', values)
+ new_repo = Repo(old_repo.id, values)
+ self.update_action._set_override_info(new_repo)
+ self.assertEquals('original', old_repo['gpgcheck'])
+ self.update_action.update_repo(old_repo, new_repo)
+ self.assertEquals('blah', old_repo['gpgcheck'])
+ self.assertEquals('some_key', old_repo['gpgkey'])
+
@patch("subscription_manager.repolib.RepoFile")
def test_update_when_new_repo(self, mock_file):
mock_file = mock_file.return_value
mock_file.section.return_value = None
def stub_content():
- return [Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')])]
+ return [Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key'), ('name', 'some name')])]
+
self.update_action.get_unique_content = stub_content
updates = self.update_action.perform()
written_repo = mock_file.add.call_args[0][0]
@@ -137,31 +143,13 @@ class UpdateActionTests(SubManFixture):
self.assertEquals(1, updates)
@patch("subscription_manager.repolib.RepoFile")
- @patch("subscription_manager.repolib.ConsumerIdentity")
- def test_update_when_registered_and_existing_repo(self, mock_ident, mock_file):
- mock_ident.existsAndValid.return_value = True
+ def test_update_when_not_registered_and_existing_repo(self, mock_file):
mock_file = mock_file.return_value
mock_file.section.return_value = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')])
def stub_content():
- return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key')])]
- self.update_action.get_unique_content = stub_content
- self.update_action.override_supported = True
- updates = self.update_action.perform()
- written_repo = mock_file.update.call_args[0][0]
- self.assertEquals('new', written_repo['gpgcheck'])
- self.assertEquals('new_key', written_repo['gpgkey'])
- self.assertEquals(1, updates)
+ return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key'), ('name', 'test')])]
- @patch("subscription_manager.repolib.RepoFile")
- @patch("subscription_manager.repolib.ConsumerIdentity")
- def test_update_when_not_registered_and_existing_repo(self, mock_ident, mock_file):
- mock_ident.existsAndValid.return_value = False
- mock_file = mock_file.return_value
- mock_file.section.return_value = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')])
-
- def stub_content():
- return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key')])]
self.update_action.get_unique_content = stub_content
self.update_action.perform()
written_repo = mock_file.update.call_args[0][0]
@@ -369,14 +357,57 @@ class UpdateActionTests(SubManFixture):
existing_repo['fake_prop'] = 'fake'
self.assertTrue(('fake_prop', 'fake') in existing_repo.items())
- @patch("subscription_manager.repolib.ConsumerIdentity")
- def test_repo_update_forbidden_when_registered(self, mock_ident):
- mock_ident.existsAndValid.return_value = True
- existing_repo = Repo('testrepo')
- existing_repo['proxy_username'] = "blah"
- incoming_repo = {'proxy_username': 'foo'}
- self.update_action.override_supported = True
- self.assertRaises(UnsupportedOperationException, self.update_action.update_repo, existing_repo, incoming_repo)
+ def test_overrides_removed_revert_to_default(self):
+ self.update_action.written_overrides.overrides = {'x': {'gpgcheck': 'blah'}}
+ self.update_action.overrides = {}
+ old_repo = Repo('x', [('gpgcheck', 'blah'), ('gpgkey', 'some_key')])
+ new_repo = Repo(old_repo.id, [('gpgcheck', 'original'), ('gpgkey', 'some_key')])
+ self.update_action._set_override_info(new_repo)
+ # The value from the current repo file (with the old override) should exist pre-update
+ self.assertEquals('blah', old_repo['gpgcheck'])
+ self.update_action.update_repo(old_repo, new_repo)
+ # Because the override has been removed, the value is reset to the default
+ self.assertEquals('original', old_repo['gpgcheck'])
+ self.assertEquals('some_key', old_repo['gpgkey'])
+
+ def test_overrides_removed_and_edited(self):
+ self.update_action.written_overrides.overrides = {'x': {'gpgcheck': 'override_value'}}
+ self.update_action.overrides = {}
+ old_repo = Repo('x', [('gpgcheck', 'hand_edit'), ('gpgkey', 'some_key')])
+ new_repo = Repo(old_repo.id, [('gpgcheck', 'original'), ('gpgkey', 'some_key')])
+ self.update_action._set_override_info(new_repo)
+ # The value from the current repo file (with the old hand edit) should exist pre-update
+ self.assertEquals('hand_edit', old_repo['gpgcheck'])
+ self.update_action.update_repo(old_repo, new_repo)
+ # Because the current value doesn't match the override, we don't modify it
+ self.assertEquals('hand_edit', old_repo['gpgcheck'])
+ self.assertEquals('some_key', old_repo['gpgkey'])
+
+ def test_non_default_overrides_added_to_existing(self):
+ '''
+ Test that overrides for values that aren't found in Repo.PROPERTIES are written
+ to existing repos
+ '''
+ self.update_action.written_overrides.overrides = {}
+ self.update_action.overrides = {'x': {'somekey': 'someval'}}
+ old_repo = Repo('x', [])
+ new_repo = Repo(old_repo.id, [])
+ self.update_action._set_override_info(new_repo)
+ self.update_action.update_repo(old_repo, new_repo)
+ self.assertEquals('someval', old_repo['somekey'])
+
+ def test_non_default_override_removed_deleted(self):
+ '''
+ Test that overrides for values that aren't found in Repo.PROPERTIES are
+ removed from redhat.repo once the override is removed
+ '''
+ self.update_action.written_overrides.overrides = {'x': {'somekey': 'someval'}}
+ self.update_action.overrides = {}
+ old_repo = Repo('x', [('somekey', 'someval')])
+ new_repo = Repo(old_repo.id, [])
+ self.update_action._set_override_info(new_repo)
+ self.update_action.update_repo(old_repo, new_repo)
+ self.assertFalse('somekey' in old_repo)
class TidyWriterTests(unittest.TestCase):