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

FreeBSD: get_tmp_exec_path() still fails #4193

Closed
igalic opened this issue Jun 20, 2023 · 4 comments · Fixed by #4222
Closed

FreeBSD: get_tmp_exec_path() still fails #4193

igalic opened this issue Jun 20, 2023 · 4 comments · Fixed by #4222
Labels
bug Something isn't working correctly incomplete Action required by submitter

Comments

@igalic
Copy link
Collaborator

igalic commented Jun 20, 2023

Bug report

This is a clean boot on Vultr (with Vultr as the only datasource)

2023-06-15 17:50:18,413 - handlers.py[DEBUG]: finish: init-local/search-Vultr: FAIL: no local data found from DataSourceVultr
2023-06-15 17:50:18,413 - util.py[WARNING]: Getting data from <class 'cloudinit.sources.DataSourceVultr.DataSourceVultr'> failed
2023-06-15 17:50:18,413 - util.py[DEBUG]: Getting data from <class 'cloudinit.sources.DataSourceVultr.DataSourceVultr'> failed
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/__init__.py", line 1009, in find_source
    if s.update_metadata_if_supported(
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/__init__.py", line 895, in update_metadata_if_supported
    result = self.get_data()
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/__init__.py", line 437, in get_data
    return_value = self._check_and_get_data()
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/__init__.py", line 366, in _check_and_get_data
    return self._get_data()
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/DataSourceVultr.py", line 56, in _get_data
    self.metadata = self.get_metadata()
  File "/usr/local/lib/python3.9/site-packages/cloudinit/sources/DataSourceVultr.py", line 98, in get_metadata
    tmp_dir=self.distro.get_tmp_exec_path(),
  File "/usr/local/lib/python3.9/site-packages/cloudinit/distros/__init__.py", line 989, in get_tmp_exec_path
    if not util.has_mount_opt(tmp_dir, "noexec"):
  File "/usr/local/lib/python3.9/site-packages/cloudinit/util.py", line 2748, in has_mount_opt
    *_, mnt_opts = get_mount_info(path, get_mnt_opts=True)
TypeError: cannot unpack non-iterable NoneType object
2023-06-15 17:50:18,421 - main.py[DEBUG]: No local datasource found

Even after #2146, this still fails.
As explained in that PR, on Linux with parse_mount_info(), we're allowed to return a parent mount for a non-existent directory / mountpoint.
On BSD, or with parse_mount() we are not allowed to do that.
After adding options parsing to parse_mount(), due to this limitation, get_tmp_exec_path() still fails with the same kind of exception on FreeBSD.

Steps to reproduce the problem

on FreeBSD,

meena@defbix ~/s/cloud-init (main)> sudo -H ipython
In [1]: from cloudinit.distros import freebsd
In [2]: f = freebsd.Distro('freebsd', {}, {})
In [3]: f.get_tmp_exec_path()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[3], line 1
----> 1 f.get_tmp_exec_path()

File /home/meena/src/cloud-init/cloudinit/distros/__init__.py:989, in Distro.get_tmp_exec_path(self)
    987 def get_tmp_exec_path(self) -> str:
    988     tmp_dir = temp_utils.get_tmp_ancestor(needs_exe=True)
--> 989     if not util.has_mount_opt(tmp_dir, "noexec"):
    990         return tmp_dir
    991     return os.path.join(self.usr_lib_exec, "cloud-init", "clouddir")

File /home/meena/src/cloud-init/cloudinit/util.py:2748, in has_mount_opt(path, opt)
   2747 def has_mount_opt(path, opt: str) -> bool:
-> 2748     *_, mnt_opts = get_mount_info(path, get_mnt_opts=True)
   2749     return opt in mnt_opts.split(",")

TypeError: cannot unpack non-iterable NoneType object

In [4]: 

Environment details

  • Cloud-init version: HEAD
  • Operating System Distribution: FreeBSD
  • Cloud provider, platform or installer type: Vultr
@igalic igalic added bug Something isn't working correctly new An issue that still needs triage labels Jun 20, 2023
@igalic
Copy link
Collaborator Author

igalic commented Jun 20, 2023

cc @CalvoM @holmanb

@aciba90
Copy link
Contributor

aciba90 commented Jun 22, 2023

Thanks @igalic for reporting this. Could you please attach the full logs?

@aciba90 aciba90 added incomplete Action required by submitter and removed new An issue that still needs triage labels Jun 22, 2023
@igalic
Copy link
Collaborator Author

igalic commented Jul 4, 2023

full logs: cloud-init.tar.gz

n.b.: The code, as-is, right now, is wrong. Here's the mount output from my test / dev machine:

% mount
meena@defbix ~> mount
zroot/ROOT/PkgBase on / (zfs, local, noatime, nfsv4acls)
devfs on /dev (devfs)
/dev/gpt/efiboot0 on /boot/efi (msdosfs, local)
zroot/home on /home (zfs, local, noatime, nfsv4acls)
zroot/poudriere on /poudriere (zfs, local, noatime, nfsv4acls)
zroot/var/log on /var/log (zfs, local, noatime, noexec, nosuid, nfsv4acls)
zroot/var/mail on /var/mail (zfs, local, nfsv4acls)
zroot/tmp on /tmp (zfs, local, noatime, nosuid, nfsv4acls)
zroot on /zroot (zfs, local, noatime, nfsv4acls)
zroot/usr/src on /usr/src (zfs, local, noatime, nfsv4acls)
zroot/var/crash on /var/crash (zfs, local, noatime, noexec, nosuid, nfsv4acls)
zroot/var/tmp on /var/tmp (zfs, local, noatime, nosuid, nfsv4acls)
zroot/usr/ports on /usr/ports (zfs, local, noatime, nosuid, nfsv4acls)
zroot/var/audit on /var/audit (zfs, local, noatime, noexec, nosuid, nfsv4acls)

and here's a series of attempts to get something sensible out of the code:

In [27]: util.parse_mount("/", get_mnt_opts=True)
Out[27]: ('zroot/ROOT/PkgBase', 'zfs', '/', 'local,noatime,nfsv4acls')

In [28]: util.parse_mount("/var", get_mnt_opts=True)
Out[28]: ('zroot/var/audit', 'zfs', '/', 'local,noatime,nfsv4acls')

In [29]: util.parse_mount("/var/tmp", get_mnt_opts=True)
Out[29]: ('zroot/var/tmp', 'zfs', '/var/tmp', 'local,noatime,nosuid,nfsv4acls')

In [25]: tmp_dir
Out[25]: '/var/tmp/cloud-init'

In [26]: util.parse_mount(tmp_dir, get_mnt_opts=True)

why does querying /var return zroot/var/audit with a mountpoint of /??
given that /var doesn't exist, the mountpoint / is correct, but the "device" zroot/var/audit is bogus.

next, we have the correct result for /var/tmp.
but the query for /var/tmp/cloud-init doesn't return anything at all… maybe because it's a level too deep?
I don't know. yet.

igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
@igalic
Copy link
Collaborator Author

igalic commented Jul 4, 2023

why does querying /var return zroot/var/audit with a mountpoint of /?? given that /var doesn't exist, the mountpoint / is correct, but the "device" zroot/var/audit is bogus.

in #4222 i added a match_devpth to make sure we track the correct devpth.

next, we have the correct result for /var/tmp. but the query for /var/tmp/cloud-init doesn't return anything at all… maybe because it's a level too deep? I don't know. yet.

there's two reasons for this: On UFS where were trying to resolve the devpth:

cloud-init/cloudinit/util.py

Lines 2645 to 2656 in c68305a

def get_freebsd_devpth(path):
(result, err) = subp.subp(["mount", "-p", path], rcs=[0, 1])
if len(err):
# find a path if the input is not a mounting point
(mnt_list, err) = subp.subp(["mount", "-p"])
path_found = get_path_dev_freebsd(path, mnt_list)
if path_found is None:
return None
result = path_found
ret = result.split()
label_part = find_freebsd_part(ret[0])
return "/dev/" + label_part

which then called

cloud-init/cloudinit/util.py

Lines 2635 to 2642 in c68305a

def get_path_dev_freebsd(path, mnt_list):
path_found = None
for line in mnt_list.split("\n"):
items = line.split()
if len(items) > 2 and os.path.exists(items[1] + path):
path_found = line
break
return path_found

This is silly for our use-case. So we're now calling get_freebsd_devpth() with the devpth instead of the path.
That drops a lot of unnecessary work that we're already doing in parse_mount()

igalic added a commit to igalic/cloud-init that referenced this issue Jul 4, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes: canonicalGH-4193

Sponsored by: The FreeBSD Foundation
TheRealFalcon pushed a commit that referenced this issue Jul 5, 2023
This puts `util.mount_parse()` now on-par with
`util.mount_parse_info()`.
Fix two now failing tests to clarify the new behaviour.

Fixes GH-4193

Sponsored by: The FreeBSD Foundation
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Jul 7, 2023
* We now have a ds-identify rc script. This will speed up config-free booting.
* canonical/cloud-init#4193 is now fixed, which gives
  us a clean boot on Vultr
* A previous fix was attempted with
  canonical/cloud-init#2146 but
  canonical/cloud-init#4222 now fixes it properly

PR:		272391
Reported by:	freebsd@igalic.co
Sponsored by:	The FreeBSD Foundation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly incomplete Action required by submitter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants