Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ issues:
- SA5011
max-issues-per-linter: 0
max-same-issues: 0
exclude-rules:
- path: '(.+)_test\.go'
linters:
- unparam
linters:
enable:
- errcheck
Expand All @@ -17,6 +21,7 @@ linters:
- misspell
- staticcheck
- testifylint
- unparam
- unused
- whitespace
linters-settings:
Expand Down
86 changes: 28 additions & 58 deletions applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,8 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque

if r.EnableProgressiveSyncs {
// trigger appropriate application syncs if RollingSync strategy is enabled
if progressiveSyncsStrategyEnabled(&applicationSetInfo, "RollingSync") {
validApps, err = r.syncValidApplications(logCtx, &applicationSetInfo, appSyncMap, appMap, validApps)
if err != nil {
_ = r.setApplicationSetStatusCondition(ctx,
&applicationSetInfo,
argov1alpha1.ApplicationSetCondition{
Type: argov1alpha1.ApplicationSetConditionErrorOccurred,
Message: err.Error(),
Reason: argov1alpha1.ApplicationSetReasonSyncApplicationError,
Status: argov1alpha1.ApplicationSetConditionStatusTrue,
}, parametersGenerated,
)
return ctrl.Result{}, err
}
if progressiveSyncsRollingSyncStrategyEnabled(&applicationSetInfo) {
validApps = r.syncValidApplications(logCtx, &applicationSetInfo, appSyncMap, appMap, validApps)
}
}

Expand Down Expand Up @@ -859,12 +847,9 @@ func (r *ApplicationSetReconciler) removeOwnerReferencesOnDeleteAppSet(ctx conte
}

func (r *ApplicationSetReconciler) performProgressiveSyncs(ctx context.Context, logCtx *log.Entry, appset argov1alpha1.ApplicationSet, applications []argov1alpha1.Application, desiredApplications []argov1alpha1.Application, appMap map[string]argov1alpha1.Application) (map[string]bool, error) {
appDependencyList, appStepMap, err := r.buildAppDependencyList(logCtx, appset, desiredApplications)
if err != nil {
return nil, fmt.Errorf("failed to build app dependency list: %w", err)
}
appDependencyList, appStepMap := r.buildAppDependencyList(logCtx, appset, desiredApplications)

_, err = r.updateApplicationSetApplicationStatus(ctx, logCtx, &appset, applications, appStepMap)
_, err := r.updateApplicationSetApplicationStatus(ctx, logCtx, &appset, applications, appStepMap)
if err != nil {
return nil, fmt.Errorf("failed to update applicationset app status: %w", err)
}
Expand All @@ -874,34 +859,27 @@ func (r *ApplicationSetReconciler) performProgressiveSyncs(ctx context.Context,
logCtx.Infof("step %v: %+v", i+1, step)
}

appSyncMap, err := r.buildAppSyncMap(ctx, appset, appDependencyList, appMap)
if err != nil {
return nil, fmt.Errorf("failed to build app sync map: %w", err)
}

appSyncMap := r.buildAppSyncMap(appset, appDependencyList, appMap)
logCtx.Infof("Application allowed to sync before maxUpdate?: %+v", appSyncMap)

_, err = r.updateApplicationSetApplicationStatusProgress(ctx, logCtx, &appset, appSyncMap, appStepMap, appMap)
_, err = r.updateApplicationSetApplicationStatusProgress(ctx, logCtx, &appset, appSyncMap, appStepMap)
if err != nil {
return nil, fmt.Errorf("failed to update applicationset application status progress: %w", err)
}

_, err = r.updateApplicationSetApplicationStatusConditions(ctx, &appset)
if err != nil {
return nil, fmt.Errorf("failed to update applicationset application status conditions: %w", err)
}
_ = r.updateApplicationSetApplicationStatusConditions(ctx, &appset)

return appSyncMap, nil
}

// this list tracks which Applications belong to each RollingUpdate step
func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, applicationSet argov1alpha1.ApplicationSet, applications []argov1alpha1.Application) ([][]string, map[string]int, error) {
func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, applicationSet argov1alpha1.ApplicationSet, applications []argov1alpha1.Application) ([][]string, map[string]int) {
if applicationSet.Spec.Strategy == nil || applicationSet.Spec.Strategy.Type == "" || applicationSet.Spec.Strategy.Type == "AllAtOnce" {
return [][]string{}, map[string]int{}, nil
return [][]string{}, map[string]int{}
}

steps := []argov1alpha1.ApplicationSetRolloutStep{}
if progressiveSyncsStrategyEnabled(&applicationSet, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(&applicationSet) {
steps = applicationSet.Spec.Strategy.RollingSync.Steps
}

Expand Down Expand Up @@ -942,7 +920,7 @@ func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, app
}
}

return appDependencyList, appStepMap, nil
return appDependencyList, appStepMap
}

func labelMatchedExpression(logCtx *log.Entry, val string, matchExpression argov1alpha1.ApplicationMatchExpression) bool {
Expand All @@ -966,7 +944,7 @@ func labelMatchedExpression(logCtx *log.Entry, val string, matchExpression argov
}

// this map is used to determine which stage of Applications are ready to be updated in the reconciler loop
func (r *ApplicationSetReconciler) buildAppSyncMap(ctx context.Context, applicationSet argov1alpha1.ApplicationSet, appDependencyList [][]string, appMap map[string]argov1alpha1.Application) (map[string]bool, error) {
func (r *ApplicationSetReconciler) buildAppSyncMap(applicationSet argov1alpha1.ApplicationSet, appDependencyList [][]string, appMap map[string]argov1alpha1.Application) map[string]bool {
appSyncMap := map[string]bool{}
syncEnabled := true

Expand Down Expand Up @@ -1003,28 +981,20 @@ func (r *ApplicationSetReconciler) buildAppSyncMap(ctx context.Context, applicat
}
}

return appSyncMap, nil
return appSyncMap
}

func appSyncEnabledForNextStep(appset *argov1alpha1.ApplicationSet, app argov1alpha1.Application, appStatus argov1alpha1.ApplicationSetApplicationStatus) bool {
if progressiveSyncsStrategyEnabled(appset, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(appset) {
// we still need to complete the current step if the Application is not yet Healthy or there are still pending Application changes
return isApplicationHealthy(app) && appStatus.Status == "Healthy"
}

return true
}

func progressiveSyncsStrategyEnabled(appset *argov1alpha1.ApplicationSet, strategyType string) bool {
if appset.Spec.Strategy == nil || appset.Spec.Strategy.Type != strategyType {
return false
}

if strategyType == "RollingSync" && appset.Spec.Strategy.RollingSync == nil {
return false
}

return true
func progressiveSyncsRollingSyncStrategyEnabled(appset *argov1alpha1.ApplicationSet) bool {
return appset.Spec.Strategy != nil && appset.Spec.Strategy.RollingSync != nil && appset.Spec.Strategy.Type == "RollingSync"
}

func isApplicationHealthy(app argov1alpha1.Application) bool {
Expand Down Expand Up @@ -1082,7 +1052,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con
}

appOutdated := false
if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) {
appOutdated = syncStatusString == "OutOfSync"
}

Expand Down Expand Up @@ -1148,7 +1118,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con
}

// check Applications that are in Waiting status and promote them to Pending if needed
func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress(ctx context.Context, logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appStepMap map[string]int, appMap map[string]argov1alpha1.Application) ([]argov1alpha1.ApplicationSetApplicationStatus, error) {
func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress(ctx context.Context, logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appStepMap map[string]int) ([]argov1alpha1.ApplicationSetApplicationStatus, error) {
now := metav1.Now()

appStatuses := make([]argov1alpha1.ApplicationSetApplicationStatus, 0, len(applicationSet.Status.ApplicationStatus))
Expand All @@ -1159,7 +1129,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress
totalCountMap := []int{}

length := 0
if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) {
length = len(applicationSet.Spec.Strategy.RollingSync.Steps)
}
for s := 0; s < length; s++ {
Expand All @@ -1171,7 +1141,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress
for _, appStatus := range applicationSet.Status.ApplicationStatus {
totalCountMap[appStepMap[appStatus.Application]] += 1

if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) {
if appStatus.Status == "Pending" || appStatus.Status == "Progressing" {
updateCountMap[appStepMap[appStatus.Application]] += 1
}
Expand All @@ -1181,7 +1151,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress
for _, appStatus := range applicationSet.Status.ApplicationStatus {
maxUpdateAllowed := true
maxUpdate := &intstr.IntOrString{}
if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") {
if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) {
maxUpdate = applicationSet.Spec.Strategy.RollingSync.Steps[appStepMap[appStatus.Application]].MaxUpdate
}

Expand Down Expand Up @@ -1225,7 +1195,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress
return appStatuses, nil
}

func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditions(ctx context.Context, applicationSet *argov1alpha1.ApplicationSet) ([]argov1alpha1.ApplicationSetCondition, error) {
func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditions(ctx context.Context, applicationSet *argov1alpha1.ApplicationSet) []argov1alpha1.ApplicationSetCondition {
appSetProgressing := false
for _, appStatus := range applicationSet.Status.ApplicationStatus {
if appStatus.Status != "Healthy" {
Expand Down Expand Up @@ -1264,7 +1234,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditio
)
}

return applicationSet.Status.Conditions, nil
return applicationSet.Status.Conditions
}

func findApplicationStatusIndex(appStatuses []argov1alpha1.ApplicationSetApplicationStatus, application string) int {
Expand Down Expand Up @@ -1374,7 +1344,7 @@ func (r *ApplicationSetReconciler) setAppSetApplicationStatus(ctx context.Contex
return nil
}

func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appMap map[string]argov1alpha1.Application, validApps []argov1alpha1.Application) ([]argov1alpha1.Application, error) {
func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appMap map[string]argov1alpha1.Application, validApps []argov1alpha1.Application) []argov1alpha1.Application {
rolloutApps := []argov1alpha1.Application{}
for i := range validApps {
pruneEnabled := false
Expand All @@ -1395,15 +1365,15 @@ func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, appl
// check appSyncMap to determine which Applications are ready to be updated and which should be skipped
if appSyncMap[validApps[i].Name] && appMap[validApps[i].Name].Status.Sync.Status == "OutOfSync" && appSetStatusPending {
logCtx.Infof("triggering sync for application: %v, prune enabled: %v", validApps[i].Name, pruneEnabled)
validApps[i], _ = syncApplication(validApps[i], pruneEnabled)
validApps[i] = syncApplication(validApps[i], pruneEnabled)
}
rolloutApps = append(rolloutApps, validApps[i])
}
return rolloutApps, nil
return rolloutApps
}

// used by the RollingSync Progressive Sync strategy to trigger a sync of a particular Application resource
func syncApplication(application argov1alpha1.Application, prune bool) (argov1alpha1.Application, error) {
func syncApplication(application argov1alpha1.Application, prune bool) argov1alpha1.Application {
operation := argov1alpha1.Operation{
InitiatedBy: argov1alpha1.OperationInitiator{
Username: "applicationset-controller",
Expand All @@ -1429,7 +1399,7 @@ func syncApplication(application argov1alpha1.Application, prune bool) (argov1al
}
application.Operation = &operation

return application, nil
return application
}

func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3666,8 +3666,7 @@ func TestBuildAppDependencyList(t *testing.T) {
KubeClientset: kubeclientset,
}

appDependencyList, appStepMap, err := r.buildAppDependencyList(log.NewEntry(log.StandardLogger()), cc.appSet, cc.apps)
require.NoError(t, err, "expected no errors, but errors occurred")
appDependencyList, appStepMap := r.buildAppDependencyList(log.NewEntry(log.StandardLogger()), cc.appSet, cc.apps)
assert.Equal(t, cc.expectedList, appDependencyList, "expected appDependencyList did not match actual")
assert.Equal(t, cc.expectedStepMap, appStepMap, "expected appStepMap did not match actual")
})
Expand Down Expand Up @@ -4257,8 +4256,7 @@ func TestBuildAppSyncMap(t *testing.T) {
KubeClientset: kubeclientset,
}

appSyncMap, err := r.buildAppSyncMap(context.TODO(), cc.appSet, cc.appDependencyList, cc.appMap)
require.NoError(t, err, "expected no errors, but errors occurred")
appSyncMap := r.buildAppSyncMap(cc.appSet, cc.appDependencyList, cc.appMap)
assert.Equal(t, cc.expectedMap, appSyncMap, "expected appSyncMap did not match actual")
})
}
Expand Down Expand Up @@ -5800,7 +5798,7 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) {
KubeClientset: kubeclientset,
}

appStatuses, err := r.updateApplicationSetApplicationStatusProgress(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.appSyncMap, cc.appStepMap, cc.appMap)
appStatuses, err := r.updateApplicationSetApplicationStatusProgress(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.appSyncMap, cc.appStepMap)

// opt out of testing the LastTransitionTime is accurate
for i := range appStatuses {
Expand Down
4 changes: 2 additions & 2 deletions cmd/argocd/commands/app_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestPrintTreeViewAppResources(t *testing.T) {
buf := &bytes.Buffer{}
w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0)

printTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, false, false, w)
printTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, w)
if err := w.Flush(); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -77,7 +77,7 @@ func TestPrintTreeViewDetailedAppResources(t *testing.T) {
buf := &bytes.Buffer{}
w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0)

printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, false, false, w)
printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, w)
if err := w.Flush(); err != nil {
t.Fatal(err)
}
Expand Down
16 changes: 8 additions & 8 deletions cmd/argocd/commands/app_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,25 +175,25 @@ func parentChildInfo(nodes []v1alpha1.ResourceNode) (map[string]v1alpha1.Resourc
return mapUidToNode, mapParentToChild, parentNode
}

func printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) {
func printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) {
for uid := range parentNodes {
detailedTreeViewAppResourcesNotOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w)
}
}

func printDetailedTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) {
func printDetailedTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) {
for uid := range parentNodes {
detailedTreeViewAppResourcesOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w)
}
}

func printTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) {
func printTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) {
for uid := range parentNodes {
treeViewAppResourcesNotOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w)
}
}

func printTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) {
func printTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) {
for uid := range parentNodes {
treeViewAppResourcesOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w)
}
Expand All @@ -206,24 +206,24 @@ func printResources(listAll bool, orphaned bool, appResourceTree *v1alpha1.Appli

if !orphaned || listAll {
mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.Nodes)
printDetailedTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w)
printDetailedTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, w)
}

if orphaned || listAll {
mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.OrphanedNodes)
printDetailedTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w)
printDetailedTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, w)
}
} else if output == "tree" {
fmt.Fprintf(w, "GROUP\tKIND\tNAMESPACE\tNAME\tORPHANED\n")

if !orphaned || listAll {
mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.Nodes)
printTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w)
printTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, w)
}

if orphaned || listAll {
mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.OrphanedNodes)
printTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w)
printTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, w)
}
} else {
headers := []interface{}{"GROUP", "KIND", "NAMESPACE", "NAME", "ORPHANED"}
Expand Down
2 changes: 1 addition & 1 deletion controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
}

// No need to care about the return value here, we just want the modified managedNs
_, err = syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy)(managedNs, liveObj)
_, err = syncNamespace(app.Spec.SyncPolicy)(managedNs, liveObj)
if err != nil {
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
failedToLoadObjs = true
Expand Down
2 changes: 1 addition & 1 deletion controller/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
}

if syncOp.SyncOptions.HasOption("CreateNamespace=true") {
opts = append(opts, sync.WithNamespaceModifier(syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy)))
opts = append(opts, sync.WithNamespaceModifier(syncNamespace(app.Spec.SyncPolicy)))
}

syncCtx, cleanup, err := sync.NewSyncContext(
Expand Down
Loading