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

Rely less on earthly and more on pure docker instructions #1939

Closed
wants to merge 1 commit into from

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Oct 23, 2023

So they can easily be moved into the dockerfiles

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

So they can easily be moved into the dockerfiles

Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka requested a review from a team October 23, 2023 08:51
END
END
# Delete not needed initramfs files
RUN rm -rf /boot/initrd.img-* || true
Copy link
Member Author

Choose a reason for hiding this comment

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

always do and not fail if its not there, we dont care

RUN rm -rf /boot/initramfs-* || true
RUN --no-cache kernel=$(ls /lib/modules | head -n1) && depmod -a "${kernel}"
# Recreate initrd and link it to /boot/initrd
RUN --no-cache if [ -f "/usr/bin/dracut" ]; then kernel=$(ls /lib/modules | head -n1) && dracut -f "/boot/initrd-${kernel}" "${kernel}" && ln -sf "initrd-${kernel}" /boot/initrd;fi
Copy link
Member Author

Choose a reason for hiding this comment

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

check in time for initrd builder

RUN --no-cache if [ -f "/usr/bin/dracut" ]; then kernel=$(ls /lib/modules | head -n1) && dracut -f "/boot/initrd-${kernel}" "${kernel}" && ln -sf "initrd-${kernel}" /boot/initrd;fi
RUN --no-cache if [ -f "/sbin/mkinitfs" ]; then kernel=$(ls /lib/modules | head -n1) && mkinitfs -o /boot/initrd $kernel; fi

# Create a symlink to the kernel to /boot/vmlinuz
Copy link
Member Author

Choose a reason for hiding this comment

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

just do all the linking here if the sources exists

Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

nice! this should make the extraction to the dockerfile easier

Comment on lines +426 to +432
RUN --no-cache if [ -e "/boot/vmlinuz-lts" ]; then ln -sf /boot/vmlinuz-lts /boot/vmlinuz; fi
RUN --no-cache if [ -e "/boot/vmlinuz-rpi4" ]; then ln -sf /boot/vmlinuz-rpi4 /boot/vmlinuz; fi
RUN --no-cache if [ -e " /boot/Image" ]; then ln -sf /boot/Image /boot/vmlinuz; fi
RUN --no-cache if [ -e "/boot/zImage" ]; then ln -sf /boot/zImage /boot/vmlinuz; fi
RUN --no-cache kernel=$(ls /boot/zImage-* 2> /dev/null | head -n1) && if [ -e "$kernel" ]; then ln -sf "${kernel#/boot/}" /boot/vmlinuz; fi
RUN --no-cache kernel=$(ls /boot/vmlinuz-* 2> /dev/null | head -n1) && if [ -e "$kernel" ]; then ln -sf "${kernel#/boot/}" /boot/vmlinuz; fi
RUN --no-cache kernel=$(ls /boot/Image-* 2> /dev/null | head -n1) && if [ -e "$kernel" ]; then ln -sf "${kernel#/boot/}" /boot/vmlinuz; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually we will have to move these to the dockerfiles and it would be good to remember which line is there for which flavor. Maybe keep that information around as comments? We can always look back in this commit but I'm afraid more changes in the future might make it hard to discover.

It should be too long until we move them though, so maybe it's fine as it is. I'm not sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it? Or should we do ALL the lines and then fail if there is no link created?

I mean, I worked them so they wont run unless it finds the file so they are mostly non-op and only one triggers.

This means we can add all of them to all the dockerfiles and they will work everywhere. Would be nicer if it was some script or whatever, but as pure docker goes, this should be valid for all the flavors in existance and should not fail anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

we can wrap these to a script you are right. But were does this script belong? Maybe it should be in the framework image. If we don't want it in the final images, we can just delete it in the dockerfiles after running it.

@jimmykarily
Copy link
Contributor

This is part of: #1897

E.g. if we move the rest of the steps to a script and we run it inside the Dockerfiles, 1897 is pretty much done.

@jimmykarily
Copy link
Contributor

Let keep this open until we investigate alternative ways (e.g. put each distro logic to its own Dockerfile, even if there is some duplication)

@jimmykarily
Copy link
Contributor

Closing in favor of: #1897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants