Nodejs express - optimize copy for buildah

[neeraj-laad]

Checklist:

We are seeing significant performance issues with all Node.js based stacks to build the final application image when using buildah (instead of docker).

This is because of a known issue with buildah 1.9.0. We can not upgrade the buildah version because of other issues.

This is a temporary fix to optimize the number of files we copy during the build process.

• Read the Code of Conduct and Contributing Guidelines.

• Followed the commit message guidelines.

• Stack adheres to Appsody stack structure.

Modifying an existing stack:

• Updated the stack version in stack.yaml

[sam-github]

Because the new code is inexplicably complex without understanding the history, the Dockerfiles could do with at least a one line comment acknowleging that a buildah issue is being worked around.

The git history can be used to trace the change back to this PR if anyone needs to, but the PR doesn't contain any links to the buildah issue, so it'll be hard to know when or if the workaround can be removed. There also isn't any info on how to use buildah to reproduce the issue to see if its still a problem. I think both of those bits of info could help future maintainers.

I made a suggestion to copy /project between build stages, rather than re-copy from host. Not sure if that will help because I can't repro :-).

[sam-github]

Isn't this a nul-op, given the next line sets the WORKDIR to something else?

[neeraj-laad]

Will change to RUN mkdir -p <path> to be clear about what we are trying to do here.

[neeraj-laad]

@sam-github Thanks for the review comments. I accidentally created the PR with changes to multiple stacks. It is easier to release stacks individually so have split the changes in 2 PRs again.

I have added comments to provide the link to the PR that has further links to original issue. I hope that helps provide some context to the issue.

[sam-github]

The issue we have today is that buildah 1.9.0 does not honour .dockerignore, and has a buggy algorithm for iterating over files. We can not move to a higher version (which has this fix) yet because of other issues.

^--- without this information, the Dockerfile isn't understandable on its own, so its likely to get refactored to break buildah. I strongly suggest that the information above be put into the dockerfile in a comment. It doesn't necessarily need a link to this PR in the comment, since that can be found from git blame.

Also, I'm a bit confused by "we can't update", who is "we"? appsody doesn't use buildah, does it? Since buildah isn't used in appsody AFAICT, its not possible to know if any future change here breaks whoever is using buildah downstream (who is using buildah downstream?). At some point in the future, I assume this structure can be deleted, but without any reproduction information here, or link to the downstream consumer who is having this issue, it won't be possible to know or ask when the workaround can be deleted.

[neeraj-laad]

Appsody uses buildah instead of docker when used in a Kubernetes environment like Che hosted IDE and Tekton build pipelines. appsody/tekton-sample repo provides sample Tekton pipelines for user's to start with and appsody-buildah provides a container image with Appsody CLI and buildah installed so these use cases.

The version of buildah that we use today 1.9.0 has this performance issue during copy. Newer version of buildah have resolved this but they have only works if newer versions of fuseoverlayfs is installed on the host (not the container). Given the changes are needed on the host it makes it difficult for Apprody team to control and fix.

[neeraj-laad]

@sam-github I have re-worked this PR, as we were copying node_modules from user-app over in the final stage of the build. This meant that dev/test dependencies from the user's development environment were getting into the final application image.

I have now moved the copying of the application over in the first stage and I delete node_modules before running npm i --production. This has also cleaned up the copy steps a bit.

Please let me know your thoughts.

[neeraj-laad]

@sam-github Would like to get a final review from you on this.

[Kamran64]

LGTM