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

feat: Add ignore-resources-tracking annotation to ignore resources update #18343

Merged
Merged
Show file tree
Hide file tree
Changes from 13 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
31 changes: 26 additions & 5 deletions controller/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/url"
"os/exec"
"reflect"
"strconv"
"strings"
"sync"
"syscall"
Expand Down Expand Up @@ -69,6 +70,9 @@ const (

// EnvClusterCacheRetryUseBackoff is the env variable to control whether to use a backoff strategy with the retry during cluster cache sync
EnvClusterCacheRetryUseBackoff = "ARGOCD_CLUSTER_CACHE_RETRY_USE_BACKOFF"

// AnnotationApplyResourcesUpdate when set to false to a resource, argocd will not proceed any update onto that resource.
AnnotationApplyResourcesUpdate = "argocd.argoproj.io/apply-resources-update"
Copy link
Member

Choose a reason for hiding this comment

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

Now that the code is written, I think it will be much easier for users to have the same terminology with the existing feature. wdyt?

Suggested change
AnnotationApplyResourcesUpdate = "argocd.argoproj.io/apply-resources-update"
AnnotationIgnoreResourceUpdates = "argocd.argoproj.io/ignore-resource-updates"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agaudreault I think AnnotationApplyResourcesUpdate is more logical to me. AnnotationIgnoreResourceUpdates seems the invert of what we are doing and therefore we need to flip our logic if we rename it.

)

// GitOps engine cluster cache tuning options
Expand Down Expand Up @@ -348,20 +352,37 @@ func skipResourceUpdate(oldInfo, newInfo *ResourceInfo) bool {
return false
}
isSameHealthStatus := (oldInfo.Health == nil && newInfo.Health == nil) || oldInfo.Health != nil && newInfo.Health != nil && oldInfo.Health.Status == newInfo.Health.Status
isSameManifest := oldInfo.manifestHash != "" && newInfo.manifestHash != "" && oldInfo.manifestHash == newInfo.manifestHash
isSameManifest := oldInfo.manifestHash == newInfo.manifestHash
Copy link
Member

Choose a reason for hiding this comment

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

This condition should not change. If a resource does not have a hash, that means the annotation was not there (or not true), therefore we should not ignore updates.

Copy link
Contributor Author

@kahoulei kahoulei Jun 10, 2024

Choose a reason for hiding this comment

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

Ok, I misunderstood your intention. So you want is the dependent object set argocd.argoproj.io/apply-resources-update=true and then use the existing ignore resources config to skip refresh. I will update the PR.

return isSameHealthStatus && isSameManifest
}

