-
Notifications
You must be signed in to change notification settings - Fork 52
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
Extract and update firstboot for better bootc compatibility #2473
base: main
Are you sure you want to change the base?
Conversation
6782d9a
to
268fea7
Compare
src/constants.ts
Outdated
@@ -220,7 +220,7 @@ export const EPEL_9_REPO_DEFINITION = { | |||
|
|||
export const DEBOUNCED_SEARCH_WAIT_TIME = 500; | |||
export const UNIQUE_VALIDATION_DELAY = 300; | |||
export const FIRST_BOOT_SERVICE_DATA = | |||
export const FIRST_BOOT_SERVICE_DATA = 'W1VuaXRdCkRlc2NyaXB0aW9uPUN1c3RvbSBmaXJzdCBib290IHNjcmlwdApDb25kaXRpb25QYXRoRXhpc3RzPSEvdmFyL2xvY2FsLy5maXJzdGJvb3QKV2FudHM9bmV0d29yay1vbmxpbmUudGFyZ2V0CkFmdGVyPW5ldHdvcmstb25saW5lLnRhcmdldApBZnRlcj1vc2J1aWxkLWZpcnN0LWJvb3Quc2VydmljZQoKW1NlcnZpY2VdClR5cGU9b25lc2hvdApFeGVjU3RhcnQ9L3Vzci9sb2NhbC9zYmluL2N1c3RvbS1maXJzdC1ib290CkV4ZWNTdGFydFBvc3Q9L3Vzci9iaW4vdG91Y2ggL3Zhci9sb2NhbC8uZmlyc3Rib290ClJlbWFpbkFmdGVyRXhpdD15ZXMKCltJbnN0YWxsXQpXYW50ZWRCeT1tdWx0aS11c2VyLnRhcmdldAo='; | |||
'W1VuaXRdCkRlc2NyaXB0aW9uPVJ1biBmaXJzdCBib290IHNjcmlwdApDb25kaXRpb25QYXRoRXhpc3RzPS91c3IvbG9jYWwvc2Jpbi9jdXN0b20tZmlyc3QtYm9vdApXYW50cz1uZXR3b3JrLW9ubGluZS50YXJnZXQKQWZ0ZXI9bmV0d29yay1vbmxpbmUudGFyZ2V0CkFmdGVyPW9zYnVpbGQtZmlyc3QtYm9vdC5zZXJ2aWNlCgpbU2VydmljZV0KVHlwZT1vbmVzaG90CkV4ZWNTdGFydD0vdXNyL2xvY2FsL3NiaW4vY3VzdG9tLWZpcnN0LWJvb3QKRXhlY1N0YXJ0UG9zdD1tdiAvdXNyL2xvY2FsL3NiaW4vY3VzdG9tLWZpcnN0LWJvb3QgL3Vzci9sb2NhbC9zYmluL2N1c3RvbS1maXJzdC1ib290LmRvbmUKCltJbnN0YWxsXQpXYW50ZWRCeT1tdWx0aS11c2VyLnRhcmdldAo='; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's here twice now - it seems the make doesn't replace the string properly 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right the original version is on two lines.
FISTBOOT_SERVICE := $(shell base64 -w0 < aux/custom-first-boot.service) | ||
|
||
src/constants.ts: aux/custom-first-boot.service | ||
sed -i "s/.*FIRST_BOOT_SERVICE_DATA.*/export const FIRST_BOOT_SERVICE_DATA = '$(FISTBOOT_SERVICE)';/" $@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a multiline string first is the variable and second line the actual string :/
We'll need bit more magic here, time for awk
? xD
Also, npm is kinda Makefile in js project, but that's secondary, happy to convert it for you once we're happy with it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do what you want, tho, curious if I would write a simple bash script would you still recommend to replace it with npm
? I mean, make
is older than bash
sure (like 50 vs 35 years) but what makes people to be so cautious about it. It is a great tool for building and still relevant in 2024. Let me create few targets so make
calls npm
and the tools could live peacefully. :-D
But if you want consistency go for it, no hard feelings. Beat my three lines! 😈
30a7777
to
af684cc
Compare
af684cc
to
342ed08
Compare
Rebased. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #2473 +/- ##
==========================================
+ Coverage 75.71% 83.88% +8.17%
==========================================
Files 33 157 +124
Lines 597 17692 +17095
Branches 144 1747 +1603
==========================================
+ Hits 452 14841 +14389
- Misses 139 2830 +2691
- Partials 6 21 +15
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
342ed08
to
0692cd7
Compare
[Install] | ||
WantedBy=basic.target | ||
WantedBy=multi-user.target | ||
WantedBy=graphical.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a report that when target is changed to grahical.target
firstboot is disabled. This ensures it will always run.
[Unit] | ||
Description=Custom first boot script | ||
ConditionFileIsExecutable=/usr/local/sbin/custom-first-boot | ||
ConditionFirstBoot=yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found a better solution - there is a built-in firstboot support in systemd. So I am changing it to that:
https://www.freedesktop.org/software/systemd/man/systemd.unit
Additional Wants/Before/After were added below as recommended in the systemd man page.
I was made aware that |
Any progress here? :) this sounds like a good idea to get done :) |
Yeah on it on Monday, if not poke me hard on Slack. |
Fistboot does not appear to work on bootc deployments, this fixes it.
Introduced a
Makefile
for easier updates and git history. Could not figure out a good place for such files to proposingaux
as for auxiliary.