diff --git a/approvals/approvals.go b/approvals/approvals.go index ecb25ca47..822399963 100644 --- a/approvals/approvals.go +++ b/approvals/approvals.go @@ -29,9 +29,6 @@ type Manager interface { // Update whole approval object Update(r *types.Approval) error - // Reset approval when votes become invalid (image digest change detected) - Reset(r *types.Approval) error - // Increases Approval votes by 1 Approve(identifier, voter string) (*types.Approval, error) // Rejects Approval @@ -267,29 +264,6 @@ func (m *DefaultManager) Update(r *types.Approval) error { return m.cache.Put(getKey(r.Identifier), bts) } -// Reset - resets votes to 0. Used by providers when image digest change is detected -func (m *DefaultManager) Reset(r *types.Approval) error { - m.mu.Lock() - defer m.mu.Unlock() - - r.Voters = []string{} - r.VotesReceived = 0 - r.UpdatedAt = time.Now() - - bts, err := m.serializer.Encode(r) - if err != nil { - return err - } - - err = m.cache.Put(getKey(r.Identifier), bts) - if err != nil { - return err - } - - // republishing event to all subscribers - return m.publishRequest(r) -} - // Approve - increase VotesReceived by 1 and returns updated version func (m *DefaultManager) Approve(identifier, voter string) (*types.Approval, error) { m.mu.Lock() diff --git a/provider/helm/approvals.go b/provider/helm/approvals.go index 1ddccff9f..a7e08d68f 100644 --- a/provider/helm/approvals.go +++ b/provider/helm/approvals.go @@ -76,13 +76,5 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error return false, err } - if event.Repository.Digest != "" && event.Repository.Digest != existing.Digest { - err = p.approvalManager.Reset(existing) - if err != nil { - return false, fmt.Errorf("failed to reset approval after changed digest, error %s", err) - } - return false, nil - } - return existing.Status() == types.ApprovalStatusApproved, nil } diff --git a/provider/helm/helm.go b/provider/helm/helm.go index ec0925e61..7455a5948 100644 --- a/provider/helm/helm.go +++ b/provider/helm/helm.go @@ -73,6 +73,9 @@ type UpdatePlan struct { CurrentVersion string // New version that's already in the deployment NewVersion string + + // ReleaseNotes is a slice of combined release notes. + ReleaseNotes []string } // keel: @@ -107,6 +110,7 @@ type ImageDetails struct { RepositoryPath string `json:"repository"` TagPath string `json:"tag"` DigestPath string `json:"digest"` + ReleaseNotes string `json:"releaseNotes"` } // Provider - helm provider, responsible for managing release updates @@ -342,9 +346,16 @@ func (p *Provider) applyPlans(plans []*UpdatePlan) error { }).Warn("provider.helm: got error while resetting approvals counter after successful update") } + var msg string + if len(plan.ReleaseNotes) == 0 { + msg = fmt.Sprintf("Successfully updated release %s/%s %s->%s (%s)", plan.Namespace, plan.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(mapToSlice(plan.Values), ", ")) + } else { + msg = fmt.Sprintf("Successfully updated release %s/%s %s->%s (%s). Release notes: %s", plan.Namespace, plan.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(mapToSlice(plan.Values), ", "), strings.Join(plan.ReleaseNotes, ", ")) + } + p.sender.Send(types.EventNotification{ Name: "update release", - Message: fmt.Sprintf("Successfully updated release %s/%s %s->%s (%s)", plan.Namespace, plan.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(mapToSlice(plan.Values), ", ")), + Message: msg, CreatedAt: time.Now(), Type: types.NotificationReleaseUpdate, Level: types.LevelSuccess, diff --git a/provider/helm/unversioned_updates.go b/provider/helm/unversioned_updates.go index 8bf848eac..5c864e9d7 100644 --- a/provider/helm/unversioned_updates.go +++ b/provider/helm/unversioned_updates.go @@ -84,6 +84,9 @@ func checkUnversionedRelease(repo *types.Repository, namespace, name string, cha plan.CurrentVersion = imageRef.Tag() plan.Config = keelCfg shouldUpdateRelease = true + if imageDetails.ReleaseNotes != "" { + plan.ReleaseNotes = append(plan.ReleaseNotes, imageDetails.ReleaseNotes) + } } return plan, shouldUpdateRelease, nil diff --git a/provider/helm/unversioned_updates_test.go b/provider/helm/unversioned_updates_test.go index 8c93811df..fbc9f3994 100644 --- a/provider/helm/unversioned_updates_test.go +++ b/provider/helm/unversioned_updates_test.go @@ -25,6 +25,24 @@ keel: - repository: image.repository tag: image.tag +` + chartValuesPolicyForceReleaseNotes := ` +name: al Rashid +where: + city: Basrah + title: caliph +image: + repository: gcr.io/v2-namespace/hello-world + tag: 1.1.0 + +keel: + policy: force + trigger: poll + images: + - repository: image.repository + tag: image.tag + releaseNotes: https://github.com/keel-hq/keel/releases + ` chartValuesPolicyMajor := ` @@ -53,6 +71,10 @@ keel: Values: &hapi_chart.Config{Raw: chartValuesPolicyMajor}, } + helloWorldChartPolicyMajorReleaseNotes := &hapi_chart.Chart{ + Values: &hapi_chart.Config{Raw: chartValuesPolicyForceReleaseNotes}, + } + type args struct { repo *types.Repository namespace string @@ -97,6 +119,38 @@ keel: wantShouldUpdateRelease: true, wantErr: false, }, + { + name: "correct force update, with release notes", + args: args{ + repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.2.0"}, + namespace: "default", + name: "release-1", + chart: helloWorldChartPolicyMajorReleaseNotes, + config: &hapi_chart.Config{Raw: ""}, + }, + wantPlan: &UpdatePlan{ + Namespace: "default", + Name: "release-1", + Chart: helloWorldChartPolicyMajorReleaseNotes, + Values: map[string]string{"image.tag": "1.2.0"}, + CurrentVersion: "1.1.0", + NewVersion: "1.2.0", + ReleaseNotes: []string{"https://github.com/keel-hq/keel/releases"}, + Config: &KeelChartConfig{ + Policy: types.PolicyTypeForce, + Trigger: types.TriggerTypePoll, + Images: []ImageDetails{ + ImageDetails{ + RepositoryPath: "image.repository", + TagPath: "image.tag", + ReleaseNotes: "https://github.com/keel-hq/keel/releases", + }, + }, + }, + }, + wantShouldUpdateRelease: true, + wantErr: false, + }, { name: "update without force", args: args{ diff --git a/provider/helm/versioned_updates.go b/provider/helm/versioned_updates.go index a5b4a7556..29fd158ad 100644 --- a/provider/helm/versioned_updates.go +++ b/provider/helm/versioned_updates.go @@ -73,6 +73,9 @@ func checkVersionedRelease(newVersion *types.Version, repo *types.Repository, na plan.CurrentVersion = imageRef.Tag() plan.Config = keelCfg shouldUpdateRelease = true + if imageDetails.ReleaseNotes != "" { + plan.ReleaseNotes = append(plan.ReleaseNotes, imageDetails.ReleaseNotes) + } log.WithFields(log.Fields{ "parsed_image": imageRef.Remote(), @@ -131,6 +134,9 @@ func checkVersionedRelease(newVersion *types.Version, repo *types.Repository, na plan.CurrentVersion = currentVersion.String() plan.Config = keelCfg shouldUpdateRelease = true + if imageDetails.ReleaseNotes != "" { + plan.ReleaseNotes = append(plan.ReleaseNotes, imageDetails.ReleaseNotes) + } log.WithFields(log.Fields{ "container_image": imageRef.Repository(), diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go index ec39287ae..90822ddc1 100644 --- a/provider/kubernetes/kubernetes.go +++ b/provider/kubernetes/kubernetes.go @@ -283,9 +283,17 @@ func (p *Provider) updateDeployments(plans []*UpdatePlan) (updated []*k8s.Generi }).Warn("provider.kubernetes: got error while resetting approvals counter after successful update") } + var msg string + releaseNotes := types.ParseReleaseNotesURL(resource.GetAnnotations()) + if releaseNotes != "" { + msg = fmt.Sprintf("Successfully updated %s %s/%s %s->%s (%s). Release notes: %s", resource.Kind(), resource.Namespace, resource.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(resource.GetImages(), ", "), releaseNotes) + } else { + msg = fmt.Sprintf("Successfully updated %s %s/%s %s->%s (%s)", resource.Kind(), resource.Namespace, resource.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(resource.GetImages(), ", ")) + } + p.sender.Send(types.EventNotification{ Name: "update resource", - Message: fmt.Sprintf("Successfully updated %s %s/%s %s->%s (%s)", resource.Kind(), resource.Namespace, resource.Name, plan.CurrentVersion, plan.NewVersion, strings.Join(resource.GetImages(), ", ")), + Message: msg, CreatedAt: time.Now(), Type: types.NotificationDeploymentUpdate, Level: types.LevelSuccess, diff --git a/provider/kubernetes/kubernetes_test.go b/provider/kubernetes/kubernetes_test.go index 3b553218e..82220b615 100644 --- a/provider/kubernetes/kubernetes_test.go +++ b/provider/kubernetes/kubernetes_test.go @@ -638,6 +638,138 @@ func TestProcessEventBuildNumber(t *testing.T) { } } +func TestEventSent(t *testing.T) { + fp := &fakeImplementer{} + fp.namespaces = &v1.NamespaceList{ + Items: []v1.Namespace{ + v1.Namespace{ + meta_v1.TypeMeta{}, + meta_v1.ObjectMeta{Name: "xxxx"}, + v1.NamespaceSpec{}, + v1.NamespaceStatus{}, + }, + }, + } + deps := []*apps_v1.Deployment{ + { + meta_v1.TypeMeta{}, + meta_v1.ObjectMeta{ + Name: "deployment-1", + Namespace: "xxxx", + Labels: map[string]string{types.KeelPolicyLabel: "all"}, + Annotations: map[string]string{}, + }, + apps_v1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Image: "gcr.io/v2-namespace/hello-world:10", + }, + }, + }, + }, + }, + apps_v1.DeploymentStatus{}, + }, + } + + grs := MustParseGRS(deps) + grc := &k8s.GenericResourceCache{} + grc.Add(grs...) + + fs := &fakeSender{} + provider, err := NewProvider(fp, fs, approver(), grc) + if err != nil { + t.Fatalf("failed to get provider: %s", err) + } + + repo := types.Repository{ + Name: "gcr.io/v2-namespace/hello-world", + Tag: "11", + } + + event := &types.Event{Repository: repo} + _, err = provider.processEvent(event) + if err != nil { + t.Errorf("got error while processing event: %s", err) + } + + if fp.updated.Containers()[0].Image != repo.Name+":"+repo.Tag { + t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image) + } + + if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11)" { + t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11)' sent message, got: %s", fs.sentEvent.Message) + } +} + +func TestEventSentWithReleaseNotes(t *testing.T) { + fp := &fakeImplementer{} + fp.namespaces = &v1.NamespaceList{ + Items: []v1.Namespace{ + v1.Namespace{ + meta_v1.TypeMeta{}, + meta_v1.ObjectMeta{Name: "xxxx"}, + v1.NamespaceSpec{}, + v1.NamespaceStatus{}, + }, + }, + } + deps := []*apps_v1.Deployment{ + { + meta_v1.TypeMeta{}, + meta_v1.ObjectMeta{ + Name: "deployment-1", + Namespace: "xxxx", + Labels: map[string]string{types.KeelPolicyLabel: "all"}, + Annotations: map[string]string{types.KeelReleaseNotesURL: "https://github.com/keel-hq/keel/releases"}, + }, + apps_v1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Image: "gcr.io/v2-namespace/hello-world:10", + }, + }, + }, + }, + }, + apps_v1.DeploymentStatus{}, + }, + } + + grs := MustParseGRS(deps) + grc := &k8s.GenericResourceCache{} + grc.Add(grs...) + + fs := &fakeSender{} + provider, err := NewProvider(fp, fs, approver(), grc) + if err != nil { + t.Fatalf("failed to get provider: %s", err) + } + + repo := types.Repository{ + Name: "gcr.io/v2-namespace/hello-world", + Tag: "11", + } + + event := &types.Event{Repository: repo} + _, err = provider.processEvent(event) + if err != nil { + t.Errorf("got error while processing event: %s", err) + } + + if fp.updated.Containers()[0].Image != repo.Name+":"+repo.Tag { + t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image) + } + + if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11). Release notes: https://github.com/keel-hq/keel/releases" { + t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11). Release notes: https://github.com/keel-hq/keel/releases' sent message, got: %s", fs.sentEvent.Message) + } +} + // Test to check how many deployments are "impacted" if we have sidecar container func TestGetImpactedTwoContainersInSameDeployment(t *testing.T) { fp := &fakeImplementer{} diff --git a/types/types.go b/types/types.go index 65ac6bf5a..5d01344b4 100644 --- a/types/types.go +++ b/types/types.go @@ -52,6 +52,9 @@ const KeelApprovalDeadlineLabel = "keel.sh/approvalDeadline" // KeelApprovalDeadlineDefault - default deadline in hours const KeelApprovalDeadlineDefault = 24 +// KeelReleasePage - optional release notes URL passed on with notification +const KeelReleaseNotesURL = "keel.sh/releaseNotes" + // KeelPodDeleteDelay - optional delay betwen killing pods // during force deploy // const KeelPodDeleteDelay = "keel.sh/forceDelay" @@ -232,6 +235,14 @@ func ParseEventNotificationChannels(annotations map[string]string) []string { return channels } +func ParseReleaseNotesURL(annotations map[string]string) string { + if annotations == nil { + return "" + } + + return annotations[KeelReleaseNotesURL] +} + // ParsePodDeleteDelay - parses pod delete delay time in seconds // from a given map of annotations // func ParsePodDeleteDelay(annotations map[string]string) int64 { diff --git a/types/types_test.go b/types/types_test.go index 91abec50f..3661c8b06 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -166,3 +166,43 @@ func TestParseEventNotificationChannels(t *testing.T) { }) } } + +func TestParseReleaseNotesURL(t *testing.T) { + type args struct { + annotations map[string]string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "nil map", + args: args{}, + want: "", + }, + { + name: "empty map", + args: args{ + make(map[string]string), + }, + want: "", + }, + { + name: "link", + args: args{ + annotations: map[string]string{ + KeelReleaseNotesURL: "http://link", + }, + }, + want: "http://link", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ParseReleaseNotesURL(tt.args.annotations); got != tt.want { + t.Errorf("ParseReleaseNotesURL() = %v, want %v", got, tt.want) + } + }) + } +}