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

BSD: add dsidentify to early startup scripts #4182

Merged
merged 8 commits into from
Jun 21, 2023

Conversation

igalic
Copy link
Collaborator

@igalic igalic commented Jun 13, 2023

Proposed Commit Message

in #4159, we removed the artifically restricted datasource_list for BSD.
This can now lead in some cases to very long boot times.
In this patch we add an RC script for ds-identify, similarly to how it's
run in systemd's generator stage.
The script is added in such a way that it will run before cloudinitlocal,
but can easily be removed by people building custom images.

Additionally, the rc scripts are now templated. This makes it now easier
for ports / pkgsrc users to move cloud-init package from the standard
`$LOCALBASE` to another location.

Sponsored by: The FreeBSD Foundation

Additional Context

Partially discussed in #4180

Test Steps

if this patch reduces start-up time, we can count it as successful

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

Copy link
Collaborator Author

@igalic igalic left a comment

Choose a reason for hiding this comment

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

this is only the first commit of three and already there's lots to comment on here…

sysvinit/freebsd/dsidentify Outdated Show resolved Hide resolved
sysvinit/freebsd/dsidentify Outdated Show resolved Hide resolved
sysvinit/freebsd/dsidentify Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@igalic igalic left a comment

Choose a reason for hiding this comment

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

some day…

#!/bin/sh

# PROVIDE: dsidentify
# REQUIRE: mountcritlocal
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

once we are correctly using paths.run_dir / paths.get_runpath() in the python code-base, we can start thinking about how to squeeze that into ds-identify itself, and then!, then we can depend on var_run instead of mountcritlocal here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this would be useful as a comment in the code? If so, we could use a template comment to avoid render it in the output file.

@igalic igalic marked this pull request as ready for review June 19, 2023 13:57
@aciba90 aciba90 self-assigned this Jun 20, 2023
Copy link
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

#!/bin/sh

# PROVIDE: dsidentify
# REQUIRE: mountcritlocal
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this would be useful as a comment in the code? If so, we could use a template comment to avoid render it in the output file.

in canonical#4159, we removed the artifically restricted datasource_list for BSD.
This can now lead in some cases to very long boot times.
In this patch we add an RC script for ds-identify, similarly to how it's
run in systemd's generator stage.

Sponsored by: The FreeBSD Foundation
- generalize --option parsing hack in setup.py
- extract --prefix from setup.py if it was passed
- extend cloudinit.tempalter and tools/render-cloudcfg to take a prefix
and add tests to validate that it works
this allows for easy relocation of LOCALBASE without having patching by
porters!
this makes it easy to just remove the rc script in scenarios where Image
builders know exactly what cloud they are targetting.
into the code, not just on GitHub.
@aciba90 aciba90 merged commit 529e108 into canonical:main Jun 21, 2023
@igalic igalic deleted the bsd/ds-identify-rc branch June 21, 2023 13:50
@igalic
Copy link
Collaborator Author

igalic commented Jun 28, 2023

something very odd is happening when i try to package this:

% python3 ./setup.py install --distro=freebsd --prefix=/usr/local --init-system=sysvinit_freebsd --root=/tmp/tmp.phnOvze98E
…
copying RENDERED_TEMPD6jnfs41_/cloudinitloca -> /tmp/tmp.phnOvze98E/usr/local/etc/rc.d
copying RENDERED_TEMPDt9kv1mz1/cloudfina -> /tmp/tmp.phnOvze98E/usr/local/etc/rc.d
copying RENDERED_TEMPD9k9k2vog/dsidentify -> /tmp/tmp.phnOvze98E/usr/local/etc/rc.d
copying RENDERED_TEMPD1k4nrvrd/cloudconfig -> /tmp/tmp.phnOvze98E/usr/local/etc/rc.d
copying RENDERED_TEMPDpdicbnjj/cloudini -> /tmp/tmp.phnOvze98E/usr/local/etc/rc.d

looks like .rstrip(".tmpl") does more than expected: https://docs.python.org/3/library/stdtypes.html#str.rstrip

@igalic
Copy link
Collaborator Author

igalic commented Jun 28, 2023

#4209

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