Fixup. Image refactoring and deletion

cmd/imageadm/list.go

@@ -12,11 +12,11 @@ var listCmd = &cobra.Command{
 	Short: "List all installed images",
 	Long:  `List all images that are currently installed`,
 	Run: func(cmd *cobra.Command, args []string) {
-		images := h.Repositories()
+		repositories := h.Repositories()
 		table := tablewriter.NewWriter(os.Stdout)
-		table.SetHeader([]string{"Name", "Repositories", "Size"})
-		for _, img := range images {
-			table.Append([]string{img.Name, img.Images.String(), img.GetSizeString()})
+		table.SetHeader([]string{"Name", "Images/Tags", "Size"})
+		for _, repo := range repositories {
+			table.Append([]string{repo.String(), repo.Images.String(), repo.GetSizeString()})
 		}
 		table.Render()
 	},

go.mod

@@ -49,7 +49,7 @@ require (
 	github.com/ztrue/tracerr v0.3.0
 	go4.org v0.0.0-20190313082347-94abd6928b1d // indirect
 	golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734
-	gotest.tools/v3 v3.0.2 // indirect
+	gotest.tools/v3 v3.0.2
 )
 
 replace github.com/sirupsen/logrus => github.com/sirupsen/logrus v1.4.2-0.20190403091019-9b3cdde74fbe

host/build.go

@@ -357,6 +357,7 @@ func (h *Host) ExportToImage(ref reference.Reference, container *pod.Container,
 	if err := repo.ImportImageFromOCIDirectory(ref, wr.ManifestDigest, true); err != nil {
 		return nil, tracerr.Wrap(err)
 	}
+
 	logrus.Info("successful")
 
 	return repo, nil

host/image.go

@@ -1,6 +1,8 @@
 package host
 
 import (
+	"fmt"
+
 	"git.wegmueller.it/opencloud/opencloud/image"
 	"git.wegmueller.it/opencloud/opencloud/image/reference"
 	"github.com/ztrue/tracerr"
@@ -25,14 +27,16 @@ func (h *Host) HasRepository(ref reference.Reference) bool {
 }
 
 func (h *Host) GetLocalRepository(ref reference.Reference) (*image.Repository, error) {
-	img, err := image.FindRepository(ref)
+	repository, err := image.FindRepository(ref)
 	if err != nil {
 		return nil, tracerr.Wrap(err)
 	}
-	if err := img.Load(); err != nil {
+
+	if err := repository.Load(); err != nil {
 		return nil, tracerr.Wrap(err)
 	}
-	return img, nil
+
+	return repository, nil
 }
 
 func (h *Host) FetchImage(ref reference.Reference) (*image.Repository, error) {
@@ -58,6 +62,10 @@ func (h *Host) CloneImage(nameRef, baseRef reference.Reference) (*image.Reposito
 
 func (h *Host) DestroyImage(ref reference.Reference) error {
 	repo, err := h.GetLocalRepository(ref)
+	if image.IsNotExistError(err) {
+		return fmt.Errorf("image %s does not exist", ref)
+	}
+
 	if err != nil {
 		return tracerr.Wrap(err)
 	}

image/destroy.go

@@ -5,18 +5,21 @@ import (
 	"os"
 
 	"git.wegmueller.it/opencloud/opencloud/image/reference"
+	"github.com/sirupsen/logrus"
 	"github.com/ztrue/tracerr"
 )
 
 // This Function destroys the image on disk and makes the Struct unusable
 // We are relying on zfs to error when an image can not be destroyed due to still dependant images
 func (r *Repository) Destroy(imageRef *reference.Reference) error {
-	for _, img := range r.Images {
-		if imageRef != nil && img.Name.String() != imageRef.String() {
+	logrus.Debugf("Starting image destroy for %s", imageRef)
+
+	for i, img := range r.Images {
+		if imageRef != nil && img.Name.Tag != imageRef.Tag {
 			continue
 		}
 
-		for i := len(img.Layers) - 1; i > 0; i-- {
+		for i := len(img.Layers) - 1; i == 0; i-- {
 			layer := img.Layers[i]
 			linkedImages, ok := r.LayerList[layer.Descriptor.Digest.Encoded()]
 			// If the layer is used in more than one place abort the deletion
@@ -45,23 +48,38 @@ func (r *Repository) Destroy(imageRef *reference.Reference) error {
 					delete(r.LayerList, layer.Descriptor.Digest.Encoded())
 				}
 			}
-			// Delete the layer dataset.
 			ds := GetLayerDS(layer.Descriptor.Digest)
+			if ds == nil {
+				continue
+			}
+
+			// Delete the layer dataset.
+			logrus.Debugf("deleting layer dataset %s", layer.Descriptor.Digest)
 			if err := ds.Destroy(true); err != nil {
 				return tracerr.Wrap(err)
 			}
 		}
+
+		delete(r.Images, i)
+
+		logrus.Debugf("Updating repository")
 		if err := r.Save(); err != nil {
 			return tracerr.Wrap(err)
 		}
 	}
 
-	if err := os.Remove(r.VFSPath() + ".json"); err != nil {
-		return tracerr.Wrap(err)
-	}
+	if len(r.Images) == 0 {
+		logrus.Debugf("Removing Repository definition")
+		if err := os.Remove(r.VFSPath() + ".json"); err != nil {
+			return tracerr.Wrap(err)
+		}
 
-	if err := os.RemoveAll(r.VFSPath()); err != nil {
-		return tracerr.Wrap(err)
+		logrus.Debugf("Removing Repository OCI path")
+		if err := os.RemoveAll(r.VFSPath()); err != nil {
+			return tracerr.Wrap(err)
+		}
+	} else {
+		logrus.Debugf("The following images still exist in repository %s: %s not removing the repository", r, r.Images)
 	}
 
 	return nil

image/find.go

@@ -1,14 +1,19 @@
 package image
 
 import (
+	"errors"
+
 	"git.wegmueller.it/opencloud/opencloud/image/reference"
-	"github.com/pkg/errors"
 	"github.com/ztrue/tracerr"
 )
 
 var doesNotExistErr = errors.New("repository does not exist")
 
 func IsNotExistError(err error) bool {
+	if errors.Is(err, doesNotExistErr) {
+		return true
+	}
+
 	return tracerr.Unwrap(err) == doesNotExistErr
 }
 

image/image.go

@@ -52,7 +52,7 @@ func OpenImageFromOCIStorage(ref reference.Reference, manifestDigest digest.Dige
 	for key, val := range i.Manifest.Annotations {
 		if strings.HasPrefix(key, volume.MetadataKeyPrefix) {
 			var vol volume.Config
-			blob, err := GetBlob(ref, digest.NewDigestFromEncoded(manifestDigest.Algorithm(), val))
+			blob, err := GetBlob(ref, digest.Digest(val))
 			if err != nil {
 				return nil, fmt.Errorf("could not load image %s: %w", i.Name, err)
 			}
@@ -82,7 +82,6 @@ func OpenImageFromOCIStorage(ref reference.Reference, manifestDigest digest.Dige
 	for _, lDesc := range i.Manifest.Layers {
 		l := Layer{
 			Descriptor: lDesc,
-			Unpacked:   false,
 		}
 
 		if err := i.openLayer(&l); err != nil {
@@ -107,7 +106,6 @@ func (i *Image) Load() error {
 
 func (i *Image) openLayer(l *Layer) error {
 	if HasLayer(l.Descriptor.Digest) {
-		l.Unpacked = true
 		l.Dataset = GetLayerDS(l.Descriptor.Digest)
 		l.volumeList = make(volume.List, 0)
 
@@ -125,7 +123,7 @@ func (i *Image) openLayer(l *Layer) error {
 }
 
 func (i *Image) GetLayers() []digest.Digest {
-	layers := make([]digest.Digest, len(i.Layers))
+	layers := make([]digest.Digest, 0)
 	for _, l := range i.Layers {
 		layers = append(layers, l.Descriptor.Digest)
 	}

image/layer.go

@@ -12,14 +12,13 @@ import (
 type Layers []Layer
 
 type Layer struct {
-	Dataset    *zfs.Dataset
-	volumeList volume.List
+	Dataset    *zfs.Dataset       `json:"-"`
 	Descriptor specsv1.Descriptor `json:"descriptor"`
-	Unpacked   bool               `json:"unpacked"`
+	volumeList volume.List
 }
 
 func (l *Layer) GetSize() int64 {
-	if !l.Unpacked {
+	if l.Dataset == nil {
 		return l.Descriptor.Size
 	}
 

image/list.go

@@ -4,7 +4,8 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
-	"strings"
+
+	"github.com/sirupsen/logrus"
 )
 
 func Repositories() []*Repository {
@@ -12,16 +13,13 @@ func Repositories() []*Repository {
 		return nil
 	}
 
-	matches, err := filepath.Glob(imagesDataset.VFSPath("images/*.json"))
+	matches, err := filepath.Glob(imagesDataset.VFSPath("*.json"))
 	if err != nil {
 		return nil
 	}
 
 	retValue := make([]*Repository, 0, len(matches))
 	for _, m := range matches {
-		if strings.Contains(m, "index.json") {
-			continue
-		}
 
 		var repo Repository
 		if err := func(fileName string, repo *Repository) error {
@@ -37,10 +35,12 @@ func Repositories() []*Repository {
 
 			return nil
 		}(m, &repo); err != nil {
+			logrus.Warnf("Failed to decode file %s into repository: %s", m, err)
 			continue
 		}
 
 		if err := repo.Load(); err != nil {
+			logrus.Warnf("Failed to decode file %s into repository: %s", m, err)
 			continue
 		}
 

image/meta.go

@@ -4,10 +4,19 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/ztrue/tracerr"
 )
 
+func (r *Repository) GetName() string {
+	return strings.ReplaceAll(r.Name, "-", "/")
+}
+
+func (r *Repository) String() string {
+	return r.GetName()
+}
+
 func (r *Repository) Exists() bool {
 	if _, err := os.Stat(imagesDataset.VFSPath(r.Name + ".json")); err != nil {
 		return false

image/oci/writer.go

@@ -362,8 +362,8 @@ func (w *Writer) Close() error {
 
 		c.History = append(c.History, specsv1.History{
 			Created:   c.Created,
-			CreatedBy: "zoneadm install",
-			Comment:   "This layer was created when importing a zone as a new image",
+			CreatedBy: "opencloud",
+			Comment:   "This layer was by opencloud software. We do not provide a history of it.",
 		})
 	}
 

image/reference/reference.go

@@ -2,6 +2,7 @@ package reference
 
 import (
 	"os"
+	"path"
 	"strings"
 )
 
@@ -30,25 +31,6 @@ func (r *Reference) GetPathSafeRepositoryName() string {
 	return strings.ReplaceAll(r.Repository+"/"+r.Name, "/", "-")
 }
 
-func NewFromSafeRepositoryName(safeName string) Reference {
-	lastIdx := strings.LastIndex(safeName, "-")
-	if lastIdx == -1 {
-		return Reference{
-			Scheme:     SchemeDocker,
-			Name:       safeName,
-			Repository: "library",
-			Tag:        "latest",
-		}
-	}
-
-	return Reference{
-		Scheme:     SchemeDocker,
-		Name:       safeName[lastIdx+1:],
-		Repository: strings.ReplaceAll(safeName[:lastIdx], "-", "/"),
-		Tag:        "latest",
-	}
-}
-
 func (r *Reference) GetFullName() string {
 	return r.Repository + "/" + r.Name
 }
@@ -109,6 +91,15 @@ func splitOf(str string, SubStr string) (string, string) {
 	return str[:idx], str[idx+1:]
 }
 
+func hasHostNameOrPort(str string) bool {
+	for _, c := range str {
+		if c == '.' || c == ':' {
+			return true
+		}
+	}
+	return false
+}
+
 // docker://ubuntu
 // docker://registry.wegmueller.it/ubuntu
 // docker:ubuntu
@@ -116,24 +107,46 @@ func splitOf(str string, SubStr string) (string, string) {
 func (r *Reference) parseDocker(refString string) {
 	r.Scheme = SchemeDocker
 	// First split off the Scheme
-	_, rest := splitOf(refString, ":")
-	// Remove //
-	if strings.HasPrefix(rest, "//") {
-		rest = rest[2:]
-		r.Host, rest = splitOf(rest, "/")
+	if strings.HasPrefix(refString, "docker:") || strings.HasPrefix(refString, "http:") || strings.HasPrefix(refString, "https:") {
+		if idx := strings.Index(refString, ":"); idx != -1 {
+			refString = refString[idx+1:]
+		}
+
+		if idx := strings.Index(refString, "//"); idx != -1 {
+			refString = refString[idx+2:]
+		}
 	}
-	if strings.Contains(rest, "/") {
-		r.Repository, rest = splitOf(rest, "/")
-	} else {
+
+	if idx := strings.Index(refString, "/"); idx != -1 {
+		// If we have a . or a : character in the string we assume it is a hostname and or port
+		if hasHostNameOrPort(refString[:idx]) {
+			r.Host = refString[:idx]
+			refString = refString[idx+1:]
+		}
+	}
+
+	//Split into dir and basename
+	r.Repository = path.Dir(refString)
+	r.Name = path.Base(refString)
+
+	if r.Repository == "." {
 		r.Repository = "library"
 	}
-	r.Name, rest = splitOf(rest, ":")
-	if r.Name == "" {
-		r.Name = rest
+
+	if r.Name == "." {
+		r.Name = r.Repository
+	}
+
+	if r.Repository == r.Name {
+		r.Repository = "library"
+	}
+
+	if idx := strings.Index(r.Name, ":"); idx != -1 {
+		r.Tag = r.Name[idx+1:]
+		r.Name = r.Name[:idx]
+	} else {
 		r.Tag = "latest"
-		return
 	}
-	r.Tag = rest
 }
 
 // oci:/full/path/to/directory

image/reference/reference_test.go

@@ -26,29 +26,94 @@ import (
 // full/path/to/file.oci[.aci]
 
 func TestReference_Parse(t *testing.T) {
-	d1 := "docker://ubuntu"
-	d2 := "docker://registry.wegmueller.it/ubuntu"
-	d3 := "docker:ubuntu"
-	d4 := "docker:toast/redis:mytag"
-	r := NewReferenceString(d1)
-	assert.Equal(t, "ubuntu", r.Name)
-	r = NewReferenceString(d2)
-	assert.Equal(t, "registry.wegmueller.it", r.Host)
-	assert.Equal(t, "ubuntu", r.Name)
-	r = NewReferenceString(d3)
-	assert.Equal(t, "ubuntu", r.Name)
-	r = NewReferenceString(d4)
-	assert.Equal(t, "toast", r.Repository)
-	assert.Equal(t, "redis", r.Name)
-	assert.Equal(t, "mytag", r.Tag)
-	o1 := "oci:/full/path/to/directory"
-	o2 := "oci:/full/path/to/file"
-	r = NewReferenceString(o1)
-	assert.Equal(t, "/full/path/to/directory", r.Path)
-	r = NewReferenceString(o2)
-	assert.Equal(t, "/full/path/to/file", r.Path)
-	f2 := "full/path/to/file.oci"
-	r = NewReferenceString(f2)
-	assert.Equal(t, "full/path/to/file.oci", r.Path)
-	assert.Equal(t, SchemeOCIArchive, r.Scheme)
+	type Expected struct {
+		name       string
+		repository string
+		tag        string
+		host       string
+		path       string
+		scheme     string
+	}
+	testCases := []struct {
+		expected Expected
+		input    string
+	}{
+		{
+			expected: Expected{
+				name:       "ubuntu",
+				repository: "library",
+				tag:        "latest",
+				host:       "",
+			},
+			input: "docker://ubuntu",
+		},
+		{
+			expected: Expected{
+				name:       "ubuntu",
+				repository: "library",
+				tag:        "latest",
+				host:       "registry.wegmueller.it",
+			},
+			input: "docker://registry.wegmueller.it/ubuntu",
+		},
+		{
+			expected: Expected{
+				name:       "ubuntu",
+				repository: "library",
+				tag:        "latest",
+				host:       "",
+			},
+			input: "docker:ubuntu",
+		},
+		{
+			expected: Expected{
+				name:       "redis",
+				repository: "toast",
+				tag:        "mytag",
+				host:       "",
+			},
+			input: "docker:toast/redis:mytag",
+		},
+		{
+			expected: Expected{
+				scheme: SchemeOCIDirectory,
+				path:   "/full/path/to/directory",
+			},
+			input: "oci:/full/path/to/directory",
+		},
+		{
+			expected: Expected{
+				scheme: SchemeOCIArchive,
+				path:   "/full/path/to/file.tar",
+			},
+			input: "oci:/full/path/to/file.tar",
+		},
+		{
+			expected: Expected{
+				repository: "openindiana",
+				name:       "hipster",
+				tag:        "latest",
+			},
+			input: "openindiana/hipster:latest",
+		},
+	}
+
+	for i, tc := range testCases {
+		r := NewReferenceString(tc.input)
+		switch r.Scheme {
+		case SchemeDocker:
+			assert.Equal(t, tc.expected.repository, r.Repository)
+			assert.Equal(t, tc.expected.name, r.Name)
+			assert.Equal(t, tc.expected.host, r.Host)
+			assert.Equal(t, tc.expected.tag, r.Tag)
+			if t.Failed() {
+				t.Fatalf("testcase %d failed with input %s:", i, tc.input)
+			}
+		case SchemeOCIArchive, SchemeOCIDirectory:
+			assert.Equal(t, tc.expected.path, r.Path)
+		default:
+			t.Fatalf("schema %s not supported by testcases yet: testcase %d: %s", r.Scheme, i, tc.input)
+		}
+	}
+
 }

image/repository.go

@@ -36,7 +36,7 @@ func OpenRepository(ref reference.Reference) (*Repository, error) {
 		}
 	}
 	r := &Repository{
-		Name:   ref.Repository,
+		Name:   ref.GetPathSafeRepositoryName(),
 		Images: make(Images),
 	}
 	if err := r.Load(); err != nil {
@@ -54,7 +54,7 @@ func CreateEmptyRepository(ref reference.Reference) (*Repository, error) {
 	}
 
 	r := &Repository{
-		Name:      ref.Repository,
+		Name:      ref.GetPathSafeRepositoryName(),
 		Images:    make(Images),
 		LayerList: make(map[string][]reference.Reference),
 	}
@@ -69,7 +69,7 @@ func CreateEmptyRepository(ref reference.Reference) (*Repository, error) {
 func (r *Repository) GetSize() int64 {
 	var size int64 = 0
 	for _, linkedImages := range r.LayerList {
-		img := r.Images[linkedImages[0].Name]
+		img := r.Images[linkedImages[0].Tag]
 		size += img.GetSize()
 	}
 

image/unpack.go

@@ -9,6 +9,7 @@ import (
 	"git.wegmueller.it/opencloud/opencloud/image/oci"
 	"git.wegmueller.it/opencloud/opencloud/volume"
 	"git.wegmueller.it/opencloud/opencloud/zfs"
+	"github.com/sirupsen/logrus"
 	"github.com/ztrue/tracerr"
 )
 
@@ -27,7 +28,14 @@ func (i *Image) UnpackLayers(rd *oci.Reader, keepLayerFile bool) (Gerr error) {
 	}()
 
 	for layerId, layer := range i.Layers {
-		if layer.Unpacked {
+		if layer.Dataset != nil {
+			continue
+		}
+
+		// safety check against runtime problems
+		// i.e if somebody forgot to load the dataset
+		if HasLayer(layer.Descriptor.Digest) {
+			logrus.Warnf("dataset for layer %s was not loaded properly", layer.Descriptor.Digest)
 			continue
 		}
 

pod/container.go

@@ -87,7 +87,11 @@ func CreateEmptyContainer(parentDataset *zfs.Dataset, m *ContainerManifest, name
 	}
 
 	if err := container.register(); err != nil {
-		return nil, err
+		return nil, tracerr.Wrap(err)
+	}
+
+	if err := container.Save(); err != nil {
+		return nil, tracerr.Wrap(err)
 	}
 
 	return container, nil
@@ -117,10 +121,6 @@ func CreateContainer(parentDataset *zfs.Dataset, m *ContainerManifest, name, tag
 		return nil, tracerr.Wrap(err)
 	}
 
-	container.Manifest.Spec.Root = &spec.Root{
-		Path: container.rootDS.Mountpoint,
-	}
-
 	if err := container.cloneImageToContainer(repo); err != nil {
 		return nil, tracerr.Wrap(err)
 	}
@@ -133,8 +133,16 @@ func CreateContainer(parentDataset *zfs.Dataset, m *ContainerManifest, name, tag
 		return nil, tracerr.Wrap(err)
 	}
 
+	container.Manifest.Spec.Root = &spec.Root{
+		Path: container.rootDS.Mountpoint,
+	}
+
 	if err := container.register(); err != nil {
-		return nil, err
+		return nil, tracerr.Wrap(err)
+	}
+
+	if err := container.Save(); err != nil {
+		return nil, tracerr.Wrap(err)
 	}
 
 	return container, nil