Skip to content

Commit c617562

Browse files
Aaron-9900crenshaw-dev
Aaron-9900
and
crenshaw-dev
authored
feat: return images from resources when sync occurs (argoproj#642)
* Add GetResourceImages Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Use require instead of assert Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Add test for empty images case Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Rename test function to match regex Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Add support for conjobs, refactor images implementation and add test for cronjobs Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Move missing images tests to single function Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Refactor test Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Add benchmark for sync Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> * Update comment on images Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> --------- Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
1 parent 370078d commit c617562

File tree

5 files changed

+260
-0
lines changed

5 files changed

+260
-0
lines changed

‎pkg/sync/common/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ func NewHookDeletePolicy(p string) (HookDeletePolicy, bool) {
136136
type ResourceSyncResult struct {
137137
// holds associated resource key
138138
ResourceKey kube.ResourceKey
139+
// Images holds the images associated with the resource. These images are collected on a best-effort basis
140+
// from fields used by known workload resources. This does not necessarily reflect the exact list of images
141+
// used by workloads in the application.
142+
Images []string
139143
// holds resource version
140144
Version string
141145
// holds the execution order

‎pkg/sync/sync_context.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,7 @@ func (sc *syncContext) setResourceResult(task *syncTask, syncStatus common.Resul
13761376

13771377
res := common.ResourceSyncResult{
13781378
ResourceKey: kubeutil.GetResourceKey(task.obj()),
1379+
Images: kubeutil.GetResourceImages(task.obj()),
13791380
Version: task.version(),
13801381
Status: task.syncStatus,
13811382
Message: task.message,

‎pkg/sync/sync_context_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import (
77
"net/http"
88
"net/http/httptest"
99
"reflect"
10+
"strings"
1011
"testing"
1112
"time"
1213

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

17+
"github.com/go-logr/logr"
1618
"github.com/stretchr/testify/assert"
1719
"github.com/stretchr/testify/require"
1820
corev1 "k8s.io/api/core/v1"
@@ -2089,3 +2091,50 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
20892091
assert.Equal(t, synccommon.ResultCodePruned, result[1].Status)
20902092
assert.Equal(t, synccommon.ResultCodePruned, result[2].Status)
20912093
}
2094+
2095+
func BenchmarkSync(b *testing.B) {
2096+
podManifest := `{
2097+
"apiVersion": "v1",
2098+
"kind": "Pod",
2099+
"metadata": {
2100+
"name": "my-pod"
2101+
},
2102+
"spec": {
2103+
"containers": [
2104+
${containers}
2105+
]
2106+
}
2107+
}`
2108+
container := `{
2109+
"image": "nginx:1.7.9",
2110+
"name": "nginx",
2111+
"resources": {
2112+
"requests": {
2113+
"cpu": "0.2"
2114+
}
2115+
}
2116+
}`
2117+
2118+
maxContainers := 10
2119+
for i := 0; i < b.N; i++ {
2120+
b.StopTimer()
2121+
containerCount := min(i+1, maxContainers)
2122+
2123+
containerStr := strings.Repeat(container+",", containerCount)
2124+
containerStr = containerStr[:len(containerStr)-1]
2125+
2126+
manifest := strings.ReplaceAll(podManifest, "${containers}", containerStr)
2127+
pod := testingutils.Unstructured(manifest)
2128+
pod.SetNamespace(testingutils.FakeArgoCDNamespace)
2129+
2130+
syncCtx := newTestSyncCtx(nil, WithOperationSettings(false, true, false, false))
2131+
syncCtx.log = logr.Discard()
2132+
syncCtx.resources = groupResources(ReconciliationResult{
2133+
Live: []*unstructured.Unstructured{nil, pod},
2134+
Target: []*unstructured.Unstructured{testingutils.NewService(), nil},
2135+
})
2136+
2137+
b.StartTimer()
2138+
syncCtx.Sync()
2139+
}
2140+
}

‎pkg/utils/kube/kube.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,49 @@ func GetDeploymentReplicas(u *unstructured.Unstructured) *int64 {
407407
return &val
408408
}
409409

410+
func GetResourceImages(u *unstructured.Unstructured) []string {
411+
var containers []any
412+
var found bool
413+
var err error
414+
var images []string
415+
416+
containerPaths := [][]string{
417+
// Resources without template, like pods
418+
{"spec", "containers"},
419+
// Resources with template, like deployments
420+
{"spec", "template", "spec", "containers"},
421+
// Cronjobs
422+
{"spec", "jobTemplate", "spec", "template", "spec", "containers"},
423+
}
424+
425+
for _, path := range containerPaths {
426+
containers, found, err = unstructured.NestedSlice(u.Object, path...)
427+
if found && err == nil {
428+
break
429+
}
430+
}
431+
432+
if !found || err != nil {
433+
return nil
434+
}
435+
436+
for _, container := range containers {
437+
containerMap, ok := container.(map[string]any)
438+
if !ok {
439+
continue
440+
}
441+
442+
image, found, err := unstructured.NestedString(containerMap, "image")
443+
if !found || err != nil {
444+
continue
445+
}
446+
447+
images = append(images, image)
448+
}
449+
450+
return images
451+
}
452+
410453
// RetryUntilSucceed keep retrying given action with specified interval until action succeed or specified context is done.
411454
func RetryUntilSucceed(ctx context.Context, interval time.Duration, desc string, log logr.Logger, action func() error) {
412455
pollErr := wait.PollUntilContextCancel(ctx, interval, true, func(_ context.Context) (bool /*done*/, error) {

‎pkg/utils/kube/kube_test.go

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,169 @@ spec:
178178
assert.Nil(t, GetDeploymentReplicas(&noDeployment))
179179
}
180180

181+
func TestGetResourceImages(t *testing.T) {
182+
testCases := []struct {
183+
manifest []byte
184+
expected []string
185+
description string
186+
}{
187+
{
188+
manifest: []byte(`
189+
apiVersion: extensions/v1beta2
190+
kind: Deployment
191+
metadata:
192+
name: nginx-deployment
193+
labels:
194+
foo: bar
195+
spec:
196+
template:
197+
metadata:
198+
labels:
199+
app: nginx
200+
spec:
201+
containers:
202+
- name: nginx
203+
image: nginx:1.7.9
204+
ports:
205+
- containerPort: 80
206+
- name: agent
207+
image: agent:1.0.0`),
208+
expected: []string{"nginx:1.7.9", "agent:1.0.0"},
209+
description: "deployment with two containers",
210+
},
211+
{
212+
manifest: []byte(`
213+
apiVersion: v1
214+
kind: Pod
215+
metadata:
216+
name: example-pod
217+
labels:
218+
app: my-app
219+
spec:
220+
containers:
221+
- name: nginx-container
222+
image: nginx:1.21
223+
ports:
224+
- containerPort: 80
225+
- name: sidecar-container
226+
image: busybox:1.35
227+
command: ["sh", "-c", "echo Hello from the sidecar; sleep 3600"]
228+
`),
229+
expected: []string{"nginx:1.21", "busybox:1.35"},
230+
description: "pod with containers",
231+
},
232+
{
233+
manifest: []byte(`
234+
apiVersion: batch/v1
235+
kind: CronJob
236+
metadata:
237+
name: hello
238+
spec:
239+
schedule: "* * * * *"
240+
jobTemplate:
241+
spec:
242+
template:
243+
spec:
244+
containers:
245+
- name: hello
246+
image: busybox:1.28
247+
`),
248+
expected: []string{"busybox:1.28"},
249+
description: "cronjob with containers",
250+
},
251+
{
252+
manifest: []byte(`
253+
apiVersion: v1
254+
kind: ConfigMap
255+
metadata:
256+
name: example-config
257+
namespace: default
258+
labels:
259+
app: my-app
260+
data:
261+
app.properties: |
262+
key1=value1
263+
key2=value2
264+
key3=value3
265+
log.level: debug
266+
`),
267+
expected: nil,
268+
description: "configmap without containers",
269+
},
270+
{
271+
manifest: []byte(`
272+
apiVersion: apps/v1
273+
kind: Deployment
274+
metadata:
275+
name: deployment-no-containers
276+
labels:
277+
foo: bar
278+
spec:
279+
replicas: 1
280+
selector:
281+
matchLabels:
282+
app: agent
283+
template:
284+
metadata:
285+
labels:
286+
app: agent
287+
spec:
288+
volumes:
289+
- name: config-volume
290+
configMap:
291+
name: config
292+
`),
293+
expected: nil,
294+
description: "deployment without containers",
295+
},
296+
{
297+
manifest: []byte(`
298+
apiVersion: apps/v1
299+
kind: Deployment
300+
metadata:
301+
name: deployment-without-image
302+
spec:
303+
template:
304+
metadata:
305+
labels:
306+
app: nginx
307+
spec:
308+
containers:
309+
- name: text-service
310+
command: ["echo", "hello"]
311+
`),
312+
expected: nil,
313+
description: "deployment with container without image",
314+
},
315+
{
316+
manifest: []byte(`
317+
apiVersion: v1
318+
kind: Pod
319+
metadata:
320+
name: example-pod
321+
labels:
322+
app: my-app
323+
spec:
324+
containers:
325+
- name: no-image-container
326+
command: ["echo", "hello"]
327+
`),
328+
expected: nil,
329+
description: "pod with container without image",
330+
},
331+
}
332+
333+
for _, tc := range testCases {
334+
t.Run(tc.description, func(t *testing.T) {
335+
resource := unstructured.Unstructured{}
336+
err := yaml.Unmarshal(tc.manifest, &resource)
337+
require.NoError(t, err)
338+
images := GetResourceImages(&resource)
339+
require.Equal(t, tc.expected, images)
340+
})
341+
}
342+
}
343+
181344
func TestSplitYAML_SingleObject(t *testing.T) {
182345
objs, err := SplitYAML([]byte(depWithLabel))
183346
require.NoError(t, err)

0 commit comments

Comments
 (0)