Cleaned up bundle creation

bundle/bundle.go

@@ -71,7 +71,7 @@ func (b *Bundle) loadSpec() (r rspecs.Spec, err error) {
 func (b *Bundle) Image() *digest.Digest {
 	if imgIdb, err := ioutil.ReadFile(b.imageFile()); err == nil {
 		d, err := digest.Parse(strings.Trim(string(imgIdb), " \n"))
-		if err == nil {
+		if err == nil && d.Validate() == nil {
 			return &d
 		}
 	}
@@ -137,25 +137,23 @@ func OpenLockedBundle(bundle Bundle) (*LockedBundle, error) {
 		return nil, err
 	}
 	if err := bundle.resetExpiryTime(); err != nil {
+		lck.Unlock()
 		return nil, errors.Wrap(err, "lock bundle")
 	}
 	return &LockedBundle{bundle, nil, nil, lck}, nil
 }
 
-func CreateLockedBundle(dir string, spec SpecGenerator, image BundleImage, update bool) (r *LockedBundle, err error) {
+func CreateLockedBundle(dir string, update bool) (r *LockedBundle, err error) {
 	defer exterrors.Wrapd(&err, "create bundle")
 
 	// Create bundle
-	id := ""
-	if id == "" {
-		id = filepath.Base(dir)
-	}
+	id := filepath.Base(dir)
 	bundle := Bundle{id, dir, time.Now()}
 
 	// Lock bundle
 	lck, err := lockBundle(&bundle)
 	if err != nil {
-		return nil, err
+		return
 	}
 	r = &LockedBundle{bundle, nil, nil, lck}
 	defer func() {
@@ -167,16 +165,13 @@ func CreateLockedBundle(dir string, spec SpecGenerator, image BundleImage, updat
 	// Create or update bundle
 	err = os.Mkdir(dir, 0770)
 	exists := err != nil && os.IsExist(err)
-	updateRootfs := true
 
 	if exists {
 		if !update {
-			return r, errors.Errorf("bundle %q directory already exists", id)
+			return r, errors.Errorf("bundle %q already exists", id)
 		}
-		lastImageId := bundle.Image()
-		rootfs := filepath.Join(dir, "rootfs")
-		if _, e := os.Stat(rootfs); e == nil && (lastImageId == nil && image == nil || lastImageId != nil && *lastImageId == image.ID()) {
-			updateRootfs = false
+		if err = bundle.resetExpiryTime(); err != nil {
+			return
 		}
 	} else {
 		if err != nil {
@@ -191,17 +186,9 @@ func CreateLockedBundle(dir string, spec SpecGenerator, image BundleImage, updat
 		}
 	}
 
-	// Update rootfs if not exists or image changed
-	if updateRootfs {
-		if err = r.UpdateRootfs(image); err != nil {
-			return
-		}
-	}
-
-	// TODO: resolve user/group names here
-	// Write spec
-	if err = r.SetSpec(spec); err != nil {
-		return
+	r.spec = &rspecs.Spec{
+		Root:        &rspecs.Root{Path: "rootfs"},
+		Annotations: map[string]string{ANNOTATION_BUNDLE_ID: id},
 	}
 
 	return
@@ -235,11 +222,30 @@ func (b *LockedBundle) Spec() (*rspecs.Spec, error) {
 	return b.spec, nil
 }
 
+// Returns the bundle's image ID
+func (b *LockedBundle) Image() *digest.Digest {
+	if b.image == nil {
+		b.image = b.bundle.Image()
+	}
+	return b.image
+}
+
+func (b *LockedBundle) Delete() (err error) {
+	err = DeleteDirSafely(b.Dir())
+	err = exterrors.Append(err, b.Close())
+	return
+}
+
+// Updates the rootfs if the image changed
 func (b *LockedBundle) UpdateRootfs(image BundleImage) (err error) {
 	var (
-		rootfs = filepath.Join(b.Dir(), "rootfs")
-		imgId  *digest.Digest
+		rootfs    = filepath.Join(b.Dir(), "rootfs")
+		imgId     *digest.Digest
+		lastImgId = b.Image()
 	)
+	if _, e := os.Stat(rootfs); e == nil && (lastImgId == nil && image == nil || lastImgId != nil && *lastImgId == image.ID()) {
+		return // don't update since the bundle is already based on the provided image
+	}
 	if image != nil {
 		id := image.ID()
 		imgId = &id
@@ -253,24 +259,32 @@ func (b *LockedBundle) UpdateRootfs(image BundleImage) (err error) {
 	return b.SetParentImageId(imgId)
 }
 
-func (b *LockedBundle) SetSpec(specgen SpecGenerator) (err error) {
-	spec, err := specgen.Spec(filepath.Join(b.Dir(), "rootfs"))
+func (b *LockedBundle) SetParentImageId(imageID *digest.Digest) (err error) {
+	if imageID == nil {
+		if e := os.Remove(b.bundle.imageFile()); e != nil && !os.IsNotExist(e) {
+			err = errors.New(e.Error())
+		}
+	} else {
+		_, err = atomic.WriteFile(b.bundle.imageFile(), bytes.NewBufferString((*imageID).String()))
+	}
+	if err == nil {
+		b.image = imageID
+	} else {
+		err = errors.Wrapf(err, "set bundle's (%s) parent image id", b.ID())
+	}
+	return
+}
+
+func (b *LockedBundle) SetSpec(spec *rspecs.Spec) (err error) {
 	if err == nil {
 		err = createVolumeDirectories(spec, b.Dir())
 	}
 	if err != nil {
 		return errors.Wrap(err, "set bundle spec")
 	}
-	if spec.Root != nil {
-		spec.Root.Path = "rootfs"
-	}
-	if spec.Annotations == nil {
-		spec.Annotations = map[string]string{}
-	}
-	spec.Annotations[ANNOTATION_BUNDLE_ID] = b.ID()
 	confFile := filepath.Join(b.Dir(), "config.json")
 	if _, err = atomic.WriteJson(confFile, spec); err != nil {
-		err = errors.Wrapf(err, "write bundle %q spec", b.ID())
+		return errors.Wrapf(err, "write bundle %q spec", b.ID())
 	}
 	b.spec = spec
 	return
@@ -306,36 +320,6 @@ func createVolumeDirectories(spec *rspecs.Spec, dir string) (err error) {
 	return
 }
 
-// Reads image ID from cached spec
-func (b *LockedBundle) Image() *digest.Digest {
-	if b.image == nil {
-		b.image = b.bundle.Image()
-	}
-	return b.image
-}
-
-func (b *LockedBundle) SetParentImageId(imageID *digest.Digest) (err error) {
-	if imageID == nil {
-		if e := os.Remove(b.bundle.imageFile()); e != nil && !os.IsNotExist(e) {
-			err = errors.New(e.Error())
-		}
-	} else {
-		_, err = atomic.WriteFile(b.bundle.imageFile(), bytes.NewBufferString((*imageID).String()))
-	}
-	if err == nil {
-		b.image = imageID
-	} else {
-		err = errors.Wrap(err, "set bundle's parent image id")
-	}
-	return
-}
-
-func (b *LockedBundle) Delete() (err error) {
-	err = DeleteDirSafely(b.Dir())
-	err = exterrors.Append(err, b.Close())
-	return
-}
-
 func lockBundle(bundle *Bundle) (l *lock.Lockfile, err error) {
 	// TODO: use tmpfs for lock file
 	if l, err = lock.LockFile(filepath.Clean(bundle.dir) + ".lock"); err == nil {

bundle/bundlebuilder.go

@@ -1,12 +1,9 @@
 package bundle
 
 import (
-	"encoding/base32"
-	"strings"
+	"path/filepath"
 
 	"github.com/mgoltzsche/ctnr/pkg/generate"
-	"github.com/pkg/errors"
-	"github.com/satori/go.uuid"
 )
 
 type BundleBuilder struct {
@@ -16,32 +13,9 @@ type BundleBuilder struct {
 }
 
 func Builder(id string) *BundleBuilder {
-	spec := generate.NewSpecBuilder()
-	spec.AddAnnotation(ANNOTATION_BUNDLE_ID, id)
-	spec.SetRootPath("rootfs")
-	return FromSpec(&spec)
-}
-
-func BuilderFromImage(id string, image BundleImage) (b *BundleBuilder, err error) {
-	spec := generate.NewSpecBuilder()
-	spec.SetRootPath("rootfs")
-	conf := image.Config()
-	spec.ApplyImage(conf)
-	spec.AddAnnotation(ANNOTATION_BUNDLE_ID, id)
-	b = FromSpec(&spec)
-	b.image = image
-	return b, errors.Wrap(err, "bundle build from image")
-}
-
-func FromSpec(spec *generate.SpecBuilder) *BundleBuilder {
-	id := ""
-	if s := spec.Generator.Spec(); s != nil && s.Annotations != nil {
-		id = s.Annotations[ANNOTATION_BUNDLE_ID]
-	}
-	if id == "" {
-		id = generateId()
-	}
-	b := &BundleBuilder{"", spec, nil}
+	specgen := generate.NewSpecBuilder()
+	specgen.SetRootPath("rootfs")
+	b := &BundleBuilder{"", &specgen, nil}
 	b.SetID(id)
 	return b
 }
@@ -59,12 +33,24 @@ func (b *BundleBuilder) GetID() string {
 	return b.id
 }
 
-func (b *BundleBuilder) Build(dir string, update bool) (*LockedBundle, error) {
-	// Create bundle directory
-	bundle, err := CreateLockedBundle(dir, b, b.image, update)
-	return bundle, errors.Wrap(err, "build bundle")
+func (b *BundleBuilder) SetImage(image BundleImage) {
+	b.ApplyImage(image.Config())
+	b.image = image
 }
 
-func generateId() string {
-	return strings.ToLower(strings.TrimRight(base32.StdEncoding.EncodeToString(uuid.NewV4().Bytes()), "="))
+func (b *BundleBuilder) Build(bundle *LockedBundle) (err error) {
+	// Prepare rootfs
+	if err = bundle.UpdateRootfs(b.image); err != nil {
+		return
+	}
+
+	// Resolve user/group names
+	rootfs := filepath.Join(bundle.Dir(), b.Generator.Spec().Root.Path)
+	spec, err := b.Spec(rootfs)
+	if err != nil {
+		return
+	}
+
+	// Apply spec
+	return bundle.SetSpec(spec)
 }

bundle/store.go

@@ -5,7 +5,7 @@ import (
 )
 
 type BundleStore interface {
-	CreateBundle(builder *BundleBuilder, update bool) (*LockedBundle, error)
+	CreateBundle(id string, update bool) (*LockedBundle, error)
 	Bundle(id string) (Bundle, error)
 	Bundles() ([]Bundle, error)
 	BundleGC(ttl time.Duration) ([]Bundle, error)

bundle/store/bundlestore.go

@@ -47,8 +47,18 @@ func (s *BundleStore) Bundle(id string) (r bundle.Bundle, err error) {
 	return bundle.NewBundle(filepath.Join(s.dir, id))
 }
 
-func (s *BundleStore) CreateBundle(builder *bundle.BundleBuilder, update bool) (b *bundle.LockedBundle, err error) {
-	return builder.Build(filepath.Join(s.dir, builder.GetID()), update)
+func (s *BundleStore) CreateBundle(id string, update bool) (b *bundle.LockedBundle, err error) {
+	dir := filepath.Join(s.dir, id)
+	if id == "" {
+		if err = os.MkdirAll(s.dir, 0770); err != nil {
+			return nil, errors.Wrap(err, "create bundle")
+		}
+		if dir, err = ioutil.TempDir(s.dir, ""); err != nil {
+			return nil, errors.Wrap(err, "create bundle")
+		}
+		update = true
+	}
+	return bundle.CreateLockedBundle(dir, update)
 }
 
 // Deletes all bundles that have not been used longer than the given TTL.

cmd/common.go

@@ -255,32 +255,34 @@ func createRuntimeBundle(service *model.Service, res model.ResourceResolver) (b
 		bundleId = ""
 	}
 
-	// Load image and bundle builder
-	var builder *bundle.BundleBuilder
-	if service.Image == "" {
-		builder = bundle.Builder(bundleId)
+	// Create bundle
+	if bundleDir != "" {
+		b, err = bundle.CreateLockedBundle(bundleDir, service.BundleUpdate)
 	} else {
+		b, err = store.CreateBundle(bundleId, service.BundleUpdate)
+	}
+	defer func() {
+		if err != nil {
+			b.Delete()
+		}
+	}()
+
+	// Apply image
+	builder := bundle.Builder(b.ID())
+	if service.Image != "" {
 		var img image.Image
 		if img, err = image.GetImage(istore, service.Image); err != nil {
-			return
-		}
-		if builder, err = bundle.BuilderFromImage(bundleId, image.NewUnpackableImage(&img, istore)); err != nil {
-			return
+			return nil, err
 		}
+		builder.SetImage(image.NewUnpackableImage(&img, istore))
 	}
 
-	// Generate config.json
-	if err = oci.ToSpec(service, res, flagRootless, flagPRootPath, builder.SpecBuilder); err != nil {
-		return
+	// Apply config.json
+	if err = oci.ToSpec(service, res, flagRootless, flagPRootPath, builder); err != nil {
+		return nil, err
 	}
 
-	// Create bundle
-	if bundleDir != "" {
-		b, err = builder.Build(bundleDir, service.BundleUpdate)
-	} else {
-		b, err = store.CreateBundle(builder, service.BundleUpdate)
-	}
-	return
+	return b, builder.Build(b)
 }
 
 func isFile(file string) bool {