correct waiting for changes that are apply=noop, wait=reconcile

pkg/kapp/clusterapply/add_or_update_change.go

@@ -5,9 +5,7 @@ import (
 	"time"
 
 	ctldiff "github.com/k14s/kapp/pkg/kapp/diff"
-	"github.com/k14s/kapp/pkg/kapp/logger"
 	ctlres "github.com/k14s/kapp/pkg/kapp/resources"
-	ctlresm "github.com/k14s/kapp/pkg/kapp/resourcesmisc"
 	"github.com/k14s/kapp/pkg/kapp/util"
 	"k8s.io/apimachinery/pkg/api/errors"
 )
@@ -32,7 +30,6 @@ type AddOrUpdateChange struct {
 	identifiedResources ctlres.IdentifiedResources
 	changeFactory       ctldiff.ChangeFactory
 	changeSetFactory    ctldiff.ChangeSetFactory
-	convergedResFactory ConvergedResourceFactory
 	opts                AddOrUpdateChangeOpts
 }
 
@@ -243,22 +240,6 @@ func (a AddOrUpdateChange) tryToUpdateAfterCreateConflict() error {
 		"due to resource conflict (tried multiple times): %s", lastUpdateErr)
 }
 
-type SpecificResource interface {
-	IsDoneApplying() ctlresm.DoneApplyState
-}
-
-func (c AddOrUpdateChange) IsDoneApplying() (ctlresm.DoneApplyState, []string, error) {
-	labeledResources := ctlres.NewLabeledResources(nil, c.identifiedResources, logger.NewTODOLogger())
-
-	// Refresh resource with latest changes from the server
-	parentRes, err := c.identifiedResources.Get(c.change.NewResource())
-	if err != nil {
-		return ctlresm.DoneApplyState{}, nil, err
-	}
-
-	return c.convergedResFactory.New(parentRes, labeledResources.GetAssociated).IsDoneApplying()
-}
-
 func (c AddOrUpdateChange) recordAppliedResource(savedRes ctlres.Resource) error {
 	savedResWithHistory := c.changeFactory.NewResourceWithHistory(savedRes)
 

pkg/kapp/clusterapply/cluster_change.go

@@ -142,8 +142,7 @@ func (c *ClusterChange) Apply() error {
 	case ClusterChangeApplyOpAdd, ClusterChangeApplyOpUpdate:
 		return c.applyErr(AddOrUpdateChange{
 			c.change, c.identifiedResources, c.changeFactory,
-			c.changeSetFactory, c.convergedResFactory,
-			c.opts.AddOrUpdateChangeOpts}.Apply())
+			c.changeSetFactory, c.opts.AddOrUpdateChangeOpts}.Apply())
 
 	case ClusterChangeApplyOpDelete:
 		return c.applyErr(DeleteChange{c.change, c.identifiedResources}.Apply())
@@ -167,10 +166,7 @@ func (c *ClusterChange) isDoneApplying() (ctlresm.DoneApplyState, []string, erro
 
 	switch op {
 	case ClusterChangeWaitOpOK:
-		return AddOrUpdateChange{
-			c.change, c.identifiedResources, c.changeFactory,
-			c.changeSetFactory, c.convergedResFactory,
-			c.opts.AddOrUpdateChangeOpts}.IsDoneApplying()
+		return ReconcilingChange{c.change, c.identifiedResources, c.convergedResFactory}.IsDoneApplying()
 
 	case ClusterChangeWaitOpDelete:
 		return DeleteChange{c.change, c.identifiedResources}.IsDoneApplying()

pkg/kapp/clusterapply/reconciling_change.go

@@ -0,0 +1,32 @@
+package clusterapply
+
+import (
+	ctldiff "github.com/k14s/kapp/pkg/kapp/diff"
+	"github.com/k14s/kapp/pkg/kapp/logger"
+	ctlres "github.com/k14s/kapp/pkg/kapp/resources"
+	ctlresm "github.com/k14s/kapp/pkg/kapp/resourcesmisc"
+)
+
+type ReconcilingChange struct {
+	change              ctldiff.Change
+	identifiedResources ctlres.IdentifiedResources
+	convergedResFactory ConvergedResourceFactory
+}
+
+type SpecificResource interface {
+	IsDoneApplying() ctlresm.DoneApplyState
+}
+
+func (c ReconcilingChange) IsDoneApplying() (ctlresm.DoneApplyState, []string, error) {
+	labeledResources := ctlres.NewLabeledResources(nil, c.identifiedResources, logger.NewTODOLogger())
+
+	// Refresh resource with latest changes from the server
+	// Pick up new or existing resource (and not just new resource),
+	// as some changes may be apply->noop, wait->reconcile.
+	parentRes, err := c.identifiedResources.Get(c.change.NewOrExistingResource())
+	if err != nil {
+		return ctlresm.DoneApplyState{}, nil, err
+	}
+
+	return c.convergedResFactory.New(parentRes, labeledResources.GetAssociated).IsDoneApplying()
+}