Push builds images without respecting the args argument

[patrobinson]

Push builds an image without respecting args argument
If you specify build and push at the same time, the image will be built twice
The resulting image built without args argument will be pushed

So if you configure the plugin like so:

- docker-compose#v3.5.0:
          build: web
          args:
            - NODE_ENV=staging
            - RACK_ENV=staging
          push: foo:bar

1. args has no effect
2. your build takes twice as long because it gets built twice

This way works:

- docker-compose#v3.5.0:
          build: web
          args:
            - NODE_ENV=staging
            - RACK_ENV=staging
          image-repository: foo
          image-name: bar

https://github.com/buildkite-plugins/docker-compose-buildkite-plugin/blob/master/commands/push.sh#L41 should probably call the same code path that build uses, allowing it respect all arguments build supports.

[toote]

This indeed looks like a bug or, at least, very poor documentation

[toote]

@patrobinson, first and foremost sorry for the delay in getting back to you. Hope that you have continued to use the plugin since and found the improvements and fixes made since you reported the issue helpful.

I believe I have finally figured out what was wrong with this scenario (in addition to the push portion of the code not respecting the same logic as the build one). To start with, mixing build and push was not officially supported until version 3.12. But most importantly, your push configuration states foo:bar which means service foo to be pushed to repository/tag bar. The matter should have been solved by changing that option to web:foo:bar. That is indeed part of the documentation examples from even before this ticket was opened and - from what I understand of the code - the build portion of the could should have even printed a warning stating that the image wouldn't have been pushed because of missing configuration variables. The rest of the behaviour is extremely dependent on your docker-compose.yml file.

As mentioned in #325 the implications of allowing multiple "stages" to be used in the same step would have unintended consequences and this one appears to be one of them. A double-build is definitely not a desired behaviour, but shouldn't delay the step too much thanks to docker's layer cache (so the second build should basically be a no-op).

If you have more information about this particular scenario we will be more than happy to continue to troubleshoot and/or get ideas for future improvements of this plugin :)

[rjl79]

I've recently experienced a very similar issue, whereby we are building and pushing the image in the same step.
The issue appears to be that the image-repository is required in order for the built image to be shared with the push step, otherwise it results in 2 different images being built.

The main issue and probably most concerning is that the args are not used in the image that is built for the push step, so it results in an image with unexpected output, and in our case for a different environment than intended.

Example config that builds the service web with the args, and then builds web again but doesn't use args which is subsequently pushed.

- docker-compose#v4.9.0:
          build: web
          args:
            - MIX_ENV=prod
          push:
            - web:foo:bar

In fact, if you just use the push command without build at all, it will also not respect the args.

- docker-compose#v4.9.0:
          args:
            - MIX_ENV=prod
          push:
            - web:foo:bar

Adding image-repository only builds once with args, and shares the image with push which has the desired effect.

- docker-compose#v4.9.0:
          build: web
          args:
            - MIX_ENV=prod
          image-repository: foo:test
          push: 
            - web:foo:bar

I think this is quite likely by design, as docs refer to args being only for build and run steps, but perhaps there should be a warning saying args that are defined will not be used in these scenarios, as it can lead to confusion.

Happy to provide more info or logs if useful.

[toote]

@rjl79 that would be correct... I'm thinking of a backwards-incompatible change in which images are not built in run nor push configurations and, while we are at it, unifying the configurations used. What would you think of that change?

[rjl79]

@toote thanks for the quick response.
I'm considering our use case of the plugin and I don't think this would cause any issues, and if it did it would highlight where potential risks exist due to expectations of how the plugin behaves.

Would you see this as requiring an explicit build configuration in a previous plugin definition or within the same one that specified run or push?

[toote]

@rjl79 there are three possible scenarios:

• requiring a build configuration: it could be implemented, but may be tricky and break a lot of currently functional scenarios in which this plugin is preferred over the regular docker plugin
• using build at the same time as run or push: currently supported but plagued with a mismatch of configuration options and inconsistent behaviours (and the scenario we want to correct)
• using build in a previous step: currently supported and, as far as I understand, works correctly; it is even tested through integration tests in the plugin's pipeline