Skip to content

Commit

Permalink
propagate the refreshtype to the diff config
Browse files Browse the repository at this point in the history
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
  • Loading branch information
leoluz committed Nov 21, 2023
1 parent 8cd28bc commit 8acc53e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
21 changes: 15 additions & 6 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,21 +580,30 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
manifestRevisions = append(manifestRevisions, manifestInfo.Revision)
}

// restore comparison using cached diff result if previous comparison was performed for the same revision
// restore comparison using cached diff result if previous comparison
// was performed for the same revision
// TODO: fix this logic as revisionChanged is always set to true
revisionChanged := len(manifestInfos) != len(sources) || !reflect.DeepEqual(app.Status.Sync.Revisions, manifestRevisions)

// TODO: fix this logic as specChanged is always set to true
specChanged := !reflect.DeepEqual(app.Status.Sync.ComparedTo, v1alpha1.ComparedTo{Source: app.Spec.GetSource(), Destination: app.Spec.Destination, Sources: sources, IgnoreDifferences: app.Spec.IgnoreDifferences})

_, refreshRequested := app.IsRefreshRequested()
noCache = noCache || refreshRequested || app.Status.Expired(m.statusRefreshTimeout) || specChanged || revisionChanged
refreshType, refreshRequested := app.IsRefreshRequested()
statusExpired := app.Status.Expired(m.statusRefreshTimeout)
noCache = noCache ||
refreshRequested ||
statusExpired ||
specChanged ||
revisionChanged

diffConfigBuilder := argodiff.NewDiffConfigBuilder().
WithDiffSettings(app.Spec.IgnoreDifferences, resourceOverrides, compareOptions.IgnoreAggregatedRoles).
WithTracking(appLabelKey, string(trackingMethod))
WithTracking(appLabelKey, string(trackingMethod)).
WithCache(m.cache, app.InstanceName(m.namespace)).
WithRefreshType(refreshType)

if noCache {
diffConfigBuilder.WithNoCache()
} else {
diffConfigBuilder.WithCache(m.cache, app.GetName())
}

gvkParser, err := m.getGVKParser(app.Spec.Destination.Server)
Expand Down
17 changes: 13 additions & 4 deletions util/argo/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"

"github.com/go-logr/logr"
log "github.com/sirupsen/logrus"

k8smanagedfields "k8s.io/apimachinery/pkg/util/managedfields"

Expand Down Expand Up @@ -63,7 +64,6 @@ func (b *DiffConfigBuilder) WithNoCache() *DiffConfigBuilder {
// WithCache sets the appstatecache.Cache and the appName in the diff config. Those the
// are two objects necessary to retrieve a cached diff.
func (b *DiffConfigBuilder) WithCache(s *appstatecache.Cache, appName string) *DiffConfigBuilder {
b.diffConfig.noCache = false
b.diffConfig.stateCache = s
b.diffConfig.appName = appName
return b
Expand Down Expand Up @@ -105,6 +105,11 @@ func (b *DiffConfigBuilder) WithServerSideDiff(ssd bool) *DiffConfigBuilder {
return b
}

func (b *DiffConfigBuilder) WithRefreshType(rt v1alpha1.RefreshType) *DiffConfigBuilder {
b.diffConfig.refreshType = rt
return b
}

// Build will first validate the current state of the diff config and return the
// DiffConfig implementation if no errors are found. Will return nil and the error
// details otherwise.
Expand Down Expand Up @@ -171,6 +176,7 @@ type diffConfig struct {
manager string
serverSideDiff bool
resourceOperations kube.ResourceOperations
refreshType v1alpha1.RefreshType
}

func (c *diffConfig) Ignores() []v1alpha1.ResourceIgnoreDifferences {
Expand Down Expand Up @@ -363,9 +369,12 @@ func (c *diffConfig) DiffFromCache(appName string) (bool, []*v1alpha1.ResourceDi
return false, nil
}
cachedDiff := make([]*v1alpha1.ResourceDiff, 0)
// TODO (SSD): Fix this code as it is swallowing errors if
// GetAppManagedResources fails to execute.
if c.stateCache != nil && c.stateCache.GetAppManagedResources(appName, &cachedDiff) == nil {
if c.stateCache != nil {
err := c.stateCache.GetAppManagedResources(appName, &cachedDiff)
if err != nil {
log.Errorf("DiffFromCache error: error getting managed resources: %s", err)
return false, nil
}
return true, cachedDiff
}
return false, nil
Expand Down

0 comments on commit 8acc53e

Please sign in to comment.