Add new Auto sync option by loosebazooka · Pull Request #3382 · GoogleContainerTools/skaffold

loosebazooka · 2019-12-13T20:34:14Z

This is the base code for #3369 and #2901, the minimum required changes to how dev works to make it with the proposed implementation of automatic sync'ing of jib projects

codecov · 2019-12-13T20:41:53Z

Codecov Report

Merging #3382 into master will decrease coverage by 0.01%.
The diff coverage is 69.23%.

Impacted Files	Coverage Δ
pkg/skaffold/schema/latest/config.go	`100% <ø> (ø)`	⬆️
pkg/skaffold/schema/defaults/defaults.go	`91.66% <100%> (+0.22%)`	⬆️
pkg/skaffold/runner/dev.go	`65.48% <33.33%> (-1.18%)`	⬇️
pkg/skaffold/build/jib/sync.go	`50.98% <33.33%> (ø)`	⬆️
pkg/skaffold/sync/sync.go	`72% <69.23%> (-0.58%)`	⬇️

balopat

It LGTM, but I'd rather have this as the last PR from the smaller PRs.
We recommend doing the "user visible" step the last one when all the other functionality is already in place. This way there is no "unimplemented" error on certain features.

pkg/skaffold/sync/sync.go

loosebazooka · 2020-02-06T21:05:56Z

Requires schema update: #3635

loosebazooka · 2020-02-28T22:36:35Z

So I think I have to initiate a new version thing or whatever, but otherwise this PR should be ready to go.

Perhaps I can move the changes made in: https://github.com/GoogleContainerTools/skaffold/pull/3382/files#diff-565d1f8c97f6be5448e4aa8873b0156dR41-R277

into a new PR though.

loosebazooka · 2020-03-04T22:46:50Z

needs update to config, will be done after next release

nkubala

this looks really good, I tested it out on the example in integration/testdata and it worked perfectly. should we update one of the jib examples we ship to show this off?

nkubala · 2020-03-10T20:46:57Z

integration/testdata/jib-sync/skaffold.yaml

+  artifacts:
+  - image: test-file-sync
+    jib:
+      type: must-use-profile


loosebazooka · 2020-03-10T21:52:07Z

@nkubala I guess I can move the testdata stuff to examples in a followup PR. ~~But also I need to figure out why this integration test is failing.~~ done

loosebazooka · 2020-03-11T16:03:37Z

~~Detect server start using output.~~: done

pkg/skaffold/schema/latest/config.go

Co-Authored-By: Balint Pato <balopat@users.noreply.github.com>

balopat · 2020-03-12T21:20:26Z

pkg/skaffold/sync/sync.go

+		return &Item{Image: tag, Copy: toCopy, Delete: toDelete}, nil
+
+	default:
+		// TODO: this error does appear a little late in the build, perhaps it could surface at first run, rather than first sync?


agreed, this could be validated upfront - for now this should be fine, that extra couple of seconds is not really worth pulling this thing only to the validation, also we should validate the config consistently then - which we don't do currently.

balopat

LGTM

corneliusweig · 2020-03-20T23:02:48Z

This is great stuff! So nice to see this land.

loosebazooka requested review from balopat, dgageot, nkubala and tejal29 as code owners December 13, 2019 20:34

googlebot added the cla: yes label Dec 13, 2019

pull-request-size bot added the size/M label Dec 13, 2019

loosebazooka force-pushed the auto-sync-setup branch from 3a3e8b7 to a028906 Compare December 16, 2019 18:38

loosebazooka mentioned this pull request Dec 17, 2019

Auto sync with Skaffold #3395

Closed

balopat suggested changes Dec 18, 2019

View reviewed changes

balopat added pr/blocked pr/lgtm labels Dec 18, 2019

briandealwis reviewed Dec 18, 2019

View reviewed changes

pkg/skaffold/sync/sync.go Outdated Show resolved Hide resolved

tejal29 assigned balopat Jan 14, 2020

