Skip to content

Conversation

placer14
Copy link
Contributor

Lily has items committed to the repo which are also listed in .dockerignore. When docker sends the context during build, it is missing files which git state interprets as "dirty".

Removing .dockerignore fixes this problem. There are also some .gitignore cleanup.

@placer14 placer14 requested a review from frrist January 31, 2022 23:42
@iand
Copy link
Contributor

iand commented Feb 1, 2022

Wouldn't it be better to align dockerignore and gitignore so nothing commited is excluded from the docker image? We don't need to include stray binaries and other artifacts in the build image. Even though they don't affect the final image copying them slows down the build,

@placer14
Copy link
Contributor Author

placer14 commented Feb 2, 2022

be better to align dockerignore and gitignore so nothing commited is excluded from the docker image

Not quite sure what you mean. Majority of the items in dockerignore are committed into the repo. Any lines on dockerignore which remove items committed within the repo will cause the gitversion to appear as dirty. Removing the dockerignore will achieve exactly what you describe (nothing committed in the repo is excluded from the docker image).

The other option we could do here is to generate the git version outside of the docker build process while the repo is still "clean". Perhaps this would be better than conflating our build payload and our build versioning together?

@placer14
Copy link
Contributor Author

placer14 commented Feb 2, 2022

And yes, I would agree with making dockerignore stricter to reduce the build payload as much as possible. I'll revert the removal and improve the rules there as well.

@iand
Copy link
Contributor

iand commented Feb 2, 2022

be better to align dockerignore and gitignore so nothing commited is excluded from the docker image

Not quite sure what you mean. Majority of the items in dockerignore are committed into the repo. Any lines on dockerignore which remove items committed within the repo will cause the gitversion to appear as dirty. Removing the dockerignore will achieve exactly what you describe (nothing committed in the repo is excluded from the docker image).

The other option we could do here is to generate the git version outside of the docker build process while the repo is still "clean". Perhaps this would be better than conflating our build payload and our build versioning together?

I think your following comment makes this redundant but just in case: I mean we should keep dockerignore and ensure that it doesn't ignore anything that is committed (by aligning it with similar rules to those in gitignore)

@placer14
Copy link
Contributor Author

placer14 commented Feb 2, 2022

My point is that we've already committed items that are also enumerated in gitignore. For example, ./build contains supporting files for the build process which deserve to be committed (albeit, not necessarily there). Because of this, if these items are present in dockerignore the build will be dirty. Most of the lines in dockerignore are committed so making it more like gitignore will not change that the build is dirty.

Since we want to tighten up dockerignore instead of removing it, I'm going to proceed with removing the version generation from within the docker build and making it an argument to docker build. Then I'm going to tighten up dockerignore as much as possible to reduce the docker context payload for build.

Copy link
Member

@frrist frrist left a comment

Choose a reason for hiding this comment

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

looks good, please merge at will once @iand's comments are addressed.

@placer14
Copy link
Contributor Author

placer14 commented Feb 3, 2022

Superseded by #835. Closing.

@placer14 placer14 closed this Feb 3, 2022
@placer14 placer14 deleted the mg/fix/dirty-docker-builds branch February 3, 2022 21:43
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