CR changes: functional option for timeout, default timeout to min/max…

… instead of erroring on incorrect values

container.go

@@ -48,7 +48,7 @@ type Container interface {
 	Terminate(context.Context) error             // terminate the container
 	Logs(context.Context) (io.ReadCloser, error) // Get logs of the container
 	FollowOutput(LogConsumer)
-	StartLogProducer(context.Context, time.Duration) error
+	StartLogProducer(context.Context, ...LogProducerOption) error
 	StopLogProducer() error
 	Name(context.Context) (string, error)                        // get container name
 	State(context.Context) (*types.ContainerState, error)        // returns container's running state

docker.go

@@ -73,6 +73,7 @@ type DockerContainer struct {
 	producerDone      chan bool
 	producerError     chan error
 	producerMutex     sync.Mutex
+	producerTimeout   *time.Duration
 	logger            Logging
 	lifecycleHooks    []ContainerLifecycleHooks
 }
@@ -614,12 +615,22 @@ func (c *DockerContainer) CopyToContainer(ctx context.Context, fileContent []byt
 	return nil
 }
 
+type LogProducerOption func(*DockerContainer)
+
+func WithLogProducerTimeout(timeout time.Duration) LogProducerOption {
+	return func(c *DockerContainer) {
+		c.producerTimeout = &timeout
+	}
+}
+
 // StartLogProducer will start a concurrent process that will continuously read logs
-// from the container and will send them to each added LogConsumer
-// timeout accepts values between 5 and 60s and will be used to set the context timeout
+// from the container and will send them to each added LogConsumer.
+// Default log producer timeout is 5s. It is used to set the context timeout
 // which means that each log-reading loop will last at least the specified timeout
-// and that it cannot be cancelled earlier
-func (c *DockerContainer) StartLogProducer(ctx context.Context, timeout time.Duration) error {
+// and that it cannot be cancelled earlier.
+// Use functional option WithLogProducerTimeout() to override default timeout. If it's
+// lower than 5s and greater than 60s it will be set to 5s or 60s respectively.
+func (c *DockerContainer) StartLogProducer(ctx context.Context, opts ...LogProducerOption) error {
 	{
 		c.producerMutex.Lock()
 		defer c.producerMutex.Unlock()
@@ -629,8 +640,23 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context, timeout time.Dur
 		}
 	}
 
-	if timeout < time.Duration(5*time.Second) || timeout > time.Duration(60*time.Second) {
-		return errors.New("timeout must be between 5 and 60 seconds")
+	for _, opt := range opts {
+		opt(c)
+	}
+
+	minProducerTimeout := time.Duration(5 * time.Second)
+	maxProducerTimeout := time.Duration(60 * time.Second)
+
+	if c.producerTimeout == nil {
+		c.producerTimeout = &minProducerTimeout
+	}
+
+	if *c.producerTimeout < minProducerTimeout {
+		c.producerTimeout = &minProducerTimeout
+	}
+
+	if *c.producerTimeout > maxProducerTimeout {
+		c.producerTimeout = &maxProducerTimeout
 	}
 
 	c.stopProducer = make(chan bool)
@@ -660,7 +686,7 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context, timeout time.Dur
 			Since:      since,
 		}
 
-		ctx, cancel := context.WithTimeout(ctx, timeout)
+		ctx, cancel := context.WithTimeout(ctx, *c.producerTimeout)
 		defer cancel()
 
 		r, err := c.provider.client.ContainerLogs(ctx, c.GetContainerID(), options)

logconsumer_test.go

@@ -81,7 +81,7 @@ func Test_LogConsumerGetsCalled(t *testing.T) {
 
 	c.FollowOutput(&g)
 
-	err = c.StartLogProducer(ctx, time.Duration(5*time.Second))
+	err = c.StartLogProducer(ctx)
 	require.NoError(t, err)
 
 	_, err = http.Get(ep + "/stdout?echo=hello")
@@ -148,7 +148,7 @@ func Test_ShouldRecognizeLogTypes(t *testing.T) {
 
 	c.FollowOutput(&g)
 
-	err = c.StartLogProducer(ctx, time.Duration(5*time.Second))
+	err = c.StartLogProducer(ctx)
 	require.NoError(t, err)
 
 	_, err = http.Get(ep + "/stdout?echo=this-is-stdout")
@@ -205,7 +205,7 @@ func Test_MultipleLogConsumers(t *testing.T) {
 	c.FollowOutput(&first)
 	c.FollowOutput(&second)
 
-	err = c.StartLogProducer(ctx, time.Duration(5*time.Second))
+	err = c.StartLogProducer(ctx)
 	require.NoError(t, err)
 
 	_, err = http.Get(ep + "/stdout?echo=mlem")
@@ -254,18 +254,18 @@ func Test_StartStop(t *testing.T) {
 
 	require.NoError(t, c.StopLogProducer(), "nothing should happen even if the producer is not started")
 
-	require.NoError(t, c.StartLogProducer(ctx, time.Duration(5*time.Second)))
+	require.NoError(t, c.StartLogProducer(ctx))
 	require.Equal(t, <-g.Accepted, "ready\n")
 
-	require.Error(t, c.StartLogProducer(ctx, time.Duration(5*time.Second)), "log producer is already started")
+	require.Error(t, c.StartLogProducer(ctx), "log producer is already started")
 
 	_, err = http.Get(ep + "/stdout?echo=mlem")
 	require.NoError(t, err)
 	require.Equal(t, <-g.Accepted, "echo mlem\n")
 
 	require.NoError(t, c.StopLogProducer())
 
-	require.NoError(t, c.StartLogProducer(ctx, time.Duration(5*time.Second)))
+	require.NoError(t, c.StartLogProducer(ctx))
 	require.Equal(t, <-g.Accepted, "ready\n")
 	require.Equal(t, <-g.Accepted, "echo mlem\n")
 
@@ -375,7 +375,7 @@ func TestContainerLogWithErrClosed(t *testing.T) {
 		Accepted: devNullAcceptorChan(),
 	}
 
-	if err = nginx.StartLogProducer(ctx, time.Duration(5*time.Second)); err != nil {
+	if err = nginx.StartLogProducer(ctx); err != nil {
 		t.Fatal(err)
 	}
 	defer func() {
@@ -454,7 +454,7 @@ func TestContainerLogsShouldBeWithoutStreamHeader(t *testing.T) {
 	assert.Equal(t, "0", strings.TrimSpace(string(b)))
 }
 
-func Test_StartLogProducerErrorsWithTooLowTimeout(t *testing.T) {
+func Test_StartLogProducerStillStartssWithTooLowTimeout(t *testing.T) {
 	ctx := context.Background()
 	req := ContainerRequest{
 		FromDockerfile: FromDockerfile{
@@ -481,12 +481,11 @@ func Test_StartLogProducerErrorsWithTooLowTimeout(t *testing.T) {
 
 	c.FollowOutput(&g)
 
-	err = c.StartLogProducer(ctx, time.Duration(4*time.Second))
-	require.Error(t, err)
-	require.Equal(t, "timeout must be between 5 and 60 seconds", err.Error())
+	err = c.StartLogProducer(ctx, WithLogProducerTimeout(4*time.Second))
+	require.NoError(t, err, "should still start with too low timeout")
 }
 
-func Test_StartLogProducerErrorsWithTooHighTimeout(t *testing.T) {
+func Test_StartLogProducerStillStartsWithTooHighTimeout(t *testing.T) {
 	ctx := context.Background()
 	req := ContainerRequest{
 		FromDockerfile: FromDockerfile{
@@ -513,7 +512,6 @@ func Test_StartLogProducerErrorsWithTooHighTimeout(t *testing.T) {
 
 	c.FollowOutput(&g)
 
-	err = c.StartLogProducer(ctx, time.Duration(61*time.Second))
-	require.Error(t, err)
-	require.Equal(t, "timeout must be between 5 and 60 seconds", err.Error())
+	err = c.StartLogProducer(ctx, WithLogProducerTimeout(61*time.Second))
+	require.NoError(t, err, "should still start with too high timeout")
 }