Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the bug with build operation in faas-cli for docker versions 23.0.0 and above #960

Closed
wants to merge 1 commit into from

Conversation

NikhilSharmaWe
Copy link
Contributor

Description

For docker versions 23.0.0 and above the env var DOCKER_BUILDKIT is equal to 1 by default for using the docker build kit, which used docker buildx build instead of docker build. This PR fixes the bug with the faas-cli build operation and docker build for docker version 23.0.0 and above. PTAL: #958 for more info.

Motivation and Context

How Has This Been Tested?

I have tested the changes. Now the faas-cli build works fine also with docker version 23.0.0 and above.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@NikhilSharmaWe NikhilSharmaWe changed the title fix the build command bug for docker versions 23.0.0 and above fix the bug with build operation in faas-cli for docker versions 23.0.0 and above Apr 25, 2023
@alexellis
Copy link
Member

My question would be - why is setting DOCKER_BUILDKIT=0 a fix?

If Docker is moving to a model where buildx replaces Docker build as a kind of alias - shouldn't we always be forcing DOCKER_BUILDKIT=1?

When would DOCKER_BUILDKIT=0 be desirable?

@NikhilSharmaWe
Copy link
Contributor Author

I think then we can update the build to also use buildx build command as publish and set DOCKER_BUILDKIT to 1 in all cases.
WDYT?

@alexellis
Copy link
Member

The buildx build command cannot be used, since it requires a specific modern docker version, right?

But what is the issue with DOCKER_BUILDKIT=1 docker build?

@NikhilSharmaWe
Copy link
Contributor Author

NikhilSharmaWe commented Apr 26, 2023

@alexellis If we set DOCKER_BUILDKIT=1 then docker buildx build is used instead of docker build, so that won't work for build in faas-cli, since it uses docker build unlike publish which uses docker buildx build.

WDYT?

@alexellis
Copy link
Member

alexellis commented Apr 27, 2023

@alexellis If we set DOCKER_BUILDKIT=1 then docker buildx build is used instead of docker build, so that won't work for build in faas-cli, since it uses docker build unlike publish which uses docker buildx build.

I often set DOCKER_BUILDKIT=1 faas-cli build and it does work for me - same for @welteki

Perhaps you need to run docker buildx install? uninstall reverts that change.

I'm wondering if we should add a flag to faas-cli build of --buildkit=true, which people can set to false if required?

@NikhilSharmaWe
Copy link
Contributor Author

@alexellis

  • After docker buildx uninstall, it works. Could be that the docker buildx and docker engine I installed were installed separately. Not sure.

  • Also, should we add a --buildkit=true flag or DOCKER_BUILDKIT=1 faas-cli build is fine for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

faas-cli might be incompatible for docker versions 23.0.0 and above
2 participants