resetting approvals on update

provider/kubernetes/approvals.go

@@ -35,6 +35,11 @@ func (p *Provider) checkForApprovals(event *types.Event, plans []*UpdatePlan) (a
 	return approvedPlans
 }
 
+// updateComplete is called after we successfully update resource
+func (p *Provider) updateComplete(plan *UpdatePlan) error {
+	return p.approvalManager.Delete(getApprovalIdentifier(plan.Resource.Identifier, plan.NewVersion))
+}
+
 func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error) {
 	labels := plan.Resource.GetLabels()
 
@@ -64,7 +69,6 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error
 		}
 	}
 
-	// identifier := getIdentifier(plan.Resource.Namespace, plan.Resource.Name, plan.NewVersion)
 	identifier := getApprovalIdentifier(plan.Resource.Identifier, plan.NewVersion)
 
 	// checking for existing approval
@@ -100,13 +104,17 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error
 		return false, err
 	}
 
-	if event.Repository.Digest != "" && event.Repository.Digest != existing.Digest {
+	// if event.Repository.Digest != "" && event.Repository.Digest != existing.Digest {
-		err = p.approvalManager.Reset(existing)
+	// 	err = p.approvalManager.Reset(existing)
-		if err != nil {
+	// 	if err != nil {
-			return false, fmt.Errorf("failed to reset approval after changed digest, error %s", err)
+	// 		return false, fmt.Errorf("failed to reset approval after changed digest, error %s", err)
-		}
+	// 	}
-		return false, nil
+	// 	return false, nil
-	}
+	// }
+	// log.WithFields(log.Fields{
+	// 	"previous": existing.Digest,
+	// 	"new":      event.Repository.Digest,
+	// }).Info("digests match")
 
 	return existing.Status() == types.ApprovalStatusApproved, nil
 }

provider/kubernetes/approvals_test.go

@@ -162,3 +162,94 @@ func TestApprovedCheck(t *testing.T) {
 		t.Errorf("expected to find 1 updated deployment but found %d", len(deps))
 	}
 }
+
+func TestApprovalsCleanup(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{},
+			},
+		},
+	}
+	deployments := []*apps_v1.Deployment{
+		{
+			meta_v1.TypeMeta{},
+			meta_v1.ObjectMeta{
+				Name:        "dep-1",
+				Namespace:   "xxxx",
+				Labels:      map[string]string{types.KeelPolicyLabel: "all", types.KeelMinimumApprovalsLabel: "1"},
+				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:1.1.1",
+							},
+						},
+					},
+				},
+			},
+			apps_v1.DeploymentStatus{},
+		},
+	}
+	grs := MustParseGRS(deployments)
+	grc := &k8s.GenericResourceCache{}
+	grc.Add(grs...)
+
+	approver := approver()
+	provider, err := NewProvider(fp, &fakeSender{}, approver, grc)
+	if err != nil {
+		t.Fatalf("failed to get provider: %s", err)
+	}
+
+	// approving event
+	err = provider.approvalManager.Create(&types.Approval{
+		Identifier:    "deployment/xxxx/dep-1:1.1.2",
+		VotesReceived: 2,
+		VotesRequired: 2,
+		Deadline:      time.Now().Add(10 * time.Second),
+	})
+	if err != nil {
+		t.Fatalf("failed to create approval: %s", err)
+	}
+
+	appr, err := provider.approvalManager.Get("deployment/xxxx/dep-1:1.1.2")
+	if err != nil {
+		t.Fatalf("failed to get approval: %s", err)
+	}
+	if appr.Status() != types.ApprovalStatusApproved {
+		t.Fatalf("approval not approved")
+	}
+
+	// creating "new version" event
+	repo := types.Repository{
+		Name: "gcr.io/v2-namespace/hello-world",
+		Tag:  "1.1.2",
+	}
+
+	deps, err := provider.processEvent(&types.Event{Repository: repo})
+	if err != nil {
+		t.Errorf("failed to get deployments: %s", err)
+	}
+
+	if len(deps) != 1 {
+		t.Errorf("expected to find 1 updated deployment but found %d", len(deps))
+	}
+
+	// no approvals expected
+
+	approvals, err := provider.approvalManager.List()
+	if err != nil {
+		t.Fatalf("failed to get a list of approvals: %s", err)
+	}
+
+	if len(approvals) != 0 {
+		t.Errorf("expected to find 0 but found %d", len(approvals))
+	}
+}

provider/kubernetes/kubernetes.go

@@ -273,6 +273,16 @@ func (p *Provider) updateDeployments(plans []*UpdatePlan) (updated []*k8s.Generi
 			continue
 		}
 
+		err = p.updateComplete(plan)
+		if err != nil {
+			log.WithFields(log.Fields{
+				"error":     err,
+				"name":      resource.Name,
+				"kind":      resource.Kind(),
+				"namespace": resource.Namespace,
+			}).Warn("provider.kubernetes: got error while resetting approvals counter after successful update")
+		}
+
 		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(), ", ")),
@@ -356,11 +366,12 @@ func (p *Provider) createUpdatePlans(repo *types.Repository) ([]*UpdatePlan, err
 			log.WithFields(log.Fields{
 				"error":          err,
 				"repository_tag": repo.Tag,
-				"deployment":     resource.Name,
+				"resource_kind":  resource.Kind(),
+				"resource":       resource.Name,
 				"namespace":      resource.Namespace,
 				"kind":           resource.Kind(),
 				"policy":         policy,
-			}).Warn("provider.kubernetes: got error while parsing repository tag")
+			}).Warn("provider.kubernetes: got error while parsing repository tag, consider using 'force' policy")
 			continue
 		}