overlord/state: marshaling tests for lanes (#2245)

overlord/state: marshaling tests for lanes

overlord/overlord_test.go

@@ -86,7 +86,7 @@ func (ovs *overlordSuite) TestNew(c *C) {
 }
 
 func (ovs *overlordSuite) TestNewWithGoodState(c *C) {
-	fakeState := []byte(fmt.Sprintf(`{"data":{"patch-level":%d,"some":"data"},"changes":null,"tasks":null,"last-change-id":0,"last-task-id":0}`, patch.Level))
+	fakeState := []byte(fmt.Sprintf(`{"data":{"patch-level":%d,"some":"data"},"changes":null,"tasks":null,"last-change-id":0,"last-task-id":0,"last-lane-id":0}`, patch.Level))
 	err := ioutil.WriteFile(dirs.SnapStateFile, fakeState, 0600)
 	c.Assert(err, IsNil)
 

overlord/snapstate/snapstate.go

@@ -433,6 +433,8 @@ func UpdateMany(st *state.State, names []string, userID int) ([]string, []*state
 			}
 			return nil, nil, err
 		}
+		ts.JoinLane(st.NewLane())
+
 		updated = append(updated, update.Name())
 		tasksets = append(tasksets, ts)
 	}

overlord/state/change.go

@@ -471,20 +471,6 @@ func (c *Change) Tasks() []*Task {
 	return c.state.tasksIn(c.taskIDs)
 }
 
-// AddLane adds a new lane ID in the change.
-func (c *Change) AddLane() int {
-	c.state.writing()
-	c.lanes++
-	return c.lanes
-}
-
-// LaneCount returns the number of lane IDs registered with the change.
-// Registered lanes may not be used on tasks currently in the change.
-func (c *Change) LaneCount() int {
-	c.state.reading()
-	return c.lanes
-}
-
 // Abort flags the change for cancellation, whether in progress or not.
 // Cancellation will proceed at the next ensure pass.
 func (c *Change) Abort() {

overlord/state/change_test.go

@@ -323,7 +323,6 @@ func (cs *changeSuite) TestMethodEntrance(c *C) {
 		func() { chg.AddTask(nil) },
 		func() { chg.AddAll(nil) },
 		func() { chg.UnmarshalJSON(nil) },
-		func() { chg.AddLane() },
 	}
 
 	reads := []func(){
@@ -335,7 +334,6 @@ func (cs *changeSuite) TestMethodEntrance(c *C) {
 		func() { chg.MarshalJSON() },
 		func() { chg.SpawnTime() },
 		func() { chg.ReadyTime() },
-		func() { chg.LaneCount() },
 	}
 
 	for i, f := range reads {
@@ -591,17 +589,3 @@ func (ts *taskRunnerSuite) TestAbortLanes(c *C) {
 		c.Assert(strings.Join(obtained, " "), Equals, strings.Join(expected, " "), Commentf("setup: %s", test.setup))
 	}
 }
-
-func (cs *changeSuite) TestAddLane(c *C) {
-	st := state.New(nil)
-	st.Lock()
-	defer st.Unlock()
-
-	chg := st.NewChange("install", "...")
-
-	c.Assert(chg.LaneCount(), Equals, 0)
-	c.Assert(chg.AddLane(), Equals, 1)
-	c.Assert(chg.LaneCount(), Equals, 1)
-	c.Assert(chg.AddLane(), Equals, 2)
-	c.Assert(chg.LaneCount(), Equals, 2)
-}

overlord/state/state.go

@@ -87,6 +87,7 @@ type State struct {
 
 	lastTaskId   int
 	lastChangeId int
+	lastLaneId   int
 
 	backend Backend
 	data    customData
@@ -146,6 +147,7 @@ type marshalledState struct {
 
 	LastChangeId int `json:"last-change-id"`
 	LastTaskId   int `json:"last-task-id"`
+	LastLaneId   int `json:"last-lane-id"`
 }
 
 // MarshalJSON makes State a json.Marshaller
@@ -158,6 +160,7 @@ func (s *State) MarshalJSON() ([]byte, error) {
 
 		LastTaskId:   s.lastTaskId,
 		LastChangeId: s.lastChangeId,
+		LastLaneId:   s.lastLaneId,
 	})
 }
 
@@ -174,6 +177,7 @@ func (s *State) UnmarshalJSON(data []byte) error {
 	s.tasks = unmarshalled.Tasks
 	s.lastChangeId = unmarshalled.LastChangeId
 	s.lastTaskId = unmarshalled.LastTaskId
+	s.lastLaneId = unmarshalled.LastLaneId
 	// backlink state again
 	for _, t := range s.tasks {
 		t.state = s
@@ -283,6 +287,13 @@ func (s *State) NewChange(kind, summary string) *Change {
 	return chg
 }
 
+// NewLane creates a new lane in the state.
+func (s *State) NewLane() int {
+	s.writing()
+	s.lastLaneId++
+	return s.lastLaneId
+}
+
 // Changes returns all changes currently known to the state.
 func (s *State) Changes() []*Change {
 	s.reading()

overlord/state/state_test.go

@@ -392,6 +392,8 @@ func (ss *stateSuite) TestNewTaskAndCheckpoint(c *C) {
 	t1.Set("a", 1)
 	t1.SetStatus(state.DoneStatus)
 	t1.SetProgress("snap", 5, 10)
+	t1.JoinLane(42)
+	t1.JoinLane(43)
 
 	t2 := st.NewTask("inst", "2...")
 	chg.AddTask(t2)
@@ -440,6 +442,8 @@ func (ss *stateSuite) TestNewTaskAndCheckpoint(c *C) {
 	c.Check(cur, Equals, 5)
 	c.Check(tot, Equals, 10)
 
+	c.Assert(task0_1.Lanes(), DeepEquals, []int{42, 43})
+
 	task0_2 := tasks0[t2ID]
 	c.Check(task0_2.WaitTasks(), DeepEquals, []*state.Task{task0_1})
 
@@ -534,6 +538,8 @@ func (ss *stateSuite) TestCheckpointPreserveLastIds(c *C) {
 	st.NewTask("download", "...")
 	st.NewTask("download", "...")
 
+	c.Assert(st.NewLane(), Equals, 1)
+
 	// implicit checkpoint
 	st.Unlock()
 
@@ -549,6 +555,9 @@ func (ss *stateSuite) TestCheckpointPreserveLastIds(c *C) {
 
 	c.Assert(st2.NewTask("download", "...").ID(), Equals, "3")
 	c.Assert(st2.NewChange("install", "...").ID(), Equals, "2")
+
+	c.Assert(st2.NewLane(), Equals, 2)
+
 }
 
 func (ss *stateSuite) TestCheckpointPreserveCleanStatus(c *C) {
@@ -647,6 +656,7 @@ func (ss *stateSuite) TestMethodEntrance(c *C) {
 		func() { st.NewChange("install", "...") },
 		func() { st.NewTask("download", "...") },
 		func() { st.UnmarshalJSON(nil) },
+		func() { st.NewLane() },
 	}
 
 	reads := []func(){

overlord/state/task.go

@@ -463,6 +463,13 @@ func (ts *TaskSet) AddAll(anotherTs *TaskSet) {
 	}
 }
 
+// JoinLane adds all the tasks in the current taskset to the given lane
+func (ts *TaskSet) JoinLane(lane int) {
+	for _, t := range ts.tasks {
+		t.JoinLane(lane)
+	}
+}
+
 // Tasks returns the tasks in the task set.
 func (ts TaskSet) Tasks() []*Task {
 	// Return something mutable, just like every other Tasks method.

tests/main/refresh-all-undo/task.yaml

@@ -0,0 +1,47 @@
+summary: Check that undo for snap refresh works
+
+systems: [-ubuntu-core-16-64, -ubuntu-core-16-arm-64, -ubuntu-core-16-arm-32]
+
+environment:
+    BLOB_DIR: $(pwd)/fake-store-blobdir
+    GOOD_SNAP: xkcd-webserver
+    BAD_SNAP: test-snapd-tools
+
+prepare: |
+    . $TESTSLIB/store.sh
+
+    echo "Given two snaps are installed"
+    for snap in $GOOD_SNAP $BAD_SNAP; do
+        snap install $snap
+    done
+
+    echo "And the daemon is configured to point to the fake store"
+    setup_store fake $BLOB_DIR
+
+restore: |
+    . $TESTSLIB/store.sh
+    teardown_store fake $BLOB_DIR
+
+execute: |
+    echo "When the store is configured to make them refreshable"
+    fakestore -make-refreshable $GOOD_SNAP,$BAD_SNAP -dir $BLOB_DIR
+
+    echo "When a snap is broken"
+    echo "i-am-broken-now" >> $BLOB_DIR/${BAD_SNAP}*fake1*.snap
+
+    echo "And a refresh is performed"
+    if snap refresh ; then
+        echo "snap refresh should fail but it did not, test is broken"
+        exit 1
+    fi
+
+    echo "Then the new version of the good snap got installed"
+    snap list | grep -Pq "${GOOD_SNAP}.*?fake1"
+
+    echo "But the bad snap did not get updated"
+    snap list | grep -P "${BAD_SNAP}"|grep -v "fake"
+
+    echo "Verify the snap change"
+    snap change 4 |grep "Undone.*Download snap \"${BAD_SNAP}\""
+    snap change 4 |grep "Done.*Download snap \"${GOOD_SNAP}\""
+    snap change 4 |grep "ERROR cannot verify snap \"test-snapd-tools\", no matching signatures found"