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

adds builds for opentnpd and dhcpcd for linuxkit images #3883

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jaxesn
Copy link
Member

@jaxesn jaxesn commented Oct 9, 2024

Issue #, if available:

Description of changes:

This adds openntpd and dhcpcd builds to linuxkit. Neither of these exists in al2/al23 so building from the source. The new base images for these are al2 based.

The openntpd build is not built with libtls so the cert validation constraint would not be supported, but based on the ntp config we are using I do not believe this is needed:

servers pool.ntp.org

I went through the dhcpcd hooks and the additional mtu script linuxkit provides to make sure I included all the utils it needs (cat/rm/set/etc). This is something we have to pay attention to when updating, I added a note to the hook readme.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jaxesn. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 9, 2024
@jaxesn jaxesn force-pushed the jgw/linuxkit-ntpd-dhcp branch 3 times, most recently from 8d0ecb3 to e1b6a39 Compare October 10, 2024 15:30
@jaxesn jaxesn force-pushed the jgw/linuxkit-ntpd-dhcp branch from e1b6a39 to 7da66e4 Compare October 10, 2024 15:32
@@ -0,0 +1,6 @@
DHCPCD:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is kind of a new pattern, we may want to break these builds out later on, for now we are only going to use them here to produce images we only use during hook build time. I did model this a bit off how we do the builder-base versions.yaml. I was thinking this would be easy to add the upgrade script to check for new versions and update.

@abhay-krishna
Copy link
Member

/test all

dhcpcd/images/% openntpd/images/%: BASE_IMAGE_NAME=eks-distro-minimal-base-glibc

$(call IMAGE_TARGETS_FOR_NAME,openntpd): openntpd-useradd/images/export
openntpd-useradd/images/export: IMAGE_USERADD_USER_NAME=_ntp
Copy link
Member

Choose a reason for hiding this comment

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

Where is this username coming from? Couldn't find it in the upstream Linuxkit openntpd Dockerfile.

$(OUTPUT_BIN_DIR)/%/dhcpcd/usr/sbin/dhcpcd: VERSION=$(shell yq '.DHCPCD.version' dependencies.yaml )
$(OUTPUT_BIN_DIR)/%/dhcpcd/usr/sbin/dhcpcd: SHA256SUM=$(shell yq '.DHCPCD.sha256sum' dependencies.yaml )
$(OUTPUT_BIN_DIR)/%/dhcpcd/usr/sbin/dhcpcd: $(GIT_PATCH_TARGET) | $$(call ENABLE_DOCKER_PLATFORM,$$(PLATFORM))
mkdir -p $(OUTPUT_DIR)/dhcpcd; \
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could move this and the one below for openntpd into a common script or Macro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants