feat: Add global build pre- and post-hooks #9047

ar3s3ru · 2023-08-27T16:17:14Z

Description
This PR adds the possibility of running pre- and post-hooks for the Build phase of a Pipeline.

The implementation is done by leveraging the pre-existing build.PipelineBuilder interface, using a wrapper that extends PreBuild and PostBuild methods, to avoid significant changes to the pre-existing code.

User facing changes

This PR adds the new functionality by introducing a new build.hooks field in the configuration.

Due to the new configuration field, I've created a new version, v4beta7, as specified in the contribution guidelines.

This is a skaffold build run from a slightly modified version of examples/getting-started/skaffold.yaml:

# skaffold.yaml
apiVersion: skaffold/v4beta7
kind: Config
build:
  hooks:
    before:
      - command: [echo, "hello from pre-hook!"]
    after:
      - command: [echo, "hello from post-hook!"]
  artifacts:
    - image: skaffold-example
  local:
    push: false
manifests:
  rawYaml:
  - k8s-pod.yaml

Here is the output, which shows how the new build.hooks is behaving:

> ./out/skaffold build
Generating tags...
 - skaffold-example -> skaffold-example:740d30e8f-dirty
Checking cache...
 - skaffold-example: Not found. Building
Starting build...
Starting pre-build hooks...
hello from pre-hook!
Completed pre-build hooks
Building [skaffold-example]...
Sending build context to Docker daemon  4.096kB
time="2023-08-27T18:06:54+02:00" level=warning msg="missing \"SKAFFOLD_GO_GCFLAGS\" build argument. Try adding \"--build-arg SKAFFOLD_GO_GCFLAGS=<VALUE>\" to the command line"
[1/2] STEP 1/6: FROM golang:1.18 AS builder
[1/2] STEP 2/6: WORKDIR /code
--> Using cache bffc72dc309d959a859d59a0aae3975abb0369b1420fc66551dbee53abe9670c
--> bffc72dc309d
[1/2] STEP 3/6: COPY main.go .
--> Using cache e9ae2def9ed085da743cddbb2dcc092a7302dc8dff53955dbd9508f166290bbe
--> e9ae2def9ed0
[1/2] STEP 4/6: COPY go.mod .
--> Using cache a22d7afc1bd669d99500bbbadd550343c390f8460a7b0767ff2c48b47de4961d
--> a22d7afc1bd6
[1/2] STEP 5/6: ARG SKAFFOLD_GO_GCFLAGS
--> Using cache 3c1e21609fff4dc761893cac7e2087705c05dc57ed84acbe4a00d13d56164f72
--> 3c1e21609fff
[1/2] STEP 6/6: RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -trimpath -o /app main.go
--> Using cache c082cefdd6ef749c854636c91f03c3098f98c13db26e81471d8461ad73a86cea
--> c082cefdd6ef
[2/2] STEP 1/4: FROM alpine:3
[2/2] STEP 2/4: ENV GOTRACEBACK=single
--> Using cache 3df39876b4561aff7cf01adab4e0535b52b7b686bea9954c77c1d4923d9f5388
--> 3df39876b456
[2/2] STEP 3/4: CMD ["./app"]
--> Using cache 4fd318e64452faa0c2461f254ee15b2bd35b9ded3439f21db8bdd9296f73c189
--> 4fd318e64452
[2/2] STEP 4/4: COPY --from=builder /app .
--> Using cache 80a5e519d652de8a28d5466706eaf6cc77be7a64ffdb358189245370f8605ca1
[2/2] COMMIT docker.io/library/skaffold-example:740d30e8f-dirty
--> 80a5e519d652
Successfully tagged docker.io/library/skaffold-example:740d30e8f-dirty
80a5e519d652de8a28d5466706eaf6cc77be7a64ffdb358189245370f8605ca1
Successfully built 80a5e519d652
Successfully tagged skaffold-example:740d30e8f-dirty
Build [skaffold-example] succeeded
Starting post-build hooks...
hello from post-hook!
Completed post-build hooks

ar3s3ru · 2023-08-27T16:19:27Z

Note for reviewers: I am a bit lost on the testing strategy.
Which kind of tests do you suggest for this new feature? Some pointer will be greatly appreciated 🙌🏻

What's more, creating a new version introduced a ton of changes -- is that ok?

ar3s3ru · 2023-08-30T11:04:54Z

Rebased to latest main since #9054 has been merged and brought in a new v4beta7 version.

ericzzzzzzz · 2023-08-30T11:49:33Z

Hi @ar3s3ru
thank you for the contribution, here is an example

skaffold/pkg/skaffold/hooks/render_test.go

Lines 1 to 75 in 88e1c17

    
           /* 
        
           Copyright 2021 The Skaffold Authors 
        
           Licensed under the Apache License, Version 2.0 (the "License"); 
        
           you may not use this file except in compliance with the License. 
        
           You may obtain a copy of the License at 
        
               http://www.apache.org/licenses/LICENSE-2.0 
        
           Unless required by applicable law or agreed to in writing, software 
        
           distributed under the License is distributed on an "AS IS" BASIS, 
        
           WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. 
        
           See the License for the specific language governing permissions and 
        
           limitations under the License. 
        
           */ 
        
           package hooks 
        
           import ( 
        
           	"bytes" 
        
           	"context" 
        
           	"testing" 
        
           	kubernetesclient "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/client" 
        
           	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" 
        
           	"github.com/GoogleContainerTools/skaffold/v2/testutil" 
        
           ) 
        
           func TestRenderHooks(t *testing.T) { 
        
           	testutil.Run(t, "TestRenderHooks", func(t *testutil.T) { 
        
           		hooks := latest.RenderHooks{ 
        
           			PreHooks: []latest.RenderHookItem{ 
        
           				{ 
        
           					HostHook: &latest.HostHook{ 
        
           						OS:      []string{"linux", "darwin"}, 
        
           						Command: []string{"sh", "-c", "echo pre-hook running with SKAFFOLD_KUBE_CONTEXT=$SKAFFOLD_KUBE_CONTEXT,SKAFFOLD_NAMESPACES=$SKAFFOLD_NAMESPACES"}, 
        
           					}, 
        
           				}, 
        
           				{ 
        
           					HostHook: &latest.HostHook{ 
        
           						OS:      []string{"windows"}, 
        
           						Command: []string{"cmd.exe", "/C", "echo pre-hook running with SKAFFOLD_KUBE_CONTEXT=%SKAFFOLD_KUBE_CONTEXT%,SKAFFOLD_NAMESPACES=%SKAFFOLD_NAMESPACES%"}, 
        
           					}, 
        
           				}, 
        
           			}, 
        
           			PostHooks: []latest.RenderHookItem{ 
        
           				{ 
        
           					HostHook: &latest.HostHook{ 
        
           						OS:      []string{"linux", "darwin"}, 
        
           						Command: []string{"sh", "-c", "echo post-hook running with SKAFFOLD_KUBE_CONTEXT=$SKAFFOLD_KUBE_CONTEXT,SKAFFOLD_NAMESPACES=$SKAFFOLD_NAMESPACES"}, 
        
           					}, 
        
           				}, 
        
           				{ 
        
           					HostHook: &latest.HostHook{ 
        
           						OS:      []string{"windows"}, 
        
           						Command: []string{"cmd.exe", "/C", "echo post-hook running with SKAFFOLD_KUBE_CONTEXT=%SKAFFOLD_KUBE_CONTEXT%,SKAFFOLD_NAMESPACES=%SKAFFOLD_NAMESPACES%"}, 
        
           					}, 
        
           				}, 
        
           			}, 
        
           		} 
        
           		preHostHookOut := "pre-hook running with SKAFFOLD_KUBE_CONTEXT=context1,SKAFFOLD_NAMESPACES=np1,np2" 
        
           		postHostHookOut := "post-hook running with SKAFFOLD_KUBE_CONTEXT=context1,SKAFFOLD_NAMESPACES=np1,np2" 
        
           		namespaces := []string{"np1", "np2"} 
        
           		opts := NewRenderEnvOpts(testKubeContext, namespaces) 
        
           		runner := newRenderRunner(hooks, &namespaces, opts) 
        
           		t.Override(&kubernetesclient.Client, fakeKubernetesClient) 
        
           		var preOut, postOut bytes.Buffer 
        
           		err := runner.RunPreHooks(context.Background(), &preOut) 
        
           		t.CheckNoError(err) 
        
           		t.CheckContains(preHostHookOut, preOut.String()) 
        
           		err = runner.RunPostHooks(context.Background(), &postOut) 
        
           		t.CheckNoError(err) 
        
           		t.CheckContains(postHostHookOut, postOut.String()) 
        
           	}) 
        
           }

for lifecycle hooks

codecov · 2023-08-30T11:58:27Z

Codecov Report

Merging #9047 (e1b0d9a) into main (290280e) will decrease coverage by 7.14%.
Report is 1023 commits behind head on main.
The diff coverage is 49.04%.

@@            Coverage Diff             @@
##             main    #9047      +/-   ##
==========================================
- Coverage   70.48%   63.34%   -7.14%     
==========================================
  Files         515      626     +111     
  Lines       23150    32188    +9038     
==========================================
+ Hits        16317    20390    +4073     
- Misses       5776    10255    +4479     
- Partials     1057     1543     +486

