From ea5120fa7fc7ddbdeab54192525171c7fb6f90a7 Mon Sep 17 00:00:00 2001 From: Srujan Bharadwaj Date: Thu, 26 Dec 2024 16:05:57 +0530 Subject: [PATCH] add linter and flag for enabling resource constraints --- .golangci.yml | 22 +++++++ README.md | 15 ++++- cmd/flags.go | 26 ++++++++ cmd/server.go | 28 +++++++-- pkg/config/config.go | 7 ++- pkg/config/config_test.go | 2 +- pkg/container/consts.go | 2 +- pkg/container/dockerclient.go | 108 ++++++++++++++++++++-------------- pkg/container/file-helper.go | 2 +- pkg/container/types.go | 7 ++- 10 files changed, 161 insertions(+), 58 deletions(-) create mode 100644 .golangci.yml create mode 100644 cmd/flags.go diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..60df208 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,22 @@ +linters: + disable-all: true + enable: + - errcheck + - goconst + - gocyclo + - gofmt + - goimports + - gosec + - govet + - ineffassign + - lll + - misspell + - nakedret + - prealloc + - staticcheck + - typecheck + - unconvert + - unparam + - unused + - whitespace + fast: false \ No newline at end of file diff --git a/README.md b/README.md index 78e50cb..1c6424b 100644 --- a/README.md +++ b/README.md @@ -71,13 +71,22 @@ To start the server, run the following command: ```sh ./server ``` -The default directory where the code files will be stored is `/tmp/` -A separate directory is created for every language. -To change the directory where the code files will be stored(eventually removed by the garbage collector), use + +### Flags +- `--code-dir` + The default directory where the code files will be stored is `/tmp/` + A separate directory is created for every language. + To change the directory where the code files will be stored(eventually removed by the garbage collector), use ```sh ./server --code-dir /path/to/code/dir ``` +- `--resource-constraints` + By default, resource constraints are turned off to improve the performance, if you want to enable it, use +```sh +./server --resource-constraints true +``` + ## API ### Submit Code diff --git a/cmd/flags.go b/cmd/flags.go new file mode 100644 index 0000000..c9ee72d --- /dev/null +++ b/cmd/flags.go @@ -0,0 +1,26 @@ +package main + +import ( + "flag" + "remote-code-engine/pkg/config" + + "go.uber.org/zap" +) + +func ParseFlags() { + flag.StringVar(&config.BaseCodePath, "code-dir", "/tmp/", "Base path to store the code files") + flag.BoolVar(&config.ResourceConstraints, "resource-constraints", false, "Enable resource constraints (default false)") + help := flag.Bool("help", false, "Display help") + + flag.Parse() + + if *help { + flag.Usage() + return + } + + logger.Info("parsed the flags", + zap.String("code-dir", config.BaseCodePath), + zap.Bool("resource-constraints", config.ResourceConstraints), + ) +} diff --git a/cmd/server.go b/cmd/server.go index d5f5d8e..fa48a67 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -2,7 +2,6 @@ package main import ( "context" - "flag" "net/http" "os" "remote-code-engine/pkg/config" @@ -55,10 +54,11 @@ func setupCodeDirectory(imageConfig config.ImageConfig) { } func main() { - defer logger.Sync() + defer func() { + _ = logger.Sync() + }() - flag.StringVar(&config.BaseCodePath, "code-dir", "/tmp/", "Base path to store the code files") - flag.Parse() + ParseFlags() // This should be given as a command line argument. imageConfig, err := config.LoadConfig("../config.yml") @@ -83,7 +83,23 @@ func main() { panic(err) } - go cli.FreeUpZombieContainers(context.Background()) + ctx, cancel := context.WithCancel(context.Background()) - StartServer(cli, imageConfig) + go func() { + err = cli.FreeUpZombieContainers(ctx) + if err != nil { + logger.Error("failed to free up zombie containers", + zap.Error(err), + ) + } + }() + + err = StartServer(cli, imageConfig) + if err != nil { + logger.Error("failed to start the server", + zap.Error(err), + ) + cancel() + panic(err) + } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 3c0d849..e5f3d0c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -17,7 +17,8 @@ const ( ) var ( - BaseCodePath string + BaseCodePath string + ResourceConstraints bool ) type LanguageConfig struct { @@ -55,3 +56,7 @@ func (c *ImageConfig) IsLanguageSupported(lang Language) bool { func GetHostLanguageCodePath(lang Language) string { return filepath.Join(BaseCodePath, string(lang)) } + +func IsResourceConstraintsEnabled() bool { + return ResourceConstraints +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index b2cc289..bdf083e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -34,7 +34,7 @@ func TestLoadConfig(t *testing.T) { // Write the sample config to a temporary file configPath := filepath.Join(tempDir, "config.yaml") - err = os.WriteFile(configPath, data, 0644) + err = os.WriteFile(configPath, data, 0600) if err != nil { t.Fatalf("failed to write sample config to file: %v", err) } diff --git a/pkg/container/consts.go b/pkg/container/consts.go index edf13cf..c742bbd 100644 --- a/pkg/container/consts.go +++ b/pkg/container/consts.go @@ -5,7 +5,7 @@ import "time" const ( MAX_EXECUTION_TIME = 60 * time.Second - // The server running this will check for every 10 minutes whether there are zombie containers - completed containers and removes them. + // The server running this will check for every 10 minutes whether there are zombie containers. GarbageCollectionTimeWindow = 5 * time.Minute // Path where the code files are mounted. diff --git a/pkg/container/dockerclient.go b/pkg/container/dockerclient.go index da89b69..45c433c 100644 --- a/pkg/container/dockerclient.go +++ b/pkg/container/dockerclient.go @@ -47,7 +47,7 @@ func NewDockerClient(opts *client.Opt, logger *zap.Logger) (ContainerClient, err } func (d *dockerClient) GetContainers(ctx context.Context, opts *container.ListOptions) ([]Container, error) { - var containersList []Container + containersList := []Container{} containers, err := d.client.ContainerList(ctx, *opts) if err != nil { @@ -66,6 +66,39 @@ func (d *dockerClient) GetContainers(ctx context.Context, opts *container.ListOp return containersList, nil } +func (d *dockerClient) getResourceConstraints() container.Resources { + if config.IsResourceConstraintsEnabled() { + return container.Resources{ + Memory: 500 * 1024 * 1024, // 500 MB + NanoCPUs: 1000000000, // 1 CPU + Ulimits: []*units.Ulimit{ + { + Name: "nproc", + Soft: 64, + Hard: 128, + }, + { + Name: "nofile", + Soft: 64, + Hard: 128, + }, + { + Name: "core", + Soft: 0, + Hard: 0, + }, + { + // Maximum file size that can be created by the process (output file in our case) + Name: "fsize", + Soft: 20 * 1024 * 1024, + Hard: 20 * 1024 * 1024, + }, + }, + } + } + return container.Resources{} +} + func (d *dockerClient) ExecuteCode(ctx context.Context, code *Code) (string, error) { codeFileName, inputFileName, err := createCodeAndInputFilesHost(code, d.logger) if err != nil { @@ -76,6 +109,8 @@ func (d *dockerClient) ExecuteCode(ctx context.Context, code *Code) (string, err zap.String("input file name", inputFileName), ) + resourceConstraints := d.getResourceConstraints() + res, err := d.client.ContainerCreate(ctx, &container.Config{ Cmd: getContainerCommand(code, codeFileName, inputFileName), Image: code.Image, @@ -92,40 +127,13 @@ func (d *dockerClient) ExecuteCode(ctx context.Context, code *Code) (string, err RestartPolicy: container.RestartPolicy{ Name: "no", }, - // We are reading the container logs to get the output. So it's better to disable and have a separate thread to delete stale containers + // We are reading the container logs to get the output. + // So it's better to disable and have a separate thread to delete stale containers AutoRemove: false, // Drop all the capabilities CapDrop: []string{"ALL"}, Privileged: false, - // Set the memory limit to 1GB - Resources: container.Resources{ - // set 500 MB as the memory limit in bytes - Memory: 500 * 1024 * 1024, - NanoCPUs: 500000000, // 0.5 CPU - Ulimits: []*units.Ulimit{ - { - Name: "nproc", - Soft: 64, - Hard: 128, - }, - { - Name: "nofile", - Soft: 64, - Hard: 128, - }, - { - Name: "core", - Soft: 0, - Hard: 0, - }, - { - // Maximum file size that can be created by the process (output file in our case) - Name: "fsize", - Soft: 20 * 1024 * 1024, - Hard: 20 * 1024 * 1024, - }, - }, - }, + Resources: resourceConstraints, }, nil, nil, getContainerName()) if err != nil { @@ -217,22 +225,36 @@ func deleteStaleFiles(dir string, logger *zap.Logger) error { } func (d *dockerClient) FreeUpZombieContainers(ctx context.Context) error { + ticker := time.NewTicker(GarbageCollectionTimeWindow) for { - pruneResults, err := d.client.ContainersPrune(ctx, filters.Args{}) - if err != nil { - d.logger.Error("failed to prune containers", - zap.Error(err), - ) - } + select { + case <-ctx.Done(): + d.logger.Info("stopping the zombie container cleanup routine") + return nil + case <-ticker.C: + pruneResults, err := d.client.ContainersPrune(ctx, filters.Args{}) + if err != nil { + d.logger.Error("failed to prune containers", + zap.Error(err), + ) + } - d.logger.Info("successfully pruned the containers:", - zap.Int("#Pruned containers", len(pruneResults.ContainersDeleted)), - ) + d.logger.Info("successfully pruned the containers:", + zap.Int("#Pruned containers", len(pruneResults.ContainersDeleted)), + ) - deleteStaleFiles(config.GetHostLanguageCodePath(config.Cpp), d.logger) - deleteStaleFiles(config.GetHostLanguageCodePath(config.Golang), d.logger) + if err = deleteStaleFiles(config.GetHostLanguageCodePath(config.Cpp), d.logger); err != nil { + d.logger.Error("failed to delete stale files", + zap.Error(err), + ) + } - time.Sleep(GarbageCollectionTimeWindow) + if err = deleteStaleFiles(config.GetHostLanguageCodePath(config.Golang), d.logger); err != nil { + d.logger.Error("failed to delete stale files", + zap.Error(err), + ) + } + } } } diff --git a/pkg/container/file-helper.go b/pkg/container/file-helper.go index 9d736ca..9a9ad8e 100644 --- a/pkg/container/file-helper.go +++ b/pkg/container/file-helper.go @@ -40,7 +40,7 @@ func createFile(filePath, base64FileContent string, logger *zap.Logger) (string, return filepath.Base(filePath), fmt.Errorf("failed to decode the file content: %w", err) } - n, err := f.Write([]byte(data)) + n, err := f.Write(data) if err != nil { return filepath.Base(filePath), fmt.Errorf("failed to write the content to the file: %w", err) } diff --git a/pkg/container/types.go b/pkg/container/types.go index 5e07792..be0f448 100644 --- a/pkg/container/types.go +++ b/pkg/container/types.go @@ -22,8 +22,11 @@ type Code struct { type ContainerClient interface { FreeUpZombieContainers(ctx context.Context) error - ExecuteCode(ctx context.Context, code *Code) (string, error) // Executes and returns the output in the string, error in case of server errors not code errors. + + // Executes and returns the output in the string, error in case of server errors not code errors. + ExecuteCode(ctx context.Context, code *Code) (string, error) // TODO: Is this even needed? - GetContainers(ctx context.Context, opts *container.ListOptions) ([]Container, error) // Remove list options if you want some other container type other than docker + // Remove list options if you want some other container type other than docker + GetContainers(ctx context.Context, opts *container.ListOptions) ([]Container, error) }