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.

Sign up for GitHub

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

Install iptables in cloud-api-adaptor daemonset #2016

Merged
merged 2 commits into from
Aug 29, 2024
Merged

Install iptables in cloud-api-adaptor daemonset #2016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e87a6f
build: Install iptables in CAA image
yoheiueda Aug 23, 2024
f0fde55
install: Enable iptables in a cloud-api-adaptor pod
yoheiueda Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/cloud-api-adaptor/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ ARG BASE=registry.fedoraproject.org/fedora:39
# that was specified with --platform. For more details see:
# https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/
FROM --platform=$BUILDPLATFORM $BUILDER_BASE AS builder-release
ARG YQ_VERSION
RUN go install github.com/mikefarah/yq/v4@$YQ_VERSION

# For `dev` builds due to CGO constraints we have to emulate the target platform
# instead of using Go's cross-compilation
FROM --platform=$TARGETPLATFORM $BUILDER_BASE AS builder-dev

ARG YQ_VERSION
RUN go install github.com/mikefarah/yq/v4@$YQ_VERSION
RUN dnf install -y libvirt-devel && dnf clean all

FROM builder-${BUILD_TYPE} AS builder
ARG RELEASE_BUILD
ARG COMMIT
ARG VERSION
ARG TARGETARCH
ARG YQ_VERSION

RUN go install github.com/mikefarah/yq/v4@$YQ_VERSION

WORKDIR /work
COPY cloud-api-adaptor/go.mod cloud-api-adaptor/go.sum ./cloud-api-adaptor/
Expand All @@ -37,8 +38,25 @@ COPY cloud-api-adaptor/proto ./proto

RUN CC=gcc make ARCH=$TARGETARCH COMMIT=$COMMIT VERSION=$VERSION RELEASE_BUILD=$RELEASE_BUILD cloud-api-adaptor

FROM builder-release AS iptables

ARG TARGETARCH

WORKDIR /iptables
RUN --mount=type=bind,target=/versions.yaml,source=cloud-api-adaptor/versions.yaml,readonly \
Copy link
Member

Choose a reason for hiding this comment

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

This format is new to me. Do you have any reference on using --mount with RUN?
Is it docker specific or can work with podman also ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can specify cache mounts and bind mounts to a RUN instruction in a Dockerfile.

https://docs.docker.com/build/guide/mounts/

Podman supports the feature as well.

https://github.com/containers/common/blob/10ced654d57245a78d1b2c3746c5fd99209370bd/docs/Containerfile.5.md?plain=1#L102

version=$(yq -r .tools.iptables-wrapper /versions.yaml) && \
GOARCH=$TARGETARCH go install "github.com/kubernetes-sigs/iptables-wrappers@$version" && \
shopt -s globstar && \
cp /go/bin/**/iptables-wrappers ./iptables-wrapper && \
curl -L -o iptables-wrapper-installer.sh "https://raw.githubusercontent.com/kubernetes-sigs/iptables-wrappers/${version#v*-*-}/iptables-wrapper-installer.sh" && \
chmod 755 iptables-wrapper-installer.sh

FROM --platform=$TARGETPLATFORM $BASE AS base-release

RUN dnf install -y iptables iptables-legacy iptables-nft nftables && dnf clean all
RUN --mount=type=cache,target=/iptables,from=iptables,source=/iptables,readonly \
cd /iptables && ./iptables-wrapper-installer.sh --no-sanity-check --no-cleanup

FROM base-release AS base-dev
RUN dnf install -y libvirt-libs /usr/bin/ssh && dnf clean all

Expand Down
13 changes: 13 additions & 0 deletions src/cloud-api-adaptor/install/yamls/caa-pod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ spec:
- mountPath: /run/netns
mountPropagation: HostToContainer
name: netns
- mountPath: /run/xtables.lock
name: xtables-lock
- mountPath: /lib/modules
name: lib-modules
readOnly: true
# # setting for cloud provider external plugin
# - mountPath: /cloud-providers
# name: provider-dir
Expand All @@ -83,6 +88,14 @@ spec:
- hostPath:
path: /run/netns
name: netns
- hostPath:
path: /run/xtables.lock
type: FileOrCreate
name: xtables-lock
- hostPath:
path: /lib/modules
type: ""
name: lib-modules
# # setting for cloud provider external plugin
# - hostPath:
# path: /opt/cloud-api-adaptor/plugins
Expand Down
1 change: 1 addition & 0 deletions src/cloud-api-adaptor/versions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ tools:
protoc: 3.15.0
packer: v1.9.4
kcli: 99.0.202407031308
iptables-wrapper: v0.0.0-20240819165702-06cad2ec6cb5
Copy link
Member

Choose a reason for hiding this comment

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

Just to double check by understanding - iptable-wrapper is a tool as it isn't itself installed into the container image, but is just used to install iptables?

Copy link
Member

Choose a reason for hiding this comment

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

Both are added to the container image to manipulate iptables rules

https://github.com/confidential-containers/cloud-api-adaptor/pull/2016/files#diff-18d9e213a3bf84ae4d6d02e3b81c7aadb80a88556e3e14d5dcf09d27069bc152R56

Also - https://github.com/kubernetes-sigs/iptables-wrappers

Copy link
Member

Choose a reason for hiding this comment

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

So does that mean it shouldn't go into the tools section, but should be in an external, or dependencies section in version.yaml? Or am I over-thinking this?

Copy link
Member

Choose a reason for hiding this comment

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

Since iptables-wrapper is a tool to manage iptables program, logically it makes sense to be in the tools section. Similar to golang, rust etc..

Copy link
Member

Choose a reason for hiding this comment

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

But I have no preference. If any other section makes more sense that's fine with me as well

Copy link
Member Author

Choose a reason for hiding this comment

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

@stevenhorsman @bpradipt

I have no preference either.

Another possible option is to put it in the git section, and build iptables-wrapper from a cloned git repo. Currently, the builder image does not include the git command, so we need to install it for this option.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Let not overthink it for now and block this work, but I think we go ahead with it in tools and can always update that later if we come up with a different section for this sort of thing. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@stevenhorsman Thank you. I agree.

I keep it in tools for now, and will update that it if necessary.

BTW, the current code to build the command using go install looks somewhat complicated due to cross compilation.

There is a discussion ongoing golang/go#44469, and we will update the code when a simpler option is available.

# Referenced Git repositories
git:
coco-operator:
Expand Down
Loading