Files Changed	Coverage Δ
cmd/skaffold/app/cmd/completion.go	`13.04% <0.00%> (-1.25%)`	⬇️
cmd/skaffold/app/cmd/config/list.go	`65.21% <ø> (ø)`
cmd/skaffold/app/cmd/config/set.go	`88.72% <ø> (ø)`
cmd/skaffold/app/cmd/config/util.go	`54.28% <ø> (ø)`
cmd/skaffold/app/cmd/credits.go	`100.00% <ø> (ø)`
cmd/skaffold/app/cmd/credits/export.go	`0.00% <0.00%> (ø)`
cmd/skaffold/app/cmd/deploy.go	`40.90% <0.00%> (-12.94%)`	⬇️
cmd/skaffold/app/cmd/generate_pipeline.go	`60.00% <ø> (ø)`
cmd/skaffold/app/cmd/inspect_modules.go	`65.00% <0.00%> (ø)`
cmd/skaffold/app/cmd/inspect_profiles.go	`66.66% <0.00%> (ø)`
... and 40 more

... and 430 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ericzzzzzzz · 2023-08-30T12:06:48Z

A similar pr for render phase hook
#7785

ar3s3ru · 2023-08-30T12:17:52Z

Hey @ericzzzzzzz, thanks for the pointers, it does look very similar indeed. I'll take a stab at it later and ping you once I have something ready 🙏🏻

…ost hooks

…thHooks to make it hash-able

…cle-hooks'

ericzzzzzzz · 2023-09-06T11:58:23Z

Hi @ar3s3ru Could you fix the linting issues? You can use make linters to test it locally

ar3s3ru · 2023-09-06T16:03:43Z

@ericzzzzzzz done, last commit should've fixed it.

ar3s3ru · 2023-09-07T15:48:58Z

Hey @ericzzzzzzz I see the unit tests are failing for linux. Seems to be a race condition non correlated to my changes.

How do you suggest to move forward? Open to anything to speed up merging this PR 🙏🏻

ar3s3ru requested a review from a team as a code owner August 27, 2023 16:17

ar3s3ru requested a review from ericzzzzzzz August 27, 2023 16:17

pull-request-size bot added the size/XXL label Aug 27, 2023

ar3s3ru force-pushed the feat/add-global-build-hooks branch from 740d30e to 171099e Compare August 30, 2023 11:03

pull-request-size bot added size/M and removed size/XXL labels Aug 30, 2023

ar3s3ru added 3 commits September 5, 2023 22:28

feat(pkg/skaffold/schema): add build.hooks to latest config version

a0c0233

feat(pkgs/skaffold/runner): wrap pipeline builders to run build pre/p…

e4a8f3d

…ost hooks

fix(pkg/skaffold/runner): use pointer semantics for pipelineBuilderWi…

77b944e

…thHooks to make it hash-able

ar3s3ru force-pushed the feat/add-global-build-hooks branch from 171099e to 77b944e Compare September 5, 2023 20:44

ar3s3ru added 2 commits September 5, 2023 23:13

test(pkg/skaffold/runner): add builder test for pipelineBuilderWithHooks

084c2dd

doc(integration/examples): add the new build.hooks example in 'lifecy…

a6c1d3d

…cle-hooks'

pull-request-size bot added size/L and removed size/M labels Sep 5, 2023

fix(pkg/skaffold/runner): add copyright to builder_test.go

e1b0d9a

ericzzzzzzz added the kokoro:run runs the kokoro jobs on a PR label Sep 6, 2023

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Sep 6, 2023

ericzzzzzzz approved these changes Sep 8, 2023

View reviewed changes

ericzzzzzzz merged commit 38b4e03 into GoogleContainerTools:main Sep 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add global build pre- and post-hooks #9047

feat: Add global build pre- and post-hooks #9047

ar3s3ru commented Aug 27, 2023

ar3s3ru commented Aug 27, 2023 •

edited

Loading

ar3s3ru commented Aug 30, 2023

ericzzzzzzz commented Aug 30, 2023

codecov bot commented Aug 30, 2023 •

edited

Loading

ericzzzzzzz commented Aug 30, 2023

ar3s3ru commented Aug 30, 2023

ericzzzzzzz commented Sep 6, 2023

ar3s3ru commented Sep 6, 2023

ar3s3ru commented Sep 7, 2023

feat: Add global build pre- and post-hooks #9047

feat: Add global build pre- and post-hooks #9047

Conversation

ar3s3ru commented Aug 27, 2023

ar3s3ru commented Aug 27, 2023 • edited Loading

ar3s3ru commented Aug 30, 2023

ericzzzzzzz commented Aug 30, 2023

codecov bot commented Aug 30, 2023 • edited Loading

Codecov Report

ericzzzzzzz commented Aug 30, 2023

ar3s3ru commented Aug 30, 2023

ericzzzzzzz commented Sep 6, 2023

ar3s3ru commented Sep 6, 2023

ar3s3ru commented Sep 7, 2023

ar3s3ru commented Aug 27, 2023 •

edited

Loading

codecov bot commented Aug 30, 2023 •

edited

Loading