// shouldHashManifest validates if the API resource needs to be hashed.
// If there's an app name from resource tracking, or if this is itself an app, we should generate a hash.
// Otherwise, the hashing should be skipped to save CPU time.
func shouldHashManifest(appName string, gvk schema.GroupVersionKind) bool {
// Only hash if the resource belongs to an app.
func shouldHashManifest(appName string, gvk schema.GroupVersionKind, un *unstructured.Unstructured) bool {
// Only hash if the resource belongs to an app OR argocd.argoproj.io/apply-resources-update is present and set to true
// Best - Only hash for resources that are part of an app or their dependencies
// (current) - Only hash for resources that are part of an app + all apps that might be from an ApplicationSet
// Orphan - If orphan is enabled, hash should be made on all resource of that namespace and a config to disable it
// Worst - Hash all resources watched by Argo
return appName != "" || (gvk.Group == application.Group && gvk.Kind == application.ApplicationKind)
belong := appName != "" || (gvk.Group == application.Group && gvk.Kind == application.ApplicationKind)

// If the resource does not belong the app, we will look up argocd.argoproj.io/apply-resources-update and decide
// whether we generate hash or not.
// If argocd.argoproj.io/apply-resources-update is presented and is true, return true
// Else return false
if !belong {
if val, ok := un.GetAnnotations()[AnnotationApplyResourcesUpdate]; ok {
applyResourcesUpdate, err := strconv.ParseBool(val)
if err != nil {
applyResourcesUpdate = false
}
return applyResourcesUpdate
}
return false
}

return belong
}

// isRetryableError is a helper method to see whether an error
Expand Down Expand Up @@ -504,7 +525,7 @@ func (c *liveStateCache) getCluster(server string) (clustercache.ClusterCache, e

gvk := un.GroupVersionKind()

if cacheSettings.ignoreResourceUpdatesEnabled && shouldHashManifest(appName, gvk) {
if cacheSettings.ignoreResourceUpdatesEnabled && shouldHashManifest(appName, gvk, un) {
hash, err := generateManifestHash(un, nil, cacheSettings.resourceOverrides, c.ignoreNormalizerOpts)
if err != nil {
log.Errorf("Failed to generate manifest hash: %v", err)
Expand Down
81 changes: 80 additions & 1 deletion controller/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package cache
import (
"context"
"errors"
"github.com/argoproj/argo-cd/v2/pkg/apis/application"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"net"
"net/url"
"sync"
Expand Down Expand Up @@ -553,7 +555,7 @@ func TestSkipResourceUpdate(t *testing.T) {
assert.False(t, skipResourceUpdate(info, nil))
})
t.Run("No hash", func(t *testing.T) {
assert.False(t, skipResourceUpdate(&ResourceInfo{}, &ResourceInfo{}))
assert.True(t, skipResourceUpdate(&ResourceInfo{}, &ResourceInfo{}))
})
t.Run("Same hash", func(t *testing.T) {
assert.True(t, skipResourceUpdate(&ResourceInfo{
Expand Down Expand Up @@ -652,3 +654,80 @@ func TestSkipResourceUpdate(t *testing.T) {
}))
})
}

func TestShouldHashManifest(t *testing.T) {
tests := []struct {
name string
appName string
gvk schema.GroupVersionKind
un *unstructured.Unstructured
annotations map[string]string
want bool
}{
{
name: "appName not empty gvk matches",
appName: "MyApp",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: application.ApplicationKind},
un: &unstructured.Unstructured{},
want: true,
},
{
name: "appName empty",
appName: "",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: application.ApplicationKind},
un: &unstructured.Unstructured{},
want: true,
},
{
name: "appName empty group not match",
appName: "",
gvk: schema.GroupVersionKind{Group: "group1", Kind: application.ApplicationKind},
un: &unstructured.Unstructured{},
want: false,
},
{
name: "appName empty kind not match",
appName: "",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: "kind1"},
un: &unstructured.Unstructured{},
want: false,
},
{
name: "argocd.argoproj.io/apply-resources-update=true",
appName: "",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: "kind1"},
un: &unstructured.Unstructured{},
annotations: map[string]string{"argocd.argoproj.io/apply-resources-update": "true"},
want: true,
},
{
name: "argocd.argoproj.io/apply-resources-update=invalid",
appName: "",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: "kind1"},
un: &unstructured.Unstructured{},
annotations: map[string]string{"argocd.argoproj.io/apply-resources-update": "invalid"},
want: false,
},
{
name: "argocd.argoproj.io/apply-resources-update=false",
appName: "",
gvk: schema.GroupVersionKind{Group: application.Group, Kind: "kind1"},
un: &unstructured.Unstructured{},
annotations: map[string]string{"argocd.argoproj.io/apply-resources-update": "false"},
want: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.annotations != nil {
test.un.SetAnnotations(test.annotations)
}
got := shouldHashManifest(test.appName, test.gvk, test.un)
if test.want != got {
t.Fatalf("test=%v want %v got %v", test.name, test.want, got)
}
})
}

}
53 changes: 53 additions & 0 deletions docs/operator-manual/reconcile.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,56 @@ data:
# actually changing in content.
- .status.conditions[].lastTransitionTime
```

## Completely ignore resources update

There is a use case that you would like to completely ignore the resources at all. This mostly happens when some dependent
resources are created by another resource (e.g. a Job and a Pod are created by a CronJob and you want to ignore the Job
and the Pod).

For this use case, the above configurations will not help because argocd will still reconcile any newly created objects.

To completely ignore a newly created object, you need to add annotation `argocd.argoproj.io/apply-resources-update=false`
to the target resource manifest.

## Example

### CronJob

```
apiVersion: batch/v1
kind: CronJob
metadata:
name: hello
namespace: test-cronjob
spec:
schedule: "* * * * *"
jobTemplate:
metadata:
annotations:
argocd.argoproj.io/apply-resources-update: "false"
spec:
template:
metadata:
annotations:
argocd.argoproj.io/apply-resources-update: "false"
spec:
containers:
- name: hello
image: busybox:1.28
imagePullPolicy: IfNotPresent
command:
- /bin/sh
- -c
- date; echo Hello from the Kubernetes cluster
restartPolicy: OnFailure
```

And you should see the following debug message (if enabled) in the `application-controller` log:

```
Ignoring change of object because none of the watched resource fields have changed
```

Note: If you add annotation to any resource with `argocd.argoproj.io/apply-resources-update=true`, argocd controller
will refresh that resource regardless if it belongs to the application or not.
1 change: 1 addition & 0 deletions docs/user-guide/annotations-and-labels.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
| argocd.argoproj.io/sync-options | any | [see sync options docs](sync-options.md) | Provides a variety of settings to determine how an Application's resources are synced. |
| argocd.argoproj.io/sync-wave | any | [see sync waves docs](sync-waves.md) | |
| argocd.argoproj.io/tracking-id | any | any | Used by Argo CD to track resources it manages. See [resource tracking docs](resource_tracking.md) for details. |
| argocd.argoproj.io/apply-resources-update | any | `"true"`, `false` | Used by Argo CD to ignore tracking resources. See [reconcile docs](..%2Foperator-manual%2Freconcile.md)reconcile_docs for details. |
| link.argocd.argoproj.io/{some link name} | any | An http(s) URL | Adds a link to the Argo CD UI for the resource. See [external URL docs](external-url.md) for details. |
| pref.argocd.argoproj.io/default-pod-sort | Application | [see UI customization docs](../operator-manual/ui-customization.md) | Sets the Application's default grouping mechanism. |
| pref.argocd.argoproj.io/default-view | Application | [see UI customization docs](../operator-manual/ui-customization.md) | Sets the Application's default view mode (e.g. "tree" or "list") |
Expand Down
Loading