Navigation Menu

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

fix #60239 Caught exception in wait_for_fun utils in OpenStack #61918

Closed

Conversation

vdloo
Copy link
Contributor

@vdloo vdloo commented Apr 5, 2022

A PR that implements the fix from #60239

After upgrading to 3004.1 we noticed we could not create any nodes anymore on OpenStack. The fix proposed by @sblaisot in #60239 seems to fix the issue.

exception_001

Looking at an earlier commit like 47ecb7a I don't think this code is thoroughly unit-tested (tests/unit/cloud/clouds/test_openstack.py maybe?)

@vdloo vdloo requested a review from a team as a code owner April 5, 2022 15:14
@vdloo vdloo requested review from garethgreenaway and removed request for a team April 5, 2022 15:14
@vdloo vdloo force-pushed the fix-openstack-driver-wait-for-fun-utils branch from 95749bf to f9d5574 Compare April 5, 2022 15:23
@sblaisot
Copy link
Contributor

sblaisot commented Apr 5, 2022

Thanks fo proposing this PR which fixes a critical bug in salt-cloud openstack driver for years.

You probably also need to import salt.utils.cloud

@vdloo
Copy link
Contributor Author

vdloo commented Apr 6, 2022

some debugging information from doing:

def _get_ips(node, addr_type="public"):
    ret = []
    for _, interface in node.addresses.items():
        for addr in interface:
            try:
                __utils__["cloud.is_public_ip"]
            except KeyError:
                import ipdb; ipdb.set_trace()
            if addr_type in ("floating", "fixed") and addr_type == addr.get(
                "OS-EXT-IPS:type"
            ):
                ret.append(addr["addr"])
            elif addr_type == "public" and __utils__["cloud.is_public_ip"](
                addr["addr"]
            ):
                ret.append(addr["addr"])
            elif addr_type == "private" and not __utils__["cloud.is_public_ip"](
                addr["addr"]
            ):
                ret.append(addr["addr"])
    return ret

looks like salt.utils.cloud is already available in this scope so import salt.utils.cloud is not needed:

ipdb> __utils__['cloud.is_public_ip']                                                          
*** KeyError: '__utils__'
ipdb> salt.utils.cloud.is_public_ip                                                            
<function is_public_ip at 0x7f219f07e680>
ipdb>

I don't know what populates this __utils__ variable but what's interesting is that not in all invocations of this function the 'cloud.is_public_ip' does not exist. An earlier invocation in the process shows:

py(444)_get_ips()
    443             import ipdb; ipdb.set_trace()
--> 444             if addr_type in ("floating", "fixed") and addr_type == addr.get(
    445                 "OS-EXT-IPS:type"

ipdb> __utils__                                                                                
<salt.loader.context.NamedLoaderContext object at 0x7f5afabe2e90>
ipdb> __utils__['cloud.is_public_ip']                                                          
<LoadedFunc name='cloud.is_public_ip'>
ipdb>                                                                                          

@aef mentions in #60239 that:

It seems __utils__ is unavailable as soon as __query_node(vm_) is run through cloud.wait_for_fun. Seems like it loses context in a threadpool or something like this.

the exact error happens here:

salt/cloud/clouds/openstack.py(452)_get_ips()
    451                 ret.append(addr["addr"])
--> 452             elif addr_type == "private" and not __utils__["cloud.is_public_ip"](
    453                 addr["addr"]

ipdb>                                                                                          
KeyError: '__utils__'

@waynew
Copy link
Contributor

waynew commented May 17, 2022

Got some tests started here:
https://github.com/waynew/salt/pull/new/tc/add-starter-tests-for-61918
There's a bit to do to finish them up, but that should be a good jumping off point 👍

@dwoz
Copy link
Contributor

dwoz commented Dec 12, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned help-wanted Community help is needed to resolve this needs-changelog-file Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants