make bookinfo sample build repeatable by krancour · Pull Request #7098 · istio/istio

krancour · 2018-07-13T20:42:53Z

I determined the exact correct set of versions required for each productinfo dependency by doing this:

docker run -it istio/examples-bookinfo-productpage-v1:1.5.0 pip freeze

For the gradle build of the reviews service, it wasn't possible to determine what version of Java and gradle were previously used because the niaquinto/gradle no longer exists. I opted to use the latest official gradle image instead. This seems to work fine.

istio-testing · 2018-07-13T20:42:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: krancour
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: frankbu

Assign the PR to them by writing /assign @frankbu in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

samples/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

istio-testing · 2018-07-13T20:43:01Z

Hi @krancour. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2018-07-13T20:55:16Z

Codecov Report

Merging #7098 into master will increase coverage by 1%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #7098   +/-   ##
======================================
+ Coverage      71%     71%   +1%     
======================================
  Files         371     370    -1     
  Lines       32001   31975   -26     
======================================
+ Hits        22472   22518   +46     
+ Misses       8596    8525   -71     
+ Partials      933     932    -1

Impacted Files	Coverage Δ
...olarwinds/internal/papertrail/papertrail_logger.go	`62% <0%> (-18%)`	⬇️
mixer/adapter/cloudwatch/cloudwatch.go	`58% <0%> (-14%)`	⬇️
mixer/adapter/rbac/controller.go	`48% <0%> (-6%)`	⬇️
mixer/adapter/rbac/rbacStore.go	`76% <0%> (-3%)`	⬇️
mixer/adapter/opa/opa.go	`79% <0%> (-1%)`	⬇️
mixer/adapter/circonus/circonus.go	`70% <0%> (-1%)`	⬇️
mixer/adapter/kubernetesenv/kubernetesenv.go	`84% <0%> (ø)`	⬇️
mixer/adapter/list/list.go	`100% <0%> (ø)`	⬇️
mixer/adapter/dogstatsd/dogstatsd.go	`100% <0%> (ø)`	⬆️
tools/hyperistio/hyperistio.go	`0% <0%> (ø)`	⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5aae308...cac62f0. Read the comment docs.

krancour · 2018-07-13T21:15:12Z

CI failure doesn't look related to these changes.

rshriram · 2018-07-13T21:57:51Z

You have to update all bookinfo images

krancour · 2018-07-13T22:20:37Z

You have to update all bookinfo images

Not sure how to go about that. I can update all references to those images, but I don't know how I could push them-- I obviously do not have permissions.

krancour · 2018-07-13T22:42:42Z

I updated all files to reference bookinfo 1.6.0 images, but they don't exist. Someone else needs to push them from the source in this branch.

vadimeisenbergibm · 2018-07-15T05:50:02Z

@krancour I see the updated docker images https://hub.docker.com/r/istio/examples-bookinfo-productpage-v1/tags/. Please merge the bookinfo changes from the release-1.0 branch.

vadimeisenbergibm

Please merge the related bookinfo changes from release-1.0. Note this PR #6988.

krancour · 2018-07-15T11:12:37Z

How did those get there?

vadimeisenbergibm · 2018-07-15T13:13:01Z

How did those get there?

By this script - https://github.com/istio/istio/blob/master/samples/bookinfo/build_push_update_images.sh .

vadimeisenbergibm · 2018-07-15T13:17:46Z

One must have permissions to upload images to https://hub.docker.com/r/istio.

krancour · 2018-07-15T19:40:52Z

Right. I understand how they were made. I was more curious about who pushed them. It doesn't seem that building/pushing these is integrated into the CI process.

vadimeisenbergibm · 2018-07-16T03:23:32Z

I pushed the 1.7.0 images by running that script.

vadimeisenbergibm · 2018-07-16T03:24:26Z

Building/pushing bookinfo images is not part of the CI process.

krancour · 2018-07-16T14:18:31Z

I pushed the 1.7.0 images by running that script.

What branch did you build and push them from? I ask because this PR would have created 1.6.0 images; not 1.7.0.

So you can probably see why I am confused...

vadimeisenbergibm · 2018-07-16T14:26:50Z

