using different identifier for approvals, cleanup

keel-hq · Apr 30, 2018 · 830dcf1 · 830dcf1
1 parent 1a69c21
commit 830dcf1
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 19 deletions.
diff --git a/provider/kubernetes/approvals.go b/provider/kubernetes/approvals.go
@@ -11,8 +11,8 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func getIdentifier(namespace, name, version string) string {
-	return namespace + "/" + name + ":" + version
+func getApprovalIdentifier(resourceIdentifier, version string) string {
+	return resourceIdentifier + ":" + version
 }
 
 // checkForApprovals - filters out deployments and only passes forward approved ones
@@ -64,7 +64,8 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error
 		}
 	}
 
-	identifier := getIdentifier(plan.Resource.Namespace, plan.Resource.Name, plan.NewVersion)
+	// identifier := getIdentifier(plan.Resource.Namespace, plan.Resource.Name, plan.NewVersion)
+	identifier := getApprovalIdentifier(plan.Resource.Identifier, plan.NewVersion)
 
 	// checking for existing approval
 	existing, err := p.approvalManager.Get(identifier)
@@ -90,7 +91,8 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error
 				approval.Delta(),
 			)
 
-			fmt.Println("requesting approval, ns: ", plan.Resource.Namespace)
+			// fmt.Println("requesting approval, identifier: ", plan.Resource.Namespace)
+			fmt.Println("requesting approval, identifier: ", identifier)
 
 			return false, p.approvalManager.Create(approval)
 		}

diff --git a/provider/kubernetes/approvals_test.go b/provider/kubernetes/approvals_test.go
@@ -73,7 +73,7 @@ func TestCheckRequestedApproval(t *testing.T) {
 	}
 
 	// checking approvals
-	approval, err := provider.approvalManager.Get("xxxx/dep-1:1.1.2")
+	approval, err := provider.approvalManager.Get("deployment/xxxx/dep-1:1.1.2")
 	if err != nil {
 		t.Fatalf("failed to find approval, err: %s", err)
 	}
@@ -130,7 +130,7 @@ func TestApprovedCheck(t *testing.T) {
 
 	// approving event
 	err = provider.approvalManager.Create(&types.Approval{
-		Identifier:    "xxxx/dep-1:1.1.2",
+		Identifier:    "deployment/xxxx/dep-1:1.1.2",
 		VotesReceived: 2,
 		VotesRequired: 2,
 		Deadline:      time.Now().Add(10 * time.Second),
@@ -139,7 +139,10 @@ func TestApprovedCheck(t *testing.T) {
 		t.Fatalf("failed to create approval: %s", err)
 	}
 
-	appr, _ := provider.approvalManager.Get("xxxx/dep-1:1.1.2")
+	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")
 	}

diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go
@@ -319,7 +319,7 @@ func (p *Provider) createUpdatePlans(repo *types.Repository) ([]*UpdatePlan, err
 		policy := policies.GetPolicy(labels)
 		if policy == types.PolicyTypeNone {
 			// skip
-			log.Infof("no policy defined, skipping: %s, labels: %s", resource.Identifier, labels)
+			log.Debugf("no policy defined, skipping: %s, labels: %s", resource.Identifier, labels)
 			continue
 		}
 

diff --git a/provider/kubernetes/unversioned_updates_test.go b/provider/kubernetes/unversioned_updates_test.go
@@ -11,9 +11,8 @@ import (
 	"github.com/keel-hq/keel/types"
 	"github.com/keel-hq/keel/util/timeutil"
 
-	"k8s.io/api/core/v1"
-	// "k8s.io/api/extensions/apps_v1"
 	apps_v1 "k8s.io/api/apps/v1"
+	"k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 

diff --git a/provider/kubernetes/versioned_updates.go b/provider/kubernetes/versioned_updates.go
@@ -38,11 +38,7 @@ func (p *Provider) checkVersionedDeployment(newVersion *types.Version, policy ty
 
 	shouldUpdateDeployment = false
 
-	// for idx, c := range deployment.Spec.Template.Spec.Containers {
 	for idx, c := range resource.Containers() {
-		// Remove version if any
-		// containerImageName := versionreg.ReplaceAllString(c.Image, "")
-
 		containerImageRef, err := image.Parse(c.Image)
 		if err != nil {
 			log.WithFields(log.Fields{
@@ -141,16 +137,11 @@ func (p *Provider) checkVersionedDeployment(newVersion *types.Version, policy ty
 		}).Info("provider.kubernetes: checked version, deciding whether to update")
 
 		if shouldUpdateContainer {
-
-			// c = updateContainer(c, conatinerImageRef, newVersion.String())
-
 			if containerImageRef.Registry() == image.DefaultRegistryHostname {
 				resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.ShortName(), newVersion.String()))
 			} else {
 				resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.Repository(), newVersion.String()))
 			}
-
-			// deployment.Spec.Template.Spec.Containers[idx] = c
 			// marking this deployment for update
 			shouldUpdateDeployment = true