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

Use debian-iptables for k8s-dns-node-cache, bump debian-base version #367

Merged
merged 1 commit into from
Oct 29, 2020
Merged

Use debian-iptables for k8s-dns-node-cache, bump debian-base version #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Dockerfile.kube-dns
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM ARG_FROM
FROM ARG_FROM_BASE

MAINTAINER Bowei Du <[email protected]>

Expand Down
14 changes: 2 additions & 12 deletions Dockerfile.node-cache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM ARG_FROM
champtar marked this conversation as resolved.
Show resolved Hide resolved
# Use --no-install-recommends to not install nftables and work around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956655
# Use iptables-legacy only until https://bugzilla.netfilter.org/show_bug.cgi?id=1422 is resolved
# once fixed we will switch to k8s.gcr.io/debian-iptables-$(ARCH) to choose iptables-legacy or iptables-nft at run time
RUN apt-get update && apt-get install -y --no-install-recommends \
iproute2 \
iptables \
ebtables \
Copy link
Contributor

Choose a reason for hiding this comment

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

will the new base image also contain ebtables?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's the nf_tables version by default, and there is no magic auto-detection script

$ podman run -ti --rm k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0 ebtables -V
ebtables 1.8.5 (nf_tables)

I will restore the update-alternatives --set ebtables /usr/sbin/ebtables-legacy to keep the current install working

I understand that this ebtables rule is an ugly hack used when you are in bridge mode, it would be way cleaner to inject a routing rule in the bridge CNI config ip r add 169.254.25.10 via <nodeip> (to have the route in each pod) and not hijack the traffic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added, rebased, build tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ docker run --rm -it --entrypoint=ebtables champtar/k8s-dns-node-cache-amd64:1.15.15-3-g8055b8f
ebtables v2.0.11 (legacy) (December 2011)
...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but it's the nf_tables version by default, and there is no magic auto-detection script

$ podman run -ti --rm k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0 ebtables -V
ebtables 1.8.5 (nf_tables)

I will restore the update-alternatives --set ebtables /usr/sbin/ebtables-legacy to keep the current install working

I understand that this ebtables rule is an ugly hack used when you are in bridge mode, it would be way cleaner to inject a routing rule in the bridge CNI config ip r add 169.254.25.10 via <nodeip> (to have the route in each pod) and not hijack the traffic

@champtar that would not work unless further changes were made (i.e. resolv.conf) with the downside of leaving all the applications broken in case only nodelocaldns is uninstalled without reverting the other changes that are not controlled by nodelocaldns.

&& update-alternatives --set iptables /usr/sbin/iptables-legacy \
&& update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy \
&& update-alternatives --set ebtables /usr/sbin/ebtables-legacy \
&& rm -rf /var/lib/apt/lists/*
FROM ARG_FROM_IPT
RUN update-alternatives --set ebtables /usr/sbin/ebtables-legacy
ADD bin/ARG_ARCH/ARG_BIN /ARG_BIN
EXPOSE 53 53/udp
EXPOSE 53 53/tcp
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.sidecar
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM ARG_FROM
FROM ARG_FROM_BASE

MAINTAINER Bowei Du <[email protected]>

Expand Down
4 changes: 2 additions & 2 deletions images/dnsmasq/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ OUTPUT_DIR := _output/$(ARCH)
# Ensure that the docker command line supports the manifest images
export DOCKER_CLI_EXPERIMENTAL=enabled

BASEIMAGE ?= us.gcr.io/k8s-artifacts-prod/build-image/debian-base-$(ARCH):v2.1.2
BASEIMAGE ?= k8s.gcr.io/build-image/debian-base-$(ARCH):buster-v1.2.0
ifeq ($(ARCH),amd64)
COMPILE_IMAGE := us.gcr.io/k8s-artifacts-prod/build-image/debian-base-$(ARCH):v2.1.2
COMPILE_IMAGE := k8s.gcr.io/build-image/debian-base-$(ARCH):buster-v1.2.0
else ifeq ($(ARCH),arm)
TRIPLE ?= arm-linux-gnueabihf
QEMUARCH := arm
Expand Down
7 changes: 4 additions & 3 deletions rules.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ export VERSION
SRC_DIRS := cmd pkg

ALL_ARCH := amd64 arm arm64 ppc64le s390x
BASEIMAGE ?= us.gcr.io/k8s-artifacts-prod/build-image/debian-base-$(ARCH):v2.1.2
BASEIMAGE ?= k8s.gcr.io/build-image/debian-base-$(ARCH):buster-v1.2.0
IPTIMAGE ?= k8s.gcr.io/build-image/debian-iptables-$(ARCH):buster-v1.3.0

# These rules MUST be expanded at reference time (hence '=') as BINARY
# is dynamically scoped.
Expand Down Expand Up @@ -125,7 +126,8 @@ define DOCKERFILE_RULE
-e 's|ARG_ARCH|$(ARCH)|g' \
-e 's|ARG_BIN|$(BINARY)|g' \
-e 's|ARG_REGISTRY|$(REGISTRY)|g' \
-e 's|ARG_FROM|$(BASEIMAGE)|g' \
-e 's|ARG_FROM_BASE|$(BASEIMAGE)|g' \
-e 's|ARG_FROM_IPT|$(IPTIMAGE)|g' \
champtar marked this conversation as resolved.
Show resolved Hide resolved
-e 's|ARG_VERSION|$(VERSION)|g' \
$$< > $$@
.$(BUILDSTAMP_NAME)-container: .$(BINARY)-$(ARCH)-dockerfile
Expand All @@ -137,7 +139,6 @@ $(foreach BINARY,$(CONTAINER_BINARIES),$(eval $(DOCKERFILE_RULE)))
define CONTAINER_RULE
.$(BUILDSTAMP_NAME)-container: bin/$(ARCH)/$(BINARY)
@echo "container: bin/$(ARCH)/$(BINARY) ($(CONTAINER_NAME))"
@docker pull $(BASEIMAGE)
@docker build \
$(DOCKER_BUILD_FLAGS) \
-t $(CONTAINER_NAME):$(VERSION) \
Expand Down