feat: return images from resources when sync occurs (argoproj#642)

* Add GetResourceImages

Signed-off-by: Aaron Hoffman <[email protected]>

* Use require instead of assert

Signed-off-by: Aaron Hoffman <[email protected]>

* Add test for empty images case

Signed-off-by: Aaron Hoffman <[email protected]>

* Rename test function to match regex

Signed-off-by: Aaron Hoffman <[email protected]>

* Add support for conjobs, refactor images implementation and add test for cronjobs

Signed-off-by: Aaron Hoffman <[email protected]>

* Move missing images tests to single function

Signed-off-by: Aaron Hoffman <[email protected]>

* Refactor test

Signed-off-by: Aaron Hoffman <[email protected]>

* Add benchmark for sync

Signed-off-by: Aaron Hoffman <[email protected]>

* Update comment on images

Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>

---------

Signed-off-by: Aaron Hoffman <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>

Author: Aaron-9900

pkg/sync/common/types.go

@@ -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

pkg/sync/sync_context.go

@@ -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,

pkg/sync/sync_context_test.go

@@ -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"
@@ -2089,3 +2091,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()
+	}
+}

pkg/utils/kube/kube.go

@@ -407,6 +407,49 @@ func GetDeploymentReplicas(u *unstructured.Unstructured) *int64 {
 	return &val
 }
 
+func GetResourceImages(u *unstructured.Unstructured) []string {
+	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) {

pkg/utils/kube/kube_test.go

@@ -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)