o/snapstate: account for remodeling when installing prereqs as well as updating them

overlord/snapstate/handlers.go

@@ -324,19 +324,28 @@ func (m *SnapManager) installOneBaseOrRequired(t *state.Task, snapName string, c
 		return nil, err
 	}
 
-	inProgress := func(snapName string) (bool, error) {
+	shouldAttemptInstall := func(snapName string) (bool, error) {
 		linkTask, err := findLinkSnapTaskForSnap(st, snapName)
 		if err != nil {
 			return false, err
 		}
 
 		if linkTask == nil {
 			// snap is not being installed
+			return true, nil
+		}
+
+		// if we are remodeling, then we should return early due to the way that
+		// tasks are ordered by the remodeling code. specifically, all snap
+		// downloads during a remodel happen prior to snap installation. thus,
+		// we cannot wait for snaps to be installed here. see remodelTasks for
+		// more information on how the tasks are ordered.
+		if deviceCtx.ForRemodeling() {
 			return false, nil
 		}
 
 		// snap is being installed, retry later
-		return true, nil
+		return false, onInFlight
 	}
 
 	if isInstalled {
@@ -346,28 +355,17 @@ func (m *SnapManager) installOneBaseOrRequired(t *state.Task, snapName string, c
 		}
 
 		// other kind of dependency, check if it's in progress
-		if ok, err := inProgress(snapName); err != nil {
+		if _, err := shouldAttemptInstall(snapName); err != nil {
 			return nil, err
-		} else if ok {
-			return nil, onInFlight
 		}
 		return nil, nil
 	}
 
 	// not installed, wait for it if it is. If not, we'll install it
-	if ok, err := inProgress(snapName); err != nil {
+	if ok, err := shouldAttemptInstall(snapName); err != nil {
 		return nil, err
-	} else if ok {
-		// if we are remodeling, then we should return early due to the way that
-		// tasks are ordered by the remodeling code. specifically, all snap
-		// downloads during a remodel happen prior to snap installation. thus,
-		// we cannot wait for snaps to be installed here. see remodelTasks for
-		// more information on how the tasks are ordered.
-		if deviceCtx.ForRemodeling() {
-			return nil, nil
-		}
-
-		return nil, onInFlight
+	} else if !ok {
+		return nil, nil
 	}
 
 	// not installed, nor queued for install -> install it

overlord/snapstate/handlers_prereq_test.go

@@ -403,6 +403,76 @@ func (s *prereqSuite) TestDoPrereqRetryWhenBaseInFlight(c *C) {
 	c.Check(chg.Status(), Equals, state.DoneStatus)
 }
 
+func (s *prereqSuite) TestDoPrereqNoRetryWhenBaseInFlightDuringRemodel(c *C) {
+	restore := snapstate.MockPrerequisitesRetryTimeout(1 * time.Millisecond)
+	defer restore()
+
+	s.runner.AddHandler("link-snap", func(task *state.Task, _ *tomb.Tomb) error {
+		st := task.State()
+		st.Lock()
+		defer st.Unlock()
+
+		snapsup, err := snapstate.TaskSnapSetup(task)
+		c.Assert(err, IsNil)
+		fmt.Println(snapsup.InstanceName())
+
+		return nil
+	}, nil)
+
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	restore = snapstatetest.MockDeviceContext(&snapstatetest.TrivialDeviceContext{
+		Remodeling: true,
+	})
+	defer restore()
+
+	// make it look like core is already installed
+	snapstate.Set(s.state, "core", &snapstate.SnapState{
+		Active: true,
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
+			{RealName: "core", Revision: snap.R(1)},
+		}),
+		Current:  snap.R(1),
+		SnapType: "os",
+	})
+
+	// pretend foo gets installed and needs core (which we will make it look
+	// like the install is in progress)
+	prereqTask := s.state.NewTask("prerequisites", "foo")
+	prereqTask.Set("snap-setup", &snapstate.SnapSetup{
+		SideInfo: &snap.SideInfo{
+			RealName: "foo",
+		},
+	})
+
+	tCore := s.state.NewTask("link-snap", "Pretend core gets installed")
+	tCore.Set("snap-setup", &snapstate.SnapSetup{
+		SideInfo: &snap.SideInfo{
+			RealName: "core",
+			Revision: snap.R(11),
+		},
+	})
+
+	// this makes sure that the prereq task runs first, but the link-snap task
+	// for core will be found by the prereq task handler
+	tCore.WaitFor(prereqTask)
+
+	chg := s.state.NewChange("sample", "...")
+	chg.AddTask(prereqTask)
+	chg.AddTask(tCore)
+
+	s.state.Unlock()
+	s.se.Ensure()
+	s.se.Wait()
+	s.state.Lock()
+
+	// prereq task is done, but the core task isn't done. this means that we
+	// didn't wait for the core task to finish, since we are remodeling
+	c.Check(prereqTask.Status(), Equals, state.DoneStatus)
+	c.Check(tCore.Status(), Equals, state.DoStatus)
+}
+
 func (s *prereqSuite) TestDoPrereqChannelEnvvars(c *C) {
 	os.Setenv("SNAPD_BASES_CHANNEL", "edge")
 	defer os.Unsetenv("SNAPD_BASES_CHANNEL")