Differential D1220

rpm_utils: remove is_installed()
ClosedPublic
Actions

Authored by kparal on Wed, Jul 12, 2:01 PM.

Details

Reviewers
jskladan
Group Reviewers
libtaskotron
Commits
rLTRN891b3194a937: rpm_utils: remove is_installed()
Summary

Due to a dnf 1.x bug (present on Fedora 25 and older), the previous
implementation with dnf list --installed wasn't working properly - it
also listed available packages, not just installed ones. Because a good
fix is not simple and because this code is going away soon anyway, I
removed the whole is_installed() method and no longer check installed
packages. The packages are now always attempted to install (in
production, under root, etc), or simply printed out as a warning on a
developer setup. It's now up to the developer to notice the warning and
install the packages if they're missing. Unfortunately it seems to be
the most reasonable way to resolve this quickly.

Test Plan

tested locally in dev mode and remotely in production mode

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal created this revision.Wed, Jul 12, 2:01 PM
jskladan accepted this revision.Wed, Jul 12, 2:03 PM
This revision is now accepted and ready to land.Wed, Jul 12, 2:03 PM
Closed by commit rLTRN891b3194a937: rpm_utils: remove is_installed() (authored by Kamil Páral <kparal@redhat.com>). · Explain WhyWed, Jul 12, 2:34 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 3093

View Options

libtaskotron/executor.py

Show All 9 Lines
10import copy 10​import copy
11import collections 11​import collections
12import pipes 12​import pipes
13 13​
14from libtaskotron import taskformula 14​from libtaskotron import taskformula
15from libtaskotron import logger 15​from libtaskotron import logger
16from libtaskotron import python_utils 16​from libtaskotron import python_utils
17from libtaskotron import config 17​from libtaskotron import config
18​from libtaskotron import os_utils
18from libtaskotron.logger import log 19​from libtaskotron.logger import log
19import libtaskotron.exceptions as exc 20​import libtaskotron.exceptions as exc
20from libtaskotron.directives import exitcode_directive 21​from libtaskotron.directives import exitcode_directive
21 22​
22try: 23​try:
23 from libtaskotron.ext.fedora import rpm_utils 24​ from libtaskotron.ext.fedora import rpm_utils
24except ImportError, e: 25​except ImportError, e:
25 raise exc.TaskotronImportError(e) 26​ raise exc.TaskotronImportError(e)
Show All 30 Lines55 def execute(self):
56 57​
57 filelog_path = os.path.join(self.arg_data['artifactsdir'], 'taskotron.log') 58​ filelog_path = os.path.join(self.arg_data['artifactsdir'], 'taskotron.log')
58 logger.add_filehandler(filelog_path=filelog_path, remove_mem_handler=True) 59​ logger.add_filehandler(filelog_path=filelog_path, remove_mem_handler=True)
59 60​
60 self._prepare_task() 61​ self._prepare_task()
61 self._run() 62​ self._run()
62 63​
63 def _prepare_task(self): 64​ def _prepare_task(self):
64 '''Prepare the environment for running a task. If there are packages, requirements or 65​ '''Prepare the environment for running a task. If there are rpms
65 groups specified in the task formula, this method tries to install them (in case of 66​ specified in the task formula, this method tries to install them (in
66 production profile) or checks whether they are installed (in case of developer profile). 67​ production profile or local mode or with enough privileges), otherwise
68​ just prints out a warning.
67 ''' 69​ '''
68 rpms = self.formula.get('environment', {}).get('rpm', []) 70​ rpms = self.formula.get('environment', {}).get('rpm', [])
69 profile = config.get_config().profile 71​ profile = config.get_config().profile
70 72​
71 if not rpm_utils.is_installed(rpms): 73​ if (profile == config.ProfileName.PRODUCTION or self.arg_data['local']
72 if profile == config.ProfileName.PRODUCTION or self.arg_data['local']: 74​ or os_utils.is_root() or os_utils.has_sudo()):
73 rpm_utils.install(rpms) 75​ rpm_utils.install(rpms)
74 else: 76​ else:
75 raise exc.TaskotronError('Some packages are not installed. Please run ' 77​ log.warn('Not enough privileges to install packages. Continuing '
76 '"dnf install %s" to install all required packages.' 78​ 'anyway, but make sure the following packages are '
77 % " ".join([pipes.quote(rpm) for rpm in rpms])) 79​ 'installed, otherwise the task is likely to crash:\n%s',
80​ " ".join([pipes.quote(rpm) for rpm in rpms]))
78 81​
79 def _run(self): 82​ def _run(self):
80 self._validate_input() 83​ self._validate_input()
81 84​
82 if not self.workdir: # create temporary workdir if needed 85​ if not self.workdir: # create temporary workdir if needed
83 self.workdir = tempfile.mkdtemp(prefix="task-", 86​ self.workdir = tempfile.mkdtemp(prefix="task-",
84 dir=config.get_config().tmpdir) 87​ dir=config.get_config().tmpdir)
85 log.debug("Current workdir: %s", self.workdir) 88​ log.debug("Current workdir: %s", self.workdir)
▲ Show 20 Lines • Show All 129 Lines • Show Last 20 Lines
View Options

