feat: enable scaffolding for host builds #1750

lkingland · 2023-05-23T04:05:09Z

🎁 creates the scaffolding package
🎁 OCI builder updated to use scaffolding package and thus function source
🐛 Fixes a bug where local jobs were not receiving context cancellation
🐛 Fixes a bug where signatures of instanced HTTP was throwing errors
🧹 adds a concurrency test which ensures only one build executes per
process

Connects the scaffolder to function source code by factoring out scaffolding into a package which is used both by the client and by other packages, allowing for the removal of the mocks.

/kind enhancement

- Fixes a bug where local jobs were sometimes not canceling immediately
- Startup timeout for local run tasks now configurable

knative-prow · 2023-05-23T04:05:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lkingland]

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

codecov · 2023-05-23T04:15:33Z

Codecov Report

Patch coverage: 85.71% and project coverage change: +1.38 🎉

Comparison is base (fea4a72) 62.04% compared to head (bb32078) 63.43%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1750      +/-   ##
==========================================
+ Coverage   62.04%   63.43%   +1.38%     
==========================================
  Files         100      105       +5     
  Lines       13311    13405      +94     
==========================================
+ Hits         8259     8503     +244     
+ Misses       4253     4088     -165     
- Partials      799      814      +15

Flag	Coverage Δ
e2e-test	`37.74% <0.00%> (-0.21%)`	⬇️
e2e-test-oncluster	`33.43% <0.00%> (-0.09%)`	⬇️
e2e-test-oncluster-runtime	`28.62% <0.00%> (?)`
e2e-test-runtime-go	`27.35% <0.00%> (?)`
e2e-test-runtime-node	`28.42% <0.00%> (?)`
e2e-test-runtime-python	`28.42% <0.00%> (?)`
e2e-test-runtime-quarkus	`28.55% <0.00%> (?)`
e2e-test-runtime-springboot	`27.41% <0.00%> (?)`
e2e-test-runtime-typescript	`28.55% <0.00%> (?)`
integration-tests	`49.93% <54.39%> (+2.08%)`	⬆️
unit-tests-macos-latest	`49.70% <85.71%> (?)`
unit-tests-ubuntu-latest	`50.56% <85.71%> (+0.46%)`	⬆️
unit-tests-windows-latest	`49.79% <85.16%> (+0.44%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/oci/errors.go	`0.00% <0.00%> (ø)`
pkg/oci/builder.go	`73.97% <73.07%> (+18.19%)`	⬆️
pkg/scaffolding/scaffold.go	`79.06% <79.06%> (ø)`
pkg/scaffolding/detectors.go	`89.58% <89.58%> (ø)`
pkg/scaffolding/errors.go	`90.90% <90.90%> (ø)`
pkg/functions/client.go	`64.55% <100.00%> (+0.28%)`	⬆️
pkg/functions/job.go	`66.66% <100.00%> (+5.02%)`	⬆️
pkg/functions/repository.go	`70.24% <100.00%> (+1.17%)`	⬆️
pkg/oci/containerize_go.go	`76.78% <100.00%> (+4.25%)`	⬆️
pkg/scaffolding/signatures.go	`100.00% <100.00%> (ø)`

... and 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

lkingland · 2023-06-05T01:30:58Z

Thanks for the review @lance. The update to the error message you noticed has been made

lance

Looking really good, @lkingland! I just have a few more comments.

pkg/functions/client_test.go

pkg/oci/builder.go

pkg/oci/errors.go

pkg/scaffolding/scaffold.go

lance · 2023-06-06T16:27:55Z

pkg/scaffolding/scaffold_test.go

+func TestWrite(t *testing.T) {
+	// The filesystem containing scaffolding is expected to conform to the
+	// structure:
+	// /[language]/scaffolding/["instanced"|"static"]-[invocation]


TODO: we will need to update existing language pack documentation with this kind of information

aslom · 2023-06-06T16:36:13Z

I have it build locally and tested OK. The code looks like good starting point with tests.

I have also tested that existing functionality is still working oK

 ../func-scaffolding-enable/func deploy --builder=pack
   🙌 Function built: docker.io/aslom/wt1:latest
   ⬆️  Deploying function to the cluster
W0606 12:36:00.466532   68896 warnings.go:70] Kubernetes default value is insecure, Knative may default this to secure in a future release: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation, spec.template.spec.containers[0].securityContext.capabi
   ✅ Function updated in namespace "default" and exposed at URL:
   http://wt1.default.127.0.0.1.sslip.io

Looking forward to see more functionality.

LGTM

Co-authored-by: Lance Ball <lball@redhat.com>

lkingland · 2023-06-06T17:19:48Z

With the exception of refactoring the in-code test helper struct (which I opened an issue for since it might be slightly more involved), all of the comments have been accounted for I believe.

Thanks very much for the reviews and verification @lance and @aslom

lance · 2023-06-06T20:15:54Z

/lgtm 🥂

lance · 2023-06-06T20:36:21Z

/lgtm

lance · 2023-06-06T20:36:34Z

/unhold

knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/enhancement labels May 23, 2023

knative-prow bot requested review from jrangelramos and vyasgun May 23, 2023 04:05

knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 23, 2023

lkingland force-pushed the scaffolding-enable branch from 77c545b to 40dcdc6 Compare May 23, 2023 04:10

lkingland force-pushed the scaffolding-enable branch from 40dcdc6 to 0480b8e Compare May 23, 2023 08:37

knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 23, 2023

lkingland force-pushed the scaffolding-enable branch from 0480b8e to 1ee740a Compare May 23, 2023 12:58

lkingland mentioned this pull request May 23, 2023

Scaffolding #1703

Closed

14 tasks

lkingland force-pushed the scaffolding-enable branch from 6478ce5 to feb1d3f Compare May 25, 2023 07:39

knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 25, 2023

lkingland removed request for jrangelramos and vyasgun May 25, 2023 07:39

lkingland marked this pull request as ready for review May 25, 2023 07:39

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 25, 2023

knative-prow bot requested review from maximilien and vyasgun May 25, 2023 07:39

lkingland requested review from lance, salaboy, zroubalik, matejvasek and aslom and removed request for maximilien and vyasgun May 25, 2023 07:43

knative-prow bot assigned lance May 30, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2023

lkingland added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 1, 2023

knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2023

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2023

knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 2, 2023

lkingland force-pushed the scaffolding-enable branch 2 times, most recently from eede3e7 to f3c2c2b Compare June 5, 2023 01:28

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2023

lkingland force-pushed the scaffolding-enable branch 2 times, most recently from 74b6912 to 22b077a Compare June 5, 2023 01:43

feat: enable scaffolding in builder

4695592

lkingland force-pushed the scaffolding-enable branch from 22b077a to 4695592 Compare June 6, 2023 05:06

fix a few typos

28a7f46

lance reviewed Jun 6, 2023

View reviewed changes

lkingland and others added 2 commits June 7, 2023 02:09

error text formatting

13c14eb

Co-authored-by: Lance Ball <lball@redhat.com>

error text formatting

ac0db66

Co-authored-by: Lance Ball <lball@redhat.com>

remove test job stop defer

bb32078

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2023

knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 6, 2023

knative-prow bot merged commit e5aff92 into knative:main Jun 6, 2023
38 checks passed

lkingland added this to the Release 1.11 milestone Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable scaffolding for host builds #1750

feat: enable scaffolding for host builds #1750

lkingland commented May 23, 2023 •

edited

Loading

knative-prow bot commented May 23, 2023

codecov bot commented May 23, 2023 •

edited

Loading

lkingland commented Jun 5, 2023 •

edited

Loading

lance left a comment

lance Jun 6, 2023

aslom commented Jun 6, 2023

lkingland commented Jun 6, 2023 •

edited

Loading

lance commented Jun 6, 2023

lance commented Jun 6, 2023

lance commented Jun 6, 2023

feat: enable scaffolding for host builds #1750

feat: enable scaffolding for host builds #1750

Conversation

lkingland commented May 23, 2023 • edited Loading

knative-prow bot commented May 23, 2023

codecov bot commented May 23, 2023 • edited Loading

Codecov Report

lkingland commented Jun 5, 2023 • edited Loading

lance left a comment

Choose a reason for hiding this comment

lance Jun 6, 2023

Choose a reason for hiding this comment

aslom commented Jun 6, 2023

lkingland commented Jun 6, 2023 • edited Loading

lance commented Jun 6, 2023

lance commented Jun 6, 2023

lance commented Jun 6, 2023

lkingland commented May 23, 2023 •

edited

Loading

codecov bot commented May 23, 2023 •

edited

Loading

lkingland commented Jun 5, 2023 •

edited

Loading

lkingland commented Jun 6, 2023 •

edited

Loading