Skip to content

Conversation

@mrodm
Copy link
Contributor

@mrodm mrodm commented Jan 18, 2024

Latest Buildkite builds that are executing the docker compose for all the commands under elastic-package looks like are not honoring the parameter --ansi never

This --ansi parameter is a valid parameter as it is mentioned in the docs: https://docs.docker.com/engine/reference/commandline/compose/#options

These are two BK buildsexamples where it can be seen these changes in the output of docker compose vs docker-compose commands:

In this PR, it is added a new parameter --progress available just for docker compose command. If used plain as value, results are similar to what --ansi never does. If used quiet as value, there is no output at all.

A new section is added to the README , to include all the environment variables available that could be used when using elastic-package CLI.

Testing this parameter locally

 $ docker compose --progress=plain down ; echo "-----" ; docker rmi ubuntu:latest ; echo "--------" ; docker compose  --progress=plain up -d --quiet-pull
 Container dockertest-ubuntu-1  Stopping
 Container dockertest-ubuntu-1  Stopped
 Container dockertest-ubuntu-1  Removing
 Container dockertest-ubuntu-1  Removed
 Network dockertest_default  Removing
 Network dockertest_default  Removed
-----
Untagged: ubuntu:latest
Untagged: ubuntu@sha256:e6173d4dc55e76b87c4af8db8821b1feae4146dd47341e4d431118c7dd060a74
Deleted: sha256:e34e831650c1bb0be9b6f61c6755749cb8ea2053ba91c6cda27fded9e089811f
Deleted: sha256:8e87ff28f1b5ff2d5131999ccfa1e674cb252631c50683f5ee43fad59cbea8e1
--------
 ubuntu Pulling 
 ubuntu Pulled 
 Network dockertest_default  Creating
 Network dockertest_default  Created
 Container dockertest-ubuntu-1  Creating
 Container dockertest-ubuntu-1  Created
 Container dockertest-ubuntu-1  Starting
 Container dockertest-ubuntu-1  Started


 $ docker compose --progress=quiet down ; echo "-----" ; docker rmi ubuntu:latest ; echo "--------" ; docker compose  --progress=quiet up -d --quiet-pull
-----
Untagged: ubuntu:latest
Untagged: ubuntu@sha256:e6173d4dc55e76b87c4af8db8821b1feae4146dd47341e4d431118c7dd060a74
Deleted: sha256:e34e831650c1bb0be9b6f61c6755749cb8ea2053ba91c6cda27fded9e089811f
Deleted: sha256:8e87ff28f1b5ff2d5131999ccfa1e674cb252631c50683f5ee43fad59cbea8e1
--------

  • Docker-compose used for the example:
version: '2.3'
services:
  ubuntu:
    image: ubuntu
    command: tail -F anything
    volumes:
      - type: bind
        source: ./test
        target: /test

}

v, ok = os.LookupEnv(DisableProgressOutputComposeEnv)
if !c.dockerComposeV1 && !c.dockerComposeStandalone && ok && strings.ToLower(v) != "false" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option is just available using docker compose

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? This behaves the same in docker-compose (trying with standalone docker-compose v2.23.3).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying with docker-compose with version 2.17.2 (same version that was set previously in CI for it). In that version there is no --progress flag.

I see that in the latest version of docker-compose standalone (v2.24.1), that version is already in place.

@mrodm mrodm force-pushed the update_params_disable_ansi branch from f76c295 to 4662354 Compare January 18, 2024 17:25
@mrodm mrodm requested a review from a team January 18, 2024 17:26
@mrodm mrodm self-assigned this Jan 18, 2024
Comment on lines 4 to 5
# ELASTIC_PACKAGE_COMPOSE_DISABLE_PROGRESS_OUTPUT: "true"
ELASTIC_PACKAGE_COMPOSE_PROGRESS_OUTPUT: "quiet"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsoriano any preference about which environment would be best to set ?

ELASTIC_PACKAGE_COMPOSE_DISABLE_PROGRESS_OUTPUT would be just to set true or false, and we would need to decide if we want plain or quiet for the --progress flag.

In ELASTIC_PACKAGE_COMPOSE_PROGRESS_OUTPUT var, a few values would be allowed (auto, plain or quiet) and the user could choose which one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For CI quite looks good 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or plain, with --quiet-pull also looks good for CI.

* `stack.serverless.region` can be used to select the region to use when starting
serverless projects.

## Useful environment variables
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK there is no docs about all the environment variables that are defined in elastic-package. Just added this section to include all these references

WDYT ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@mrodm mrodm force-pushed the update_params_disable_ansi branch from 4662354 to 00c34ad Compare January 18, 2024 17:31
Comment on lines 40 to 41
DisableProgressOutputComposeEnv = environment.WithElasticPackagePrefix("COMPOSE_DISABLE_PROGRESS_OUTPUT")
ProgressOutputComposeEnv = environment.WithElasticPackagePrefix("COMPOSE_PROGRESS_OUTPUT")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should decide which one to use here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COMPOSE_DISABLE_PROGRESS_OUTPUT sounds good to me, we don't need to expose all the options.

