Skip to content

feat: return images from resources when sync occurs #642

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

Merged
merged 9 commits into from
Mar 27, 2025
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
4 changes: 4 additions & 0 deletions pkg/sync/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ func NewHookDeletePolicy(p string) (HookDeletePolicy, bool) {
type ResourceSyncResult struct {
// holds associated resource key
ResourceKey kube.ResourceKey
// Images holds the images associated with the resource. These images are collected on a best-effort basis
// from fields used by known workload resources. This does not necessarily reflect the exact list of images
// used by workloads in the application.
Images []string
// holds resource version
Version string
// holds the execution order
Expand Down
1 change: 1 addition & 0 deletions pkg/sync/sync_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,7 @@ func (sc *syncContext) setResourceResult(task *syncTask, syncStatus common.Resul

res := common.ResourceSyncResult{
ResourceKey: kubeutil.GetResourceKey(task.obj()),
Images: kubeutil.GetResourceImages(task.obj()),
Version: task.version(),
Status: task.syncStatus,
Message: task.message,
Expand Down
49 changes: 49 additions & 0 deletions pkg/sync/sync_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"strings"
"testing"
"time"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -2061,3 +2063,50 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
assert.Equal(t, synccommon.ResultCodePruned, result[1].Status)
assert.Equal(t, synccommon.ResultCodePruned, result[2].Status)
}

func BenchmarkSync(b *testing.B) {
podManifest := `{
"apiVersion": "v1",
"kind": "Pod",
"metadata": {
"name": "my-pod"
},
"spec": {
"containers": [
${containers}
]
}
}`
container := `{
"image": "nginx:1.7.9",
"name": "nginx",
"resources": {
"requests": {
"cpu": "0.2"
}
}
}`

maxContainers := 10
for i := 0; i < b.N; i++ {
b.StopTimer()
containerCount := min(i+1, maxContainers)

containerStr := strings.Repeat(container+",", containerCount)
containerStr = containerStr[:len(containerStr)-1]

manifest := strings.ReplaceAll(podManifest, "${containers}", containerStr)
pod := testingutils.Unstructured(manifest)
pod.SetNamespace(testingutils.FakeArgoCDNamespace)

syncCtx := newTestSyncCtx(nil, WithOperationSettings(false, true, false, false))
syncCtx.log = logr.Discard()
syncCtx.resources = groupResources(ReconciliationResult{
Live: []*unstructured.Unstructured{nil, pod},
Target: []*unstructured.Unstructured{testingutils.NewService(), nil},
})

b.StartTimer()
syncCtx.Sync()
}
}
43 changes: 43 additions & 0 deletions pkg/utils/kube/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,49 @@ func GetDeploymentReplicas(u *unstructured.Unstructured) *int64 {
return &val
}

func GetResourceImages(u *unstructured.Unstructured) []string {

Choose a reason for hiding this comment

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

For CronJobs it seems that the image is under spec.jobTemplate.spec.template.spec.containers. Should we also check there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6b9063a

var containers []any
var found bool
var err error
var images []string

containerPaths := [][]string{
// Resources without template, like pods
{"spec", "containers"},
// Resources with template, like deployments
{"spec", "template", "spec", "containers"},
// Cronjobs
{"spec", "jobTemplate", "spec", "template", "spec", "containers"},
}

for _, path := range containerPaths {
containers, found, err = unstructured.NestedSlice(u.Object, path...)
if found && err == nil {
break
}
}

if !found || err != nil {
return nil
}

for _, container := range containers {
containerMap, ok := container.(map[string]any)
if !ok {
continue
}

image, found, err := unstructured.NestedString(containerMap, "image")
if !found || err != nil {
continue
}

images = append(images, image)
}

return images
}

// RetryUntilSucceed keep retrying given action with specified interval until action succeed or specified context is done.
func RetryUntilSucceed(ctx context.Context, interval time.Duration, desc string, log logr.Logger, action func() error) {
pollErr := wait.PollUntilContextCancel(ctx, interval, true, func(_ context.Context) (bool /*done*/, error) {
Expand Down
163 changes: 163 additions & 0 deletions pkg/utils/kube/kube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,169 @@ spec:
assert.Nil(t, GetDeploymentReplicas(&noDeployment))
}

func TestGetResourceImages(t *testing.T) {
testCases := []struct {
manifest []byte
expected []string
description string
}{
{
manifest: []byte(`
apiVersion: extensions/v1beta2
kind: Deployment
metadata:
name: nginx-deployment
labels:
foo: bar
spec:
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.7.9
ports:
- containerPort: 80
- name: agent
image: agent:1.0.0`),
expected: []string{"nginx:1.7.9", "agent:1.0.0"},
description: "deployment with two containers",
},
{
manifest: []byte(`
apiVersion: v1
kind: Pod
metadata:
name: example-pod
labels:
app: my-app
spec:
containers:
- name: nginx-container
image: nginx:1.21
ports:
- containerPort: 80
- name: sidecar-container
image: busybox:1.35
command: ["sh", "-c", "echo Hello from the sidecar; sleep 3600"]
`),
expected: []string{"nginx:1.21", "busybox:1.35"},
description: "pod with containers",
},
{
manifest: []byte(`
apiVersion: batch/v1
kind: CronJob
metadata:
name: hello
spec:
schedule: "* * * * *"
jobTemplate:
spec:
template:
spec:
containers:
- name: hello
image: busybox:1.28
`),
expected: []string{"busybox:1.28"},
description: "cronjob with containers",
},
{
manifest: []byte(`
apiVersion: v1
kind: ConfigMap
metadata:
name: example-config
namespace: default
labels:
app: my-app
data:
app.properties: |
key1=value1
key2=value2
key3=value3
log.level: debug
`),
expected: nil,
description: "configmap without containers",
},
{
manifest: []byte(`
apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-no-containers
labels:
foo: bar
spec:
replicas: 1
selector:
matchLabels:
app: agent
template:
metadata:
labels:
app: agent
spec:
volumes:
- name: config-volume
configMap:
name: config
`),
expected: nil,
description: "deployment without containers",
},
{
manifest: []byte(`
apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment-without-image
spec:
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: text-service
command: ["echo", "hello"]
`),
expected: nil,
description: "deployment with container without image",
},
{
manifest: []byte(`
apiVersion: v1
kind: Pod
metadata:
name: example-pod
labels:
app: my-app
spec:
containers:
- name: no-image-container
command: ["echo", "hello"]
`),
expected: nil,
description: "pod with container without image",
},
}

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
resource := unstructured.Unstructured{}
err := yaml.Unmarshal(tc.manifest, &resource)
require.NoError(t, err)
images := GetResourceImages(&resource)
require.Equal(t, tc.expected, images)
})
}
}

func TestSplitYAML_SingleObject(t *testing.T) {
objs, err := SplitYAML([]byte(depWithLabel))
require.NoError(t, err)
Expand Down