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

docker: do not remove build dep which might be needed by extension #8291

Merged
merged 1 commit into from
Jan 22, 2019

Conversation

yugangw-msft
Copy link
Contributor

Fix #8284
Note: the image size increases from 497MB to 666MB, but extension installation must work so build dep has to stay

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@tjprescott
Copy link
Member

But if an extension has a dependency, shouldn't the extension declare its own dependency so that it is installed? We can't expect the CLI to know all the dependencies an extension might need and carry them.

@tjprescott
Copy link
Member

I might be misunderstanding the issue, but we should consult with @williexu

Copy link
Member

@marstr marstr left a comment

Choose a reason for hiding this comment

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

I agree with @tjprescott. Indiscriminately adding build dependencies will bloat our image more than in helps support extensions. We should continue to work with the authors of extensions that aren't working to ensure they've accurately described all of their dependencies.

@yugangw-msft
Copy link
Contributor Author

yugangw-msft commented Jan 19, 2019

The workaround for now is:
docker run -it microsoft/azure-cli sh -c "apk add --no-cache --virtual .build-deps gcc make openssl-dev libffi-dev musl-dev && bash"

There is no simple patch can be done at the extension end, afaik. To maintain the sandbox model, CLI installs all dependencies to extension’s own folder, which means the build tools must be around. This also means, in a nutshell, that any extensions has azure-mgmt-* or msrest-azure called out in setup.py will be broken. For context, most extensions don’t have any dependencies specified at all in setup.py, guess that is why they are still surviving.

For the payload increase of not removing the build deps, I would not worry too much, once this issue #7387 gets addressed, we would sure find some balances back.

Copy link
Member

@marstr marstr left a comment

Choose a reason for hiding this comment

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

We chatted about this offline for quite a while. We agreed that being able to install extensions is enough of a core-scenario that it's worth increasing the size of the image to facilitate it.

Dockerfile Show resolved Hide resolved
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.

None yet

3 participants