libtaskotron/ext/fedora/rpm_utils.py

Show First 20 Lines • Show All 163 Lines • ▼ Show 20 Line(s)161 else:
164 return 164​ return
165 165​
166 # safeguard against infinite loop, we should never reach this code 166​ # safeguard against infinite loop, we should never reach this code
167 log.critical("Infinite loop detected in 'install()'") 167​ log.critical("Infinite loop detected in 'install()'")
168 assert False, "Infinite loop detected in 'install()'" 168​ assert False, "Infinite loop detected in 'install()'"
169 return 169​ return
170 170​
171 171​
172def is_installed(pkgs): ​
173 '''Check if packages are installed using DNF. No elevated permissions needed. ​
174 ​
175 :param pkgs: packages to be checked whether they are installed, e.g. ``['pidgin']``. Package ​
176 names or file paths are supported. ​
177 :type pkgs: list of str ​
178 :return: True if all specified packages are installed, False otherwise ​
179 :rtype: bool ​
180 ''' ​
181 if not pkgs: ​
182 return True ​
183 ​
184 log.info('Checking installed state of %d packages...', len(pkgs)) ​
185 cmd = ['dnf', 'list', '--installed', '--cacheonly'] ​
186 missing = [] ​
187 ​
188 for pkg in pkgs: ​
189 pkg_cmd = cmd + [pkg] ​
190 log.debug('Running: %s', ' '.join([pipes.quote(c) for c in pkg_cmd])) ​
191 try: ​
192 subprocess.check_output(pkg_cmd, stderr=subprocess.STDOUT) ​
193 except subprocess.CalledProcessError as e: ​
194 log.debug(u"✘ Package '%s' is not installed. DNF returned exit code %d " ​
195 "and output:\n%s", pkg, e.returncode, e.output.rstrip()) ​
196 missing.append(pkg) ​
197 ​
198 if missing: ​
199 log.info('✘ Required packages not installed: %s', ' '.join(missing)) ​
200 return False ​
201 else: ​
202 log.info(u'✔ All specified packages are installed') ​
203 return True ​
204 ​
205 ​
206def dnf_cache_available(): 172​def dnf_cache_available():
207 '''Determine whether DNF cache is available. 173​ '''Determine whether DNF cache is available.
208 174​
209 If it is, we can run read-only actions solely from the cache, and try to use it even for 175​ If it is, we can run read-only actions solely from the cache, and try to use it even for
210 ``install`` action (provided the repos haven't changed much). This way we avoid contacting 176​ ``install`` action (provided the repos haven't changed much). This way we avoid contacting
211 DNF repos and downloading new metadata if they changed. 177​ DNF repos and downloading new metadata if they changed.
212 178​
213 This tests the system cache (if running as root or with sudo access) or the user cache 179​ This tests the system cache (if running as root or with sudo access) or the user cache
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

testing/test_executor.py

Show First 20 Lines • Show All 423 Lines • ▼ Show 20 Line(s)
424 424​
425 425​
426@pytest.mark.usefixtures('setup') 426​@pytest.mark.usefixtures('setup')
427class TestExecutorPrepareTask(): 427​class TestExecutorPrepareTask():
428 428​
429 @pytest.fixture 429​ @pytest.fixture
430 def setup(self, monkeypatch): 430​ def setup(self, monkeypatch):
431 self.install_stub = Dingus() 431​ self.install_stub = Dingus()
432 self.is_installed_stub = Dingus() ​
433 self.stub_config = Dingus() 432​ self.stub_config = Dingus()
434 self.stub_get_config = Dingus() 433​ self.stub_get_config = Dingus()
435 434​
436 self.stub_get_config.return_value = self.stub_config 435​ self.stub_get_config.return_value = self.stub_config
437 monkeypatch.setattr(rpm_utils, 'install', self.install_stub) 436​ monkeypatch.setattr(rpm_utils, 'install', self.install_stub)
438 monkeypatch.setattr(rpm_utils, 'is_installed', self.is_installed_stub) ​
439 monkeypatch.setattr(config, 'get_config', self.stub_get_config) 437​ monkeypatch.setattr(config, 'get_config', self.stub_get_config)
440 438​
441 self.test_executor = executor.Executor({}, {'local': False, 'ssh': None, 'uuid': ''}) 439​ self.test_executor = executor.Executor({}, {'local': False, 'ssh': None, 'uuid': ''})
442 440​
443 def test_devel_profile_installed(self): 441​ def test_devel_profile(self):
444 self.is_installed_stub.return_value = True ​
445 self.stub_config.profile = 'devel' 442​ self.stub_config.profile = 'devel'
446 ​
447 self.test_executor._prepare_task() 443​ self.test_executor._prepare_task()
448 444​ assert len(self.install_stub.calls()) == 0
449 assert len(self.install_stub.calls()) == 0 and len(self.is_installed_stub.calls()) == 1 ​
450 ​
451 def test_devel_profile_not_installed(self): ​
452 self.is_installed_stub.return_value = False ​
453 self.stub_config.profile = 'devel' ​
454 ​
455 with pytest.raises(exc.TaskotronError): ​
456 self.test_executor._prepare_task() ​
457 ​
458 assert len(self.install_stub.calls()) == 0 and len(self.is_installed_stub.calls()) == 1 ​
459 445​
460 def test_prod_profile(self): 446​ def test_prod_profile(self):
461 self.is_installed_stub.return_value = False ​
462 self.stub_config.profile = 'production' 447​ self.stub_config.profile = 'production'
463 ​
464 self.test_executor._prepare_task() 448​ self.test_executor._prepare_task()
465 449​ assert len(self.install_stub.calls()) == 1
466 assert len(self.install_stub.calls()) == 1 and len(self.is_installed_stub.calls()) == 1 ​
View Options

