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

hook: fix vlan handling #238

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Conversation

rpardini
Copy link
Contributor

@rpardini rpardini commented Aug 17, 2024

hook: vlan.sh: fix 'parse_cmdline' bug; if no hw_addr specified, default ifname to eth0

  • parse_cmdline is actually parse_kernel_cmdline_for
  • no reason to double-newline results
  • allow for simple vlan_id=xxx without hwaddr for single-interface or first-interface VLAN scenarios

Signed-off-by: Ricardo Pardini [email protected]

hook: introduce hook-ip container for vlan.sh

  • Based on linuxkit/ip pkg, sans wireguard stuff; add GNU sed needed for /proc/cmdline parsing

Signed-off-by: Ricardo Pardini [email protected]

…ult ifname to eth0

- `parse_cmdline` is actually `parse_kernel_cmdline_for`
- no reason to double-newline results
- allow for simple vlan_id=xxx without hwaddr for single-interface or first-interface VLAN scenarios

Signed-off-by: Ricardo Pardini <[email protected]>
- Based on linuxkit/ip pkg, sans wireguard stuff; add GNU sed needed for /proc/cmdline parsing

Signed-off-by: Ricardo Pardini <[email protected]>
Comment on lines +1 to +23
FROM linuxkit/alpine:146f540f25cd92ec8ff0c5b0c98342a9a95e479e AS mirror
RUN mkdir -p /out/etc/apk && cp -r /etc/apk/* /out/etc/apk/
RUN apk add curl
RUN apk add --no-cache --initdb -p /out \
alpine-baselayout \
bash \
busybox \
iproute2 \
iptables \
ebtables \
ipvsadm \
bridge-utils \
musl \
sed

# Remove apk residuals
RUN rm -rf /out/etc/apk /out/lib/apk /out/var/cache

FROM scratch
ENTRYPOINT []
CMD []
WORKDIR /
COPY --from=mirror /out/ /
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, why do we need this new image? i dont see any of these tools being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I just copied linuxkit/ip -- previously used directly -- from https://github.com/linuxkit/linuxkit/blob/master/pkg/ip/Dockerfile -- just to add sed (GNU sed) required for proper kernel cmdline parsing in the bash code. I discovered busybox's sed didn't really work when I tried actually passing a vlan_id=876 parameter and that failed.

As for the tools: I kept the original ones we had there, minus wireguard. I agree that probably most of them aren't used either.

Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

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

Thank @rpardini , FYI, I added the hook-ip repo in quay.io https://quay.io/repository/tinkerbell/hook-ip

@jacobweinstock jacobweinstock added the ready-to-merge Signal to Mergify to merge the PR. label Aug 27, 2024
@mergify mergify bot merged commit 526b4a3 into tinkerbell:main Aug 27, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants