From 249315b40be4a1892b88cbb57195ae69c2acbd93 Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Thu, 14 Nov 2024 11:00:01 +0900 Subject: [PATCH] Implement checkImageChange and tests (#5333) * Implement checkImageChange and tests Signed-off-by: Shinnosuke Sawada-Dazai * Add test case for the container order change Signed-off-by: Shinnosuke Sawada-Dazai --------- Signed-off-by: Shinnosuke Sawada-Dazai --- .../plugin/kubernetes/deployment/determine.go | 23 +++ .../kubernetes/deployment/determine_test.go | 179 ++++++++++++++++++ 2 files changed, 202 insertions(+) diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/determine.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/determine.go index fb5218b2a7..af792554cc 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/deployment/determine.go +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/determine.go @@ -23,6 +23,7 @@ import ( "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config" "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider" "github.com/pipe-cd/pipecd/pkg/model" + "github.com/pipe-cd/pipecd/pkg/plugin/diff" ) type containerImage struct { @@ -170,3 +171,25 @@ func findConfigsAndSecrets(manifests []provider.Manifest) map[provider.ResourceK } return configs } + +func checkImageChange(ns diff.Nodes) (string, bool) { + const containerImageQuery = `^spec\.template\.spec\.containers\.\d+.image$` + nodes, _ := ns.Find(containerImageQuery) + if len(nodes) == 0 { + return "", false + } + + images := make([]string, 0, len(ns)) + for _, n := range nodes { + beforeImg := parseContainerImage(n.StringX()) + afterImg := parseContainerImage(n.StringY()) + + if beforeImg.name == afterImg.name { + images = append(images, fmt.Sprintf("image %s from %s to %s", beforeImg.name, beforeImg.tag, afterImg.tag)) + } else { + images = append(images, fmt.Sprintf("image %s:%s to %s:%s", beforeImg.name, beforeImg.tag, afterImg.name, afterImg.tag)) + } + } + desc := fmt.Sprintf("Sync progressively because of updating %s", strings.Join(images, ", ")) + return desc, true +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/determine_test.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/determine_test.go index 9b539a1cdc..8a481ac400 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/deployment/determine_test.go +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/determine_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" "sigs.k8s.io/yaml" "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config" @@ -1064,3 +1065,181 @@ data: }) } } + +func TestCheckImageChange(t *testing.T) { + tests := []struct { + name string + old string + new string + want string + wantOk bool + }{ + { + name: "image updated", + old: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 +`, + new: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.4 +`, + want: "Sync progressively because of updating image nginx from 1.19.3 to 1.19.4", + wantOk: true, + }, + { + name: "image name changed", + old: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 +`, + new: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: redis + image: redis:6.0.9 +`, + want: "Sync progressively because of updating image nginx:1.19.3 to redis:6.0.9", + wantOk: true, + }, + { + name: "no image change", + old: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 +`, + new: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 +`, + want: "", + wantOk: false, + }, + { + name: "multiple image updates", + old: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 + - name: redis + image: redis:6.0.9 +`, + new: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.4 + - name: redis + image: redis:6.0.10 +`, + want: "Sync progressively because of updating image nginx from 1.19.3 to 1.19.4, image redis from 6.0.9 to 6.0.10", + wantOk: true, + }, + { + name: "change the order cause multi-image update", + old: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.19.3 + - name: redis + image: redis:6.0.9 +`, + new: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app-deployment +spec: + template: + spec: + containers: + - name: redis + image: redis:6.0.9 + - name: nginx + image: nginx:1.19.3 +`, + want: "Sync progressively because of updating image nginx:1.19.3 to redis:6.0.9, image redis:6.0.9 to nginx:1.19.3", + wantOk: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldManifests := mustParseManifests(t, tt.old) + newManifests := mustParseManifests(t, tt.new) + logger := zap.NewNop() // or use a real logger if available + diffs, err := provider.Diff(oldManifests[0], newManifests[0], logger) + require.NoError(t, err) + + got, ok := checkImageChange(diffs.Nodes()) + assert.Equal(t, tt.wantOk, ok) + assert.Equal(t, tt.want, got) + }) + } +}