balopat added the needs-rebase label Jan 15, 2020

loosebazooka force-pushed the auto-sync-setup branch from a028906 to 605a5b9 Compare February 5, 2020 21:38

loosebazooka requested review from TadCordle and chanseokoh as code owners February 5, 2020 21:38

loosebazooka force-pushed the auto-sync-setup branch from 605a5b9 to 7b4da99 Compare February 5, 2020 21:41

loosebazooka removed the needs-rebase label Feb 6, 2020

loosebazooka force-pushed the auto-sync-setup branch from 7b4da99 to c752386 Compare February 19, 2020 22:39

loosebazooka requested review from balopat and briandealwis February 19, 2020 22:44

loosebazooka force-pushed the auto-sync-setup branch from c752386 to acceea6 Compare February 26, 2020 16:04

loosebazooka force-pushed the auto-sync-setup branch from 2190adc to c271e82 Compare February 28, 2020 22:37

loosebazooka mentioned this pull request Feb 28, 2020

Add profile option to RunBuilder in test helper #3761

Merged

loosebazooka force-pushed the auto-sync-setup branch from c271e82 to b841f24 Compare March 4, 2020 18:33

loosebazooka force-pushed the auto-sync-setup branch from b841f24 to 332851a Compare March 10, 2020 18:12

loosebazooka removed the pr/blocked label Mar 10, 2020

nkubala approved these changes Mar 10, 2020

View reviewed changes

integration/testdata/jib-sync/skaffold.yaml

artifacts:

- image: test-file-sync

jib:

type: must-use-profile

Copy link
Copy Markdown

Contributor

nkubala Mar 10, 2020

Choose a reason for hiding this comment

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

:)

loosebazooka force-pushed the auto-sync-setup branch from 2963c63 to ab0c881 Compare March 11, 2020 02:54

loosebazooka added 5 commits March 10, 2020 23:28

Add new Auto sync option and tie into jib impl

c5fd629

add integration test

806bf9c

update schemas

131f696

remove reference to personal project

b3370cb

give server a chance to warm up, cache .m2

b991157

loosebazooka force-pushed the auto-sync-setup branch from 71d247a to b991157 Compare March 11, 2020 03:28

maybe 5 wasn't enough

683ff3e

loosebazooka added 2 commits March 12, 2020 10:58

remove delay and watch output instead

6bb0f93

oops, remove personal projects

87a85e3

balopat reviewed Mar 12, 2020

View reviewed changes

pkg/skaffold/schema/latest/config.go Outdated Show resolved Hide resolved

Update pkg/skaffold/schema/latest/config.go

2a917c1

Co-Authored-By: Balint Pato <balopat@users.noreply.github.com>

balopat reviewed Mar 12, 2020

View reviewed changes

balopat approved these changes Mar 12, 2020

View reviewed changes

update schemas

af175f9

loosebazooka merged commit d72e304 into master Mar 12, 2020

loosebazooka mentioned this pull request Mar 13, 2020

Spring Boot DevTools support #2357

Open

fmotrifork mentioned this pull request Mar 20, 2020

skaffold 1.6.0 fishworks/fish-food#635

Merged

briandealwis deleted the auto-sync-setup branch April 14, 2020 15:59

Conversation

loosebazooka commented Dec 13, 2019

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

balopat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

loosebazooka commented Feb 6, 2020

Uh oh!

loosebazooka commented Feb 28, 2020

Uh oh!

loosebazooka commented Mar 4, 2020

Uh oh!

nkubala left a comment

Choose a reason for hiding this comment

Uh oh!

nkubala Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

loosebazooka commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loosebazooka commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

balopat Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

balopat left a comment

Choose a reason for hiding this comment

Uh oh!

corneliusweig commented Mar 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Dec 13, 2019 •

edited

Loading

balopat left a comment •

edited

Loading

loosebazooka commented Mar 10, 2020 •

edited

Loading

loosebazooka commented Mar 11, 2020 •

edited

Loading