Skip to content

Commit

Permalink
Adds Prune=false and IgnoreExtraneous options (argoproj#1680)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexec authored Jun 7, 2019
1 parent bdabd5b commit 0088955
Show file tree
Hide file tree
Showing 19 changed files with 319 additions and 17 deletions.
4 changes: 4 additions & 0 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ const (
// LabelValueSecretTypeCluster indicates a secret type of cluster
LabelValueSecretTypeCluster = "cluster"

// AnnotationCompareOptions is a comma-separated list of options for comparison
AnnotationCompareOptions = "argocd.argoproj.io/compare-options"
// AnnotationSyncOptions is a comma-separated list of options for syncing
AnnotationSyncOptions = "argocd.argoproj.io/sync-options"
// AnnotationSyncWave indicates which wave of the sync the resource or hook should be in
AnnotationSyncWave = "argocd.argoproj.io/sync-wave"
// AnnotationKeyHook contains the hook type of a resource
Expand Down
19 changes: 12 additions & 7 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,11 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, revision st
syncCode := v1alpha1.SyncStatusCodeSynced
managedResources := make([]managedResource, len(targetObjs))
resourceSummaries := make([]v1alpha1.ResourceStatus, len(targetObjs))
for i := 0; i < len(targetObjs); i++ {
obj := managedLiveObj[i]
for i, targetObj := range targetObjs {
liveObj := managedLiveObj[i]
obj := liveObj
if obj == nil {
obj = targetObjs[i]
obj = targetObj
}
if obj == nil {
continue
Expand All @@ -319,13 +320,17 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, revision st
diffResult := diffResults.Diffs[i]
if resState.Hook || resource.Ignore(obj) {
// For resource hooks, don't store sync status, and do not affect overall sync status
} else if diffResult.Modified || targetObjs[i] == nil || managedLiveObj[i] == nil {
} else if diffResult.Modified || targetObj == nil || liveObj == nil {
// Set resource state to OutOfSync since one of the following is true:
// * target and live resource are different
// * target resource not defined and live resource is extra
// * target resource present but live resource is missing
resState.Status = v1alpha1.SyncStatusCodeOutOfSync
syncCode = v1alpha1.SyncStatusCodeOutOfSync
// we ignore the status if the obj needs pruning AND we have the annotation
needsPruning := targetObj == nil && liveObj != nil
if !(needsPruning && resource.HasAnnotationOption(obj, common.AnnotationCompareOptions, "IgnoreExtraneous")) {
syncCode = v1alpha1.SyncStatusCodeOutOfSync
}
} else {
resState.Status = v1alpha1.SyncStatusCodeSynced
}
Expand All @@ -335,8 +340,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, revision st
Group: resState.Group,
Kind: resState.Kind,
Version: resState.Version,
Live: managedLiveObj[i],
Target: targetObjs[i],
Live: liveObj,
Target: targetObj,
Diff: diffResult,
Hook: resState.Hook,
}
Expand Down
27 changes: 27 additions & 0 deletions controller/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,33 @@ func TestCompareAppStateHook(t *testing.T) {
assert.Equal(t, 0, len(compRes.conditions))
}

// checks that ignore resources are detected, but excluded from status
func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) {
pod := test.NewPod()
pod.SetAnnotations(map[string]string{common.AnnotationCompareOptions: "IgnoreExtraneous"})
app := newFakeApp()
data := fakeData{
apps: []runtime.Object{app},
manifestResponse: &repository.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
}
ctrl := newFakeController(&data)

compRes, err := ctrl.appStateManager.CompareAppState(app, "", app.Spec.Source, false)

assert.NoError(t, err)
assert.NotNil(t, compRes)
assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
assert.Len(t, compRes.resources, 0)
assert.Len(t, compRes.managedResources, 0)
assert.Len(t, compRes.conditions, 0)
}

// TestCompareAppStateExtraHook tests when there is an extra _hook_ object in live but not defined in git
func TestCompareAppStateExtraHook(t *testing.T) {
pod := test.NewPod()
Expand Down
10 changes: 7 additions & 3 deletions controller/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ import (
"k8s.io/client-go/dynamic"
"k8s.io/client-go/rest"

"github.com/argoproj/argo-cd/common"
"github.com/argoproj/argo-cd/controller/metrics"
"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
listersv1alpha1 "github.com/argoproj/argo-cd/pkg/client/listers/application/v1alpha1"
"github.com/argoproj/argo-cd/util/argo"
"github.com/argoproj/argo-cd/util/health"
"github.com/argoproj/argo-cd/util/hook"
"github.com/argoproj/argo-cd/util/kube"
"github.com/argoproj/argo-cd/util/resource"
)

type syncContext struct {
Expand Down Expand Up @@ -473,7 +475,11 @@ func (sc *syncContext) applyObject(targetObj *unstructured.Unstructured, dryRun

// pruneObject deletes the object if both prune is true and dryRun is false. Otherwise appropriate message
func (sc *syncContext) pruneObject(liveObj *unstructured.Unstructured, prune, dryRun bool) (v1alpha1.ResultCode, string) {
if prune {
if !prune {
return v1alpha1.ResultCodePruneSkipped, "ignored (requires pruning)"
} else if resource.HasAnnotationOption(liveObj, common.AnnotationSyncOptions, "Prune=false") {
return v1alpha1.ResultCodePruneSkipped, "ignored (no prune)"
} else {
if dryRun {
return v1alpha1.ResultCodePruned, "pruned (dry run)"
} else {
Expand All @@ -487,8 +493,6 @@ func (sc *syncContext) pruneObject(liveObj *unstructured.Unstructured, prune, dr
}
return v1alpha1.ResultCodePruned, "pruned"
}
} else {
return v1alpha1.ResultCodePruneSkipped, "ignored (requires pruning)"
}
}

Expand Down
20 changes: 20 additions & 0 deletions controller/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,26 @@ func TestDontSyncOrPruneHooks(t *testing.T) {
assert.Equal(t, v1alpha1.OperationSucceeded, syncCtx.opState.Phase)
}

// make sure that we do not prune resources with Prune=false
func TestDontPrunePruneFalse(t *testing.T) {
syncCtx := newTestSyncCtx()
pod := test.NewPod()
pod.SetAnnotations(map[string]string{common.AnnotationSyncOptions: "Prune=false"})
pod.SetNamespace(test.FakeArgoCDNamespace)
syncCtx.compareResult = &comparisonResult{managedResources: []managedResource{{Live: pod}}}

syncCtx.sync()

assert.Equal(t, v1alpha1.OperationRunning, syncCtx.opState.Phase)
assert.Len(t, syncCtx.syncRes.Resources, 1)
assert.Equal(t, v1alpha1.ResultCodePruneSkipped, syncCtx.syncRes.Resources[0].Status)
assert.Equal(t, "ignored (no prune)", syncCtx.syncRes.Resources[0].Message)

syncCtx.sync()

assert.Equal(t, v1alpha1.OperationSucceeded, syncCtx.opState.Phase)
}

func TestSelectiveSyncOnly(t *testing.T) {
syncCtx := newTestSyncCtx()
pod1 := test.NewPod()
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/assets/sync-option-no-prune.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
34 changes: 34 additions & 0 deletions docs/user-guide/compare-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Compare Options

## Ignoring Resources That Are Extraneous

You may wish to exclude resources from the app's overall sync status under certain circumstances. E.g. if they are generated by a tool. This can be done by adding this annotation:

```yaml
metadata:
annotations:
argocd.argoproj.io/compare-options: IgnoreExtraneous
```
![compare option needs pruning](../assets/compare-option-ignore-needs-pruning.png)
!!! note
This only affects the sync status. If the resource's health is degraded, then the app will also be degraded.
Kustomize has a feature that allows you to generate config maps ([read more ⧉](https://github.com/kubernetes-sigs/kustomize/blob/master/examples/configGeneration.md)). You can set `generatorOptions` to add this annotation so that your app remains in sync:

```yaml
configMapGenerator:
- name: my-map
literals:
- foo=bar
generatorOptions:
annotations:
argocd.argoproj.io/compare-options: IgnoreExtraneous
kind: Kustomization
```

!!! note
`generatorOptions` adds annotations to both config maps and secrets ([read more ⧉](https://github.com/kubernetes-sigs/kustomize/blob/master/examples/generatorOptions.md)).

You may wish to combine this with the [`Prune=false` sync option](sync-options.md).
5 changes: 4 additions & 1 deletion docs/user-guide/kustomize.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ You have three configuration options for Kustomize:
* `imageTags` is a list of Kustomize 1.0 image tag overrides
* `images` is a list of Kustomize 2.0 image overrides

To use Kustomize with an overlay, point your path to the overlay.
To use Kustomize with an overlay, point your path to the overlay.

!!! tip
If you're generating resources, you should read up how to ignore those generated resources using the [`IgnoreExtraneous` compare option](compare-options.md).
24 changes: 24 additions & 0 deletions docs/user-guide/sync-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Sync Options

## No Prune Resources

You may wish to prevent an object from being pruned:

```yaml
metadata:
annotations:
argocd.argoproj.io/sync-options: Prune=false
```
In the UI, the pod will simply appear as out-of-sync:
![sync option no prune](../assets/sync-option-no-prune.png)
The sync-status panel shows that pruning was skipped, and why:
![sync option no prune](../assets/sync-option-no-prune-sync-status.png)
!!! note
The app will be out of sync if Argo CD expects a resource to be pruned. You may wish to use this along with [compare options](compare-options.md).
2 changes: 2 additions & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ nav:
- user-guide/private-repositories.md
- user-guide/auto_sync.md
- user-guide/diffing.md
- user-guide/compare-options.md
- user-guide/sync-options.md
- user-guide/parameters.md
- user-guide/tracking_strategies.md
- user-guide/resource_hooks.md
Expand Down
64 changes: 59 additions & 5 deletions test/e2e/app_management_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,15 +306,19 @@ func TestResourceDiffing(t *testing.T) {
}

func TestDeprecatedExtensions(t *testing.T) {
testEdgeCasesApplicationResources(t, "deprecated-extensions")
testEdgeCasesApplicationResources(t, "deprecated-extensions", OperationRunning, HealthStatusProgressing)
}

func TestCRDs(t *testing.T) {
testEdgeCasesApplicationResources(t, "crd-creation")
testEdgeCasesApplicationResources(t, "crd-creation", OperationSucceeded, HealthStatusHealthy)
}

func TestDuplicatedResources(t *testing.T) {
testEdgeCasesApplicationResources(t, "duplicated-resources")
testEdgeCasesApplicationResources(t, "duplicated-resources", OperationSucceeded, HealthStatusHealthy)
}

func TestConfigMap(t *testing.T) {
testEdgeCasesApplicationResources(t, "config-map", OperationSucceeded, HealthStatusHealthy)
}

func TestFailedConversion(t *testing.T) {
Expand All @@ -323,17 +327,19 @@ func TestFailedConversion(t *testing.T) {
errors.FailOnErr(fixture.Run("", "kubectl", "delete", "apiservice", "v1beta1.metrics.k8s.io"))
}()

testEdgeCasesApplicationResources(t, "failed-conversion")
testEdgeCasesApplicationResources(t, "failed-conversion", OperationSucceeded, HealthStatusHealthy)
}

func testEdgeCasesApplicationResources(t *testing.T, appPath string) {
func testEdgeCasesApplicationResources(t *testing.T, appPath string, phase OperationPhase, statusCode HealthStatusCode) {
Given(t).
Path(appPath).
When().
Create().
Sync().
Then().
Expect(OperationPhaseIs(phase)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
Expect(HealthIs(statusCode)).
And(func(app *Application) {
diffOutput, err := fixture.RunCli("app", "diff", app.Name, "--local", path.Join("testdata", appPath))
assert.Empty(t, diffOutput)
Expand Down Expand Up @@ -504,3 +510,51 @@ func TestPermissions(t *testing.T) {
assert.True(t, destinationErrorExist)
assert.True(t, sourceErrorExist)
}

// make sure that if we deleted a resource from the app, it is not pruned if annotated with Prune=false
func TestSyncOptionPruneFalse(t *testing.T) {
Given(t).
Path("two-nice-pods").
When().
PatchFile("pod-1.yaml", `[{"op": "add", "path": "/metadata/annotations", "value": {"argocd.argoproj.io/sync-options": "Prune=false"}}]`).
Create().
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
When().
DeleteFile("pod-1.yaml").
Refresh(RefreshTypeHard).
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
Expect(ResourceSyncStatusIs("Pod", "pod-1", SyncStatusCodeOutOfSync))
}

// make sure that, if we have a resource that needs pruning, but we're ignoring it, the app is in-sync
func TestCompareOptionIgnoreExtraneous(t *testing.T) {
Given(t).
Path("two-nice-pods").
When().
PatchFile("pod-1.yaml", `[{"op": "add", "path": "/metadata/annotations", "value": {"argocd.argoproj.io/compare-options": "IgnoreExtraneous"}}]`).
Create().
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
When().
DeleteFile("pod-1.yaml").
Refresh(RefreshTypeHard).
Then().
Expect(SyncStatusIs(SyncStatusCodeSynced)).
And(func(app *Application) {
assert.Len(t, app.Status.Resources, 2)
assert.Equal(t, SyncStatusCodeOutOfSync, app.Status.Resources[1].Status)
}).
When().
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeSynced))
}
2 changes: 1 addition & 1 deletion test/e2e/fixture/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func EnsureCleanState(t *testing.T) {

// new random ID
id = dnsFriendly(t.Name())
repoUrl = fmt.Sprintf("file:///%s", repoDirectory())
repoUrl = fmt.Sprintf("file://%s", repoDirectory())

// create tmp dir
FailOnErr(Run("", "mkdir", "-p", tmpDir))
Expand Down
53 changes: 53 additions & 0 deletions test/e2e/kustomize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,56 @@ func TestKustomize2AppSource(t *testing.T) {
And(patchLabelMatchesFor("Service")).
And(patchLabelMatchesFor("Deployment"))
}

// when we have a config map generator, AND the ignore annotation, it is ignored in the app's sync status
func TestSyncStatusOptionIgnore(t *testing.T) {
var mapName string
Given(t).
Path("kustomize-cm-gen").
When().
Create().
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
Expect(HealthIs(HealthStatusHealthy)).
And(func(app *Application) {
resourceStatus := app.Status.Resources[0]
assert.Contains(t, resourceStatus.Name, "my-map-")
assert.Equal(t, SyncStatusCodeSynced, resourceStatus.Status)

mapName = resourceStatus.Name
}).
When().
// we now force generation of a second CM
PatchFile("kustomization.yaml", `[{"op": "replace", "path": "/configMapGenerator/0/literals/0", "value": "foo=baz"}]`).
Refresh(RefreshTypeHard).
Then().
// this is standard logging from the command - tough one - true statement
When().
Sync().
Then().
Expect(Error("1 resources require pruning")).
Expect(OperationPhaseIs(OperationSucceeded)).
// this is a key check - we expect the app to be healthy because, even though we have a resources that needs
// pruning, because it is annotated with IgnoreExtraneous it should not contribute to the sync status
Expect(SyncStatusIs(SyncStatusCodeSynced)).
Expect(HealthIs(HealthStatusHealthy)).
And(func(app *Application) {
assert.Equal(t, 2, len(app.Status.Resources))
// new map in-sync
{
resourceStatus := app.Status.Resources[0]
assert.Contains(t, resourceStatus.Name, "my-map-")
// make sure we've a new map with changed name
assert.NotEqual(t, mapName, resourceStatus.Name)
assert.Equal(t, SyncStatusCodeSynced, resourceStatus.Status)
}
// old map is out of sync
{
resourceStatus := app.Status.Resources[1]
assert.Equal(t, mapName, resourceStatus.Name)
assert.Equal(t, SyncStatusCodeOutOfSync, resourceStatus.Status)
}
})
}
Loading

0 comments on commit 0088955

Please sign in to comment.