From 81d27aa23969b77f5e7e565b0b69234537b0503e Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Mon, 14 Nov 2016 11:30:33 +0000 Subject: [PATCH 1/5] Add SIGCHLD handler to pause container This allows pause to reap zombies in the upcoming Shared PID namespace (#1615). Uses the better defined sigaction() instead of signal() for all signals both for consistency (SIGCHLD handler avoids SA_RESTART) and to avoid the implicit signal()->sigaction() translation of various libc versions. Also makes warnings errors and includes a tool to make orphaned zombies for manual testing. --- build-tools/pause/orphan.c | 35 +++++++++++++++++++++++++++++++++++ build/pause/Makefile | 15 +++++++++++++-- build/pause/pause.c | 19 ++++++++++++++++--- 3 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 build-tools/pause/orphan.c diff --git a/build-tools/pause/orphan.c b/build-tools/pause/orphan.c new file mode 100644 index 00000000000..b2d1a6cd58b --- /dev/null +++ b/build-tools/pause/orphan.c @@ -0,0 +1,35 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* Creates a zombie to be reaped by init. Useful for testing. */ + +#include +#include + +int main() { + pid_t pid; + pid = fork(); + if ( pid == 0 ) { + while ( getppid() > 1 ); + printf("Child exiting: pid=%d ppid=%d\n", getpid(), getppid()); + return 0; + } else if ( pid > 0 ) { + printf("Parent exiting: pid=%d ppid=%d\n", getpid(), getppid()); + return 0; + } + perror("Could not create child"); + return 1; +} diff --git a/build/pause/Makefile b/build/pause/Makefile index 81f4174cc3b..d8b0a1f7290 100644 --- a/build/pause/Makefile +++ b/build/pause/Makefile @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -.PHONY: all push push-legacy container clean +.PHONY: all push push-legacy container clean orphan REGISTRY ?= gcr.io/google_containers IMAGE = $(REGISTRY)/pause-$(ARCH) @@ -25,7 +25,7 @@ ARCH ?= amd64 ALL_ARCH = amd64 arm arm64 ppc64le s390x -CFLAGS = -Os -Wall -static +CFLAGS = -Os -Wall -Werror -static KUBE_CROSS_IMAGE ?= gcr.io/google_containers/kube-cross KUBE_CROSS_VERSION ?= $(shell cat ../build-image/cross/VERSION) @@ -97,5 +97,16 @@ ifeq ($(ARCH),amd64) endif touch $@ +# Useful for testing, not automatically included in container image +orphan: bin/orphan-$(ARCH) +bin/orphan-$(ARCH): orphan.c + mkdir -p bin + docker run -u $$(id -u):$$(id -g) -v $$(pwd):/build \ + $(KUBE_CROSS_IMAGE):$(KUBE_CROSS_VERSION) \ + /bin/bash -c "\ + cd /build && \ + $(TRIPLE)-gcc $(CFLAGS) -o $@ $^ && \ + $(TRIPLE)-strip $@" + clean: rm -rf .container-* .push-* bin/ diff --git a/build/pause/pause.c b/build/pause/pause.c index ffbceb69fbe..cdf6637ce0c 100644 --- a/build/pause/pause.c +++ b/build/pause/pause.c @@ -17,6 +17,8 @@ limitations under the License. #include #include #include +#include +#include #include static void sigdown(int signo) { @@ -24,12 +26,23 @@ static void sigdown(int signo) { exit(0); } +static void sigreap(int signo) { + while (waitpid(-1, NULL, WNOHANG) > 0); +} + int main() { - if (signal(SIGINT, sigdown) == SIG_ERR) + if (getpid() != 1) { + fprintf(stderr, "Warning: pause should be the first process in a pod\n"); + } + + if (sigaction(SIGINT, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) return 1; - if (signal(SIGTERM, sigdown) == SIG_ERR) + if (sigaction(SIGTERM, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) return 2; - signal(SIGKILL, sigdown); + if (sigaction(SIGCHLD, &(struct sigaction){.sa_handler = sigreap, .sa_flags = SA_NOCLDSTOP}, NULL) < 0) + return 3; + sigaction(SIGKILL, &(struct sigaction){.sa_handler = sigdown}, NULL); + for (;;) pause(); fprintf(stderr, "error: infinite loop terminated\n"); return 42; From c1520e15ff35fa7ec19b6aa999362adef5963b9d Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 22 Nov 2016 23:13:03 +0000 Subject: [PATCH 2/5] Address style comments for pause.c Run clang-format on the C files and capitalize error messages. --- build-tools/pause/orphan.c | 25 +++++++++++++------------ build/pause/pause.c | 35 +++++++++++++++++++---------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/build-tools/pause/orphan.c b/build-tools/pause/orphan.c index b2d1a6cd58b..07f490de91d 100644 --- a/build-tools/pause/orphan.c +++ b/build-tools/pause/orphan.c @@ -20,16 +20,17 @@ limitations under the License. #include int main() { - pid_t pid; - pid = fork(); - if ( pid == 0 ) { - while ( getppid() > 1 ); - printf("Child exiting: pid=%d ppid=%d\n", getpid(), getppid()); - return 0; - } else if ( pid > 0 ) { - printf("Parent exiting: pid=%d ppid=%d\n", getpid(), getppid()); - return 0; - } - perror("Could not create child"); - return 1; + pid_t pid; + pid = fork(); + if (pid == 0) { + while (getppid() > 1) + ; + printf("Child exiting: pid=%d ppid=%d\n", getpid(), getppid()); + return 0; + } else if (pid > 0) { + printf("Parent exiting: pid=%d ppid=%d\n", getpid(), getppid()); + return 0; + } + perror("Could not create child"); + return 1; } diff --git a/build/pause/pause.c b/build/pause/pause.c index cdf6637ce0c..4dac4d1b5af 100644 --- a/build/pause/pause.c +++ b/build/pause/pause.c @@ -22,28 +22,31 @@ limitations under the License. #include static void sigdown(int signo) { - psignal(signo, "shutting down, got signal"); - exit(0); + psignal(signo, "Shutting down, got signal"); + exit(0); } static void sigreap(int signo) { - while (waitpid(-1, NULL, WNOHANG) > 0); + while (waitpid(-1, NULL, WNOHANG) > 0) + ; } int main() { - if (getpid() != 1) { - fprintf(stderr, "Warning: pause should be the first process in a pod\n"); - } + if (getpid() != 1) + fprintf(stderr, "Warning: pause should be the first process in a pod\n"); - if (sigaction(SIGINT, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) - return 1; - if (sigaction(SIGTERM, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) - return 2; - if (sigaction(SIGCHLD, &(struct sigaction){.sa_handler = sigreap, .sa_flags = SA_NOCLDSTOP}, NULL) < 0) - return 3; - sigaction(SIGKILL, &(struct sigaction){.sa_handler = sigdown}, NULL); + if (sigaction(SIGINT, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) + return 1; + if (sigaction(SIGTERM, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) + return 2; + if (sigaction(SIGCHLD, &(struct sigaction){.sa_handler = sigreap, + .sa_flags = SA_NOCLDSTOP}, + NULL) < 0) + return 3; + sigaction(SIGKILL, &(struct sigaction){.sa_handler = sigdown}, NULL); - for (;;) pause(); - fprintf(stderr, "error: infinite loop terminated\n"); - return 42; + for (;;) + pause(); + fprintf(stderr, "Error: infinite loop terminated\n"); + return 42; } From 7910a3f315b89747b1a9a0bc8e74ae390f877001 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 16 Dec 2016 02:36:32 +0000 Subject: [PATCH 3/5] Move orphan.c to catch up to build-tools rename --- {build-tools => build}/pause/orphan.c | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {build-tools => build}/pause/orphan.c (100%) diff --git a/build-tools/pause/orphan.c b/build/pause/orphan.c similarity index 100% rename from build-tools/pause/orphan.c rename to build/pause/orphan.c From c706a052ef823db1b689d1ee0a716e47f3db9673 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Thu, 22 Dec 2016 21:51:10 +0000 Subject: [PATCH 4/5] pause.c: Document intended PID 1 behavior --- build/pause/pause.c | 1 + 1 file changed, 1 insertion(+) diff --git a/build/pause/pause.c b/build/pause/pause.c index 4dac4d1b5af..7bb383dc77f 100644 --- a/build/pause/pause.c +++ b/build/pause/pause.c @@ -33,6 +33,7 @@ static void sigreap(int signo) { int main() { if (getpid() != 1) + /* Not an error because pause sees use outside of infra containers. */ fprintf(stderr, "Warning: pause should be the first process in a pod\n"); if (sigaction(SIGINT, &(struct sigaction){.sa_handler = sigdown}, NULL) < 0) From 68262ad5f46673f61a8d11a8ed87f58df50b89d4 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Thu, 19 Jan 2017 22:50:21 +0000 Subject: [PATCH 5/5] Remove SIGKILL handler from pause.c --- build/pause/pause.c | 1 - 1 file changed, 1 deletion(-) diff --git a/build/pause/pause.c b/build/pause/pause.c index 7bb383dc77f..f9853915f82 100644 --- a/build/pause/pause.c +++ b/build/pause/pause.c @@ -44,7 +44,6 @@ int main() { .sa_flags = SA_NOCLDSTOP}, NULL) < 0) return 3; - sigaction(SIGKILL, &(struct sigaction){.sa_handler = sigdown}, NULL); for (;;) pause();