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 problem when api_host set in config. #183

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zhangdaolong
Copy link

when api_host is config ,command gwcli may be hold.

gwcli.8 Outdated Show resolved Hide resolved
@@ -2621,6 +2621,8 @@ def target_ready(gateway_list):
"summary": ''}

for gw in gateway_list:
if gw == "localhost"
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a ":" at the end of the line.

if gw == "localhost":

Copy link
Contributor

Choose a reason for hiding this comment

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

One question just to make sure I understood the localhost check. Is it for cases like rbd-target-api:clientauth():

gateways.insert(0, 'localhost')
The check you are adding allows callers to just use localhost instead of doing

gateways.insert(0, settings.config.api_host)

right?

Copy link
Author

@zhangdaolong zhangdaolong Apr 3, 2020

Choose a reason for hiding this comment

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

One question just to make sure I understood the localhost check. Is it for cases like rbd-target-api:clientauth():

gateways.insert(0, 'localhost')
The check you are adding allows callers to just use localhost instead of doing

gateways.insert(0, settings.config.api_host)

right?

yes ,that's what I thought.

@@ -2673,6 +2675,9 @@ def call_api(gateway_list, endpoint, element, http_method='put', api_vars=None):
logger.debug("gateway update order is {}".format(','.join(gateway_list)))

for gw in gateway_list:
if gw == "localhost"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

"api_host": StrSetting("api_host", "::"),
# if the api_host has a colon, you must wrap it with square brackets.
# i.e:[::]
"api_host": StrSetting("api_host", "localhost"),
"api_port": IntSetting("api_port", 1, 65535, 5000),
"api_secure": BoolSetting("api_secure", True),
"api_ssl_verify": BoolSetting("api_ssl_verify", False),
Copy link
Contributor

Choose a reason for hiding this comment

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

You will need to add api_host to exclude_from_hash in ceph_iscsi_config/setting.py, because that value can be different on each node.

Copy link
Author

Choose a reason for hiding this comment

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

ok i add it

@zhangdaolong zhangdaolong force-pushed the fix-api-block branch 2 times, most recently from 75b7fb4 to acaf08a Compare April 3, 2020 02:56
@zhangdaolong
Copy link
Author

@mikechristie
I have revised it, please take a review, thank you

"api_host": StrSetting("api_host", "::"),
# if the api_host has a colon, you must wrap it with square brackets.
# i.e:[::]
"api_host": StrSetting("api_host", "localhost"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey

Sorry for the delay on this, but I just can't get it to work and I'm not sure if its my setup or the patch.

  1. If I use the defaults set in the patch, and I do not override anything, then I get "Connection refused" errors when trying to do any command to the the daemon.

Does using the default values work for you? If so, when yo do:

ping localhost

do you see localhost resolving to ::1 or 127.0.1.

  1. If you use "[::]" like documented in the code comments above, then rbd-target-api crashes.

  2. If I use "::" as the default above or in iscsi-gateway.cfg for the api_host value it works ok for me then.

Copy link
Author

@zhangdaolong zhangdaolong Apr 17, 2020

Choose a reason for hiding this comment

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

Hey

Sorry for the delay on this, but I just can't get it to work and I'm not sure if its my setup or the patch.

1. If I use the defaults set in the patch, and I do not override anything, then I get "Connection refused" errors when trying to do any command to the the daemon.

Does using the default values work for you? If so, when yo do:

ping localhost

do you see localhost resolving to ::1 or 127.0.1.

It is better to use ip such as the host name, because there is no mapping configured for localhost。
If localhost host mapping is configured on /etc/hosts, there should be no problem。
ie:/etc/hosts
[root@ceph ~]# cat /etc/hosts
127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4
::1 localhost localhost.localdomain localhost6 localhost6.localdomain6

[root@ceph ~]# ping localhost
PING localhost (127.0.0.1) 56(84) bytes of data.
64 bytes from localhost (127.0.0.1): icmp_seq=1 ttl=64 time=0.014 ms
^C
--- localhost ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.014/0.014/0.014/0.000 ms
[root@ceph ~]#

1. If you use "[::]" like documented in the code comments above, then rbd-target-api crashes.
2. If I use "::"  as the default above or in iscsi-gateway.cfg for the api_host value it works ok for me then.

flask does not support IP that contain [and] characters,so i change it
...........
app.run(host=settings.config.api_host.strip('[').strip(']'),
.........

I test it,rbd-target-api and gwcli ,it works for me

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.

2 participants