allow digested images to be 'run' #2445

gauron99 · 2024-07-26T11:24:46Z

In order to match the deploy command, make it possible to run digested images as well when building into the container

changes

run (`func run`) images directly ~ for convenience

Ive added a way to use digested/ valid undigested images to be run using func run cmd. With--build=false one can also run the undigested images directly via skipping the build and using the --image for the image name that they want to deploy.

tests

Ive also added tests that cover which image gets passed on to .Build.Image (which is the image used for the runner) and therefore what image actually gets "run".

new `client.Push` return value

Added a return value to client.Push which now matches build function (bool indicating if the current push/build was successful or not)

todo:

digested image deploys correct image (.Build.Image) when --container=true
undigested images direct deploy only with --build=false flag

knative-prow · 2024-07-26T11:24:49Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-07-26T11:30:04Z

Codecov Report

Attention: Patch coverage is 68.96552% with 18 lines in your changes missing coverage. Please review.

Project coverage is 66.73%. Comparing base (4775ad9) to head (4c3bbb4).
Report is 3 commits behind head on main.

Files	Patch %	Lines
cmd/deploy.go	71.87%	5 Missing and 4 partials ⚠️
cmd/run.go	78.94%	2 Missing and 2 partials ⚠️
pkg/functions/client.go	33.33%	2 Missing and 2 partials ⚠️
cmd/build.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2445      +/-   ##
==========================================
+ Coverage   61.22%   66.73%   +5.51%     
==========================================
  Files         130      130              
  Lines       15354    11950    -3404     
==========================================
- Hits         9400     7975    -1425     
+ Misses       5029     3012    -2017     
- Partials      925      963      +38

Flag	Coverage Δ
e2e-test	`36.67% <24.13%> (-0.11%)`	⬇️
e2e-test-oncluster	`33.28% <18.96%> (-0.03%)`	⬇️
e2e-test-oncluster-runtime	`29.38% <5.17%> (?)`
e2e-test-runtime-go	`26.67% <18.96%> (?)`
e2e-test-runtime-node	`26.06% <18.96%> (?)`
e2e-test-runtime-python	`26.06% <18.96%> (?)`
e2e-test-runtime-quarkus	`26.16% <18.96%> (?)`
e2e-test-runtime-rust	`25.17% <18.96%> (?)`
e2e-test-runtime-springboot	`25.24% <18.96%> (?)`
e2e-test-runtime-typescript	`26.15% <18.96%> (?)`
integration-tests	`52.78% <68.96%> (+2.37%)`	⬆️
unit-tests	`50.97% <68.96%> (?)`
unit-tests-macos-latest	`?`
unit-tests-ubuntu-latest	`?`
unit-tests-windows-latest	`?`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lkingland

I particularly like the new unit tests.

Looks great as-is! I added a couple of questions/suggestions.

/hold for optional changes from review comments

cmd/deploy.go

cmd/deploy_test.go

cmd/run.go

lkingland · 2024-08-13T05:01:09Z

pkg/docker/runner_int_test.go

+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute*10)
+	t.Cleanup(cancel)
+
+	image := testImageWithDigest


Would it be possible to first build the image, get its digested image name, and then use that digested image for testing "func run" (from a new temporary directory such that the build stamp from .func is not considered)? That would slow the test a bit, but would remove the need for a pre-built external container for the test

linux-foundation-easycla · 2024-08-26T11:42:38Z

The committers listed above are authorized under a signed CLA.

✅ login: gauron99 / name: David Fridrich (0dfb178, 6d8b1fe, 82f7459, 21e1b56, eba5c03, 0ddc30a, 34f237e, a5617d4, 298f71b, a0a00e3, 25dba57, 9d20af7, 4c3bbb4, d094e34, d095a39, ea39639)

Signed-off-by: gauron99 <[email protected]>

Signed-off-by: David Fridrich <[email protected]>

lkingland · 2024-08-27T14:10:09Z

/approve

Looking good

Signed-off-by: David Fridrich <[email protected]>

knative-prow · 2024-08-27T14:33:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauron99, 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~~ [gauron99,lkingland]

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

gauron99 · 2024-08-28T05:59:17Z

/unhold

* init fix Signed-off-by: gauron99 <[email protected]> * dont override direct deploy tag, more tests Signed-off-by: gauron99 <[email protected]> * fix Signed-off-by: gauron99 <[email protected]> * dont validate with tagged image, fix comment Signed-off-by: gauron99 <[email protected]> * init run fix for --image Signed-off-by: gauron99 <[email protected]> * init Signed-off-by: gauron99 <[email protected]> * int test, add valid untdigested images to run Signed-off-by: gauron99 <[email protected]> * check images passed to runner for func run command Signed-off-by: gauron99 <[email protected]> * fix build/deploy image passing bug add test Signed-off-by: gauron99 <[email protected]> * fix Signed-off-by: gauron99 <[email protected]> * remove extra printing Signed-off-by: gauron99 <[email protected]> * merge functions to digested Signed-off-by: gauron99 <[email protected]> * misspell Signed-off-by: gauron99 <[email protected]> * simplify Signed-off-by: David Fridrich <[email protected]> * quick fix Signed-off-by: David Fridrich <[email protected]> * remove prints, comment Signed-off-by: David Fridrich <[email protected]> --------- Signed-off-by: gauron99 <[email protected]> Signed-off-by: David Fridrich <[email protected]>

* test: adding GO s2i to e2e test suite (#2401) * Remove superfluous equals sign from envvar (#2446) * Remove superfluous equals sign from envvar Signed-off-by: Matej Vašek <[email protected]> * Update rust templates Cargo.lock Signed-off-by: Matej Vašek <[email protected]> --------- Signed-off-by: Matej Vašek <[email protected]> * allow digested images to be 'run' (#2445) * init fix Signed-off-by: gauron99 <[email protected]> * dont override direct deploy tag, more tests Signed-off-by: gauron99 <[email protected]> * fix Signed-off-by: gauron99 <[email protected]> * dont validate with tagged image, fix comment Signed-off-by: gauron99 <[email protected]> * init run fix for --image Signed-off-by: gauron99 <[email protected]> * init Signed-off-by: gauron99 <[email protected]> * int test, add valid untdigested images to run Signed-off-by: gauron99 <[email protected]> * check images passed to runner for func run command Signed-off-by: gauron99 <[email protected]> * fix build/deploy image passing bug add test Signed-off-by: gauron99 <[email protected]> * fix Signed-off-by: gauron99 <[email protected]> * remove extra printing Signed-off-by: gauron99 <[email protected]> * merge functions to digested Signed-off-by: gauron99 <[email protected]> * misspell Signed-off-by: gauron99 <[email protected]> * simplify Signed-off-by: David Fridrich <[email protected]> * quick fix Signed-off-by: David Fridrich <[email protected]> * remove prints, comment Signed-off-by: David Fridrich <[email protected]> --------- Signed-off-by: gauron99 <[email protected]> Signed-off-by: David Fridrich <[email protected]> * On cluster s2i build for Go (alternative version) (#2471) * Prepare util image to accomodate multiple cmds As of now func-util image has only one command of our own -- "deploy". This commits prepares grounds for one additional command named "scaffolding". The commands will be implemented in one binary and dispatched over argv[0] using symlinks. Kinda like busybox does. Signed-off-by: Matej Vašek <[email protected]> * Add scaffolding for on-cluster build Added new build step in tekton pipeline that scaffolds main() for Go porject when using s2i builder. Signed-off-by: Matej Vašek <[email protected]> * Better docker build caching This will cache dependencies between docker builds. Signed-off-by: Matej Vašek <[email protected]> * fixup: perms Signed-off-by: Matej Vašek <[email protected]> * fixup: remove broken check Signed-off-by: Matej Vašek <[email protected]> * fixup: remove test because of 'no space left on device' Signed-off-by: Matej Vašek <[email protected]> * Make func-util rootfull Signed-off-by: Matej Vašek <[email protected]> --------- Signed-off-by: Matej Vašek <[email protected]> * Incorporate s2i CLI into func-utils image. (#2473) Signed-off-by: Matej Vašek <[email protected]> * chore: update Quarkus platform version to 3.14.1 (#2474) Co-authored-by: Knative Automation <[email protected]> Signed-off-by: Matej Vašek <[email protected]> * chore: update Springboot platform version to 3.3.3 (#2465) Co-authored-by: Knative Automation <[email protected]> Signed-off-by: Matej Vašek <[email protected]> --------- Signed-off-by: Matej Vašek <[email protected]> Signed-off-by: gauron99 <[email protected]> Signed-off-by: David Fridrich <[email protected]> Co-authored-by: Jefferson Ramos <[email protected]> Co-authored-by: David Fridrich <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Knative Automation <[email protected]>

knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 26, 2024

knative-prow bot requested review from jrangelramos and maximilien July 26, 2024 11:24

knative-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 26, 2024

gauron99 removed request for maximilien and jrangelramos July 26, 2024 11:25

gauron99 added the kind/enhancement label Jul 29, 2024

gauron99 marked this pull request as ready for review August 7, 2024 08:06

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 Aug 7, 2024

knative-prow bot requested review from jrangelramos and vyasgun August 7, 2024 08:06

gauron99 requested review from lkingland and matejvasek and removed request for jrangelramos and vyasgun August 7, 2024 08:06

lkingland approved these changes Aug 13, 2024

View reviewed changes

knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2024

knative-prow bot assigned lkingland Aug 13, 2024

knative-prow bot added lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 13, 2024

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 Aug 26, 2024

gauron99 added 2 commits August 26, 2024 13:43

init fix

34f237e

Signed-off-by: gauron99 <[email protected]>

dont override direct deploy tag, more tests

a0a00e3

Signed-off-by: gauron99 <[email protected]>

gauron99 added 11 commits August 26, 2024 13:43

fix

21e1b56

Signed-off-by: gauron99 <[email protected]>

dont validate with tagged image, fix comment

6d8b1fe

Signed-off-by: gauron99 <[email protected]>

init run fix for --image

0ddc30a

Signed-off-by: gauron99 <[email protected]>

init

25dba57

Signed-off-by: gauron99 <[email protected]>

int test, add valid untdigested images to run

a5617d4

Signed-off-by: gauron99 <[email protected]>

check images passed to runner for func run command

0dfb178

Signed-off-by: gauron99 <[email protected]>

fix build/deploy image passing bug add test

ea39639

Signed-off-by: gauron99 <[email protected]>

fix

9d20af7

Signed-off-by: gauron99 <[email protected]>

remove extra printing

d094e34

Signed-off-by: gauron99 <[email protected]>

merge functions to digested

82f7459

Signed-off-by: gauron99 <[email protected]>

misspell

298f71b

Signed-off-by: gauron99 <[email protected]>

gauron99 force-pushed the bug-fix-run branch from 1ceeadc to 054bf6e Compare August 26, 2024 11:43

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 Aug 26, 2024

simplify

eba5c03

Signed-off-by: David Fridrich <[email protected]>

gauron99 force-pushed the bug-fix-run branch from 39bf359 to eba5c03 Compare August 26, 2024 12:08

quick fix

d095a39

Signed-off-by: David Fridrich <[email protected]>

remove prints, comment

4c3bbb4

Signed-off-by: David Fridrich <[email protected]>

lkingland approved these changes Aug 27, 2024

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2024

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

knative-prow bot merged commit ca61712 into knative:main Aug 28, 2024
39 checks passed

matejvasek mentioned this pull request Sep 4, 2024

[release-1.15] Release 1.15.1 #2480

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow digested images to be 'run' #2445

allow digested images to be 'run' #2445

gauron99 commented Jul 26, 2024 •

edited

Loading

knative-prow bot commented Jul 26, 2024

codecov bot commented Jul 26, 2024 •

edited

Loading

lkingland left a comment

lkingland Aug 13, 2024

linux-foundation-easycla bot commented Aug 26, 2024 •

edited

Loading

lkingland commented Aug 27, 2024

knative-prow bot commented Aug 27, 2024

gauron99 commented Aug 28, 2024

allow digested images to be 'run' #2445

allow digested images to be 'run' #2445

Conversation

gauron99 commented Jul 26, 2024 • edited Loading

changes

run (func run) images directly ~ for convenience

tests

new client.Push return value

knative-prow bot commented Jul 26, 2024

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

lkingland left a comment

Choose a reason for hiding this comment

lkingland Aug 13, 2024

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Aug 26, 2024 • edited Loading

lkingland commented Aug 27, 2024

knative-prow bot commented Aug 27, 2024

gauron99 commented Aug 28, 2024

gauron99 commented Jul 26, 2024 •

edited

Loading

run (`func run`) images directly ~ for convenience

new `client.Push` return value

codecov bot commented Jul 26, 2024 •

edited

Loading

linux-foundation-easycla bot commented Aug 26, 2024 •

edited

Loading