From c5b502f231c9832cbf2985b48b2832d9f79e4349 Mon Sep 17 00:00:00 2001 From: David Sauer Date: Sun, 15 Nov 2020 23:04:39 +0100 Subject: [PATCH] added timeout / implemeted resource limits --- cmd/wedding/main.go | 4 +- hack/sniffer/main.go | 16 +++--- pkg/build.go | 116 +++++++++++++++++++++++-------------------- pkg/pull.go | 19 ++++++- pkg/push.go | 19 ++++++- pkg/service.go | 12 ++++- pkg/tag.go | 19 ++++++- 7 files changed, 132 insertions(+), 73 deletions(-) diff --git a/cmd/wedding/main.go b/cmd/wedding/main.go index 1774fae2..f33d7ab9 100644 --- a/cmd/wedding/main.go +++ b/cmd/wedding/main.go @@ -108,9 +108,8 @@ func run(c *cli.Context) error { awaitShutdown() - ctx, cancel := context.WithTimeout(context.Background(), time.Hour) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) defer cancel() - // TODO configure pod to allow a grace period of one hour err = shutdown(ctx, svcServer) if err != nil { @@ -188,7 +187,6 @@ func setupKubernetesClient() (*kubernetes.Clientset, string, error) { func httpServer(h http.Handler, addr string) *http.Server { httpServer := &http.Server{ - // TODO do not timeout builds ReadTimeout: 30 * time.Minute, WriteTimeout: 30 * time.Minute, } diff --git a/hack/sniffer/main.go b/hack/sniffer/main.go index d59f086d..a790500e 100644 --- a/hack/sniffer/main.go +++ b/hack/sniffer/main.go @@ -32,14 +32,13 @@ func sniffer(w http.ResponseWriter, r *http.Request) { } func serveReverseProxy(target string, w http.ResponseWriter, r *http.Request) error { - _, err := printRequest(os.Stdout, r) + err := printRequest(os.Stdout, r) if err != nil { return err } url, err := url.Parse(target) if err != nil { - // TODO Avoid panicing during runtime, parse url during start up once. return fmt.Errorf("parse url %s: %v", target, err) } @@ -60,20 +59,17 @@ func serveReverseProxy(target string, w http.ResponseWriter, r *http.Request) er return nil } -func printRequest(w io.Writer, r *http.Request) (*bytes.Reader, error) { - // TODO To avoid OOM, check body size and use disk to cache. - - body, err := ioutil.ReadAll(r.Body) +func printRequest(w io.Writer, r *http.Request) error { + body, err := r.GetBody() if err != nil { - return nil, fmt.Errorf("reading body: %v", err) + return fmt.Errorf("create copy of body: %v", err) } w.Write([]byte(fmt.Sprintf("req: %v\n\n", r))) - r.Body = ioutil.NopCloser(bytes.NewBuffer(body)) - reader := bytes.NewReader(body) + r.Body = body - return reader, nil + return nil } func printRespose(w io.Writer, resp *http.Response) error { diff --git a/pkg/build.go b/pkg/build.go index ad9a0650..ba6ad8d8 100644 --- a/pkg/build.go +++ b/pkg/build.go @@ -3,11 +3,11 @@ package wedding import ( "bytes" "context" - "encoding/json" "fmt" "io" "log" "net/http" + "path/filepath" "regexp" "strconv" "time" @@ -17,6 +17,7 @@ import ( "github.com/aws/aws-sdk-go/service/s3/s3manager" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -27,15 +28,13 @@ wedding builds only support these arguments: context, tag, buildargs, cachefrom, ) type buildConfig struct { - buildArgs map[string]string - labels map[string]string - cacheRepo string + //buildArgs map[string]string + //labels map[string]string cpuMilliseconds int - dockerfile string + dockerfile string // TODO test path/Dockerfile memoryBytes int - target string - tags []string - noCache bool + target string // TODO test + tags []string // TODO test registryAuth dockerConfig contextFilePath string } @@ -86,7 +85,7 @@ func buildParameters(r *http.Request) (*buildConfig, error) { "cpusetcpus": "", "cpusetmems": "", "cpushares": "0", - // "dockerfile": "use-case-1%2FDockerfile", + // "dockerfile": "Dockerfile", // "labels": "{}", // "memory": "1000", "memswap": "0", @@ -96,6 +95,7 @@ func buildParameters(r *http.Request) (*buildConfig, error) { // "target": "", "ulimits": "null", // "version": "1", // needs two ignored values + "nocache": "", } for k, v := range asserts { @@ -104,62 +104,53 @@ func buildParameters(r *http.Request) (*buildConfig, error) { } } + cachefrom := r.URL.Query().Get("cachefrom") + if cachefrom != "[]" && cachefrom != "null" { // docker uses "[]", tilt uses "null" by default + return cfg, fmt.Errorf("unsupported argument cachefrom set to '%s'", cachefrom) + } + networkmode := r.URL.Query().Get("networkmode") - if networkmode != "default" && networkmode != "" { // docker uses "default", tilt uses "" + if networkmode != "default" && networkmode != "" { // docker uses "default", tilt uses "" by default return cfg, fmt.Errorf("unsupported argument networkmode set to '%s'", networkmode) } version := r.URL.Query().Get("version") - if version != "1" && version != "2" { // docker uses "1", tilt uses "2" + if version != "1" && version != "2" { // docker uses "1", tilt uses "2" by default return cfg, fmt.Errorf("unsupported argument version set to '%s'", version) } rm := r.URL.Query().Get("rm") - if rm != "1" && rm != "0" { // docker uses "1", tilt uses 02" + if rm != "1" && rm != "0" { // docker uses "1", tilt uses "0" by default return cfg, fmt.Errorf("unsupported argument rm set to '%s'", rm) } - err := json.Unmarshal([]byte(r.URL.Query().Get("buildargs")), &cfg.buildArgs) - if err != nil { - return cfg, fmt.Errorf("decode buildargs: %v", err) - } + // TODO implement + // err := json.Unmarshal([]byte(r.URL.Query().Get("buildargs")), &cfg.buildArgs) + // if err != nil { + // return cfg, fmt.Errorf("decode buildargs: %v", err) + // } - err = json.Unmarshal([]byte(r.URL.Query().Get("labels")), &cfg.labels) - if err != nil { - return cfg, fmt.Errorf("decode labels: %v", err) - } + // TODO implement + // err = json.Unmarshal([]byte(r.URL.Query().Get("labels")), &cfg.labels) + // if err != nil { + // return cfg, fmt.Errorf("decode labels: %v", err) + // } - // cache repo - cachefrom := []string{} - err = json.Unmarshal([]byte(r.URL.Query().Get("cachefrom")), &cachefrom) + // cpu limit + cpuquota, err := strconv.Atoi(r.URL.Query().Get("cpuquota")) if err != nil { - return cfg, fmt.Errorf("decode cachefrom: %v", err) - } - - if len(cachefrom) > 1 { - return cfg, fmt.Errorf("wedding only supports one cachefrom image") + return cfg, fmt.Errorf("parse cpu quota to int: %v", err) } - if len(cachefrom) == 1 { - cfg.cacheRepo = cachefrom[0] + if cpuquota == 0 { + cpuquota = buildCPUQuota } - // TODO set default cache from tag - - // cpu limit cpuperiod, err := strconv.Atoi(r.URL.Query().Get("cpuperiod")) if err != nil { return cfg, fmt.Errorf("parse cpu period to int: %v", err) } if cpuperiod == 0 { - cpuperiod = 100_000 // results in 1 cpu - } - - cpuquota, err := strconv.Atoi(r.URL.Query().Get("cpuquota")) - if err != nil { - return cfg, fmt.Errorf("parse cpu quota to int: %v", err) - } - if cpuperiod == 0 { - cpuperiod = 100_000 // 100ms is the default of docker + cpuperiod = buildCPUPeriod } cfg.cpuMilliseconds = int(1000 * float64(cpuquota) / float64(cpuperiod)) @@ -187,10 +178,6 @@ func buildParameters(r *http.Request) (*buildConfig, error) { // image tag cfg.tags = r.URL.Query()["t"] - // disable cache - nocache := r.URL.Query().Get("nocache") - cfg.noCache = nocache == "1" - // registry authentitation dockerCfg, err := xRegistryConfig(r.Header.Get("X-Registry-Config")).toDockerConfig() if err != nil { @@ -303,26 +290,35 @@ func (s Service) executeBuild(ctx context.Context, cfg *buildConfig, w http.Resp destination = fmt.Sprintf("--output type=image,push=true,\"name=%s\"", imageNames) } - // TODO add timeout for script + dockerfileName := filepath.Base(cfg.dockerfile) + dockerfileDir := filepath.Dir(cfg.dockerfile) + + target := "" + if cfg.target != "" { + target = fmt.Sprintf("--opt target=%s", cfg.target) + } + buildScript := fmt.Sprintf(` -set -euxo pipefail +set -euo pipefail +echo download bulid context mkdir ~/context && cd ~/context - wget -O - "%s" | tar -xf - +set -x export BUILDKITD_FLAGS="--oci-worker-no-process-sandbox" export BUILDCTL_CONNECT_RETRIES_MAX=100 buildctl-daemonless.sh \ build \ --frontend dockerfile.v0 \ --local context=. \ - --local dockerfile=. \ - --opt filename=Dockerfile \ + --local dockerfile=%s \ + --opt filename=%s \ + %s \ %s \ --export-cache=type=registry,ref=wedding-registry:5000/cache-repo,mode=max \ --import-cache=type=registry,ref=wedding-registry:5000/cache-repo -`, presignedContextURL, destination) +`, presignedContextURL, dockerfileDir, dockerfileName, target, destination) pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -331,9 +327,13 @@ buildctl-daemonless.sh \ Spec: corev1.PodSpec{ Containers: []corev1.Container{ { - Image: "moby/buildkit:v0.7.2-rootless", + Image: buildkitImage, Name: "buildkit", Command: []string{ + "timeout", + strconv.Itoa(maxBuildTime), + }, + Args: []string{ "sh", "-c", buildScript, @@ -348,6 +348,16 @@ buildctl-daemonless.sh \ Name: "buildkitd-config", }, }, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(fmt.Sprintf("%dm", cfg.cpuMilliseconds)), + corev1.ResourceMemory: resource.MustParse(strconv.Itoa(cfg.memoryBytes)), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(fmt.Sprintf("%dm", cfg.cpuMilliseconds)), + corev1.ResourceMemory: resource.MustParse(strconv.Itoa(cfg.memoryBytes)), + }, + }, }, }, Volumes: []corev1.Volume{ diff --git a/pkg/pull.go b/pkg/pull.go index 88a84cae..33f66bc9 100644 --- a/pkg/pull.go +++ b/pkg/pull.go @@ -5,9 +5,11 @@ import ( "fmt" "log" "net/http" + "strconv" "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -93,7 +95,6 @@ func (s Service) pullImage(w http.ResponseWriter, r *http.Request) { } }() - // TODO add timeout for script buildScript := fmt.Sprintf(` set -euxo pipefail @@ -108,8 +109,12 @@ skopeo copy --dest-tls-verify=false docker://%s docker://%s Containers: []corev1.Container{ { Name: "skopeo", - Image: "mrliptontea/skopeo:1.2.0", + Image: skopeoImage, Command: []string{ + "timeout", + strconv.Itoa(maxBuildTime), + }, + Args: []string{ "sh", "-c", buildScript, @@ -120,6 +125,16 @@ skopeo copy --dest-tls-verify=false docker://%s docker://%s Name: "docker-config", }, }, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + }, }, }, Volumes: []corev1.Volume{ diff --git a/pkg/push.go b/pkg/push.go index ec7eb740..ab5036ce 100644 --- a/pkg/push.go +++ b/pkg/push.go @@ -5,10 +5,12 @@ import ( "fmt" "log" "net/http" + "strconv" "time" "github.com/gorilla/mux" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -56,7 +58,6 @@ func (s Service) pushImage(w http.ResponseWriter, r *http.Request) { } }() - // TODO add timeout for script buildScript := fmt.Sprintf(` set -euxo pipefail @@ -71,8 +72,12 @@ skopeo copy --src-tls-verify=false --dest-tls-verify=false docker://%s docker:// Containers: []corev1.Container{ { Name: "skopeo", - Image: "mrliptontea/skopeo:1.2.0", + Image: skopeoImage, Command: []string{ + "timeout", + strconv.Itoa(maxBuildTime), + }, + Args: []string{ "sh", "-c", buildScript, @@ -83,6 +88,16 @@ skopeo copy --src-tls-verify=false --dest-tls-verify=false docker://%s docker:// Name: "docker-config", }, }, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + }, }, }, Volumes: []corev1.Volume{ diff --git a/pkg/service.go b/pkg/service.go index 9465467d..e27475e7 100644 --- a/pkg/service.go +++ b/pkg/service.go @@ -9,7 +9,17 @@ import ( "k8s.io/client-go/kubernetes" ) -const apiVersion = "1.40" +const ( + apiVersion = "1.40" + maxBuildTime = 1800 + buildkitImage = "moby/buildkit:v0.7.2-rootless" + skopeoImage = "mrliptontea/skopeo:1.2.0" + buildMemory = "2147483648" // 2Gi default + buildCPUQuota = 100_000 // results in 1 cpu + buildCPUPeriod = 100_000 // 100ms is the default of docker + skopeoMemory = "100Mi" + skopeoCPU = "200m" +) // Service runs the wedding server. type Service struct { diff --git a/pkg/tag.go b/pkg/tag.go index a8461d50..42ce7289 100644 --- a/pkg/tag.go +++ b/pkg/tag.go @@ -7,10 +7,12 @@ import ( "log" "net/http" "regexp" + "strconv" "strings" "github.com/gorilla/mux" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -34,7 +36,6 @@ func (s Service) tagImage(w http.ResponseWriter, r *http.Request) { escapePort(fmt.Sprintf("%s:%s", args.Get("repo"), tag)), ) - // TODO add timeout for script buildScript := fmt.Sprintf(` set -euxo pipefail @@ -49,12 +50,26 @@ skopeo copy --src-tls-verify=false --dest-tls-verify=false docker://%s docker:// Containers: []corev1.Container{ { Name: "skopeo", - Image: "mrliptontea/skopeo:1.2.0", + Image: skopeoImage, Command: []string{ + "timeout", + strconv.Itoa(maxBuildTime), + }, + Args: []string{ "sh", "-c", buildScript, }, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(skopeoCPU), + corev1.ResourceMemory: resource.MustParse(skopeoMemory), + }, + }, }, }, RestartPolicy: corev1.RestartPolicyNever,