Fixed subp import

[sshedi]

Made minor code improvements in getting subp module

Signed-off-by: Shreenidhi Shedi [email protected]

[jfroche]

You are removing the test if hasattr(subp, 'subp'): which verifies if subp package has a subp function.
We are running Ubuntu 18.04 with cloud-init 20.2-45 and we have this:

>>> from cloudinit import subp
>>> subp.subp
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'cloudinit.subp' has no attribute 'subp'
>>> from cloudinit import util
>>> util.subp
<function subp at 0x7fae4157a730>

So we have a subp package but no subp function, so we should be using cloudinit.util

[sshedi]

@jfroche thanks, I missed it. Will send an update soon. It's weird what's been done in cloud-init, not moving a module fully in one shot. Simple added to further complications.

[yvespp]

Maybe this would be cleaner/easier to read:

try:
    from cloudinit.subp import subp, ProcessExecutionError
except ImportError:
    from cloudinit.util import subp, ProcessExecutionError

and call the methods directly (remove all subp_module references)

[skyscooby]

Could we prioritise merging one of these fixes please? master is broken on latest ubuntu 18.04 cloud-init version at the moment.

[skyscooby]

It would really help the users of this module to make #2 happen.. remember everyone's OS moves forward as they yum/apt update their code.. this module just sits in the python directory unmaintained and gets stale.. either this project needs to start building deb/rpm and host a repository so fixes can be released... or we need to get this code into the cloud-init project proper so they can deal with these module changes directly before releasing.

Anyone relying on this module at the moment who updates their cloud-init package is in for serious pain on their next reboot.

[akutz]

We are trying. The problem is fixing it so that it works across all the various distros (see my earlier note about setting up automated testing pipelines).

I try very much to make this DS backwards compatible so that it will work with all versions of CI on all distros. It is just challenging to cover everything.

I merged two PRs already, but each time new issues were identified issues on additional other distro/cloud-init combinations.

So this time I’m making sure we have are not merging a fix that just breaks something else.

[sshedi]

Maybe this would be cleaner/easier to read:

try:
    from cloudinit.subp import subp, ProcessExecutionError
except ImportError:
    from cloudinit.util import subp, ProcessExecutionError

and call the methods directly (remove all subp_module references)

👍 will update my PR, thanks for the input

[sshedi]

@yvespp @akutz @jfroche Please give it a try, let's finish this one.

[yvespp]

Looks got to me now, tested it with Ubuntu 20.04

[LinAnt]

Can confirm this works with Ubuntu 20.04 on vSphere 6.7u3

[angelbarrera92]

One more here to confirm it works with Ubuntu 18.04. Is there any plan to release it?

[nickvth]

@akutz and @yvespp can you review this change and merge if possible?
We are waiting for this fix.

[yvespp]

@akutz and @yvespp can you review this change and merge if possible?
We are waiting for this fix.

I don't have merge permission.

[akutz]

@akutz and @yvespp can you review this change and merge if possible?
We are waiting for this fix.

I don't have merge permission.

I was waiting to ensure this was properly tested this time. Merged.

[nickvth]

@akutz and @yvespp can you review this change and merge if possible?
We are waiting for this fix.

I don't have merge permission.

I was waiting to ensure this was properly tested this time. Merged.

Thanks @akutz @yvespp @sshedi

[sshedi]

Thanks all for getting involved in this and all the valuable suggestions. Liked it. It was a good learning experience.