testing/test_rpm_utils.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2014, Red Hat, Inc. 2​# Copyright 2009-2014, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6'''Unit tests for libtaskotron.ext.fedora.rpm_utils''' 6​'''Unit tests for libtaskotron.ext.fedora.rpm_utils'''
7 7​
8import pytest 8​import pytest
9import subprocess 9​import subprocess
10 10​
11from libtaskotron.ext.fedora.rpm_utils import (cmpNEVR, dnf_cache_available, get_dist_tag, install, 11​from libtaskotron.ext.fedora.rpm_utils import (cmpNEVR, dnf_cache_available, get_dist_tag, install,
12 is_installed, rpmformat) 12​ rpmformat)
13from libtaskotron.ext.fedora import rpm_utils 13​from libtaskotron.ext.fedora import rpm_utils
14from libtaskotron import exceptions as exc 14​from libtaskotron import exceptions as exc
15from libtaskotron import os_utils 15​from libtaskotron import os_utils
16 16​
17import mock 17​import mock
18 18​
19 19​
20class TestRpmformat: 20​class TestRpmformat:
▲ Show 20 Lines • Show All 200 Lines • ▼ Show 20 Line(s)220 def test_run_twice_second_ok(self):
221 '''If installation is tried twice and second one succeeds, the whole operation should 221​ '''If installation is tried twice and second one succeeds, the whole operation should
222 succeed.''' 222​ succeed.'''
223 self.mock_check_output.side_effect = (self.err, mock.DEFAULT) 223​ self.mock_check_output.side_effect = (self.err, mock.DEFAULT)
224 install(['foo']) 224​ install(['foo'])
225 assert self.mock_check_output.call_count == 2 225​ assert self.mock_check_output.call_count == 2
226 226​
227 227​
228@pytest.mark.usefixtures('setup') 228​@pytest.mark.usefixtures('setup')
229class TestIsInstalled(object): ​
230 '''Test rpm_utils.is_installed()''' ​
231 ​
232 @pytest.fixture ​
233 def setup(self, monkeypatch): ​
234 self.mock_is_root = mock.Mock(return_value=True) ​
235 monkeypatch.setattr(os_utils, 'is_root', self.mock_is_root) ​
236 self.mock_has_sudo = mock.Mock(return_value=True) ​
237 monkeypatch.setattr(os_utils, 'has_sudo', self.mock_has_sudo) ​
238 ​
239 self.mock_check_output = mock.Mock(return_value='') ​
240 monkeypatch.setattr(subprocess, 'check_output', self.mock_check_output) ​
241 self.err = subprocess.CalledProcessError(1, 'cmd', output='') ​
242 ​
243 def test_installed(self): ​
244 assert is_installed(['foo']) is True ​
245 ​
246 def test_not_installed(self): ​
247 self.mock_check_output.side_effect = self.err ​
248 assert is_installed(['foo']) is False ​
249 ​
250 def test_no_permissions_is_ok(self): ​
251 self.mock_is_root.return_value = False ​
252 self.mock_has_sudo.return_value = False ​
253 assert is_installed(['foo']) is True ​
254 ​
255 def test_dont_add_sudo(self): ​
256 is_installed(['foo']) ​
257 assert 'sudo' not in self.mock_check_output.call_args[0][0] ​
258 ​
259 def test_no_pkgs(self): ​
260 assert is_installed([]) is True ​
261 assert self.mock_check_output.call_count == 0 ​
262 ​
263 def test_cacheonly(self): ​
264 is_installed(['foo']) ​
265 assert '--cacheonly' in self.mock_check_output.call_args_list[0][0][0] ​
266 ​
267 ​
268@pytest.mark.usefixtures('setup') ​
269class TestDnfCacheAvailable(object): 229​class TestDnfCacheAvailable(object):
270 '''Test rpm_utils.dnf_cache_available()''' 230​ '''Test rpm_utils.dnf_cache_available()'''
271 231​
272 @pytest.fixture 232​ @pytest.fixture
273 def setup(self, monkeypatch): 233​ def setup(self, monkeypatch):
274 self.mock_is_root = mock.Mock(return_value=True) 234​ self.mock_is_root = mock.Mock(return_value=True)
275 monkeypatch.setattr(os_utils, 'is_root', self.mock_is_root) 235​ monkeypatch.setattr(os_utils, 'is_root', self.mock_is_root)
276 self.mock_has_sudo = mock.Mock(return_value=True) 236​ self.mock_has_sudo = mock.Mock(return_value=True)
▲ Show 20 Lines • Show All 44 Lines • Show Last 20 Lines