Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: monorepo selfheal should not happens if manifest is the same #322

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Changes from 1 commit
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
7 changes: 5 additions & 2 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
@@ -1959,8 +1959,11 @@ func alreadyAttemptedSync(app *appv1.Application, commitSHA string, commitSHAsMS
return false, ""
}
} else {
if app.Status.OperationState.SyncResult.Revision != commitSHA {
return false, ""
manifestsChangedMap := app.Status.Sync.ManifestsChanged
if manifestsChangedMap[commitSHA] {
if app.Status.OperationState.SyncResult.Revision != commitSHA {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make fix for multi-sourced also

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will do it when will open pr to oss, for now it is optional

return false, ""
}
}
}

2 changes: 1 addition & 1 deletion controller/hook.go
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Applicat
revisions = append(revisions, src.TargetRevision)
}

targets, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj)
targets, _, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj)
if err != nil {
return false, err
}
52 changes: 31 additions & 21 deletions controller/state.go
Original file line number Diff line number Diff line change
@@ -73,7 +73,7 @@
type AppStateManager interface {
CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool) (*comparisonResult, error)
SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState)
GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, error)
GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, map[string]bool, error)
}

// comparisonResult holds the state of an application after the reconciliation
@@ -126,51 +126,51 @@
// task to the repo-server. It returns the list of generated manifests as unstructured
// objects. It also returns the full response from all calls to the repo server as the
// second argument.
func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, error) {
func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, map[string]bool, error) {
ts := stats.NewTimingStats()
helmRepos, err := m.db.ListHelmRepositories(context.Background())
if err != nil {
return nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
return nil, nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
}
permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos)
if err != nil {
return nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
return nil, nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
}

ts.AddCheckpoint("repo_ms")
helmRepositoryCredentials, err := m.db.GetAllHelmRepositoryCredentials(context.Background())
if err != nil {
return nil, nil, fmt.Errorf("failed to get Helm credentials: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Helm credentials: %w", err)
}
permittedHelmCredentials, err := argo.GetPermittedReposCredentials(proj, helmRepositoryCredentials)
if err != nil {
return nil, nil, fmt.Errorf("failed to get permitted Helm credentials for project %q: %w", proj.Name, err)
return nil, nil, nil, fmt.Errorf("failed to get permitted Helm credentials for project %q: %w", proj.Name, err)
}

enabledSourceTypes, err := m.settingsMgr.GetEnabledSourceTypes()
if err != nil {
return nil, nil, fmt.Errorf("failed to get enabled source types: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get enabled source types: %w", err)
}
ts.AddCheckpoint("plugins_ms")

kustomizeSettings, err := m.settingsMgr.GetKustomizeSettings()
if err != nil {
return nil, nil, fmt.Errorf("failed to get Kustomize settings: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Kustomize settings: %w", err)
}

helmOptions, err := m.settingsMgr.GetHelmSettings()
if err != nil {
return nil, nil, fmt.Errorf("failed to get Helm settings: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Helm settings: %w", err)
}

ts.AddCheckpoint("build_options_ms")
serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(app.Spec.Destination.Server)
if err != nil {
return nil, nil, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err)
return nil, nil, nil, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err)
}
conn, repoClient, err := m.repoClientset.NewRepoServerClient()
if err != nil {
return nil, nil, fmt.Errorf("failed to connect to repo server: %w", err)
return nil, nil, nil, fmt.Errorf("failed to connect to repo server: %w", err)
}
defer io.Close(conn)

@@ -180,21 +180,23 @@
// Store the map of all sources having ref field into a map for applications with sources field
refSources, err := argo.GetRefSources(context.Background(), app.Spec, m.db)
if err != nil {
return nil, nil, fmt.Errorf("failed to get ref sources: %v", err)
return nil, nil, nil, fmt.Errorf("failed to get ref sources: %v", err)
}

manifestsChanges := make(map[string]bool)

Check failure on line 187 in controller/state.go

GitHub Actions / Lint Go code

