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: return images from resources when sync occurs #642

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Aaron-9900
Copy link

@Aaron-9900 Aaron-9900 commented Nov 25, 2024

Implementing the Gitops side of this issue: argoproj/argo-cd#20896

Adds an api to kube.go to get the images from a resource. Adds those images to ResourceSyncResult

Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
@Aaron-9900 Aaron-9900 changed the title Add GetResourceImages feat: return images from resources when sync occurs Nov 25, 2024
Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
Copy link
Contributor

@andrii-korotkov-verkada andrii-korotkov-verkada left a comment

Choose a reason for hiding this comment

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

Can you also add a test to when there are no images, please?

Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
for _, container := range containers {
containerMap, ok := container.(map[string]interface{})
if !ok {
return nil

Choose a reason for hiding this comment

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

Should we continue instead of returning here? It might be something related to the single container only

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense to extract as much data as possible. Added continue on 5b2c46f but maybe @andrii-korotkov-verkada has a different take

@@ -404,6 +404,34 @@ 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
Author

Choose a reason for hiding this comment

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

Done in 6b9063a

…for cronjobs

Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
pkg/utils/kube/kube_test.go Outdated Show resolved Hide resolved
Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
Signed-off-by: Aaron Hoffman <31711338+Aaron-9900@users.noreply.github.com>
Copy link

sonarcloud bot commented Nov 27, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants