horizon: build docker image with --no-cache flag

[jacekn]

What

Add --no-cache flag to horizon docker build.

Why

We provide stellar-horizon package version as parameter but we don't do that for core.
This means we may end up using cached layer for package installs and this may result
in older core being installed

Testing

The change was tested locally using:

VERSION=2.17.0-267 make docker-build

[Shaptic]

We either need to create a separate branch for this, or cherry-pick this change from master into a separate branch. Otherwise, the build we want (2.17.0 w/ 19.0.1) will not happen, since it will contain all of the changes since the last Horizon tag.

[leighmcculloch]

I think this change will unfairly punish folks using the image. I would rather see the stellar-core version become a parameter like the horizon version. Doing so would mean the image is more deterministic.

For the specific 19.0.1 case I think you should add in a command to clear the cache in the Jenkins pipeline file. It won't require tagging a new Horizon to do it. Or, @stellar/ops-team could ssh into the Jenkins workers and simply clear the docker cache.

[Shaptic]

That may probably a better holistic approach, but that's a huge break in how this image worked prior. Every tag is tied to a specific build of Horizon and the "latest" version of Core at the time it's built. It's basically a snapshot of the world at the time of release. I think it's okay to update that snapshot with a new picture of the world since the old picture was found to be broken. Having yet another place to forget to bump versions is frustrating.

Anyway, I'm not entirely opposed, but if we opt for that moving forward, we'd need to tag images with both Horizon and Core versions, so instead of 2.17.0-267 it would be something like 2.17.0-267~19.0.1-[xxx].

[leighmcculloch]

we'd need to tag images with both Horizon and Core versions, so instead of 2.17.0-267 it would be something like 2.17.0-267~19.0.1-[xxx].

You could simply bump the stellar-horizon version each time you need to change the stellar-core version it depends on. i.e. build the specific stellar-core version into the stellar-horizon Dockerfile. So you create a clear and solid 1-1 mapping between the version of stellar-core that stellar-horizon is intended to require. But I get that may also have downsides.

cc @ire-and-curses in case he has ideas.

[2opremio]

To solve the docker build caching problems we have, I think we should just add a build parameter with core's version. This will make docker cache things correctly.

Also, the horizon docker image tag should probably reflect the core version included (otherwise we may end up releasing mutable tags, i.e. different images with the same tag but different core versions).

[Shaptic]

This goes back to "What is the intent of the stellar/stellar-horizon image?"

To me, the intent seems pretty clear: "This is stable Horizon vX.Y.Z packaged with a working stable Core." If that means replacing the image because that Core version wasn't working, that's completely fine. There's no tangible benefit to having broken images in the registry, and I can easily imagine us dealing with people downloading broken images and it being hard to track down why their Horizon isn't working.

If the intent is different, i.e. "This is Horizon vX.Y.Z packaged with Core vA.B.C," then fine, but that seems... less useful.

[leighmcculloch]

That's a good point. We should also ask ourselves what would we do if we discovered another arbitrary dependency in the Docker image contained a vulnerability or bug we wanted to patch for: We aren't going to include the version of that dependency in the image version, we're probably just going to rebuild it and retag it with the same Horizon version. So I agree, including the core version in the horizon image version is unnecessary.

For anyone who wants a fixed Docker image that never changes they should be using the sha256 digest hash alongside the tag name anyway so that they download a specific version of a Docker tag.

[jacekn]

To make docker builds fast by default but uncached when needed we could add optional environment variable with support in the Makefile.
Maybe something like this:

DOCKER_OPTS="--no-cache" make docker-build

I think that addresses @leighmcculloch concern about impacting wider audience while also giving us easy way to have fresh, uncached builds

[leighmcculloch]

@jacekn Yup. Alternatively, clear the layer cache on the worker before building, which requires no changes here.

[satyamz]

LGTM!

[2opremio]

Regarding tags, @Shaptic you have a point, but in practice that means that if there is a problem with core (like there has been now) users won't really know whether they are using the right version or not.

Sure, users can always pull, but that brings confusion and problems (due to the mutable tags).

If we don't want to add the full core version, we can add a docker image version which we append whenever there is an update to the same Horizon version.

[jacekn]

@jacekn Yup. Alternatively, clear the layer cache on the worker before building, which requires no changes here.

I pushed new commit to add optional docker flags. It's very low overhead and default behaviour will remain the same. Also maybe in the future we'll use it for some other purpose so I think there is no harm in having it here.

@Shaptic @leighmcculloch @2opremio your points about rebuilding on core updates and immutable tags are very good for sure. I think it's worth discussing further but maybe separately from this PR? WDYT?

[Shaptic]

Agreed @jacekn - I think having add'l docker args is useful regardless of the conclusion we come to around tagging.

Do we want related PRs that will expose this in Jenkins, etc. before merging this?

[Shaptic]

Agreed @jacekn - I think having add'l docker args is useful regardless of the conclusion we come to there.

Do we want related PRs that will expose this in Jenkins, etc. before merging this?