@mrodm mrodm marked this pull request as ready for review January 18, 2024 17:32
@mrodm mrodm force-pushed the update_params_disable_ansi branch from 00c34ad to f8f7578 Compare January 18, 2024 17:41
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest Buildkite builds that are executing the docker compose for all the commands under elastic-package looks like are not honoring the parameter --ansi never.

I think it is not a matter of docker-compose vs docker compose but about a regression(?) on recent versions. Trying locally with standalone docker-compose v2.23.3 it neither honors --ansi never.

Given the differences between the different versions I wonder if instead of exposing the different flags, we should have a single flag to enable or disable this verbose output, that under the hood uses any flag known to work for each version.

Comment on lines 4 to 5
# ELASTIC_PACKAGE_COMPOSE_DISABLE_PROGRESS_OUTPUT: "true"
ELASTIC_PACKAGE_COMPOSE_PROGRESS_OUTPUT: "quiet"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For CI quite looks good 👍

* `stack.serverless.region` can be used to select the region to use when starting
serverless projects.

## Useful environment variables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment on lines 4 to 5
# ELASTIC_PACKAGE_COMPOSE_DISABLE_PROGRESS_OUTPUT: "true"
ELASTIC_PACKAGE_COMPOSE_PROGRESS_OUTPUT: "quiet"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or plain, with --quiet-pull also looks good for CI.

Comment on lines 40 to 41
DisableProgressOutputComposeEnv = environment.WithElasticPackagePrefix("COMPOSE_DISABLE_PROGRESS_OUTPUT")
ProgressOutputComposeEnv = environment.WithElasticPackagePrefix("COMPOSE_PROGRESS_OUTPUT")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COMPOSE_DISABLE_PROGRESS_OUTPUT sounds good to me, we don't need to expose all the options.

}

v, ok = os.LookupEnv(DisableProgressOutputComposeEnv)
if !c.dockerComposeV1 && !c.dockerComposeStandalone && ok && strings.ToLower(v) != "false" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? This behaves the same in docker-compose (trying with standalone docker-compose v2.23.3).

@mrodm
Copy link
Contributor Author

mrodm commented Jan 22, 2024

I think it is not a matter of docker-compose vs docker compose but about a regression(?) on recent versions. Trying locally with standalone docker-compose v2.23.3 it neither honors --ansi never.

Just tried with the latest version (v2.24.1) and it happens the same, --ansi never flag is not taken into account.

Given the differences between the different versions I wonder if instead of exposing the different flags, we should have a single flag to enable or disable this verbose output, that under the hood uses any flag known to work for each version.

Should we use a new flag, for instance just ELASTIC_PACKAGE_COMPOSE_VERBOSE_OUTPUT and remove the flags ELASTIC_PACKAGE_COMPOSE_DISABLE_ANSI and ELASTIC_PACKAGE_COMPOSE_PROGRESS_OUTPUT in this PR ? Should it be removed ELASTIC_PACKAGE_COMPOSE_DISABLE_PULL_PROGRESS_INFORMATION?
That new flag in case of docker-compose v1 would just add --ansi never in docker-compose v2 depending on the version --progress flag exists or not.

Not sure if adding just one variable would just force to update docker-compose versions to users. If it is kept different environment variables, probably it is easier to set up by users depending on their versions. Or maybe having all these environment variables make more difficult to know which one to use... WDYT ? @jsoriano

And checking with the latest-version, it looks also that --quiet-pull is ignored in docker-copmpose version v2.24.1 in my tests. Example:

# docker-compose v2.24.1
 $ docker-compose --ansi=never down ; echo "-----" ; docker rmi ubuntu:latest ; echo "--------" ; docker-compose  --ansi=never up -d --quiet-pull
[+] Running 2/2
 ✔ Container dockertest-ubuntu-1  Removed                                                         10.3s 
 ✔ Network dockertest_default     Removed                                                          0.4s 
-----
Untagged: ubuntu:latest
Untagged: ubuntu@sha256:e6173d4dc55e76b87c4af8db8821b1feae4146dd47341e4d431118c7dd060a74
Deleted: sha256:e34e831650c1bb0be9b6f61c6755749cb8ea2053ba91c6cda27fded9e089811f
Deleted: sha256:8e87ff28f1b5ff2d5131999ccfa1e674cb252631c50683f5ee43fad59cbea8e1
--------
[+] Running 1/1
 ✔ ubuntu Pulled                                                                                   5.8s 
[+] Running 1/2
 ⠴ Network dockertest_default     Created                                                          0.4s 
 ✔ Container dockertest-ubuntu-1  Started    

@mrodm mrodm requested a review from jsoriano January 22, 2024 11:56
@mrodm
Copy link
Contributor Author

mrodm commented Jan 22, 2024

buildkite test this

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm merged commit 2ec9be9 into elastic:main Jan 23, 2024
@mrodm mrodm deleted the update_params_disable_ansi branch January 23, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants