Merge pull request #37 from fgimenez/timestamp-as-comparable-version

Add timestamp as version for non-15.04 images

pkg/cloud/cloud.go

@@ -28,6 +28,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	log "github.com/Sirupsen/logrus"
 
@@ -149,7 +150,18 @@ func extractVersion(imageID string) (ver int, err error) {
 func GetImageID(options *flags.Options, version int) (name string) {
 	options.Release = removeDot(options.Release)
 	imageNamePrefix := imgTemplate(options)
-	return fmt.Sprintf("%s-%d-%s", imageNamePrefix, version, imageNameSufix)
+
+	finalVersion := strconv.Itoa(version)
+	// The numeric version makes sense for system-image based images, on all-snaps
+	// the version of the image, if any, should be determined by the versions of
+	// the snaps that form it. For the time being we assume by convention that
+	// version == 0 means all-snaps, and we replace it by a timestamp so that we are
+	// able to sort images by date
+	if version == 0 {
+		finalVersion = time.Now().Format("20060102150405.000000")
+	}
+
+	return fmt.Sprintf("%s-%s-%s", imageNamePrefix, finalVersion, imageNameSufix)
 }
 
 // Delete calls the cli command to remove the given images

pkg/cloud/cloud_test.go

@@ -214,6 +214,27 @@ func (s *cloudSuite) TestGetImageID(c *check.C) {
 	}
 }
 
+func (s *cloudSuite) TestGetImageIDAssignsVersionOnZeroVersionGiven(c *check.C) {
+	output := GetImageID(s.defaultOptions, 0)
+
+	ver := getVersionFromImageID(output)
+
+	c.Assert(ver != "0", check.Equals, true)
+}
+
+func (s *cloudSuite) TestGetImageIDAssignsIncreasingVersionNumbersOnZeroVersionGiven(c *check.C) {
+	var currVer, prevVer string
+	for i := 0; i < 100; i++ {
+		output := GetImageID(s.defaultOptions, 0)
+
+		currVer = getVersionFromImageID(output)
+
+		c.Check(currVer > prevVer, check.Equals, true)
+		prevVer = currVer
+		fmt.Println(currVer)
+	}
+}
+
 func (s *cloudSuite) TestDeleteCallsCli(c *check.C) {
 	testCases := []struct {
 		images       []string
@@ -404,3 +425,8 @@ func getImageID(options *flags.Options, ver int) string {
 	return fmt.Sprintf("ubuntu-core/%s/ubuntu-%s-snappy-core-%s-%s-%d-disk1.img",
 		options.ImageType, options.Release, options.Arch, options.Channel, ver)
 }
+
+func getVersionFromImageID(imageID string) string {
+	parts := strings.Split(imageID, "-")
+	return parts[7]
+}

pkg/image/image.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2015 Canonical Ltd
+ * Copyright (C) 2015, 2016 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -86,11 +86,15 @@ func (u *UDFQcow2) Create(options *flags.Options, ver int) (path string, err err
 	if options.Arch == "arm" {
 		archFlag = "--oem beagleblack"
 	}
-	cmds := []string{"sudo", "ubuntu-device-flash",
-		"--revision=" + strconv.Itoa(ver),
+	cmds := []string{"sudo", "ubuntu-device-flash"}
+
+	if options.Release == "15.04" {
+		cmds = append(cmds, "--revision="+strconv.Itoa(ver))
+	}
+	cmds = append(cmds, []string{
 		"core", options.Release,
 		"--channel", options.Channel,
-	}
+	}...)
 	if options.Release != "15.04" {
 		cmds = append(cmds, []string{
 			"--os", options.OS,

pkg/image/image_test.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2015 Canonical Ltd
+ * Copyright (C) 2015, 2016 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -100,9 +100,9 @@ func (s *imageSuite) TestCreateCallsUDF(c *check.C) {
 		version                                    int
 		expectedCall                               string
 	}{
-		{"16.04", "edge", "amd64", "os1", "kernel1", "gadget1", 100, "sudo ubuntu-device-flash --revision=100 core 16.04 --channel edge --os os1 --kernel kernel1 --gadget gadget1 --developer-mode  -o " + filename},
-		{"rolling", "stable", "amd64", "os2", "kernel2", "gadget2", 100, "sudo ubuntu-device-flash --revision=100 core rolling --channel stable --os os2 --kernel kernel2 --gadget gadget2 --developer-mode  -o " + filename},
-		{"17.10", "alpha", "arm", "os3", "kernel3", "gadget3", 56, "sudo ubuntu-device-flash --revision=56 core 17.10 --channel alpha --os os3 --kernel kernel3 --gadget gadget3 --developer-mode --oem beagleblack -o " + filename},
+		{"16.04", "edge", "amd64", "os1", "kernel1", "gadget1", 100, "sudo ubuntu-device-flash core 16.04 --channel edge --os os1 --kernel kernel1 --gadget gadget1 --developer-mode  -o " + filename},
+		{"rolling", "stable", "amd64", "os2", "kernel2", "gadget2", 100, "sudo ubuntu-device-flash core rolling --channel stable --os os2 --kernel kernel2 --gadget gadget2 --developer-mode  -o " + filename},
+		{"17.10", "alpha", "arm", "os3", "kernel3", "gadget3", 56, "sudo ubuntu-device-flash core 17.10 --channel alpha --os os3 --kernel kernel3 --gadget gadget3 --developer-mode --oem beagleblack -o " + filename},
 	}
 
 	for _, item := range testCases {
@@ -125,6 +125,20 @@ func (s *imageSuite) TestCreateCallsUDF(c *check.C) {
 	}
 }
 
+func (s *imageSuite) TestCreateCallsUDFWithoutRevisionForNon1504(c *check.C) {
+	s.cli.output = tmpDirName
+	filename := tmpRawFileName()
+
+	expectedCall := fmt.Sprintf("sudo ubuntu-device-flash core %s --channel %s --os %s --kernel %s --gadget %s --developer-mode  -o "+filename,
+		s.defaultOptions.Release, s.defaultOptions.Channel, s.defaultOptions.OS, s.defaultOptions.Kernel, s.defaultOptions.Gadget)
+
+	_, err := s.subject.Create(s.defaultOptions, testDefaultVer)
+
+	c.Check(err, check.IsNil)
+	c.Assert(len(s.cli.execCommandCalls) > 0, check.Equals, true)
+	c.Check(s.cli.execCommandCalls[expectedCall], check.Equals, 1)
+}
+
 func (s *imageSuite) TestCreateCallsUDFWithoutAllSnapsParamsFor1504(c *check.C) {
 	s.cli.output = tmpDirName
 	filename := tmpRawFileName()
@@ -160,8 +174,8 @@ func (s *imageSuite) TestCreateDoesNotCallUDFOnMktempError(c *check.C) {
 
 	s.subject.Create(s.defaultOptions, testDefaultVer)
 
-	expectedCall := fmt.Sprintf("sudo ubuntu-device-flash --revision=%d core %s --channel %s --os %s --kernel %s --gadget %s --developer-mode  -o %s",
-		100, testDefaultRelease, testDefaultChannel, testDefaultOS, testDefaultKernel, testDefaultGadget, filename)
+	expectedCall := fmt.Sprintf("sudo ubuntu-device-flash core %s --channel %s --os %s --kernel %s --gadget %s --developer-mode  -o %s",
+		testDefaultRelease, testDefaultChannel, testDefaultOS, testDefaultKernel, testDefaultGadget, filename)
 
 	c.Assert(s.cli.execCommandCalls[expectedCall], check.Equals, 0)
 }

pkg/runner/runner.go

@@ -81,14 +81,16 @@ func (r *Runner) Exec(options *flags.Options) (err error) {
 func (r *Runner) create(options *flags.Options) (err error) {
 	log.Infof("Checking current versions for release %s, channel %s and arch %s", options.Release, options.Channel, options.Arch)
 	var siVersion, cloudVersion int
-	siVersion, cloudVersion, err = r.getVersions(options)
-	if err != nil {
-		return
-	}
-	if siVersion <= cloudVersion {
-		return &ErrVersion{siVersion, cloudVersion}
-	}
 
+	if options.Release == "15.04" {
+		siVersion, cloudVersion, err = r.getVersions(options)
+		if err != nil {
+			return
+		}
+		if siVersion <= cloudVersion {
+			return &ErrVersion{siVersion, cloudVersion}
+		}
+	}
 	var path string
 	path, err = r.imgDriver.Create(options, siVersion)
 	defer os.Remove(path)

pkg/runner/runner_test.go

@@ -215,7 +215,8 @@ func (s *runnerPurgeSuite) SetUpTest(c *check.C) {
 	s.options.Action = "purge"
 }
 
-func (s *runnerCreateSuite) TestExecCreateGetsSIVersion(c *check.C) {
+func (s *runnerCreateSuite) TestExecCreateGetsSIVersionFor1504(c *check.C) {
+	s.options.Release = "15.04"
 	err := s.subject.Exec(s.options)
 
 	c.Assert(err, check.IsNil)
@@ -224,6 +225,16 @@ func (s *runnerCreateSuite) TestExecCreateGetsSIVersion(c *check.C) {
 	c.Assert(s.siClient.getVersionCalls[key], check.Equals, 1)
 }
 
+func (s *runnerCreateSuite) TestExecCreateDoesNotGetSIVersionForNon1504(c *check.C) {
+	s.options.Release = "non-15.04"
+	err := s.subject.Exec(s.options)
+
+	c.Assert(err, check.IsNil)
+
+	key := getFakeKey(s.options)
+	c.Assert(s.siClient.getVersionCalls[key], check.Equals, 0)
+}
+
 func (s *runnerCreateSuite) TestExecDoesNotGetSIVersionOnNonCreateAction(c *check.C) {
 	s.options.Action = "non-create"
 	err := s.subject.Exec(s.options)
@@ -241,8 +252,7 @@ func (s *runnerCreateSuite) TestExecReturnsGetSIVersionError(c *check.C) {
 	c.Assert(err.Error(), check.Equals, siVersionError)
 }
 
-func (s *runnerCreateSuite) TestExecGetsCloudLatestVersion(c *check.C) {
-	s.options.Release = "1504"
+func (s *runnerCreateSuite) TestExecGetsCloudLatestVersionFor1504(c *check.C) {
 	err := s.subject.Exec(s.options)
 
 	c.Assert(err, check.IsNil)
@@ -251,6 +261,16 @@ func (s *runnerCreateSuite) TestExecGetsCloudLatestVersion(c *check.C) {
 	c.Assert(s.cloudClient.getLatestVersionCalls[key], check.Equals, 1)
 }
 
+func (s *runnerCreateSuite) TestExecDoesNotGetCloudLatestVersionForNon1504(c *check.C) {
+	s.options.Release = "non15.04"
+	err := s.subject.Exec(s.options)
+
+	c.Assert(err, check.IsNil)
+
+	key := getFakeKey(s.options)
+	c.Assert(s.cloudClient.getLatestVersionCalls[key], check.Equals, 0)
+}
+
 func (s *runnerCreateSuite) TestExecDoesNotGetCloudVersionOnNonCreateAction(c *check.C) {
 	s.options.Action = "non-create"
 	err := s.subject.Exec(s.options)
@@ -294,7 +314,7 @@ func (s *runnerCreateSuite) TestExecReturnsErrVersionIfCloudVersionNotLessThanSI
 	}
 }
 
-func (s *runnerCreateSuite) TestExecCallsDriverCreate(c *check.C) {
+func (s *runnerCreateSuite) TestExecCallsDriverCreateWithSIVersionFor1504(c *check.C) {
 	err := s.subject.Exec(s.options)
 
 	c.Assert(err, check.IsNil)
@@ -303,6 +323,16 @@ func (s *runnerCreateSuite) TestExecCallsDriverCreate(c *check.C) {
 	c.Assert(s.udfDriver.createCalls[key], check.Equals, 1)
 }
 
+func (s *runnerCreateSuite) TestExecCallsDriverCreateWithZeroForNon1504(c *check.C) {
+	s.options.Release = "non15.04"
+	err := s.subject.Exec(s.options)
+
+	c.Assert(err, check.IsNil)
+
+	key := getCreateKey(s.options, 0)
+	c.Assert(s.udfDriver.createCalls[key], check.Equals, 1)
+}
+
 func (s *runnerCreateSuite) TestExecDoesNotCallDriverCreateOnNonCreateAction(c *check.C) {
 	s.options.Action = "non-create"
 	err := s.subject.Exec(s.options)
@@ -330,6 +360,17 @@ func (s *runnerCreateSuite) TestExecCallsCloudCreate(c *check.C) {
 	c.Assert(s.cloudClient.createCalls[key], check.Equals, 1)
 }
 
+func (s *runnerCreateSuite) TestExecCallsCloudCreateWithZeroVersionForNon1504(c *check.C) {
+	s.options.Release = "non15.04"
+	s.udfDriver.path = "mypath"
+	err := s.subject.Exec(s.options)
+
+	c.Assert(err, check.IsNil)
+
+	key := getFullCreateKey("mypath", s.options, 0)
+	c.Assert(s.cloudClient.createCalls[key], check.Equals, 1)
+}
+
 func (s *runnerCreateSuite) TestExecDoesNotCallCloudCreateOnNonCreateAction(c *check.C) {
 	s.options.Action = "non-create"
 	err := s.subject.Exec(s.options)