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

update fetch_externally_routable_ips to use any network device #314

Merged
merged 8 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 51 additions & 9 deletions ducktape/cluster/linux_remoteaccount.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

from ducktape.cluster.cluster_spec import LINUX
from ducktape.cluster.remoteaccount import RemoteAccount
from ducktape.cluster.remoteaccount import RemoteAccount, RemoteAccountError


class LinuxRemoteAccount(RemoteAccount):
Expand All @@ -30,11 +30,53 @@ def local(self):
This is an imperfect heuristic, but should work for simple local testing."""
return self.hostname == "localhost" and self.user is None and self.ssh_config is None

def fetch_externally_routable_ip(self, is_aws):
if is_aws:
cmd = "/sbin/ifconfig eth0 "
else:
cmd = "/sbin/ifconfig eth1 "
cmd += r"| grep 'inet ' | tail -n 1 | egrep -o '[0-9\.]+' | head -n 1 2>&1"
output = "".join(self.ssh_capture(cmd))
return output.strip()
def get_network_devices(self):
"""
Utility to get all network devices on a linux account
"""
return [
device
for device in self.sftp_client.listdir('/sys/class/net')
]

def get_external_accessible_network_devices(self):
"""
gets the subset of devices accessible through an external conenction
"""
return [
device
for device in self.get_network_devices()
if device != 'lo' # do not include local device
and ("eth" in device or "ens" in device) # filter out other devices
]

# deprecated, please use the self.externally_routable_ip that is set in your cluster,
# not explicitly deprecating it as it's used by vagrant cluster
def fetch_externally_routable_ip(self, is_aws=None):
if is_aws is not None:
self.logger.warning("fetch_externally_routable_ip: is_aws is a deprecated flag, and does nothing")

devices = self.get_external_accessible_network_devices()

self.logger.debug("found devices: {}".format(devices))

if not devices:
raise RemoteAccountError("Couldn't find any network devices")

fmt_cmd = (
"/sbin/ifconfig {device} | "
"grep 'inet ' | "
"tail -n 1 | "
r"egrep -o '[0-9\.]+' | "
"head -n 1 2>&1"
)

ips = [
imcdo marked this conversation as resolved.
Show resolved Hide resolved
"".join(
self.ssh_capture(fmt_cmd.format(device=device))
).strip()
for device in devices
]
self.logger.debug("found ips: {}".format(ips))
self.logger.debug("returning the first ip found")
return next(iter(ips))
imcdo marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 1 addition & 17 deletions ducktape/cluster/vagrant.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class VagrantCluster(JsonCluster):
"""

def __init__(self, *args, **kwargs):
self._is_aws = None
is_read_from_file = False
self.ssh_exception_checks = kwargs.get("ssh_exception_checks")
cluster_file = kwargs.get("cluster_file")
Expand Down Expand Up @@ -83,7 +82,7 @@ def _get_nodes_from_vagrant(self):
account = None
try:
account = JsonCluster.make_remote_account(ssh_config, ssh_exception_checks=self.ssh_exception_checks)
externally_routable_ip = account.fetch_externally_routable_ip(self.is_aws)
externally_routable_ip = account.fetch_externally_routable_ip()
finally:
if account:
account.close()
Expand All @@ -102,18 +101,3 @@ def _vagrant_ssh_config(self):
# Force to text mode in py2/3 compatible way
universal_newlines=True).communicate()
return ssh_config_info, error

@property
def is_aws(self):
"""Heuristic to detect whether the slave nodes are local or aws.

Return true if they are running on aws.
"""
if self._is_aws is None:
proc = subprocess.Popen("vagrant status", shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
close_fds=True,
# Force to text mode in py2/3 compatible way
universal_newlines=True)
output, _ = proc.communicate()
self._is_aws = output.find("aws") >= 0
return self._is_aws
5 changes: 1 addition & 4 deletions ducktape/cluster/windows_remoteaccount.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,7 @@ def winrm_client(self):

return self._winrm_client

def fetch_externally_routable_ip(self, is_aws):
if not is_aws:
raise NotImplementedError("Windows is only supported in AWS.")

def fetch_externally_routable_ip(self, is_aws=None):
Copy link
Member

Choose a reason for hiding this comment

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

So do we support windows in aws now?

Copy link
Member Author

Choose a reason for hiding this comment

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

in the past we only supported windows in aws for vagrant clusters, whats not made clear is this code path is only executed from the vagrant cluster file, so its not a great representation of whats actually supported. This pr makes things a bit more generic.

# EC2 windows machines aren't given an externally routable IP. Use the hostname instead.
return self.ssh_config.hostname

Expand Down
3 changes: 1 addition & 2 deletions tests/cluster/check_vagrant.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ def teardown_method(self, _):

def _set_monkeypatch_attr(self, monkeypatch):
monkeypatch.setattr("ducktape.cluster.vagrant.VagrantCluster._vagrant_ssh_config", lambda vc: (TWO_HOSTS, None))
monkeypatch.setattr("ducktape.cluster.vagrant.VagrantCluster.is_aws", lambda vc: False)
monkeypatch.setattr(
"ducktape.cluster.linux_remoteaccount.LinuxRemoteAccount.fetch_externally_routable_ip",
lambda vc, node_account: "127.0.0.1")
lambda vc: "127.0.0.1")

def check_pickleable(self, monkeypatch):
self._set_monkeypatch_attr(monkeypatch)
Expand Down