feat: return images from resources when sync occurs #642
Conversation
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
from
922aecd to
3f1734a
Compare
Aaron-9900
changed the title
andrii-korotkov-verkada
left a comment
andrii-korotkov-verkada
left a comment
There was a problem hiding this comment.
Can you also add a test to when there are no images, please?
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
from
5281e57 to
ae4000b
Compare
pkg/utils/kube/kube.go
Outdated
| for _, container := range containers { | ||
| containerMap, ok := container.(map[string]interface{}) | ||
| if !ok { | ||
| return nil |
There was a problem hiding this comment.
Should we continue instead of returning here? It might be something related to the single container only
There was a problem hiding this comment.
Makes sense to extract as much data as possible. Added continue on 5b2c46f but maybe @andrii-korotkov-verkada has a different take
| return &val | ||
| } | ||
|
|
||
| func GetResourceImages(u *unstructured.Unstructured) []string { |
There was a problem hiding this comment.
For CronJobs it seems that the image is under spec.jobTemplate.spec.template.spec.containers. Should we also check there?
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #642 +/- ##
==========================================
- Coverage 54.26% 53.55% -0.72%
==========================================
Files 64 64
Lines 6164 6442 +278
==========================================
+ Hits 3345 3450 +105
- Misses 2549 2715 +166
- Partials 270 277 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
3 times, most recently
from
50c907f to
a989b44
Compare
Signed-off-by: Aaron Hoffman <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>
…for cronjobs Signed-off-by: Aaron Hoffman <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>
Signed-off-by: Aaron Hoffman <[email protected]>
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
from
a989b44 to
e6f8fbe
Compare
|
Can you create a PR to Argo CD updating the EDIT: Nvm, found it argoproj/argo-cd#20954. Do the e2e pass? |
|
@rumstead done in argoproj/argo-cd#20954, tests are now passing |
Signed-off-by: Aaron Hoffman <[email protected]>
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
from
f14771b to
914b89c
Compare
|
Added new benchmark test for the sync operation. These are the results: command: 3 images5 images10 imagesOverall, we see a small increase in time/operation for a small amount of images (11% for 3 images), and it gets larger as the images grow (36% for 10 images). This is expected, since we are now iterating over images. Even with this growth, the total value is still quite low. |
Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: Aaron Hoffman <[email protected]>
Aaron-9900
force-pushed
the
feat/add-images-to-resources
branch
from
b985995 to
4fbe25e
Compare
|
6 participants
Implementing the Gitops side of this issue: argoproj/argo-cd#20896
Adds an api to
kube.goto get the images from a resource. Adds those images toResourceSyncResult. This API accounts for how different k8s resources define the images.