Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework the NetworkManagerUpdateClientId actor #69

Closed
wants to merge 1 commit into from

Conversation

bengal
Copy link
Contributor

@bengal bengal commented Feb 12, 2019

Move the external tool code into a library and convert it to use D-Bus
instead of the GObject introspection library. The main reason for this
is to avoid the large dependency chain needed by python-gobject. Also,
add the python-dbus dependency.

@bengal
Copy link
Contributor Author

bengal commented Feb 12, 2019

I could only test this locally using snactor. When I copy my actor code to the vagrant box and launch an upgrade, after the reboot leapp fails because the python-dbus dependency in RHEL8 is missing. Is there a way I can test a full upgrade with my code and with new dependencies?

@pirat89
Copy link
Member

pirat89 commented Feb 18, 2019

@bengal Hi Beniamino, yes it is. However in your case it looks like issue with CI so I will have to prepare builds manually. I will send you an email with intructions in several minutes.

@pirat89
Copy link
Member

pirat89 commented Feb 18, 2019

leapp-ci build

@pirat89
Copy link
Member

pirat89 commented Feb 18, 2019

From the CI:

17:13:28 17:13:28 Traceback (most recent call last):
17:13:28 17:13:28   File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
17:13:28 17:13:28     self.run()
17:13:28 17:13:28   File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run
17:13:28 17:13:28     self._target(*self._args, **self._kwargs)
17:13:28 17:13:28   File "/tmp/repo/tut/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 26, in inspect_actor
17:13:28 17:13:28     definition.load()
17:13:28 17:13:28   File "/tmp/repo/tut/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 135, in load
17:13:28 17:13:28     with self.injected_context():
17:13:28 17:13:28   File "/usr/lib64/python2.7/contextlib.py", line 17, in __enter__
17:13:28 17:13:28     return self.gen.next()
17:13:28 17:13:28   File "/tmp/repo/tut/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 245, in injected_context
17:13:28 17:13:28     map(lambda x: os.path.join(self._repo_dir, self.directory, x), self.libraries))
17:13:28 17:13:28   File "/tmp/repo/tut/lib/python2.7/site-packages/leapp/repository/loader.py", line 19, in library_loader
17:13:28 17:13:28     imported = importer.find_module(name).load_module(name)
17:13:28 17:13:28   File "/usr/lib64/python2.7/pkgutil.py", line 246, in load_module
17:13:28 17:13:28     mod = imp.load_module(fullname, self.file, self.filename, self.etc)
17:13:28 17:13:28   File "/tmp/repo/repos/system_upgrade/el7toel8/actors/networkmanagerupdateclientid/libraries/networkmanagerupdateclientid.py", line 1, in <module>
17:13:28 17:13:28     import dbus
17:13:28 17:13:28 ImportError: No module named dbus

Regarding the PR changes in spec file, probably you need to require such package for tests as well. You can do that by Makefile inside the actor directory with content like this:

install-deps:
    yum install python-dbus

See e.g. this one.

Move the external tool code into a library and convert it to use D-Bus
instead of the GObject introspection library. The main reason for this
is to avoid the large dependency chain needed by python-gobject. Also,
add the python-dbus dependency.
@@ -0,0 +1,66 @@
import dbus
Copy link
Member

@vinzenz vinzenz Feb 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that this in the ApplicationsPhase and I can't see a python2-dbus in el8 I wonder if will run into some trouble with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, I couldn't test it due to other issues I had with the upgrade.

So, everything that runs in ApplicationsPhase uses python2, right?

The actor needs to talk with NM to update connection profiles. The option available are:

  1. use D-Bus: we need python2-dbus, which seems not available in RHEL8

  2. use libnm through GObject-introspection: we need python2-gobject not available on RHEL8

  3. use nmcli commands: this is the worst option because it's inefficient and requires additional string parsing

As I understand it, we're left with 3), right? Or do you think in some ways we could use one of the other two?

Copy link
Member

@pirat89 pirat89 Mar 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bengal We will probably switch into the python3 after the UpgradePhase is done. So you would be able to (ok , you will have to - in case we will do that) use Python3 libraries in actor for ApplicationsPhase. But in that case, you need to wait for implementation. Not sure when it would be resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then I'll wait the switch to python 3 for the upgrade phase.

Obsoletes: leapp-repository-deps

Requires: dnf >= 4
Requires: pciutils
Requires: dbus-python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no such thing on el8. will this will work?

@bengal
Copy link
Contributor Author

bengal commented Mar 25, 2019

Closing for now, will reopen when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants