Skip to content

Commit

Permalink
chore: change custom actions pull logic, to check if an image exists …
Browse files Browse the repository at this point in the history 
…locally first before triggering a pull (#9147)

* chore: check if an image exists locally before triggering a pull when running a local custom action

* chore: add new config to control the pull behaviour in custom actions to check or not the local daemon before pull

* docs: update schema docs with new field

* tests: unit tests to check if image exists locally logic before pull
  • Loading branch information
@renzodavid9
renzodavid9 authored Oct 24, 2023
1 parent f625fcf commit 8d20ed0
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 17 deletions.
12 changes: 12 additions & 0 deletions docs-v2/content/en/schemas/v4beta8.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3240,6 +3240,18 @@
"x-intellij-html-description": "<em>beta</em> describes how to do a build on the local docker daemon and optionally push to a repository."
},
"LocalVerifier": {
"properties": {
"useLocalImages": {
"type": "boolean",
"description": "if true, will first check if the containers images exist locally before triggering a pull. Defaults to false.",
"x-intellij-html-description": "if true, will first check if the containers images exist locally before triggering a pull. Defaults to false.",
"default": "false"
}
},
"preferredOrder": [
"useLocalImages"
],
"additionalProperties": false,
"type": "object",
"description": "uses the `docker` CLI to create verify test case containers on the host machine in Docker.",
"x-intellij-html-description": "uses the <code>docker</code> CLI to create verify test case containers on the host machine in Docker."
Expand Down
25 changes: 16 additions & 9 deletions pkg/skaffold/actions/docker/exec_env.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,10 @@ func (e ExecEnv) createTasks(ctx context.Context, out io.Writer, aCfgs latest.Ac
var tracked []graph.Artifact
containerCfgs := aCfgs.Containers
timeout := *aCfgs.Config.Timeout
useLocalImages := aCfgs.ExecutionModeConfig.LocalExecutionMode.UseLocalImages

for _, cCfg := range containerCfgs {
art, err := e.pullArtifact(ctx, out, builts, cCfg)
art, err := e.pullArtifact(ctx, out, builts, useLocalImages, cCfg)
if err != nil {
return nil, nil, err
}
Expand All @@ -185,22 +186,28 @@ func (e ExecEnv) createTasks(ctx context.Context, out io.Writer, aCfgs latest.Ac
return ts, tracked, nil
}

func (e ExecEnv) pullArtifact(ctx context.Context, out io.Writer, allbuilds map[string]graph.Artifact, cfg latest.VerifyContainer) (*graph.Artifact, error) {
ba, found := allbuilds[cfg.Image]
func (e ExecEnv) pullArtifact(ctx context.Context, out io.Writer, allbuilds map[string]graph.Artifact, useLocalImg bool, cfg latest.VerifyContainer) (*graph.Artifact, error) {
ba, foundInBuiltImgs := allbuilds[cfg.Image]
tag := cfg.Image
shouldPullImg := true
imgID := ""
var err error

if found {
if foundInBuiltImgs {
tag = ba.Tag
imgID, err := e.client.ImageID(ctx, tag)
useLocalImg = true
}

if useLocalImg {
imgID, err = e.client.ImageID(ctx, tag)
if err != nil {
return nil, fmt.Errorf("getting imageID for %q: %w", tag, err)
}
shouldPullImg = imgID == ""
}

if shouldPullImg {
if err := e.client.Pull(ctx, out, tag, v1.Platform{}); err != nil {
notFoundLocally := imgID == ""

if notFoundLocally {
if err = e.client.Pull(ctx, out, tag, v1.Platform{}); err != nil {
return nil, err
}
}
Expand Down
80 changes: 73 additions & 7 deletions pkg/skaffold/actions/docker/exec_env_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (fd *fakeDockerDaemon) ImageID(ctx context.Context, ref string) (string, er
return img, nil
}

func getActionCfg(aName string, containers []latest.VerifyContainer) latest.Action {
func getActionCfg(aName string, containers []latest.VerifyContainer, useLocalImgs bool) latest.Action {
return latest.Action{
Name: aName,
Config: latest.ActionConfig{
Expand All @@ -61,7 +61,9 @@ func getActionCfg(aName string, containers []latest.VerifyContainer) latest.Acti
},
ExecutionModeConfig: latest.ActionExecutionModeConfig{
VerifyExecutionModeType: latest.VerifyExecutionModeType{
LocalExecutionMode: &latest.LocalVerifier{},
LocalExecutionMode: &latest.LocalVerifier{
UseLocalImages: useLocalImgs,
},
},
},
Containers: containers,
Expand All @@ -82,7 +84,7 @@ func TestExecEnv_PrepareActions(t *testing.T) {
shouldFail: true,
errMsg: "action not-created-action not found for local execution mode",
availableAcsCfgs: []latest.Action{
getActionCfg("action1", []latest.VerifyContainer{}),
getActionCfg("action1", []latest.VerifyContainer{}, false),
},
},
{
Expand All @@ -93,11 +95,11 @@ func TestExecEnv_PrepareActions(t *testing.T) {
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock:latest"},
{Name: "container2", Image: "gcr.io/k8s-skaffold/mock:latest"},
}),
}, false),
getActionCfg("action2", []latest.VerifyContainer{
{Name: "container3", Image: "gcr.io/k8s-skaffold/mock:latest"},
{Name: "container4", Image: "gcr.io/k8s-skaffold/mock:latest"},
}),
}, false),
},
},
}
Expand Down Expand Up @@ -154,7 +156,7 @@ func TestExecEnv_PullImages(t *testing.T) {
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock1:latest"},
{Name: "container2", Image: "gcr.io/k8s-skaffold/mock2:latest"},
}),
}, true),
},
expectedPulledImgs: []string{"gcr.io/k8s-skaffold/mock1:latest", "gcr.io/k8s-skaffold/mock2:latest"},
},
Expand All @@ -165,7 +167,7 @@ func TestExecEnv_PullImages(t *testing.T) {
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "mock1"},
{Name: "container2", Image: "mock2:latest"},
}),
}, false),
},
expectedPulledImgs: []string{"mock2:latest"},
builtImgs: []graph.Artifact{
Expand All @@ -178,6 +180,70 @@ func TestExecEnv_PullImages(t *testing.T) {
"mock1:latest": "id1234",
},
},
{
description: "prepare action, force check if images exists locally - none of them exists",
actionToExec: "action1",
availableAcsCfgs: []latest.Action{
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock1:latest"},
{Name: "container2", Image: "gcr.io/k8s-skaffold/mock2:latest"},
}, true),
},
expectedPulledImgs: []string{"gcr.io/k8s-skaffold/mock1:latest", "gcr.io/k8s-skaffold/mock2:latest"},
},
{
description: "prepare action, force check if images exists locally - both exists",
actionToExec: "action1",
availableAcsCfgs: []latest.Action{
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock1:latest"},
{Name: "container2", Image: "gcr.io/k8s-skaffold/mock2:latest"},
}, true),
},
imagesInDaemon: map[string]string{
"gcr.io/k8s-skaffold/mock2:latest": "id1111",
"gcr.io/k8s-skaffold/mock1:latest": "id2222",
},
},
{
description: "prepare action, force check if images exists locally - one exists other not",
actionToExec: "action1",
availableAcsCfgs: []latest.Action{
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock1:latest"},
{Name: "container2", Image: "gcr.io/k8s-skaffold/mock2:latest"},
}, true),
},
expectedPulledImgs: []string{
"gcr.io/k8s-skaffold/mock2:latest",
},
imagesInDaemon: map[string]string{
"gcr.io/k8s-skaffold/mock1:latest": "id2222",
},
},
{
description: "prepare action, one built locally and one external",
actionToExec: "action1",
availableAcsCfgs: []latest.Action{
getActionCfg("action1", []latest.VerifyContainer{
{Name: "container1", Image: "gcr.io/k8s-skaffold/mock1:latest"},
{Name: "container2", Image: "mock2"},
}, false),
},
builtImgs: []graph.Artifact{
{
ImageName: "mock2",
Tag: "mock2:latest",
},
},
expectedPulledImgs: []string{
"gcr.io/k8s-skaffold/mock1:latest",
},
imagesInDaemon: map[string]string{
"gcr.io/k8s-skaffold/mock1:latest": "id1111",
"mock2:latest": "id2222",
},
},
}

for _, test := range tests {
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/runner/actions_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ func cfgsByExecMode(aCfgs []latest.Action) (dockerCfgs []latest.Action, k8sCfgs
k8sCfgs = append(k8sCfgs, cfg)
continue
}

if cfg.ExecutionModeConfig.LocalExecutionMode == nil {
cfg.ExecutionModeConfig.LocalExecutionMode = &latest.LocalVerifier{}
}

dockerCfgs = append(dockerCfgs, cfg)
}
return
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/schema/latest/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,11 @@ type VerifyExecutionModeType struct {
}

// LocalVerifier uses the `docker` CLI to create verify test case containers on the host machine in Docker.
type LocalVerifier struct{}
type LocalVerifier struct {
// UseLocalImages if true, will first check if the containers images exist locally before triggering a pull.
// Defaults to false.
UseLocalImages bool `yaml:"useLocalImages,omitempty"`
}

// KubernetesClusterVerifier uses the `kubectl` CLI to create veriy test case
// container in a kubernetes cluster.
Expand Down

0 comments on commit 8d20ed0

Please sign in to comment.