From 61ad1e458bca9c071f7f6acfd44c52681e3e3895 Mon Sep 17 00:00:00 2001 From: Mizuki Urushida Date: Wed, 25 Oct 2017 17:09:51 +0900 Subject: [PATCH 1/5] Fix a names-generator binary To ensure that namesgenerator binary outputs random name by initializing Seed. Signed-off-by: Mizuki Urushida not use init function. Signed-off-by: Mizuki Urushida Upstream-commit: eaab2f715039e212dd67c71c30f1f8a8cfc03ded Component: engine --- .../engine/pkg/namesgenerator/cmd/names-generator/main.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/engine/pkg/namesgenerator/cmd/names-generator/main.go b/components/engine/pkg/namesgenerator/cmd/names-generator/main.go index 18a939b70b..7fd5955beb 100644 --- a/components/engine/pkg/namesgenerator/cmd/names-generator/main.go +++ b/components/engine/pkg/namesgenerator/cmd/names-generator/main.go @@ -2,10 +2,13 @@ package main import ( "fmt" + "math/rand" + "time" "github.com/docker/docker/pkg/namesgenerator" ) func main() { + rand.Seed(time.Now().UnixNano()) fmt.Println(namesgenerator.GetRandomName(0)) } From 8efb0e1631eea1e71ced34999cedeb0518aa1e75 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 27 Sep 2017 11:49:22 -0700 Subject: [PATCH 2/5] ContainerWait on remove: don't stuck on rm fail Currently, if a container removal has failed for some reason, any client waiting for removal (e.g. `docker run --rm`) is stuck, waiting for removal to succeed while it has failed already. For more details and the reproducer, please check https://github.com/moby/moby/issues/34945 This commit addresses that by allowing `ContainerWait()` with `container.WaitCondition == "removed"` argument to return an error in case of removal failure. The `ContainerWaitOKBody` stucture returned to a client is amended with a pointer to `struct Error`, containing an error message string, and the `Client.ContainerWait()` is modified to return the error, if any, to the client. Note that this feature is only available for API version >= 1.34. In order for the old clients to be unstuck, we just close the connection without writing anything -- this causes client's error. Now, docker-cli would need a separate commit to bump the API to 1.34 and to show an error returned, if any. [v2: recreate the waitRemove channel after closing] [v3: document; keep legacy behavior for older clients] [v4: convert Error from string to pointer to a struct] [v5: don't emulate old behavior, send empty response in error case] [v6: rename legacy* vars to include version suffix] Signed-off-by: Kir Kolyshkin Upstream-commit: f963500c544daa3c158c0ca3d2985295c875cb6b Component: engine --- .../router/container/container_routes.go | 22 ++++++++++++++++--- components/engine/api/swagger.yaml | 7 ++++++ .../api/types/container/container_wait.go | 12 ++++++++++ components/engine/container/state.go | 17 ++++++++++++-- components/engine/daemon/delete.go | 8 +++++-- components/engine/docs/api/version-history.md | 9 +++++++- 6 files changed, 67 insertions(+), 8 deletions(-) diff --git a/components/engine/api/server/router/container/container_routes.go b/components/engine/api/server/router/container/container_routes.go index 95bbe0e533..106a7087cd 100644 --- a/components/engine/api/server/router/container/container_routes.go +++ b/components/engine/api/server/router/container/container_routes.go @@ -280,11 +280,12 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons // Behavior changed in version 1.30 to handle wait condition and to // return headers immediately. version := httputils.VersionFromContext(ctx) - legacyBehavior := versions.LessThan(version, "1.30") + legacyBehaviorPre130 := versions.LessThan(version, "1.30") + legacyRemovalWaitPre134 := false // The wait condition defaults to "not-running". waitCondition := containerpkg.WaitConditionNotRunning - if !legacyBehavior { + if !legacyBehaviorPre130 { if err := httputils.ParseForm(r); err != nil { return err } @@ -293,6 +294,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons waitCondition = containerpkg.WaitConditionNextExit case container.WaitConditionRemoved: waitCondition = containerpkg.WaitConditionRemoved + legacyRemovalWaitPre134 = versions.LessThan(version, "1.34") } } @@ -306,7 +308,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons w.Header().Set("Content-Type", "application/json") - if !legacyBehavior { + if !legacyBehaviorPre130 { // Write response header immediately. w.WriteHeader(http.StatusOK) if flusher, ok := w.(http.Flusher); ok { @@ -317,8 +319,22 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons // Block on the result of the wait operation. status := <-waitC + // With API < 1.34, wait on WaitConditionRemoved did not return + // in case container removal failed. The only way to report an + // error back to the client is to not write anything (i.e. send + // an empty response which will be treated as an error). + if legacyRemovalWaitPre134 && status.Err() != nil { + return nil + } + + var waitError *container.ContainerWaitOKBodyError + if status.Err() != nil { + waitError = &container.ContainerWaitOKBodyError{Message: status.Err().Error()} + } + return json.NewEncoder(w).Encode(&container.ContainerWaitOKBody{ StatusCode: int64(status.ExitCode()), + Error: waitError, }) } diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index c3b9c29244..b0c0575fc0 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -5723,6 +5723,13 @@ paths: description: "Exit code of the container" type: "integer" x-nullable: false + Error: + description: "container waiting error, if any" + type: "object" + properties: + Message: + description: "Details of an error" + type: "string" 404: description: "no such container" schema: diff --git a/components/engine/api/types/container/container_wait.go b/components/engine/api/types/container/container_wait.go index 77ecdbaf7a..47fb17578a 100644 --- a/components/engine/api/types/container/container_wait.go +++ b/components/engine/api/types/container/container_wait.go @@ -7,10 +7,22 @@ package container // See hack/generate-swagger-api.sh // ---------------------------------------------------------------------------- +// ContainerWaitOKBodyError container waiting error, if any +// swagger:model ContainerWaitOKBodyError +type ContainerWaitOKBodyError struct { + + // Details of an error + Message string `json:"Message,omitempty"` +} + // ContainerWaitOKBody container wait o k body // swagger:model ContainerWaitOKBody type ContainerWaitOKBody struct { + // error + // Required: true + Error *ContainerWaitOKBodyError `json:"Error"` + // Exit code of the container // Required: true StatusCode int64 `json:"StatusCode"` diff --git a/components/engine/container/state.go b/components/engine/container/state.go index cdf51d37d2..b624fe1dee 100644 --- a/components/engine/container/state.go +++ b/components/engine/container/state.go @@ -29,7 +29,7 @@ type State struct { Dead bool Pid int ExitCodeValue int `json:"ExitCode"` - ErrorMsg string `json:"Error"` // contains last known error when starting the container + ErrorMsg string `json:"Error"` // contains last known error during container start or remove StartedAt time.Time FinishedAt time.Time Health *Health @@ -319,7 +319,10 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) { // know the error that occurred when container transits to another state // when inspecting it func (s *State) SetError(err error) { - s.ErrorMsg = err.Error() + s.ErrorMsg = "" + if err != nil { + s.ErrorMsg = err.Error() + } } // IsPaused returns whether the container is paused or not. @@ -385,8 +388,18 @@ func (s *State) IsDead() bool { // closes the internal waitRemove channel to unblock callers waiting for a // container to be removed. func (s *State) SetRemoved() { + s.SetRemovalError(nil) +} + +// SetRemovalError is to be called in case a container remove failed. +// It sets an error and closes the internal waitRemove channel to unblock +// callers waiting for the container to be removed. +func (s *State) SetRemovalError(err error) { + s.SetError(err) s.Lock() close(s.waitRemove) // Unblock those waiting on remove. + // Recreate the channel so next ContainerWait will work + s.waitRemove = make(chan struct{}) s.Unlock() } diff --git a/components/engine/daemon/delete.go b/components/engine/daemon/delete.go index 3009400c09..b5dd258a66 100644 --- a/components/engine/daemon/delete.go +++ b/components/engine/daemon/delete.go @@ -120,12 +120,16 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo metadata, err := daemon.stores[container.OS].layerStore.ReleaseRWLayer(container.RWLayer) layer.LogReleaseMetadata(metadata) if err != nil && err != layer.ErrMountDoesNotExist && !os.IsNotExist(errors.Cause(err)) { - return errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID) + e := errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID) + container.SetRemovalError(e) + return e } } if err := system.EnsureRemoveAll(container.Root); err != nil { - return errors.Wrapf(err, "unable to remove filesystem for %s", container.ID) + e := errors.Wrapf(err, "unable to remove filesystem for %s", container.ID) + container.SetRemovalError(e) + return e } linkNames := daemon.linkIndex.delete(container) diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 9ac31dcaaf..77b8545bcc 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -17,12 +17,19 @@ keywords: "API, Docker, rcli, REST, documentation" [Docker Engine API v1.34](https://docs.docker.com/engine/api/v1.34/) documentation +* `POST /containers/(name)/wait?condition=removed` now also also returns + in case of container removal failure. A pointer to a structure named + `Error` added to the response JSON in order to indicate a failure. + If `Error` is `null`, container removal has succeeded, otherwise + the test of an error message indicating why container removal has failed + is available from `Error.Message` field. + ## v1.33 API changes [Docker Engine API v1.33](https://docs.docker.com/engine/api/v1.33/) documentation * `GET /events` now supports filtering 4 more kinds of events: `config`, `node`, -`secret` and `service`. +`secret` and `service`. ## v1.32 API changes From b238ed565f919f606a6de586ece163cfc23110d0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 27 Oct 2017 09:59:09 +0200 Subject: [PATCH 3/5] Improve devicemapper driver-status output Do not print "Data file" and "Metadata file" if they're not used, and sort/group output. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 8f702de9b705ced68b6244239ac81d86ebdd6b0a Component: engine --- .../daemon/graphdriver/devmapper/driver.go | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index f41afa2ae7..b485096bc8 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/components/engine/daemon/graphdriver/devmapper/driver.go @@ -73,19 +73,14 @@ func (d *Driver) Status() [][2]string { {"Pool Blocksize", units.HumanSize(float64(s.SectorSize))}, {"Base Device Size", units.HumanSize(float64(s.BaseDeviceSize))}, {"Backing Filesystem", s.BaseDeviceFS}, - {"Data file", s.DataFile}, - {"Metadata file", s.MetadataFile}, - {"Data Space Used", units.HumanSize(float64(s.Data.Used))}, - {"Data Space Total", units.HumanSize(float64(s.Data.Total))}, - {"Data Space Available", units.HumanSize(float64(s.Data.Available))}, - {"Metadata Space Used", units.HumanSize(float64(s.Metadata.Used))}, - {"Metadata Space Total", units.HumanSize(float64(s.Metadata.Total))}, - {"Metadata Space Available", units.HumanSize(float64(s.Metadata.Available))}, - {"Thin Pool Minimum Free Space", units.HumanSize(float64(s.MinFreeSpace))}, {"Udev Sync Supported", fmt.Sprintf("%v", s.UdevSyncSupported)}, - {"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)}, - {"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)}, - {"Deferred Deleted Device Count", fmt.Sprintf("%v", s.DeferredDeletedDeviceCount)}, + } + + if len(s.DataFile) > 0 { + status = append(status, [2]string{"Data file", s.DataFile}) + } + if len(s.MetadataFile) > 0 { + status = append(status, [2]string{"Metadata file", s.MetadataFile}) } if len(s.DataLoopback) > 0 { status = append(status, [2]string{"Data loop file", s.DataLoopback}) @@ -93,6 +88,20 @@ func (d *Driver) Status() [][2]string { if len(s.MetadataLoopback) > 0 { status = append(status, [2]string{"Metadata loop file", s.MetadataLoopback}) } + + status = append(status, [][2]string{ + {"Data Space Used", units.HumanSize(float64(s.Data.Used))}, + {"Data Space Total", units.HumanSize(float64(s.Data.Total))}, + {"Data Space Available", units.HumanSize(float64(s.Data.Available))}, + {"Metadata Space Used", units.HumanSize(float64(s.Metadata.Used))}, + {"Metadata Space Total", units.HumanSize(float64(s.Metadata.Total))}, + {"Metadata Space Available", units.HumanSize(float64(s.Metadata.Available))}, + {"Thin Pool Minimum Free Space", units.HumanSize(float64(s.MinFreeSpace))}, + {"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)}, + {"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)}, + {"Deferred Deleted Device Count", fmt.Sprintf("%v", s.DeferredDeletedDeviceCount)}, + }...) + if vStr, err := devicemapper.GetLibraryVersion(); err == nil { status = append(status, [2]string{"Library Version", vStr}) } From 67f37f6e34dc6e4c75f4a273b1fc8e4d225a3e6e Mon Sep 17 00:00:00 2001 From: John Howard Date: Thu, 26 Oct 2017 14:16:43 -0700 Subject: [PATCH 4/5] Fixes LCOW after containerd 1.0 introduced regressions Signed-off-by: John Howard Upstream-commit: 71651e0b801ae874b4a899e3c47add9e3fbc2400 Component: engine --- components/engine/libcontainerd/client_local_windows.go | 2 +- components/engine/oci/defaults.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/libcontainerd/client_local_windows.go b/components/engine/libcontainerd/client_local_windows.go index 209b00db67..c33e346a7a 100644 --- a/components/engine/libcontainerd/client_local_windows.go +++ b/components/engine/libcontainerd/client_local_windows.go @@ -513,7 +513,7 @@ func (c *client) createLinux(id string, spec *specs.Spec, runtimeOptions interfa ctr := &container{ id: id, execs: make(map[string]*process), - isWindows: true, + isWindows: false, ociSpec: spec, hcsContainer: hcsContainer, status: StatusCreated, diff --git a/components/engine/oci/defaults.go b/components/engine/oci/defaults.go index 667dd4147a..0cc07ffa13 100644 --- a/components/engine/oci/defaults.go +++ b/components/engine/oci/defaults.go @@ -65,6 +65,7 @@ func DefaultLinuxSpec() specs.Spec { Effective: defaultCapabilities(), }, }, + Root: &specs.Root{}, } s.Mounts = []specs.Mount{ { From be3cbac37e04a6eca9628b3ddf2845282fdd1dd6 Mon Sep 17 00:00:00 2001 From: chaowang Date: Sat, 28 Oct 2017 08:28:19 +0800 Subject: [PATCH 5/5] Separate the GenerateRandomAlphaOnlyString function from stringutils Signed-off-by: chaowang Upstream-commit: 7c35a2418265336a572976e2ced378ef4b6f1666 Component: engine --- .../cli/build/fakestorage/storage.go | 6 ++-- .../integration-cli/docker_cli_build_test.go | 4 +-- .../integration-cli/docker_cli_run_test.go | 4 +-- .../integration-cli/docker_cli_tag_test.go | 4 +-- .../engine/integration-cli/utils_test.go | 4 +-- .../engine/internal/testutil/stringutils.go | 14 ++++++++ .../internal/testutil/stringutils_test.go | 33 +++++++++++++++++++ .../engine/pkg/stringutils/stringutils.go | 11 ------- .../pkg/stringutils/stringutils_test.go | 8 ----- 9 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 components/engine/internal/testutil/stringutils.go create mode 100644 components/engine/internal/testutil/stringutils_test.go diff --git a/components/engine/integration-cli/cli/build/fakestorage/storage.go b/components/engine/integration-cli/cli/build/fakestorage/storage.go index 25cd872f50..eb0363628d 100644 --- a/components/engine/integration-cli/cli/build/fakestorage/storage.go +++ b/components/engine/integration-cli/cli/build/fakestorage/storage.go @@ -14,7 +14,7 @@ import ( "github.com/docker/docker/integration-cli/cli/build/fakecontext" "github.com/docker/docker/integration-cli/request" "github.com/docker/docker/internal/test/environment" - "github.com/docker/docker/pkg/stringutils" + "github.com/docker/docker/internal/testutil" "github.com/stretchr/testify/require" ) @@ -124,8 +124,8 @@ func (f *remoteFileServer) Close() error { func newRemoteFileServer(t testingT, ctx *fakecontext.Fake) *remoteFileServer { var ( - image = fmt.Sprintf("fileserver-img-%s", strings.ToLower(stringutils.GenerateRandomAlphaOnlyString(10))) - container = fmt.Sprintf("fileserver-cnt-%s", strings.ToLower(stringutils.GenerateRandomAlphaOnlyString(10))) + image = fmt.Sprintf("fileserver-img-%s", strings.ToLower(testutil.GenerateRandomAlphaOnlyString(10))) + container = fmt.Sprintf("fileserver-cnt-%s", strings.ToLower(testutil.GenerateRandomAlphaOnlyString(10))) ) ensureHTTPServerImage(t) diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index f6ab1923b9..89e62c14e0 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -23,8 +23,8 @@ import ( "github.com/docker/docker/integration-cli/cli/build/fakecontext" "github.com/docker/docker/integration-cli/cli/build/fakegit" "github.com/docker/docker/integration-cli/cli/build/fakestorage" + "github.com/docker/docker/internal/testutil" "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/stringutils" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" digest "github.com/opencontainers/go-digest" @@ -3185,7 +3185,7 @@ func (s *DockerSuite) TestBuildOnBuildOutput(c *check.C) { // FIXME(vdemeester) should be a unit test func (s *DockerSuite) TestBuildInvalidTag(c *check.C) { - name := "abcd:" + stringutils.GenerateRandomAlphaOnlyString(200) + name := "abcd:" + testutil.GenerateRandomAlphaOnlyString(200) buildImage(name, build.WithDockerfile("FROM "+minimalBaseImage()+"\nMAINTAINER quux\n")).Assert(c, icmd.Expected{ ExitCode: 125, Err: "invalid reference format", diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 8198fded72..eb01e34634 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -26,10 +26,10 @@ import ( "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/cli/build" "github.com/docker/docker/integration-cli/cli/build/fakecontext" + "github.com/docker/docker/internal/testutil" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/parsers/kernel" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/stringutils" "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" "github.com/docker/libnetwork/resolvconf" @@ -1828,7 +1828,7 @@ func testRunWriteSpecialFilesAndNotCommit(c *check.C, name, path string) { } func eqToBaseDiff(out string, c *check.C) bool { - name := "eqToBaseDiff" + stringutils.GenerateRandomAlphaOnlyString(32) + name := "eqToBaseDiff" + testutil.GenerateRandomAlphaOnlyString(32) dockerCmd(c, "run", "--name", name, "busybox", "echo", "hello") cID := getIDByName(c, name) baseDiff, _ := dockerCmd(c, "diff", cID) diff --git a/components/engine/integration-cli/docker_cli_tag_test.go b/components/engine/integration-cli/docker_cli_tag_test.go index 907977fc06..ee94a9b14e 100644 --- a/components/engine/integration-cli/docker_cli_tag_test.go +++ b/components/engine/integration-cli/docker_cli_tag_test.go @@ -6,8 +6,8 @@ import ( "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli/build" + "github.com/docker/docker/internal/testutil" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/stringutils" "github.com/go-check/check" ) @@ -34,7 +34,7 @@ func (s *DockerSuite) TestTagInvalidUnprefixedRepo(c *check.C) { // ensure we don't allow the use of invalid tags; these tag operations should fail func (s *DockerSuite) TestTagInvalidPrefixedRepo(c *check.C) { - longTag := stringutils.GenerateRandomAlphaOnlyString(121) + longTag := testutil.GenerateRandomAlphaOnlyString(121) invalidTags := []string{"repo:fo$z$", "repo:Foo@3cc", "repo:Foo$3", "repo:Foo*3", "repo:Fo^3", "repo:Foo!3", "repo:%goodbye", "repo:#hashtagit", "repo:F)xcz(", "repo:-foo", "repo:..", longTag} diff --git a/components/engine/integration-cli/utils_test.go b/components/engine/integration-cli/utils_test.go index d176c7f062..1146e1b2fc 100644 --- a/components/engine/integration-cli/utils_test.go +++ b/components/engine/integration-cli/utils_test.go @@ -7,7 +7,7 @@ import ( "path/filepath" "strings" - "github.com/docker/docker/pkg/stringutils" + "github.com/docker/docker/internal/testutil" "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" "github.com/pkg/errors" @@ -60,7 +60,7 @@ func RandomTmpDirPath(s string, platform string) string { if platform == "windows" { tmp = os.Getenv("TEMP") } - path := filepath.Join(tmp, fmt.Sprintf("%s.%s", s, stringutils.GenerateRandomAlphaOnlyString(10))) + path := filepath.Join(tmp, fmt.Sprintf("%s.%s", s, testutil.GenerateRandomAlphaOnlyString(10))) if platform == "windows" { return filepath.FromSlash(path) // Using \ } diff --git a/components/engine/internal/testutil/stringutils.go b/components/engine/internal/testutil/stringutils.go new file mode 100644 index 0000000000..76cf8d86a9 --- /dev/null +++ b/components/engine/internal/testutil/stringutils.go @@ -0,0 +1,14 @@ +package testutil + +import "math/rand" + +// GenerateRandomAlphaOnlyString generates an alphabetical random string with length n. +func GenerateRandomAlphaOnlyString(n int) string { + // make a really long string + letters := []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + b := make([]byte, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} diff --git a/components/engine/internal/testutil/stringutils_test.go b/components/engine/internal/testutil/stringutils_test.go new file mode 100644 index 0000000000..2985a06553 --- /dev/null +++ b/components/engine/internal/testutil/stringutils_test.go @@ -0,0 +1,33 @@ +package testutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func testLengthHelper(generator func(int) string, t *testing.T) { + expectedLength := 20 + s := generator(expectedLength) + assert.Equal(t, expectedLength, len(s)) +} + +func testUniquenessHelper(generator func(int) string, t *testing.T) { + repeats := 25 + set := make(map[string]struct{}, repeats) + for i := 0; i < repeats; i = i + 1 { + str := generator(64) + assert.Equal(t, 64, len(str)) + _, ok := set[str] + assert.False(t, ok, "Random number is repeated") + set[str] = struct{}{} + } +} + +func TestGenerateRandomAlphaOnlyStringLength(t *testing.T) { + testLengthHelper(GenerateRandomAlphaOnlyString, t) +} + +func TestGenerateRandomAlphaOnlyStringUniqueness(t *testing.T) { + testUniquenessHelper(GenerateRandomAlphaOnlyString, t) +} diff --git a/components/engine/pkg/stringutils/stringutils.go b/components/engine/pkg/stringutils/stringutils.go index 8c4c39875e..b294de2c23 100644 --- a/components/engine/pkg/stringutils/stringutils.go +++ b/components/engine/pkg/stringutils/stringutils.go @@ -7,17 +7,6 @@ import ( "strings" ) -// GenerateRandomAlphaOnlyString generates an alphabetical random string with length n. -func GenerateRandomAlphaOnlyString(n int) string { - // make a really long string - letters := []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - b := make([]byte, n) - for i := range b { - b[i] = letters[rand.Intn(len(letters))] - } - return string(b) -} - // GenerateRandomASCIIString generates an ASCII random string with length n. func GenerateRandomASCIIString(n int) string { chars := "abcdefghijklmnopqrstuvwxyz" + diff --git a/components/engine/pkg/stringutils/stringutils_test.go b/components/engine/pkg/stringutils/stringutils_test.go index 8af2bdcc0b..15b3cf8e86 100644 --- a/components/engine/pkg/stringutils/stringutils_test.go +++ b/components/engine/pkg/stringutils/stringutils_test.go @@ -34,14 +34,6 @@ func isASCII(s string) bool { return true } -func TestGenerateRandomAlphaOnlyStringLength(t *testing.T) { - testLengthHelper(GenerateRandomAlphaOnlyString, t) -} - -func TestGenerateRandomAlphaOnlyStringUniqueness(t *testing.T) { - testUniquenessHelper(GenerateRandomAlphaOnlyString, t) -} - func TestGenerateRandomAsciiStringLength(t *testing.T) { testLengthHelper(GenerateRandomASCIIString, t) }