Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop caching COPY layers #1408

Merged
merged 1 commit into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ as a remote image destination:
### Caching

#### Caching Layers
kaniko can cache layers created by `RUN` and `COPY` commands in a remote repository.
kaniko can cache layers created by `RUN` commands in a remote repository.
Before executing a command, kaniko checks the cache for the layer.
If it exists, kaniko will pull and extract the cached layer instead of executing the command.
If not, kaniko will execute the command and then push the newly created layer to the cache.
Expand Down
2 changes: 2 additions & 0 deletions pkg/commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type DockerCommand interface {

RequiresUnpackedFS() bool

// Whether the output layer of this command should be cached in and
// retrieved from the layer cache.
ShouldCacheOutput() bool

// ShouldDetectDeletedFiles returns true if the command could delete files.
Expand Down
81 changes: 0 additions & 81 deletions pkg/commands/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package commands

import (
"fmt"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -142,90 +141,10 @@ func (c *CopyCommand) RequiresUnpackedFS() bool {
return true
}

func (c *CopyCommand) ShouldCacheOutput() bool {
return true
}

// CacheCommand returns true since this command should be cached
func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand {

return &CachingCopyCommand{
img: img,
cmd: c.cmd,
buildcontext: c.buildcontext,
extractFn: util.ExtractFile,
}
}

func (c *CopyCommand) From() string {
return c.cmd.From
}

type CachingCopyCommand struct {
BaseCommand
caching
img v1.Image
extractedFiles []string
cmd *instructions.CopyCommand
buildcontext string
extractFn util.ExtractFunction
}

func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
logrus.Infof("Found cached layer, extracting to filesystem")
var err error

if cr.img == nil {
return errors.New(fmt.Sprintf("cached command image is nil %v", cr.String()))
}

layers, err := cr.img.Layers()
if err != nil {
return errors.Wrapf(err, "retrieve image layers")
}

if len(layers) != 1 {
return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
}

cr.layer = layers[0]
cr.extractedFiles, err = util.GetFSFromLayers(kConfig.RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout())

logrus.Debugf("extractedFiles: %s", cr.extractedFiles)
if err != nil {
return errors.Wrap(err, "extracting fs from image")
}

return nil
}

func (cr *CachingCopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) {
return copyCmdFilesUsedFromContext(config, buildArgs, cr.cmd, cr.buildcontext)
}

func (cr *CachingCopyCommand) FilesToSnapshot() []string {
f := cr.extractedFiles
logrus.Debugf("%d files extracted by caching copy command", len(f))
logrus.Tracef("Extracted files: %s", f)

return f
}

func (cr *CachingCopyCommand) MetadataOnly() bool {
return false
}

func (cr *CachingCopyCommand) String() string {
if cr.cmd == nil {
return "nil command"
}
return cr.cmd.String()
}

func (cr *CachingCopyCommand) From() string {
return cr.cmd.From
}

func resolveIfSymlink(destPath string) (string, error) {
if !filepath.IsAbs(destPath) {
return "", errors.New("dest path must be abs")
Expand Down
154 changes: 0 additions & 154 deletions pkg/commands/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
package commands

import (
"archive/tar"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -236,159 +235,6 @@ func Test_resolveIfSymlink(t *testing.T) {
}
}

func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
tempDir := setupTestTemp()

tarContent, err := prepareTarFixture([]string{"foo.txt"})
if err != nil {
t.Errorf("couldn't prepare tar fixture %v", err)
}

config := &v1.Config{}
buildArgs := &dockerfile.BuildArgs{}

type testCase struct {
desctiption string
expectLayer bool
expectErr bool
count *int
expectedCount int
command *CachingCopyCommand
extractedFiles []string
contextFiles []string
}
testCases := []testCase{
func() testCase {
err = ioutil.WriteFile(filepath.Join(tempDir, "foo.txt"), []byte("meow"), 0644)
if err != nil {
t.Errorf("couldn't write tempfile %v", err)
t.FailNow()
}

c := &CachingCopyCommand{
img: fakeImage{
ImageLayers: []v1.Layer{
fakeLayer{TarContent: tarContent},
},
},
buildcontext: tempDir,
cmd: &instructions.CopyCommand{
SourcesAndDest: []string{
"foo.txt", "foo.txt",
},
},
}
count := 0
tc := testCase{
desctiption: "with valid image and valid layer",
count: &count,
expectedCount: 1,
expectLayer: true,
extractedFiles: []string{"/foo.txt"},
contextFiles: []string{"foo.txt"},
}
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
*tc.count++
return nil
}
tc.command = c
return tc
}(),
func() testCase {
c := &CachingCopyCommand{}
tc := testCase{
desctiption: "with no image",
expectErr: true,
}
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
return nil
}
tc.command = c
return tc
}(),
func() testCase {
c := &CachingCopyCommand{
img: fakeImage{},
}
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
return nil
}
return testCase{
desctiption: "with image containing no layers",
expectErr: true,
command: c,
}
}(),
func() testCase {
c := &CachingCopyCommand{
img: fakeImage{
ImageLayers: []v1.Layer{
fakeLayer{},
},
},
}
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
return nil
}
tc := testCase{
desctiption: "with image one layer which has no tar content",
expectErr: false, // this one probably should fail but doesn't because of how ExecuteCommand and util.GetFSFromLayers are implemented - cvgw- 2019-11-25
expectLayer: true,
}
tc.command = c
return tc
}(),
}

for _, tc := range testCases {
t.Run(tc.desctiption, func(t *testing.T) {
c := tc.command
err := c.ExecuteCommand(config, buildArgs)
if !tc.expectErr && err != nil {
t.Errorf("Expected err to be nil but was %v", err)
} else if tc.expectErr && err == nil {
t.Error("Expected err but was nil")
}

if tc.count != nil {
if *tc.count != tc.expectedCount {
t.Errorf("Expected extractFn to be called %v times but was called %v times", tc.expectedCount, *tc.count)
}
for _, file := range tc.extractedFiles {
match := false
cFiles := c.FilesToSnapshot()
for _, cFile := range cFiles {
if file == cFile {
match = true
break
}
}
if !match {
t.Errorf("Expected extracted files to include %v but did not %v", file, cFiles)
}
}

cmdFiles, err := c.FilesUsedFromContext(
config, buildArgs,
)
if err != nil {
t.Errorf("failed to get files used from context from command %v", err)
}

if len(cmdFiles) != len(tc.contextFiles) {
t.Errorf("expected files used from context to equal %v but was %v", tc.contextFiles, cmdFiles)
}
}

if c.layer == nil && tc.expectLayer {
t.Error("expected the command to have a layer set but instead was nil")
} else if c.layer != nil && !tc.expectLayer {
t.Error("expected the command to have no layer set but instead found a layer")
}
})
}
}

func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
setupDirs := func(t *testing.T) (string, string) {
testDir, err := ioutil.TempDir("", "")
Expand Down
2 changes: 0 additions & 2 deletions pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,6 @@ func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string
switch v := command.(type) {
case *commands.CopyCommand:
compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey)
case *commands.CachingCopyCommand:
compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey)
}

srcCtx := s.opts.SrcContext
Expand Down
Loading