Skip to content

Commit

Permalink
MAISTRA-1862 Fix NilPointerException when images can't be pulled (#183)
Browse files Browse the repository at this point in the history
This also improves logging by dropping some loglevels and always
logging errors when calling pullStrategy.GetImage()

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
dgn and mergify[bot] committed Oct 2, 2020
1 parent 367fa9a commit 5687393
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 9 deletions.
5 changes: 2 additions & 3 deletions mec/pkg/model/types.go
Expand Up @@ -45,11 +45,10 @@ const (
)

func (i *ImageRef) String() string {
tag := i.Tag
if i.SHA256 != "" {
tag = fmt.Sprintf("sha256:%s", i.SHA256)
return fmt.Sprintf("%s/%s@sha256:%s", i.Hub, i.Repository, i.SHA256)
}
return fmt.Sprintf("%s/%s:%s", i.Hub, i.Repository, tag)
return fmt.Sprintf("%s/%s:%s", i.Hub, i.Repository, i.Tag)
}

func StringToImageRef(ref string) *ImageRef {
Expand Down
28 changes: 24 additions & 4 deletions mec/pkg/pullstrategy/ossm/strategy.go
Expand Up @@ -102,11 +102,11 @@ func (p *ossmPullStrategy) GetImage(image *model.ImageRef) (model.Image, error)
if image.SHA256 == "" {
return nil, fmt.Errorf("getImage() only works for pinned images")
}
imageID, err := podman.GetImageID(image.String())
imageID, err := podman.GetImageID(image.SHA256)
if err != nil {
return nil, err
} else if imageID == "" {
return nil, fmt.Errorf("failed to read imageID from podman CLI")
return nil, nil
}
manifest, err := extractManifest(imageID)
if err != nil {
Expand All @@ -130,19 +130,39 @@ func (p *ossmPullStrategy) PullImage(image *model.ImageRef) (model.Image, error)
if errors.IsNotFound(err) {
createdIsi, err := p.createImageStreamImport(image)
if err != nil {
log.Errorf("failed to create ImageStreamImport: %s, attempt %d", err, attempt)
log.Warnf("failed to create ImageStreamImport: %s, attempt %d", err, attempt)
continue
}
log.Infof("Created ImageStreamImport %s", createdIsi.Name)
}
if err != nil {
log.Errorf("failed to Get() ImageStream: %s, attempt %d", err, attempt)
log.Warnf("failed to Get() ImageStream: %s, attempt %d", err, attempt)
continue
}
if imageStream != nil {
for _, tag := range imageStream.Spec.Tags {
if tag.From.Name == image.String() {
break
}
}
createdIsi, err := p.createImageStreamImport(image)
if err != nil {
log.Warnf("failed to create ImageStreamImport: %s, attempt %d", err, attempt)
continue
}
log.Infof("Created ImageStreamImport %s", createdIsi.Name)
}
}
if err != nil {
return nil, err
}

for _, condition := range imageStream.Status.Tags[0].Conditions {
if condition.Status == corev1.ConditionFalse {
return nil, fmt.Errorf("failed to pull image: %s", condition.Message)
}
}

// TODO implement importing always when ImagePullPolicy == Always
repo := imageStream.Status.DockerImageRepository
sha := imageStream.Status.Tags[0].Items[0].Image
Expand Down
11 changes: 9 additions & 2 deletions mec/pkg/server/worker.go
Expand Up @@ -88,8 +88,15 @@ func (w *Worker) processEvent(event ExtensionEvent) {
imageRef.SHA256 = extension.Status.Deployment.ContainerSHA256
}

img, err := w.pullStrategy.GetImage(imageRef)
if err != nil {
var img model.Image
var err error
if imageRef.SHA256 != "" {
img, err = w.pullStrategy.GetImage(imageRef)
if err != nil {
log.Errorf("Failed to check whether image is already present: %s", err)
}
}
if img == nil {
log.Infof("Image %s not present. Pulling", imageRef.String())
img, err = w.pullStrategy.PullImage(imageRef)
if err != nil {
Expand Down

0 comments on commit 5687393

Please sign in to comment.