Skip to content

Reintroduce "Remove ccache from all docker images"#15483

Merged
jtattermusch merged 2 commits intogrpc:masterfrom
jtattermusch:reintroduce_15466
May 23, 2018
Merged

Reintroduce "Remove ccache from all docker images"#15483
jtattermusch merged 2 commits intogrpc:masterfrom
jtattermusch:reintroduce_15466

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch commented May 21, 2018

No description provided.

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

matt-kwong
matt-kwong approved these changes May 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@matt-kwong matt-kwong left a comment

Choose a reason for hiding this comment

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

LGTM if tests pass

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[microbenchmarks] No significant performance differences

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented May 21, 2018

Introducing this would break grpc-node tests again - @nicolasnoble what would be the proper fix?

@murgatroid99
Copy link
Copy Markdown
Member

murgatroid99 commented May 21, 2018

Are the docker images that are impacted by this change saved anywhere, or are they rebuilt whenever the tests are run?

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented May 21, 2018

they are saved in dockerhub (and they are differentiated based on sha1 of the Dockerfile)
https://hub.docker.com/u/grpctesting/

by computing sha1 of the Dockerfile before and after you can get to the exact images used by kokoro.

@murgatroid99
Copy link
Copy Markdown
Member

murgatroid99 commented May 21, 2018

OK, I think I know what the problem is. It's not related to ccache, but rather to the fact that the docker image is getting rebuilt. I think this should be fixed by #15491.

@murgatroid99
Copy link
Copy Markdown
Member

murgatroid99 commented May 22, 2018

I have merged #15491. I think you will have to merge master into your branch and republish the docker images, but then the tests should pass.

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented May 23, 2018

@murgatroid99, thanks for the update! Testing now.

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 23, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 23, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 23, 2018 

[microbenchmarks] No significant performance differences

@jtattermusch
Copy link
Copy Markdown
Contributor Author

jtattermusch commented May 23, 2018

Known failures:
#15458

@jtattermusch jtattermusch merged commit d4f572d into grpc:master May 23, 2018
@jtattermusch jtattermusch mentioned this pull request Oct 30, 2018
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nicolasnoble nicolasnoble Awaiting requested review from nicolasnoble

1 more reviewer

@matt-kwong matt-kwong matt-kwong approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jtattermusch @grpc-testing @murgatroid99 @matt-kwong