manifest: Store layers inside manifests consistently as values.

Commit 1829fb61 ("manifest: Fix crash on startup when trying to clean up
unused files (#5840)") changed the config layer stored in manifests
from a pointer to a value. This was done in order to avoid potential
nil pointer dereferences after it is deserialized from JSON in the
event that the field is missing.

This changes the Layers slice to also be stored by value. This enables
consistency in handling across the two objects.
This commit is contained in:
Jesse Gross 2024-08-07 14:22:17 -07:00
parent 97ec8cfd4e
commit 7edaf6e7e8
6 changed files with 33 additions and 33 deletions

View file

@ -373,7 +373,7 @@ func CreateModel(ctx context.Context, name model.Name, modelFileDir, quantizatio
var messages []*api.Message var messages []*api.Message
parameters := make(map[string]any) parameters := make(map[string]any)
var layers []*Layer var layers []Layer
for _, c := range modelfile.Commands { for _, c := range modelfile.Commands {
mediatype := fmt.Sprintf("application/vnd.ollama.image.%s", c.Name) mediatype := fmt.Sprintf("application/vnd.ollama.image.%s", c.Name)
@ -499,7 +499,7 @@ func CreateModel(ctx context.Context, name model.Name, modelFileDir, quantizatio
if c.Name != "license" { if c.Name != "license" {
// replace // replace
layers = slices.DeleteFunc(layers, func(layer *Layer) bool { layers = slices.DeleteFunc(layers, func(layer Layer) bool {
if layer.MediaType != mediatype { if layer.MediaType != mediatype {
return false return false
} }
@ -545,7 +545,7 @@ func CreateModel(ctx context.Context, name model.Name, modelFileDir, quantizatio
} }
var err2 error var err2 error
layers = slices.DeleteFunc(layers, func(layer *Layer) bool { layers = slices.DeleteFunc(layers, func(layer Layer) bool {
switch layer.MediaType { switch layer.MediaType {
case "application/vnd.ollama.image.message": case "application/vnd.ollama.image.message":
// if there are new messages, remove the inherited ones // if there are new messages, remove the inherited ones
@ -839,10 +839,10 @@ func PushModel(ctx context.Context, name string, regOpts *registryOptions, fn fu
return err return err
} }
var layers []*Layer var layers []Layer
layers = append(layers, manifest.Layers...) layers = append(layers, manifest.Layers...)
if manifest.Config.Digest != "" { if manifest.Config.Digest != "" {
layers = append(layers, &manifest.Config) layers = append(layers, manifest.Config)
} }
for _, layer := range layers { for _, layer := range layers {
@ -911,10 +911,10 @@ func PullModel(ctx context.Context, name string, regOpts *registryOptions, fn fu
return fmt.Errorf("pull model manifest: %s", err) return fmt.Errorf("pull model manifest: %s", err)
} }
var layers []*Layer var layers []Layer
layers = append(layers, manifest.Layers...) layers = append(layers, manifest.Layers...)
if manifest.Config.Digest != "" { if manifest.Config.Digest != "" {
layers = append(layers, &manifest.Config) layers = append(layers, manifest.Config)
} }
skipVerify := make(map[string]bool) skipVerify := make(map[string]bool)

View file

@ -16,15 +16,15 @@ type Layer struct {
status string status string
} }
func NewLayer(r io.Reader, mediatype string) (*Layer, error) { func NewLayer(r io.Reader, mediatype string) (Layer, error) {
blobs, err := GetBlobsPath("") blobs, err := GetBlobsPath("")
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
temp, err := os.CreateTemp(blobs, "sha256-") temp, err := os.CreateTemp(blobs, "sha256-")
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
defer temp.Close() defer temp.Close()
defer os.Remove(temp.Name()) defer os.Remove(temp.Name())
@ -32,28 +32,28 @@ func NewLayer(r io.Reader, mediatype string) (*Layer, error) {
sha256sum := sha256.New() sha256sum := sha256.New()
n, err := io.Copy(io.MultiWriter(temp, sha256sum), r) n, err := io.Copy(io.MultiWriter(temp, sha256sum), r)
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
if err := temp.Close(); err != nil { if err := temp.Close(); err != nil {
return nil, err return Layer{}, err
} }
digest := fmt.Sprintf("sha256:%x", sha256sum.Sum(nil)) digest := fmt.Sprintf("sha256:%x", sha256sum.Sum(nil))
blob, err := GetBlobsPath(digest) blob, err := GetBlobsPath(digest)
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
status := "using existing layer" status := "using existing layer"
if _, err := os.Stat(blob); err != nil { if _, err := os.Stat(blob); err != nil {
status = "creating new layer" status = "creating new layer"
if err := os.Rename(temp.Name(), blob); err != nil { if err := os.Rename(temp.Name(), blob); err != nil {
return nil, err return Layer{}, err
} }
} }
return &Layer{ return Layer{
MediaType: mediatype, MediaType: mediatype,
Digest: digest, Digest: digest,
Size: n, Size: n,
@ -61,22 +61,22 @@ func NewLayer(r io.Reader, mediatype string) (*Layer, error) {
}, nil }, nil
} }
func NewLayerFromLayer(digest, mediatype, from string) (*Layer, error) { func NewLayerFromLayer(digest, mediatype, from string) (Layer, error) {
if digest == "" { if digest == "" {
return nil, errors.New("creating new layer from layer with empty digest") return Layer{}, errors.New("creating new layer from layer with empty digest")
} }
blob, err := GetBlobsPath(digest) blob, err := GetBlobsPath(digest)
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
fi, err := os.Stat(blob) fi, err := os.Stat(blob)
if err != nil { if err != nil {
return nil, err return Layer{}, err
} }
return &Layer{ return Layer{
MediaType: mediatype, MediaType: mediatype,
Digest: digest, Digest: digest,
Size: fi.Size(), Size: fi.Size(),
@ -109,7 +109,7 @@ func (l *Layer) Remove() error {
} }
for _, m := range ms { for _, m := range ms {
for _, layer := range append(m.Layers, &m.Config) { for _, layer := range append(m.Layers, m.Config) {
if layer.Digest == l.Digest { if layer.Digest == l.Digest {
// something is using this layer // something is using this layer
return nil return nil

View file

@ -17,7 +17,7 @@ type Manifest struct {
SchemaVersion int `json:"schemaVersion"` SchemaVersion int `json:"schemaVersion"`
MediaType string `json:"mediaType"` MediaType string `json:"mediaType"`
Config Layer `json:"config"` Config Layer `json:"config"`
Layers []*Layer `json:"layers"` Layers []Layer `json:"layers"`
filepath string filepath string
fi os.FileInfo fi os.FileInfo
@ -25,7 +25,7 @@ type Manifest struct {
} }
func (m *Manifest) Size() (size int64) { func (m *Manifest) Size() (size int64) {
for _, layer := range append(m.Layers, &m.Config) { for _, layer := range append(m.Layers, m.Config) {
size += layer.Size size += layer.Size
} }
@ -46,7 +46,7 @@ func (m *Manifest) Remove() error {
} }
func (m *Manifest) RemoveLayers() error { func (m *Manifest) RemoveLayers() error {
for _, layer := range append(m.Layers, &m.Config) { for _, layer := range append(m.Layers, m.Config) {
if layer.Digest != "" { if layer.Digest != "" {
if err := layer.Remove(); errors.Is(err, os.ErrNotExist) { if err := layer.Remove(); errors.Is(err, os.ErrNotExist) {
slog.Debug("layer does not exist", "digest", layer.Digest) slog.Debug("layer does not exist", "digest", layer.Digest)
@ -95,7 +95,7 @@ func ParseNamedManifest(n model.Name) (*Manifest, error) {
return &m, nil return &m, nil
} }
func WriteManifest(name model.Name, config *Layer, layers []*Layer) error { func WriteManifest(name model.Name, config Layer, layers []Layer) error {
manifests, err := GetManifestPath() manifests, err := GetManifestPath()
if err != nil { if err != nil {
return err return err
@ -115,7 +115,7 @@ func WriteManifest(name model.Name, config *Layer, layers []*Layer) error {
m := Manifest{ m := Manifest{
SchemaVersion: 2, SchemaVersion: 2,
MediaType: "application/vnd.docker.distribution.manifest.v2+json", MediaType: "application/vnd.docker.distribution.manifest.v2+json",
Config: *config, Config: config,
Layers: layers, Layers: layers,
} }

View file

@ -26,7 +26,7 @@ import (
var intermediateBlobs map[string]string = make(map[string]string) var intermediateBlobs map[string]string = make(map[string]string)
type layerGGML struct { type layerGGML struct {
*Layer Layer
*llm.GGML *llm.GGML
} }

View file

@ -98,7 +98,7 @@ func TestDeleteDuplicateLayers(t *testing.T) {
} }
// create a manifest with duplicate layers // create a manifest with duplicate layers
if err := WriteManifest(n, config, []*Layer{config}); err != nil { if err := WriteManifest(n, config, []Layer{config}); err != nil {
t.Fatal(err) t.Fatal(err)
} }

View file

@ -26,7 +26,7 @@ import (
var blobUploadManager sync.Map var blobUploadManager sync.Map
type blobUpload struct { type blobUpload struct {
*Layer Layer
Total int64 Total int64
Completed atomic.Int64 Completed atomic.Int64
@ -362,7 +362,7 @@ func (p *progressWriter) Rollback() {
p.written = 0 p.written = 0
} }
func uploadBlob(ctx context.Context, mp ModelPath, layer *Layer, opts *registryOptions, fn func(api.ProgressResponse)) error { func uploadBlob(ctx context.Context, mp ModelPath, layer Layer, opts *registryOptions, fn func(api.ProgressResponse)) error {
requestURL := mp.BaseURL() requestURL := mp.BaseURL()
requestURL = requestURL.JoinPath("v2", mp.GetNamespaceRepository(), "blobs", layer.Digest) requestURL = requestURL.JoinPath("v2", mp.GetNamespaceRepository(), "blobs", layer.Digest)