Skip to content

Commit

Permalink
update the image status when start pulling images & compare(without …
Browse files Browse the repository at this point in the history
…defaultDomain and officialRepoName) name and tag of image (#1280)

add ut for ContainsImage



revert import

Signed-off-by: liuzhenwei <dui_zhang@163.com>
  • Loading branch information
diannaowa committed May 11, 2023
1 parent 441afd9 commit ccd94b2
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 23 deletions.
27 changes: 4 additions & 23 deletions pkg/daemon/criruntime/imageruntime/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"encoding/json"
"fmt"
"io"
"strings"

dockermessage "github.com/docker/docker/pkg/jsonmessage"
daemonutil "github.com/openkruise/kruise/pkg/daemon/util"
Expand Down Expand Up @@ -230,31 +229,13 @@ func (r *imagePullStatusReader) mainloop() {

func (c ImageInfo) ContainsImage(name string, tag string) bool {
for _, repoTag := range c.RepoTags {
imageRepo, imageTag := parseRepositoryTag(repoTag)
// We should remove defaultDomain and officialRepoName in RepoTags by NormalizeImageRefToNameTag method,
// Because if the user needs to download the image from hub.docker.com, CRI.PullImage will automatically add these when downloading the image
// Ref: https://github.com/openkruise/kruise/issues/1273
imageRepo, imageTag, _ := daemonutil.NormalizeImageRefToNameTag(repoTag)
if imageRepo == name && imageTag == tag {
return true
}
}
return false
}

// parseRepositoryTag gets a repos name and returns the right reposName + tag|digest
// The tag can be confusing because of a port in a repository name.
//
// Ex: localhost.localdomain:5000/samalba/hipache:latest
// Digest ex: localhost:5000/foo/bar@sha256:bc8813ea7b3603864987522f02a76101c17ad122e1c46d790efc0fca78ca7bfb
func parseRepositoryTag(repos string) (string, string) {
n := strings.Index(repos, "@")
if n >= 0 {
parts := strings.Split(repos, "@")
return parts[0], parts[1]
}
n = strings.LastIndex(repos, ":")
if n < 0 {
return repos, ""
}
if tag := repos[n+1:]; !strings.Contains(tag, "/") {
return repos[:n], tag
}
return repos, ""
}
73 changes: 73 additions & 0 deletions pkg/daemon/criruntime/imageruntime/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,76 @@ func TestMatchRegistryAuths(t *testing.T) {
})
}
}

func TestContainsImage(t *testing.T) {
cases := []struct {
name string
ImageName string
Tag string
ImageInfos []ImageInfo
Expect bool
}{
{
name: "test_nginx",
ImageName: "nginx",
Tag: "latest",
ImageInfos: []ImageInfo{{
RepoTags: []string{"docker.io/library/nginx:latest"},
},
},
Expect: true,
},
{
name: "test_test/nginx:1.0",
ImageName: "test/nginx",
Tag: "1.0",
ImageInfos: []ImageInfo{{
RepoTags: []string{"docker.io/test/nginx:1.0"},
},
},
Expect: true,
},
{
name: "test_test/nginx:1.0_false",
ImageName: "test/nginx",
Tag: "1.0",
ImageInfos: []ImageInfo{{
RepoTags: []string{"docker.io/library/nginx:1.0"},
},
},
Expect: false,
},

{
name: "test_docker.io/library/nginx",
ImageName: "docker.io/library/nginx",
Tag: "latest",
ImageInfos: []ImageInfo{{
RepoTags: []string{"docker.io/library/nginx:latest"},
},
},
Expect: false,
},
{
name: "test_1.1.1.1:8080/test/nginx",
ImageName: "1.1.1.1:8080/test/nginx",
Tag: "latest",
ImageInfos: []ImageInfo{{
RepoTags: []string{"1.1.1.1:8080/test/nginx:latest"},
},
},
Expect: true,
},
}

for _, cs := range cases {
t.Run(cs.name, func(t *testing.T) {
for _, info := range cs.ImageInfos {
res := info.ContainsImage(cs.ImageName, cs.Tag)
if res != cs.Expect {
t.Fatalf("ContainsImage failed")
}
}
})
}
}
7 changes: 7 additions & 0 deletions pkg/daemon/imagepuller/imagepuller_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,13 @@ func (w *pullWorker) Run() {
StartTime: &startTime,
Version: w.tagSpec.Version,
}

// We should update the image status when we start pulling images,
// which can meet the scenario that some large size images cannot return the result from CRI.PullImage within 60s. For one reason:
// For nodeimage controller will mark image:tag task failed (not responded for a long time) if daemon does not report status in 60s.
// Ref: https://github.com/openkruise/kruise/issues/1273
w.statusUpdater.UpdateStatus(newStatus)

defer func() {
cost := time.Since(startTime.Time)
if newStatus.Phase == appsv1alpha1.ImagePhaseFailed {
Expand Down

0 comments on commit ccd94b2

Please sign in to comment.