File is not `gofmt`-ed with `-s` (gofmt)
for i, source := range sources {
if len(revisions) < len(sources) || revisions[i] == "" {
revisions[i] = source.TargetRevision
}
ts.AddCheckpoint("helm_ms")
repo, err := m.db.GetRepository(context.Background(), source.RepoURL)
if err != nil {
return nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
return nil, nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
}
kustomizeOptions, err := kustomizeSettings.GetOptions(source, m.settingsMgr.GetKustomizeSetNamespaceEnabled())
if err != nil {
return nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
}

syncedRevision := app.Status.Sync.Revision
@@ -209,7 +211,7 @@
val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]
if !source.IsHelm() && syncedRevision != "" && ok && val != "" {
// Validate the manifest-generate-path annotation to avoid generating manifests if it has not changed.
_, err = repoClient.UpdateRevisionForPaths(context.Background(), &apiclient.UpdateRevisionForPathsRequest{
updateRevisionResponse, err := repoClient.UpdateRevisionForPaths(context.Background(), &apiclient.UpdateRevisionForPathsRequest{
Repo: repo,
Revision: revisions[i],
SyncedRevision: syncedRevision,
@@ -224,8 +226,12 @@
RefSources: refSources,
HasMultipleSources: app.Spec.HasMultipleSources(),
})
// not need to change, if we already found at least one cached revision that has no changes
if updateRevisionResponse != nil {
manifestsChanges[syncedRevision] = updateRevisionResponse.Changes
}
if err != nil {
return nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err)
}
}

@@ -256,13 +262,13 @@
ApplicationMetadata: &app.ObjectMeta,
})
if err != nil {
return nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
}

targetObj, err := unmarshalManifests(manifestInfo.GetCompiledManifests())

if err != nil {
return nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err)
}
targetObjs = append(targetObjs, targetObj...)

@@ -276,7 +282,7 @@
}
logCtx = logCtx.WithField("time_ms", time.Since(ts.StartTime).Milliseconds())
logCtx.Info("GetRepoObjs stats")
return targetObjs, manifestInfos, nil
return targetObjs, manifestInfos, manifestsChanges, nil
}

func unmarshalManifests(manifests []string) ([]*unstructured.Unstructured, error) {
@@ -444,6 +450,8 @@
var manifestInfos []*apiclient.ManifestResponse
targetNsExists := false

var manifestChanged map[string]bool

if len(localManifests) == 0 {
// If the length of revisions is not same as the length of sources,
// we take the revisions from the sources directly for all the sources.
@@ -453,8 +461,8 @@
revisions = append(revisions, source.TargetRevision)
}
}

targetObjs, manifestInfos, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
targetObjs, manifestInfos, manifestChanged, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
if err != nil {
targetObjs = make([]*unstructured.Unstructured, 0)
msg := fmt.Sprintf("Failed to load target state: %s", err.Error())
@@ -830,6 +838,7 @@
},
Status: syncCode,
Revisions: manifestRevisions,
ManifestsChanged: manifestChanged,
}
} else {
syncStatus = v1alpha1.SyncStatus{
@@ -840,6 +849,7 @@
},
Status: syncCode,
Revision: revision,
ManifestsChanged: manifestChanged,
}
}

7 changes: 7 additions & 0 deletions manifests/crds/application-crd.yaml
Original file line number Diff line number Diff line change
@@ -5035,6 +5035,13 @@ spec:
required:
- destination
type: object
manifestsChanged:
additionalProperties:
type: boolean
description: ManifestsChanged indicates whether the manifests
have changed base on argocd.argoproj.io/manifest-generate-paths
annotation
type: object
revision:
description: Revision contains information about the revision
the comparison has been performed to
2 changes: 2 additions & 0 deletions pkg/apis/application/v1alpha1/types.go
Original file line number Diff line number Diff line change
@@ -1518,6 +1518,8 @@ type SyncStatus struct {
Revision string `json:"revision,omitempty" protobuf:"bytes,3,opt,name=revision"`
// Revisions contains information about the revisions of multiple sources the comparison has been performed to
Revisions []string `json:"revisions,omitempty" protobuf:"bytes,4,opt,name=revisions"`
// ManifestsChanged indicates whether the manifests have changed base on argocd.argoproj.io/manifest-generate-paths annotation
ManifestsChanged map[string]bool `json:"manifestsChanged,omitempty" protobuf:"bytes,5,opt,name=manifestsChanged"`
}

// HealthStatus contains information about the currently observed health state of an application or resource
Loading