From 6addc470c606069e687fdf218b4799c6d4701169 Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Mon, 10 Dec 2018 22:46:54 +0000 Subject: [PATCH 1/5] #323 optional cluster name override --- trigger/pubsub/util.go | 27 +++++++++++++++------------ trigger/pubsub/util_test.go | 13 +++++++++++-- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/trigger/pubsub/util.go b/trigger/pubsub/util.go index 15f2bb64..40a7feb4 100644 --- a/trigger/pubsub/util.go +++ b/trigger/pubsub/util.go @@ -11,23 +11,26 @@ import ( // MetadataEndpoint - default metadata server for gcloud pubsub const MetadataEndpoint = "http://metadata/computeMetadata/v1/instance/attributes/cluster-name" -func containerRegistrySubName(projectID, topic string) string { - cluster := "unknown" - clusterName, err := clusterName(MetadataEndpoint) - if err != nil { - log.WithFields(log.Fields{ - "error": err, - "metadata_endpoint": MetadataEndpoint, - }).Warn("trigger.pubsub.containerRegistrySubName: got error while retrieving cluster metadata, messages might be lost if more than one Keel instance is created") - } else { - cluster = clusterName +func containerRegistrySubName(clusterName, projectID, topic string) string { + + if clusterName == "" { + var err error + clusterName, err = getClusterName(MetadataEndpoint) + if err != nil { + log.WithFields(log.Fields{ + "error": err, + "metadata_endpoint": MetadataEndpoint, + }).Warn("trigger.pubsub.containerRegistrySubName: got error while retrieving cluster metadata, messages might be lost if more than one Keel instance is created") + } else { + clusterName = "unknown" + } } - return "keel-" + cluster + "-" + projectID + "-" + topic + return "keel-" + clusterName + "-" + projectID + "-" + topic } // https://cloud.google.com/compute/docs/storing-retrieving-metadata -func clusterName(metadataEndpoint string) (string, error) { +func getClusterName(metadataEndpoint string) (string, error) { req, err := http.NewRequest(http.MethodGet, metadataEndpoint, nil) if err != nil { return "", err diff --git a/trigger/pubsub/util_test.go b/trigger/pubsub/util_test.go index e89178f7..491079b8 100644 --- a/trigger/pubsub/util_test.go +++ b/trigger/pubsub/util_test.go @@ -63,7 +63,7 @@ func TestClusterName(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(handler)) defer ts.Close() - name, err := clusterName(ts.URL) + name, err := getClusterName(ts.URL) if err != nil { t.Errorf("unexpected error while getting cluster name") } @@ -75,9 +75,18 @@ func TestClusterName(t *testing.T) { func TestGetContainerRegistryURI(t *testing.T) { - name := containerRegistrySubName("project-1", "topic-1") + name := containerRegistrySubName("", "project-1", "topic-1") if name != "keel-unknown-project-1-topic-1" { t.Errorf("unexpected topic name: %s", name) } } + +func TestGetContainerRegistryURIWithClusterNameSet(t *testing.T) { + + name := containerRegistrySubName("testxxx", "project-1", "topic-1") + + if name != "keel-testxxx-project-1-topic-1" { + t.Errorf("unexpected topic name: %s", name) + } +} From ebc5bb472ed00c9c86e1450e0734fca5fc1e594e Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Mon, 10 Dec 2018 22:47:04 +0000 Subject: [PATCH 2/5] #323 passing in cluster name to the manager --- trigger/pubsub/manager.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/trigger/pubsub/manager.go b/trigger/pubsub/manager.go index a4794a37..18655e18 100644 --- a/trigger/pubsub/manager.go +++ b/trigger/pubsub/manager.go @@ -25,6 +25,11 @@ type DefaultManager struct { // projectID is required to correctly set GCR subscriptions projectID string + // clusterName is used to create unique names for the subscriptions. Each subscription + // has to have a unique name in order to receive all events (otherwise, if it is the same, + // only 1 keel instance will receive a GCR event after a push event) + clusterName string + // scanTick - scan interval in seconds, defaults to 60 seconds scanTick int @@ -39,11 +44,12 @@ type Subscriber interface { } // NewDefaultManager - creates new pubsub manager to create subscription for deployments -func NewDefaultManager(projectID string, providers provider.Providers, subClient Subscriber) *DefaultManager { +func NewDefaultManager(clusterName, projectID string, providers provider.Providers, subClient Subscriber) *DefaultManager { return &DefaultManager{ providers: providers, client: subClient, projectID: projectID, + clusterName: clusterName, subscribers: make(map[string]context.Context), mu: &sync.Mutex{}, scanTick: 60, @@ -116,7 +122,7 @@ func (s *DefaultManager) ensureSubscription(gcrURI string) { if !ok { ctx, cancel := context.WithCancel(s.ctx) s.subscribers[gcrURI] = ctx - subName := containerRegistrySubName(s.projectID, gcrURI) + subName := containerRegistrySubName(s.clusterName, s.projectID, gcrURI) go func() { defer cancel() err := s.client.Subscribe(s.ctx, gcrURI, subName) From 3b8a98b6c3090e0fff6827910dcab10c6644bbd4 Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Mon, 10 Dec 2018 22:47:30 +0000 Subject: [PATCH 3/5] #323 new CLUSTER_NAME env variable for setting up cluster name --- cmd/keel/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/keel/main.go b/cmd/keel/main.go index 68e5806e..d6f5c07a 100644 --- a/cmd/keel/main.go +++ b/cmd/keel/main.go @@ -53,6 +53,7 @@ const ( EnvTriggerPubSub = "PUBSUB" // set to 1 or something to enable pub/sub trigger EnvTriggerPoll = "POLL" // set to 1 or something to enable poll trigger EnvProjectID = "PROJECT_ID" + EnvClusterName = "CLUSTER_NAME" EnvNamespace = "NAMESPACE" // Keel's namespace @@ -298,7 +299,7 @@ func setupTriggers(ctx context.Context, providers provider.Providers, approvalsM return } - subManager := pubsub.NewDefaultManager(projectID, providers, ps) + subManager := pubsub.NewDefaultManager(os.Getenv(EnvClusterName), projectID, providers, ps) go subManager.Start(ctx) } From ed40874c462ee8fde5e4f4b637fea7466ff7c64e Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Tue, 11 Dec 2018 00:17:18 +0000 Subject: [PATCH 4/5] #323 setting name to 'unknown' if we fail to get the metadata --- trigger/pubsub/util.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/trigger/pubsub/util.go b/trigger/pubsub/util.go index 40a7feb4..e8347964 100644 --- a/trigger/pubsub/util.go +++ b/trigger/pubsub/util.go @@ -17,12 +17,11 @@ func containerRegistrySubName(clusterName, projectID, topic string) string { var err error clusterName, err = getClusterName(MetadataEndpoint) if err != nil { + clusterName = "unknown" log.WithFields(log.Fields{ "error": err, "metadata_endpoint": MetadataEndpoint, }).Warn("trigger.pubsub.containerRegistrySubName: got error while retrieving cluster metadata, messages might be lost if more than one Keel instance is created") - } else { - clusterName = "unknown" } } From 2e64effc7b45dff021ddf56e0757ae90e318ddd4 Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Tue, 11 Dec 2018 23:21:27 +0000 Subject: [PATCH 5/5] cleanup --- cache/memory/memory.go | 7 ------- provider/kubernetes/approvals.go | 3 --- 2 files changed, 10 deletions(-) diff --git a/cache/memory/memory.go b/cache/memory/memory.go index 7313fa8e..2f444a7d 100644 --- a/cache/memory/memory.go +++ b/cache/memory/memory.go @@ -5,8 +5,6 @@ import ( "sync" "github.com/keel-hq/keel/cache" - - log "github.com/sirupsen/logrus" ) type Cache struct { @@ -59,11 +57,6 @@ func (c *Cache) List(prefix string) (map[string][]byte, error) { dst := make([]byte, len(v)) copy(dst, v) values[k] = dst - - log.WithFields(log.Fields{ - "KEY": k, - "VALUE": string(dst), - }).Info("CACHE LIST") } } diff --git a/provider/kubernetes/approvals.go b/provider/kubernetes/approvals.go index 79dfce9a..d6eec43e 100644 --- a/provider/kubernetes/approvals.go +++ b/provider/kubernetes/approvals.go @@ -95,9 +95,6 @@ func (p *Provider) isApproved(event *types.Event, plan *UpdatePlan) (bool, error approval.Delta(), ) - // fmt.Println("requesting approval, identifier: ", plan.Resource.Namespace) - fmt.Println("requesting approval, identifier: ", identifier) - return false, p.approvalManager.Create(approval) }