Initial support for Knative Build #851

dgageot · 2018-07-24T16:52:18Z

This PR brings initial support for https://github.com/knative/build. It currently focused on supporting Docker build artifacts. Bazel artifacts should be easy to add.

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

AkihiroSuda · 2018-07-25T03:18:20Z

pkg/skaffold/build/knative/build.go

+}
+
+func waitForCompletion(builds build_v1alpha1.BuildInterface, build *v1alpha1.Build) error {
+	for {


why busy wait?

@AkihiroSuda I'm not sure I understand your comment

Does not this loop consume CPU too much?

No, it's done just after the logs are done tailing so it usually succeeds i getting the status the first time. But since there's a few ms between the logs being done and the status being updated, it sometimes requires a retry.

AkihiroSuda · 2018-07-25T03:19:50Z

pkg/skaffold/build/knative/build.go

+				Custom: &v1.Container{
+					Image:   "gcr.io/cloud-builders/gsutil", // TODO(dgageot) way too big
+					Command: []string{"sh", "-c"},
+					Args:    []string{fmt.Sprintf("gsutil cp gs://%s/%s - | tar xvz", cfg.GCSBucket, tarName)},


Are these escaped?

This piece needs fixing.

codecov-io · 2018-08-08T08:56:20Z

Codecov Report

Merging #851 into master will decrease coverage by 0.04%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master    #851      +/-   ##
=========================================
- Coverage   43.55%   43.5%   -0.05%     
=========================================
  Files          63      63              
  Lines        2645    2648       +3     
=========================================
  Hits         1152    1152              
- Misses       1373    1376       +3     
  Partials      120     120

Impacted Files	Coverage Δ
pkg/skaffold/runner/runner.go	`55.31% <0%> (-0.9%)`	⬇️

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 0592b2b...d0de097. Read the comment docs.

bobcatfish · 2018-08-08T17:24:20Z

very cool @dgageot ! How would you feel about including some docs in this PR that explain how to use it? (makes reviewing easier too)

bobcatfish · 2018-08-08T17:25:28Z

pkg/skaffold/build/knative.go

+	var builds []Artifact
+
+	for _, artifact := range artifacts {
+		fmt.Fprintf(out, "Building [%s]...\n", artifact.ImageName)


what do you think about outputting this in color (blue?)?

dlorenc · 2018-09-07T14:47:54Z

Is this ready for a look yet?

Currently focused on supporting Docker build artifacts Signed-off-by: David Gageot <david@gageot.net>

dlorenc · 2018-09-17T17:05:07Z

Ping, what's left here?

ajbouh · 2018-09-25T20:23:59Z

I think this is the right approach to use for using skaffold in a kubernetes-hosted CI job.

Have other folks tried this PR yet?

dgageot · 2018-09-25T20:26:51Z

I need to rebase this code, sync it with recent knative build and fix the TODOs. Might have time to do that this week

balopat · 2018-10-29T21:43:17Z

ping @dgageot are you okay with me to fork this and continue where you left it?

bkuschel · 2018-10-30T17:42:34Z

This is a pretty tight integration. It would be neat if skaffold had the capability to use pre-existing Build and BuildTemplate resources rather than create fixed implementations with gcs/kaniko.

bkuschel · 2018-11-09T14:48:36Z

I can take a look at this as well now. I'll pull this code down and see if i can help take it forward.

bkuschel · 2018-11-09T18:30:40Z

@dgageot so i'm thinking about adding support for knative build resources. Tricky part is generating the input from the local build context. This current implementation uses an object store as a drop-off.
Since we have access to the k8s control plane, it could be feasible transfer the files to a custom "skaffold" knative initcontainer responsible for populating the knative build workspace.

So the sequence would be something like:

Skaffold starts the build, tgz's the build context
It loads the build resource specified in the skaffold.yaml. Ensure the build input source is using a "skaffold" "custom" input container. ( Not sure what to do if an input source specified therein, as it would need a very specific setup here, perhaps it will be overwritten. Perhaps some flexibility of using an object store drop-off etc. ).
Skaffold creates the build resource in k8s
knative build starts up and "skaffold" input container immediately goes into a waiting state, waiting for a tar process to start
Skaffold execs into the waiting "skaffold" container, having it untar the tgz build context that was sent to it
"skaffold" input container sees the tar process exit and exits

Thoughts?

dgageot · 2018-11-09T18:52:48Z

@bkuschel Hi! I just heard that Knative Build is going to evolve a LOT in the upcoming weeks. You might want to hold on any effort and see how it turns out

csantanapr · 2018-12-06T15:02:22Z

@dgageot Is there some doc or example on how to use it with knative?

dgageot requested review from dlorenc, r2d4 and viglesiasce as code owners July 24, 2018 16:52

dgageot force-pushed the knative branch from 4eb3360 to f514733 Compare July 24, 2018 20:47

AkihiroSuda mentioned this pull request Jul 25, 2018

[Proposal] CBI: Container Builder Interface (Docker, BuildKit, Buildah, kaniko, img, Google Cloud Container Builder, OpenShift Source-to-Image...) #596

Closed

AkihiroSuda reviewed Jul 25, 2018

View reviewed changes

dgageot force-pushed the knative branch 4 times, most recently from 2fd02c4 to 78a16f7 Compare July 28, 2018 18:38

dgageot force-pushed the knative branch from 78a16f7 to ef190a0 Compare August 4, 2018 06:35

dgageot requested a review from balopat as a code owner August 4, 2018 06:35

dgageot force-pushed the knative branch 7 times, most recently from 04d0bbd to 1639bf6 Compare August 8, 2018 08:01

bobcatfish reviewed Aug 8, 2018

View reviewed changes

dgageot force-pushed the knative branch 2 times, most recently from e17d117 to eeef9c2 Compare August 15, 2018 09:59

dgageot force-pushed the knative branch 5 times, most recently from 568fd6c to a4a1a3b Compare August 24, 2018 08:41

dgageot added the wip label Aug 24, 2018

dgageot force-pushed the knative branch 2 times, most recently from 6cdd90a to d0de097 Compare September 6, 2018 04:28

Support Knative Build CRD

1653893

Currently focused on supporting Docker build artifacts Signed-off-by: David Gageot <david@gageot.net>

dgageot force-pushed the knative branch from d0de097 to 1653893 Compare September 8, 2018 06:17

balopat mentioned this pull request Nov 15, 2018

[Proposal] Introduce v1beta1 config + remove ACR builder #1283

Closed

dgageot closed this Nov 16, 2018

csantanapr mentioned this pull request Dec 6, 2018

Docs for knative usage? #1369

Closed

dgageot deleted the knative branch December 28, 2018 07:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support for Knative Build #851

Initial support for Knative Build #851

dgageot commented Jul 24, 2018

AkihiroSuda Jul 25, 2018

dgageot Jul 25, 2018

AkihiroSuda Jul 25, 2018

dgageot Jul 25, 2018

AkihiroSuda Jul 25, 2018

dgageot Jul 25, 2018

codecov-io commented Aug 8, 2018 •

edited

Loading

bobcatfish commented Aug 8, 2018

bobcatfish Aug 8, 2018

dlorenc commented Sep 7, 2018

dlorenc commented Sep 17, 2018

ajbouh commented Sep 25, 2018

dgageot commented Sep 25, 2018

balopat commented Oct 29, 2018 •

edited

Loading

bkuschel commented Oct 30, 2018

bkuschel commented Nov 9, 2018

bkuschel commented Nov 9, 2018 •

edited

Loading

dgageot commented Nov 9, 2018

csantanapr commented Dec 6, 2018

Initial support for Knative Build #851

Initial support for Knative Build #851

Conversation

dgageot commented Jul 24, 2018

AkihiroSuda Jul 25, 2018

Choose a reason for hiding this comment

dgageot Jul 25, 2018

Choose a reason for hiding this comment

AkihiroSuda Jul 25, 2018

Choose a reason for hiding this comment

dgageot Jul 25, 2018

Choose a reason for hiding this comment

AkihiroSuda Jul 25, 2018

Choose a reason for hiding this comment

dgageot Jul 25, 2018

Choose a reason for hiding this comment

codecov-io commented Aug 8, 2018 • edited Loading

Codecov Report

bobcatfish commented Aug 8, 2018

bobcatfish Aug 8, 2018

Choose a reason for hiding this comment

dlorenc commented Sep 7, 2018

dlorenc commented Sep 17, 2018

ajbouh commented Sep 25, 2018

dgageot commented Sep 25, 2018

balopat commented Oct 29, 2018 • edited Loading

bkuschel commented Oct 30, 2018

bkuschel commented Nov 9, 2018

bkuschel commented Nov 9, 2018 • edited Loading

dgageot commented Nov 9, 2018

csantanapr commented Dec 6, 2018

codecov-io commented Aug 8, 2018 •

edited

Loading

balopat commented Oct 29, 2018 •

edited

Loading

bkuschel commented Nov 9, 2018 •

edited

Loading