I built the images from the release-1.0 branch. Please merge this PR #6988 into your branch.

krancour · 2018-07-16T14:31:42Z

I've also just pulled the 1.7.0 productpage image that you recently pushed and found that it definitely does not include anything that was in this branch.

vadimeisenbergibm · 2018-07-16T14:37:33Z

I've also just pulled the 1.7.0 productpage image that you recently pushed and found that it definitely does not include anything that was in this branch

By this branch, do you mean krancour:repeatable-bookinfo-build or release-1.0? I built the images from release-1.0.

krancour · 2018-07-16T15:07:09Z

By this branch, do you mean krancour:repeatable-bookinfo-build or release-1.0? I built the images from release-1.0.

I mean krancour:repeatable-bookinfo-build.

Again-- I'm confused. I don't understand what the release-1.0 branch has to do with this PR.

Just trying to understand the process here.

krancour · 2018-07-16T15:13:35Z

Also-- slight tangent-- you were probably only able to build these images in the first place because of things that already exist in your Docker (and maybe pip) cache. Absent the changes from this PR, producing new images for productpage and reviews shouldn't even be possible without a primed cache. i.e. This process seems as if it is currently dependent on your machine.

vadimeisenbergibm · 2018-07-16T15:14:28Z

Hmm, let me merge the PR #6988 to master first, and then you merge it from the master. The issue is that while you are working on bookinfo, I already changed it in istio/istio, in the release-1.0 branch.

krancour · 2018-07-16T15:15:25Z

Hmm, let me merge the PR #6988 to master first, and then you merge it from the master. The issue is that while you are working on bookinfo, I already changed it in istio/istio, in the release-1.0 branch.

Ok... that may simplify the equation here to something I can understand. :) . Appreciate it.

vadimeisenbergibm · 2018-07-16T15:21:04Z

@krancour Please wait until #7151 is merged and then merge the master from istio into your branch.

krancour · 2018-07-16T15:24:37Z

@vadimeisenbergibm sgtm. Thanks.

rshriram · 2018-07-16T15:42:39Z

rebase to release-1.0 branch

krancour · 2018-07-16T16:31:43Z

@rshriram, I am assuming then that release-1.0 is also the desired merge target for this?

If so, I'll have to close this and open a new PR. Happy to do it.

krancour · 2018-07-16T16:47:38Z

Superseded by #7152

istio-testing requested review from rshriram and vadimeisenbergibm July 13, 2018 20:42

googlebot added the cla: yes label Jul 13, 2018

istio-testing added the needs-ok-to-test label Jul 13, 2018

make bookinfo sample build repeatable

4898726

vadimeisenbergibm approved these changes Jul 13, 2018

View reviewed changes

point to new bookinfo images

cac62f0

krancour mentioned this pull request Jul 13, 2018

bookinfo fixes #7109

Merged

vadimeisenbergibm suggested changes Jul 15, 2018

View reviewed changes

krancour mentioned this pull request Jul 16, 2018

make bookinfo sample build repeatable #7152

Merged

krancour closed this Jul 16, 2018

Conversation

krancour commented Jul 13, 2018

Uh oh!

istio-testing commented Jul 13, 2018

Uh oh!

istio-testing commented Jul 13, 2018

Uh oh!

codecov bot commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

krancour commented Jul 13, 2018

Uh oh!

rshriram commented Jul 13, 2018

Uh oh!

krancour commented Jul 13, 2018

Uh oh!

krancour commented Jul 13, 2018

Uh oh!

vadimeisenbergibm commented Jul 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vadimeisenbergibm left a comment

Choose a reason for hiding this comment

Uh oh!

krancour commented Jul 15, 2018

Uh oh!

vadimeisenbergibm commented Jul 15, 2018

Uh oh!

vadimeisenbergibm commented Jul 15, 2018

Uh oh!

krancour commented Jul 15, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

vadimeisenbergibm commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

rshriram commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

krancour commented Jul 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Jul 13, 2018 •

edited

Loading

vadimeisenbergibm commented Jul 15, 2018 •

edited

Loading

vadimeisenbergibm commented Jul 16, 2018 •

edited

Loading