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

duplicated build events #3812

Closed
balopat opened this issue Mar 11, 2020 · 11 comments · Fixed by #4038
Closed

duplicated build events #3812

balopat opened this issue Mar 11, 2020 · 11 comments · Fixed by #4038
Assignees
Labels
area/api kind/bug Something isn't working priority/p2 May take a couple of releases

Comments

@balopat
Copy link
Contributor

balopat commented Mar 11, 2020

{"result":{"timestamp":"2020-03-11T02:45:43.851603Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version:v1.5.0-16-gc9be4210c ConfigVersion:skaffold/v2beta1 GitVersion: GitCommit:c9be4210cfa10fae8057e5bd61c6129c70f698e3 GitTreeState:clean BuildDate:2020-03-10T19:44:39Z GoVersion:go1.13.8 Compiler:gc Platform:darwin/amd64}"}}}}
{"result":{"timestamp":"2020-03-11T02:45:44.669571Z","event":{"buildEvent":{"artifact":"leeroy-web","status":"In Progress"}},"entry":"Build started for artifact leeroy-web"}}
{"result":{"timestamp":"2020-03-11T02:45:44.669613Z","event":{"buildEvent":{"artifact":"leeroy-app","status":"In Progress"}},"entry":"Build started for artifact leeroy-app"}}
{"result":{"timestamp":"2020-03-11T02:45:47.261407Z","event":{"buildEvent":{"artifact":"leeroy-web","status":"In Progress"}},"entry":"Build started for artifact leeroy-web"}}

Not sure why but in the event API, we report two build events for In progress for the microservices example...

@balopat
Copy link
Contributor Author

balopat commented Mar 11, 2020

for jib:

{"result":{"timestamp":"2020-03-11T02:47:53.764202Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version:v1.5.0-16-gc9be4210c ConfigVersion:skaffold/v2beta1 GitVersion: GitCommit:c9be4210cfa10fae8057e5bd61c6129c70f698e3 GitTreeState:clean BuildDate:2020-03-10T19:44:39Z GoVersion:go1.13.8 Compiler:gc Platform:darwin/amd64}"}}}}
{"result":{"timestamp":"2020-03-11T02:47:54.241400Z","event":{"buildEvent":{"artifact":"skaffold-jib","status":"In Progress"}},"entry":"Build started for artifact skaffold-jib"}}
{"result":{"timestamp":"2020-03-11T02:47:57.747318Z","event":{"buildEvent":{"artifact":"skaffold-jib","status":"In Progress"}},"entry":"Build started for artifact skaffold-jib"}}

@dgageot dgageot added area/api kind/bug Something isn't working labels Mar 20, 2020
@dgageot
Copy link
Contributor

dgageot commented Mar 26, 2020

@balopat I can't reproduce. I used curl as a client to the http api. Can you share how you ran your test?

@nkubala
Copy link
Contributor

nkubala commented Mar 27, 2020

hmm, yeah I see this too. just ran skaffold dev in the microservices example and grabbed events with curl

➜  ~ curl http://localhost:50052/v1/events
{"result":{"timestamp":"2020-03-27T18:19:39.905177Z","event":{"metaEvent":{"entry":"Starting Skaffold: \u0026{Version:v1.5.0-71-gee9091bd3 ConfigVersion:skaffold/v2beta1 GitVersion: GitCommit:ee9091bd365fc9bbe8ca25458efdd6a45ba1438e GitTreeState:clean BuildDate:2020-03-27T11:15:52Z GoVersion:go1.14 Compiler:gc Platform:darwin/amd64}"}}}}
{"result":{"timestamp":"2020-03-27T18:19:40.154122Z","event":{"buildEvent":{"artifact":"leeroy-app","status":"In Progress"}},"entry":"Build started for artifact leeroy-app"}}
{"result":{"timestamp":"2020-03-27T18:19:40.154121Z","event":{"buildEvent":{"artifact":"leeroy-web","status":"In Progress"}},"entry":"Build started for artifact leeroy-web"}}
{"result":{"timestamp":"2020-03-27T18:19:42.141648Z","event":{"buildEvent":{"artifact":"leeroy-web","status":"In Progress"}},"entry":"Build started for artifact leeroy-web"}}
{"result":{"timestamp":"2020-03-27T18:19:56.250200Z","event":{"buildEvent":{"artifact":"leeroy-web","status":"Complete"}},"entry":"Build completed for artifact leeroy-web"}}
{"result":{"timestamp":"2020-03-27T18:19:56.250224Z","event":{"buildEvent":{"artifact":"leeroy-app","status":"In Progress"}},"entry":"Build started for artifact leeroy-app"}}
{"result":{"timestamp":"2020-03-27T18:20:12.240964Z","event":{"buildEvent":{"artifact":"leeroy-app","status":"Complete"}},"entry":"Build completed for artifact leeroy-app"}}

@dgageot
Copy link
Contributor

dgageot commented Mar 27, 2020

I'll try with skaffold dev too then

@dgageot dgageot self-assigned this Mar 27, 2020
@tstromberg tstromberg added the priority/p2 May take a couple of releases label Apr 20, 2020
@dgageot
Copy link
Contributor

dgageot commented Apr 23, 2020

I'm giving up. Can't reproduce.

@dgageot dgageot removed their assignment Apr 23, 2020
@nkubala
Copy link
Contributor

nkubala commented Apr 23, 2020

that's so weird, I repro'd this on my first try...were you trying on OS X?

@dgageot
Copy link
Contributor

dgageot commented Apr 27, 2020

@nkubala yes, osx

@dgageot dgageot self-assigned this Apr 27, 2020
@dgageot
Copy link
Contributor

dgageot commented Apr 27, 2020

I finally managed to reproduce it!

@dgageot
Copy link
Contributor

dgageot commented Apr 27, 2020

What happens is that a "build In Progress" event is sent when the cache is checked for a given artifact and then another one is sent if the artifact needs to be built.

I'm not sure what we should do exactly to solve that.

  • Should we send a "build in progress" only when the artifact needs to be actually built?
  • Should we send a "build in progress" when the cache is checked and a "build completed" once the cache returns a hit or the artifact is finished building.

@nkubala @balopat wdyt?

@nkubala
Copy link
Contributor

nkubala commented Apr 27, 2020

ah good find! it seems like we should only be sending an "in progress" event if the build is actually started, so my preference would be your first idea. to me it makes more sense that the builder itself sends the success/failure event for the build - your second option would mean the cache itself is sending a "success" event, though functionally I guess it's the same 🤷

it does also feel a little weird that the cache checking is considered part of the "build phase" of skaffold....but that's a different conversation :)

@dgageot
Copy link
Contributor

dgageot commented Apr 28, 2020

I'll remove the "build in progress" event that's sent when checking the cache for an artifact.

dgageot added a commit to dgageot/skaffold that referenced this issue Apr 28, 2020
Not the cache.

Fixes GoogleContainerTools#3812

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Apr 29, 2020
Not the cache.

Fixes GoogleContainerTools#3812

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit that referenced this issue May 4, 2020
* Only the builder sends “Build in progress” events.

Not the cache.

Fixes #3812

Signed-off-by: David Gageot <david@gageot.net>

* Fix integration tests

Signed-off-by: David Gageot <david@gageot.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api kind/bug Something isn't working priority/p2 May take a couple of releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants