From 960fb7326893ddd5d97e637d70a0291f85774e27 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 14:38:39 +0900 Subject: [PATCH 01/12] Add ArtifactVersion to KubernetesApp Deployment --- pkg/app/piped/controller/planner.go | 1 + pkg/app/piped/controller/scheduler.go | 1 + .../piped/planner/kubernetes/kubernetes.go | 54 +++++++++++++ .../planner/kubernetes/kubernetes_test.go | 80 +++++++++++++++++++ pkg/app/piped/planner/planner.go | 1 + pkg/app/piped/trigger/deployment.go | 1 + pkg/app/server/grpcapi/piped_api.go | 3 +- .../server/service/pipedservice/service.proto | 1 + .../web/src/__fixtures__/dummy-deployment.ts | 1 + pkg/datastore/deploymentstore.go | 9 ++- pkg/datastore/deploymentstore_test.go | 12 ++- pkg/model/application.proto | 14 +--- pkg/model/deployment.proto | 13 +++ 13 files changed, 172 insertions(+), 19 deletions(-) diff --git a/pkg/app/piped/controller/planner.go b/pkg/app/piped/controller/planner.go index 4e2baa75e0..fc6519eb8c 100644 --- a/pkg/app/piped/controller/planner.go +++ b/pkg/app/piped/controller/planner.go @@ -228,6 +228,7 @@ func (p *planner) reportDeploymentPlanned(ctx context.Context, out pln.Output) e RunningCommitHash: p.lastSuccessfulCommitHash, RunningConfigFilename: p.lastSuccessfulConfigFilename, Version: out.Version, + Versions: out.Versions, Stages: out.Stages, DeploymentChainId: p.deployment.DeploymentChainId, DeploymentChainBlockIndex: p.deployment.DeploymentChainBlockIndex, diff --git a/pkg/app/piped/controller/scheduler.go b/pkg/app/piped/controller/scheduler.go index a1bfa51b02..32e4e77fb1 100644 --- a/pkg/app/piped/controller/scheduler.go +++ b/pkg/app/piped/controller/scheduler.go @@ -679,6 +679,7 @@ func (s *scheduler) reportMostRecentlySuccessfulDeployment(ctx context.Context) Trigger: s.deployment.Trigger, Summary: s.deployment.Summary, Version: s.deployment.Version, + Versions: s.deployment.Versions, ConfigFilename: s.deployment.GitPath.GetApplicationConfigFilename(), StartedAt: s.deployment.CreatedAt, CompletedAt: s.deployment.CompletedAt, diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 8cb053c047..57d0e2f627 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -97,6 +97,18 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu out.Version = version } + if versions, e := determineVersions(newManifests); e != nil { + in.Logger.Error("unable to determine versions", zap.Error(e)) + out.Versions = []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: versionUnknown, + }, + } + } else { + out.Versions = versions + } + autoRollback := *cfg.Input.AutoRollback // In case the strategy has been decided by trigger. @@ -480,3 +492,45 @@ func determineVersion(manifests []provider.Manifest) (string, error) { return b.String(), nil } + +// determineVersions decide artifact versions of an application. +// For example, in case some containers are used in one application, those image version are retured +func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, error) { + versions := []*model.ArtifactVersion{} + + for _, m := range manifests { + if !m.Key.IsDeployment() { + continue + } + data, err := m.MarshalJSON() + if err != nil { + return nil, err + } + var d resource.Deployment + if err := json.Unmarshal(data, &d); err != nil { + return nil, err + } + + containers := d.Spec.Template.Spec.Containers + for _, c := range containers { + image := parseContainerImage(c.Image) + versions = append(versions, &model.ArtifactVersion{ + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: image.tag, + Name: image.name, + Url: c.Image, + }) + } + } + + if len(versions) == 0 { + return []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_UNKNOWN, + Version: versionUnknown, + }, + }, nil + } + + return versions, nil +} diff --git a/pkg/app/piped/planner/kubernetes/kubernetes_test.go b/pkg/app/piped/planner/kubernetes/kubernetes_test.go index 4ab81d9b08..dce8d78298 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes_test.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes_test.go @@ -9,6 +9,7 @@ import ( provider "github.com/pipe-cd/pipecd/pkg/app/piped/cloudprovider/kubernetes" "github.com/pipe-cd/pipecd/pkg/config" + "github.com/pipe-cd/pipecd/pkg/model" ) func TestDecideStrategy(t *testing.T) { @@ -438,6 +439,85 @@ func TestDetermineVersion(t *testing.T) { } } +func TestDetermineVersions(t *testing.T) { + testcases := []struct { + name string + manifests string + expected []*model.ArtifactVersion + expectedError error + }{ + { + name: "no workload", + manifests: "testdata/version_no_workload.yaml", + expected: []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_UNKNOWN, + Version: versionUnknown, + }, + }, + }, + { + name: "single container", + manifests: "testdata/version_single_container.yaml", + expected: []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v1.0.0", + Name: "helloworld", + Url: "gcr.io/pipecd/helloworld:v1.0.0", + }, + }, + }, + { + name: "multiple containers", + manifests: "testdata/version_multi_containers.yaml", + expected: []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v1.0.0", + Name: "helloworld", + Url: "gcr.io/pipecd/helloworld:v1.0.0", + }, + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v0.6.0", + Name: "my-service", + Url: "gcr.io/pipecd/my-service:v0.6.0", + }, + }, + }, + { + name: "multiple workloads", + manifests: "testdata/version_multi_workloads.yaml", + expected: []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v1.0.0", + Name: "helloworld", + Url: "gcr.io/pipecd/helloworld:v1.0.0", + }, + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v0.5.0", + Name: "my-service", + Url: "gcr.io/pipecd/my-service:v0.5.0", + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + manifests, err := provider.LoadManifestsFromYAMLFile(tc.manifests) + require.NoError(t, err) + + versions, err := determineVersions(manifests) + assert.Equal(t, tc.expected, versions) + assert.Equal(t, tc.expectedError, err) + }) + } +} + func TestCheckImageChange(t *testing.T) { testcases := []struct { name string diff --git a/pkg/app/piped/planner/planner.go b/pkg/app/piped/planner/planner.go index 076344e094..38d248e2dd 100644 --- a/pkg/app/piped/planner/planner.go +++ b/pkg/app/piped/planner/planner.go @@ -58,6 +58,7 @@ type Input struct { type Output struct { Version string + Versions []*model.ArtifactVersion SyncStrategy model.SyncStrategy Summary string Stages []*model.PipelineStage diff --git a/pkg/app/piped/trigger/deployment.go b/pkg/app/piped/trigger/deployment.go index ece84c3800..0e5a1a5584 100644 --- a/pkg/app/piped/trigger/deployment.go +++ b/pkg/app/piped/trigger/deployment.go @@ -118,6 +118,7 @@ func reportMostRecentlyTriggeredDeployment(ctx context.Context, client apiClient Trigger: d.Trigger, Summary: d.Summary, Version: d.Version, + Versions: d.Versions, StartedAt: d.CreatedAt, CompletedAt: d.CompletedAt, }, diff --git a/pkg/app/server/grpcapi/piped_api.go b/pkg/app/server/grpcapi/piped_api.go index d9d00fe6e6..7a7e2793f6 100644 --- a/pkg/app/server/grpcapi/piped_api.go +++ b/pkg/app/server/grpcapi/piped_api.go @@ -54,7 +54,7 @@ type pipedApiDeploymentStore interface { Add(ctx context.Context, app *model.Deployment) error Get(ctx context.Context, id string) (*model.Deployment, error) List(ctx context.Context, opts datastore.ListOptions) ([]*model.Deployment, string, error) - UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, stages []*model.PipelineStage) error + UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, versions []*model.ArtifactVersion, stages []*model.PipelineStage) error UpdateToCompleted(ctx context.Context, id string, status model.DeploymentStatus, stageStatuses map[string]model.StageStatus, reason string, completedAt int64) error UpdateStatus(ctx context.Context, id string, status model.DeploymentStatus, reason string) error UpdateStageStatus(ctx context.Context, id, stageID string, status model.StageStatus, reason string, requires []string, visible bool, retriedCount int32, completedAt int64) error @@ -403,6 +403,7 @@ func (a *PipedAPI) ReportDeploymentPlanned(ctx context.Context, req *pipedservic req.RunningCommitHash, req.RunningConfigFilename, req.Version, + req.Versions, req.Stages, ); err != nil { return nil, gRPCEntityOperationError(err, fmt.Sprintf("update deployment %s as planned", req.DeploymentId)) diff --git a/pkg/app/server/service/pipedservice/service.proto b/pkg/app/server/service/pipedservice/service.proto index 4a3ccee9df..64d57e9c3e 100644 --- a/pkg/app/server/service/pipedservice/service.proto +++ b/pkg/app/server/service/pipedservice/service.proto @@ -276,6 +276,7 @@ message ReportDeploymentPlannedRequest { string running_config_filename = 9; // The application version this deployment is trying to deploy. string version = 5; + repeated model.ArtifactVersion versions = 10; // The planned stages. // Empty means nothing has changed compared to when the deployment was created. repeated model.PipelineStage stages = 6; diff --git a/pkg/app/web/src/__fixtures__/dummy-deployment.ts b/pkg/app/web/src/__fixtures__/dummy-deployment.ts index 10235ba84f..692e105c84 100644 --- a/pkg/app/web/src/__fixtures__/dummy-deployment.ts +++ b/pkg/app/web/src/__fixtures__/dummy-deployment.ts @@ -22,6 +22,7 @@ export const dummyDeployment: Deployment.AsObject = { statusReason: "good", trigger: dummyTrigger, version: "0.0.0", + versionsList: [], cloudProvider: "kube-1", labelsMap: [], createdAt: createdAt.unix(), diff --git a/pkg/datastore/deploymentstore.go b/pkg/datastore/deploymentstore.go index 394664341b..c444828f26 100644 --- a/pkg/datastore/deploymentstore.go +++ b/pkg/datastore/deploymentstore.go @@ -70,7 +70,7 @@ func (d *deploymentCollection) Encode(e interface{}) (map[Shard][]byte, error) { } var ( - toPlannedUpdateFunc = func(summary, statusReason, runningCommitHash, runningConfigFilename, version string, stages []*model.PipelineStage) func(*model.Deployment) error { + toPlannedUpdateFunc = func(summary, statusReason, runningCommitHash, runningConfigFilename, version string, versions []*model.ArtifactVersion, stages []*model.PipelineStage) func(*model.Deployment) error { return func(d *model.Deployment) error { d.Status = model.DeploymentStatus_DEPLOYMENT_PLANNED d.Summary = summary @@ -78,6 +78,7 @@ var ( d.RunningCommitHash = runningCommitHash d.RunningConfigFilename = runningConfigFilename d.Version = version + d.Versions = versions d.Stages = stages return nil } @@ -134,7 +135,7 @@ type DeploymentStore interface { Add(ctx context.Context, d *model.Deployment) error Get(ctx context.Context, id string) (*model.Deployment, error) List(ctx context.Context, opts ListOptions) ([]*model.Deployment, string, error) - UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, stages []*model.PipelineStage) error + UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, versions []*model.ArtifactVersion, stages []*model.PipelineStage) error UpdateToCompleted(ctx context.Context, id string, status model.DeploymentStatus, stageStatuses map[string]model.StageStatus, reason string, completedAt int64) error UpdateStatus(ctx context.Context, id string, status model.DeploymentStatus, reason string) error UpdateStageStatus(ctx context.Context, id, stageID string, status model.StageStatus, reason string, requires []string, visible bool, retriedCount int32, completedAt int64) error @@ -222,8 +223,8 @@ func (s *deploymentStore) update(ctx context.Context, id string, updater func(*m }) } -func (s *deploymentStore) UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, stages []*model.PipelineStage) error { - updater := toPlannedUpdateFunc(summary, reason, runningCommitHash, runningConfigFilename, version, stages) +func (s *deploymentStore) UpdateToPlanned(ctx context.Context, id, summary, reason, runningCommitHash, runningConfigFilename, version string, versions []*model.ArtifactVersion, stages []*model.PipelineStage) error { + updater := toPlannedUpdateFunc(summary, reason, runningCommitHash, runningConfigFilename, version, versions, stages) return s.update(ctx, id, updater) } diff --git a/pkg/datastore/deploymentstore_test.go b/pkg/datastore/deploymentstore_test.go index 5be2cacddd..c6e86ea26e 100644 --- a/pkg/datastore/deploymentstore_test.go +++ b/pkg/datastore/deploymentstore_test.go @@ -35,7 +35,15 @@ func TestDeploymentToPlannedUpdater(t *testing.T) { expectedRunningCommitHash = "update-running-commit-hash" expectedRunningConfigFilename = "update-running-config-filename" expectedVersion = "update-version" - expectedStages = []*model.PipelineStage{ + expectedVersions = []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "update-version", + Name: "update-image-name", + Url: "dummy-registry/update-image-name:update-version", + }, + } + expectedStages = []*model.PipelineStage{ { Id: "stage-id1", Name: "stage1", @@ -63,6 +71,7 @@ func TestDeploymentToPlannedUpdater(t *testing.T) { expectedRunningCommitHash, expectedRunningConfigFilename, expectedVersion, + expectedVersions, expectedStages, ) ) @@ -75,6 +84,7 @@ func TestDeploymentToPlannedUpdater(t *testing.T) { assert.Equal(t, expectedRunningCommitHash, d.RunningCommitHash) assert.Equal(t, expectedRunningConfigFilename, d.RunningConfigFilename) assert.Equal(t, expectedVersion, d.Version) + assert.Equal(t, expectedVersions, d.Versions) assert.Equal(t, expectedStages, d.Stages) } diff --git a/pkg/model/application.proto b/pkg/model/application.proto index cf3c516d13..1e72cff82b 100644 --- a/pkg/model/application.proto +++ b/pkg/model/application.proto @@ -85,25 +85,13 @@ message ApplicationSyncState { int64 timestamp = 5 [(validate.rules).int64.gt = 0]; } -message ArtifactVersion { - enum Kind { - UNKNOWN = 0; - CONTAINER_IMAGE = 1; - } - - Kind kind = 1 [(validate.rules).enum.defined_only = true]; - string version = 2 [(validate.rules).string.min_len = 1]; - string name = 3; - string url = 4; -} - message ApplicationDeploymentReference { string deployment_id = 1 [(validate.rules).string.min_len = 1]; DeploymentTrigger trigger = 2 [(validate.rules).message.required = true]; string summary = 3; string version = 4; string config_filename = 5; - repeated ArtifactVersion versions = 6; + repeated model.ArtifactVersion versions = 6; int64 started_at = 14 [(validate.rules).int64.gt = 0]; int64 completed_at = 15 [(validate.rules).int64.gte = 0]; diff --git a/pkg/model/deployment.proto b/pkg/model/deployment.proto index e54be43174..90846ae78e 100644 --- a/pkg/model/deployment.proto +++ b/pkg/model/deployment.proto @@ -77,6 +77,7 @@ message Deployment { // e.g. Update image from v1.5.0 to v1.6.0. string summary = 22; string version = 23; + repeated ArtifactVersion versions = 103; // Hash value of the most recently successfully deployed commit. string running_commit_hash = 21; @@ -146,3 +147,15 @@ message Commit { string url = 6; int64 created_at = 14 [(validate.rules).int64.gt = 0]; } + +message ArtifactVersion { + enum Kind { + UNKNOWN = 0; + CONTAINER_IMAGE = 1; + } + + Kind kind = 1 [(validate.rules).enum.defined_only = true]; + string version = 2 [(validate.rules).string.min_len = 1]; + string name = 3; + string url = 4; +} From 5a48e20eac79425eea13614c6e2bdb80de92622b Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:10:35 +0900 Subject: [PATCH 02/12] Remove duplicate images on multiple manifests --- .../piped/planner/kubernetes/kubernetes.go | 20 +++--- .../planner/kubernetes/kubernetes_test.go | 12 ++++ .../version_multi_workloads_same_image.yaml | 63 +++++++++++++++++++ 3 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 pkg/app/piped/planner/kubernetes/testdata/version_multi_workloads_same_image.yaml diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 57d0e2f627..9b7942ec7b 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -497,6 +497,7 @@ func determineVersion(manifests []provider.Manifest) (string, error) { // For example, in case some containers are used in one application, those image version are retured func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, error) { versions := []*model.ArtifactVersion{} + imageMap := map[string]string{} for _, m := range manifests { if !m.Key.IsDeployment() { @@ -512,17 +513,22 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, } containers := d.Spec.Template.Spec.Containers + // remove duplicate images on multiple manifests for _, c := range containers { - image := parseContainerImage(c.Image) - versions = append(versions, &model.ArtifactVersion{ - Kind: model.ArtifactVersion_CONTAINER_IMAGE, - Version: image.tag, - Name: image.name, - Url: c.Image, - }) + imageMap[c.Image] = "" } } + for i, _ := range imageMap { + image := parseContainerImage(i) + versions = append(versions, &model.ArtifactVersion{ + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: image.tag, + Name: image.name, + Url: i, + }) + } + if len(versions) == 0 { return []*model.ArtifactVersion{ { diff --git a/pkg/app/piped/planner/kubernetes/kubernetes_test.go b/pkg/app/piped/planner/kubernetes/kubernetes_test.go index dce8d78298..bd4ecc5d19 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes_test.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes_test.go @@ -504,6 +504,18 @@ func TestDetermineVersions(t *testing.T) { }, }, }, + { + name: "multiple workloads using same container image", + manifests: "testdata/version_multi_workloads_same_image.yaml", + expected: []*model.ArtifactVersion{ + { + Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Version: "v1.0.0", + Name: "helloworld", + Url: "gcr.io/pipecd/helloworld:v1.0.0", + }, + }, + }, } for _, tc := range testcases { diff --git a/pkg/app/piped/planner/kubernetes/testdata/version_multi_workloads_same_image.yaml b/pkg/app/piped/planner/kubernetes/testdata/version_multi_workloads_same_image.yaml new file mode 100644 index 0000000000..21abdf39d5 --- /dev/null +++ b/pkg/app/piped/planner/kubernetes/testdata/version_multi_workloads_same_image.yaml @@ -0,0 +1,63 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: simple + labels: + app: simple + pipecd.dev/managed-by: piped +spec: + replicas: 2 + selector: + matchLabels: + app: simple + template: + metadata: + labels: + app: simple + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v1.0.0 + args: + - hello + - hi + ports: + - containerPort: 9085 +--- +apiVersion: v1 +kind: Service +metadata: + name: my-service +spec: + selector: + app: MyApp + ports: + - protocol: TCP + port: 80 + targetPort: 9376 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-service + labels: + pipecd.dev/managed-by: piped + app: simple +spec: + replicas: 2 + selector: + matchLabels: + app: simple + template: + metadata: + labels: + app: simple + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v1.0.0 + args: + - hi + - hello + ports: + - containerPort: 9085 From f42f70cc3bec0ce966f50a94465e42726842c38e Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:12:01 +0900 Subject: [PATCH 03/12] Use ArtifactVersion_UNKNOWN when to unable to determine versions --- pkg/app/piped/planner/kubernetes/kubernetes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 9b7942ec7b..644770efac 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -101,7 +101,7 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu in.Logger.Error("unable to determine versions", zap.Error(e)) out.Versions = []*model.ArtifactVersion{ { - Kind: model.ArtifactVersion_CONTAINER_IMAGE, + Kind: model.ArtifactVersion_UNKNOWN, Version: versionUnknown, }, } From 99b5c35d4808d0cc7ea4a4db77d2c8ece88c40d3 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:12:44 +0900 Subject: [PATCH 04/12] Fix comment --- pkg/app/piped/planner/kubernetes/kubernetes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 644770efac..20b16c45f2 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -493,8 +493,8 @@ func determineVersion(manifests []provider.Manifest) (string, error) { return b.String(), nil } -// determineVersions decide artifact versions of an application. -// For example, in case some containers are used in one application, those image version are retured +// determineVersions decides artifact versions of an application. +// It finds all container images that are being specified in the workload manifests then returns their names, version numbers, and urls. func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, error) { versions := []*model.ArtifactVersion{} imageMap := map[string]string{} From d276e4586219e2f052d88d0c92769c9796e124ea Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:23:27 +0900 Subject: [PATCH 05/12] Move ArtifactVersion to common.proto --- pkg/model/common.proto | 12 ++++++++++++ pkg/model/deployment.proto | 12 ------------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/model/common.proto b/pkg/model/common.proto index a2a03cf83e..c4fc5fc034 100644 --- a/pkg/model/common.proto +++ b/pkg/model/common.proto @@ -74,3 +74,15 @@ message ApplicationInfo { // This field will be no longer needed as labels can be an alternative. string env_name = 14 [deprecated=true]; } + +message ArtifactVersion { + enum Kind { + UNKNOWN = 0; + CONTAINER_IMAGE = 1; + } + + Kind kind = 1 [(validate.rules).enum.defined_only = true]; + string version = 2 [(validate.rules).string.min_len = 1]; + string name = 3; + string url = 4; +} diff --git a/pkg/model/deployment.proto b/pkg/model/deployment.proto index 90846ae78e..5015ea30f3 100644 --- a/pkg/model/deployment.proto +++ b/pkg/model/deployment.proto @@ -147,15 +147,3 @@ message Commit { string url = 6; int64 created_at = 14 [(validate.rules).int64.gt = 0]; } - -message ArtifactVersion { - enum Kind { - UNKNOWN = 0; - CONTAINER_IMAGE = 1; - } - - Kind kind = 1 [(validate.rules).enum.defined_only = true]; - string version = 2 [(validate.rules).string.min_len = 1]; - string name = 3; - string url = 4; -} From 08317b973ddae647b7fa9f19dd70268e887e6584 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:28:13 +0900 Subject: [PATCH 06/12] Small fix --- pkg/app/piped/planner/kubernetes/kubernetes.go | 2 +- pkg/model/application.proto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 20b16c45f2..ba1930f988 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -519,7 +519,7 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, } } - for i, _ := range imageMap { + for i := range imageMap { image := parseContainerImage(i) versions = append(versions, &model.ArtifactVersion{ Kind: model.ArtifactVersion_CONTAINER_IMAGE, diff --git a/pkg/model/application.proto b/pkg/model/application.proto index 1e72cff82b..44bf9afe36 100644 --- a/pkg/model/application.proto +++ b/pkg/model/application.proto @@ -91,7 +91,7 @@ message ApplicationDeploymentReference { string summary = 3; string version = 4; string config_filename = 5; - repeated model.ArtifactVersion versions = 6; + repeated ArtifactVersion versions = 6; int64 started_at = 14 [(validate.rules).int64.gt = 0]; int64 completed_at = 15 [(validate.rules).int64.gte = 0]; From acbc9466e94b5e3a01db9870294684e38f6ad864 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:44:44 +0900 Subject: [PATCH 07/12] Use strruct{}{} to have smaller size --- pkg/app/piped/planner/kubernetes/kubernetes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index ba1930f988..a5cfa2cc84 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -497,7 +497,7 @@ func determineVersion(manifests []provider.Manifest) (string, error) { // It finds all container images that are being specified in the workload manifests then returns their names, version numbers, and urls. func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, error) { versions := []*model.ArtifactVersion{} - imageMap := map[string]string{} + imageMap := map[string]struct{}{} for _, m := range manifests { if !m.Key.IsDeployment() { @@ -515,7 +515,7 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, containers := d.Spec.Template.Spec.Containers // remove duplicate images on multiple manifests for _, c := range containers { - imageMap[c.Image] = "" + imageMap[c.Image] = struct{}{} } } From 569159b0e298f4214398d4c50bb8112fdd6b9412 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:47:29 +0900 Subject: [PATCH 08/12] Add comment --- pkg/app/piped/planner/kubernetes/kubernetes.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index a5cfa2cc84..d69128a25a 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -500,6 +500,7 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, imageMap := map[string]struct{}{} for _, m := range manifests { + // TODO: Determine container image version from other workload kinds such as StatefulSet, Pod, Daemon, CronJob... if !m.Key.IsDeployment() { continue } From 093087646f2529314422cb9e67d4594699b71dc5 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 16:51:28 +0900 Subject: [PATCH 09/12] Return empty list when image couldn't find --- pkg/app/piped/planner/kubernetes/kubernetes.go | 11 +---------- pkg/app/piped/planner/kubernetes/kubernetes_test.go | 7 +------ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index d69128a25a..33a607d0da 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -97,7 +97,7 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu out.Version = version } - if versions, e := determineVersions(newManifests); e != nil { + if versions, e := determineVersions(newManifests); e != nil || len(versions) == 0 { in.Logger.Error("unable to determine versions", zap.Error(e)) out.Versions = []*model.ArtifactVersion{ { @@ -530,14 +530,5 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, }) } - if len(versions) == 0 { - return []*model.ArtifactVersion{ - { - Kind: model.ArtifactVersion_UNKNOWN, - Version: versionUnknown, - }, - }, nil - } - return versions, nil } diff --git a/pkg/app/piped/planner/kubernetes/kubernetes_test.go b/pkg/app/piped/planner/kubernetes/kubernetes_test.go index bd4ecc5d19..8da6f6b4c0 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes_test.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes_test.go @@ -449,12 +449,7 @@ func TestDetermineVersions(t *testing.T) { { name: "no workload", manifests: "testdata/version_no_workload.yaml", - expected: []*model.ArtifactVersion{ - { - Kind: model.ArtifactVersion_UNKNOWN, - Version: versionUnknown, - }, - }, + expected: []*model.ArtifactVersion{}, }, { name: "single container", From 3275aa5589c905b54c008a69fe037530c0725eb1 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 19:26:48 +0900 Subject: [PATCH 10/12] Fix proto field number --- pkg/model/deployment.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/model/deployment.proto b/pkg/model/deployment.proto index 5015ea30f3..e4ff23d813 100644 --- a/pkg/model/deployment.proto +++ b/pkg/model/deployment.proto @@ -77,7 +77,7 @@ message Deployment { // e.g. Update image from v1.5.0 to v1.6.0. string summary = 22; string version = 23; - repeated ArtifactVersion versions = 103; + repeated ArtifactVersion versions = 24; // Hash value of the most recently successfully deployed commit. string running_commit_hash = 21; From f969aa8d687497521434739c45d1a73ba63376a4 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 19:31:12 +0900 Subject: [PATCH 11/12] Move initialization versions slice --- pkg/app/piped/planner/kubernetes/kubernetes.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 33a607d0da..1f54dc20b5 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -496,9 +496,7 @@ func determineVersion(manifests []provider.Manifest) (string, error) { // determineVersions decides artifact versions of an application. // It finds all container images that are being specified in the workload manifests then returns their names, version numbers, and urls. func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, error) { - versions := []*model.ArtifactVersion{} imageMap := map[string]struct{}{} - for _, m := range manifests { // TODO: Determine container image version from other workload kinds such as StatefulSet, Pod, Daemon, CronJob... if !m.Key.IsDeployment() { @@ -520,6 +518,7 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, } } + versions := make([]*model.ArtifactVersion, 0, len(imageMap)) for i := range imageMap { image := parseContainerImage(i) versions = append(versions, &model.ArtifactVersion{ From c23e18a9c8aac5e9fbd39fdc51a54256d4b58f40 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Fri, 4 Mar 2022 19:49:08 +0900 Subject: [PATCH 12/12] Fix comment --- pkg/app/piped/planner/kubernetes/kubernetes.go | 2 +- pkg/model/deployment.proto | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/app/piped/planner/kubernetes/kubernetes.go b/pkg/app/piped/planner/kubernetes/kubernetes.go index 1f54dc20b5..b415b630cb 100644 --- a/pkg/app/piped/planner/kubernetes/kubernetes.go +++ b/pkg/app/piped/planner/kubernetes/kubernetes.go @@ -512,7 +512,7 @@ func determineVersions(manifests []provider.Manifest) ([]*model.ArtifactVersion, } containers := d.Spec.Template.Spec.Containers - // remove duplicate images on multiple manifests + // Remove duplicate images on multiple manifests. for _, c := range containers { imageMap[c.Image] = struct{}{} } diff --git a/pkg/model/deployment.proto b/pkg/model/deployment.proto index e4ff23d813..596c483865 100644 --- a/pkg/model/deployment.proto +++ b/pkg/model/deployment.proto @@ -76,6 +76,7 @@ message Deployment { // e.g. Scale from 10 to 100 replicas. // e.g. Update image from v1.5.0 to v1.6.0. string summary = 22; + // TODO: Remove version from deployment model. string version = 23; repeated ArtifactVersion versions = 24;