From e11815c22e36f13a92869b338e87059050dd4e40 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 27 Dec 2018 15:34:19 +0200 Subject: [PATCH 1/4] Use ClusterIP service name for connecting to memcached - replace memcached headless service with ClusterIP - make Flux use the memcached ClusterIP name instead of SRV records --- chart/flux/templates/memcached.yaml | 1 - deploy/memcache-svc.yaml | 3 --- registry/cache/memcached/memcached.go | 18 ++---------------- 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/chart/flux/templates/memcached.yaml b/chart/flux/templates/memcached.yaml index c13a3fecc..639359e3b 100755 --- a/chart/flux/templates/memcached.yaml +++ b/chart/flux/templates/memcached.yaml @@ -60,7 +60,6 @@ metadata: release: {{ .Release.Name }} heritage: {{ .Release.Service }} spec: - clusterIP: None ports: - port: 11211 targetPort: memcached diff --git a/deploy/memcache-svc.yaml b/deploy/memcache-svc.yaml index 284254a39..d90f0cd65 100644 --- a/deploy/memcache-svc.yaml +++ b/deploy/memcache-svc.yaml @@ -4,9 +4,6 @@ kind: Service metadata: name: memcached spec: - # The memcache client uses DNS to get a list of memcached servers and then - # uses a consistent hash of the key to determine which server to pick. - clusterIP: None ports: - name: memcached port: 11211 diff --git a/registry/cache/memcached/memcached.go b/registry/cache/memcached/memcached.go index e8dfd6b68..d6e8b1fb1 100644 --- a/registry/cache/memcached/memcached.go +++ b/registry/cache/memcached/memcached.go @@ -14,8 +14,6 @@ package memcached import ( "encoding/binary" "fmt" - "net" - "sort" "sync" "time" @@ -158,20 +156,8 @@ func (c *MemcacheClient) updateLoop(updateInterval time.Duration) { } } -// updateMemcacheServers sets a memcache server list from SRV records. SRV -// priority & weight are ignored. +// updateMemcacheServers sets the memcache server address using the K8s service name func (c *MemcacheClient) updateMemcacheServers() error { - _, addrs, err := net.LookupSRV(c.service, "tcp", c.hostname) - if err != nil { - return err - } - var servers []string - for _, srv := range addrs { - servers = append(servers, fmt.Sprintf("%s:%d", srv.Target, srv.Port)) - } - // ServerList deterministically maps keys to _index_ of the server list. - // Since DNS returns records in different order each time, we sort to - // guarantee best possible match between nodes. - sort.Strings(servers) + servers := []string{fmt.Sprintf("%s:11211", c.hostname)} return c.serverList.SetServers(servers...) } From bc8b47ccc75a45bfcfb46346d0db0c8452b2f942 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 27 Dec 2018 17:02:07 +0200 Subject: [PATCH 2/4] Use memcached ClusterIP name when memcached-service flag is empty --- cmd/fluxd/main.go | 14 ++++++++++++-- registry/cache/memcached/memcached.go | 18 ++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/cmd/fluxd/main.go b/cmd/fluxd/main.go index f41a5fd4d..4444559d5 100644 --- a/cmd/fluxd/main.go +++ b/cmd/fluxd/main.go @@ -282,14 +282,24 @@ func main() { { // Cache client, for use by registry and cache warmer var cacheClient cache.Client - memcacheClient := registryMemcache.NewMemcacheClient(registryMemcache.MemcacheConfig{ + var memcacheClient *registryMemcache.MemcacheClient + memcacheConfig := registryMemcache.MemcacheConfig{ Host: *memcachedHostname, Service: *memcachedService, Timeout: *memcachedTimeout, UpdateInterval: 1 * time.Minute, Logger: log.With(logger, "component", "memcached"), MaxIdleConns: *registryBurst, - }) + } + + // if no memcached service is specified use the ClusterIP name instead of SRV records + if *memcachedService == "" { + memcacheClient = registryMemcache.NewFixedServerMemcacheClient(memcacheConfig, + fmt.Sprintf("%s:11211", *memcachedHostname)) + } else { + memcacheClient = registryMemcache.NewMemcacheClient(memcacheConfig) + } + defer memcacheClient.Stop() cacheClient = cache.InstrumentClient(memcacheClient) diff --git a/registry/cache/memcached/memcached.go b/registry/cache/memcached/memcached.go index d6e8b1fb1..e8dfd6b68 100644 --- a/registry/cache/memcached/memcached.go +++ b/registry/cache/memcached/memcached.go @@ -14,6 +14,8 @@ package memcached import ( "encoding/binary" "fmt" + "net" + "sort" "sync" "time" @@ -156,8 +158,20 @@ func (c *MemcacheClient) updateLoop(updateInterval time.Duration) { } } -// updateMemcacheServers sets the memcache server address using the K8s service name +// updateMemcacheServers sets a memcache server list from SRV records. SRV +// priority & weight are ignored. func (c *MemcacheClient) updateMemcacheServers() error { - servers := []string{fmt.Sprintf("%s:11211", c.hostname)} + _, addrs, err := net.LookupSRV(c.service, "tcp", c.hostname) + if err != nil { + return err + } + var servers []string + for _, srv := range addrs { + servers = append(servers, fmt.Sprintf("%s:%d", srv.Target, srv.Port)) + } + // ServerList deterministically maps keys to _index_ of the server list. + // Since DNS returns records in different order each time, we sort to + // guarantee best possible match between nodes. + sort.Strings(servers) return c.serverList.SetServers(servers...) } From b100474efb24cd04e5edd3dd6613f9ec732f685b Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 27 Dec 2018 17:05:20 +0200 Subject: [PATCH 3/4] Make memcached ClusterIP optional in Helm chart --- chart/flux/templates/deployment.yaml | 3 +++ chart/flux/templates/memcached.yaml | 3 +++ chart/flux/values.yaml | 1 + deploy/flux-deployment.yaml | 7 +++++-- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/chart/flux/templates/deployment.yaml b/chart/flux/templates/deployment.yaml index e2197f954..8c538ee69 100644 --- a/chart/flux/templates/deployment.yaml +++ b/chart/flux/templates/deployment.yaml @@ -76,6 +76,9 @@ spec: - --ssh-keygen-dir=/var/fluxd/keygen - --k8s-secret-name={{ template "flux.fullname" . }}-git-deploy - --memcached-hostname={{ template "flux.fullname" . }}-memcached + {{- if .Values.memcached.createClusterIP }} + - --memcached-service= + {{- end }} - --git-url={{ .Values.git.url }} - --git-branch={{ .Values.git.branch }} - --git-path={{ .Values.git.path }} diff --git a/chart/flux/templates/memcached.yaml b/chart/flux/templates/memcached.yaml index 639359e3b..8800576f2 100755 --- a/chart/flux/templates/memcached.yaml +++ b/chart/flux/templates/memcached.yaml @@ -60,6 +60,9 @@ metadata: release: {{ .Release.Name }} heritage: {{ .Release.Service }} spec: + {{- if eq .Values.memcached.createClusterIP false }} + clusterIP: None + {{- end }} ports: - port: 11211 targetPort: memcached diff --git a/chart/flux/values.yaml b/chart/flux/values.yaml index ca4646908..704119385 100644 --- a/chart/flux/values.yaml +++ b/chart/flux/values.yaml @@ -126,6 +126,7 @@ registry: memcached: repository: memcached tag: 1.4.25 + createClusterIP: true verbose: false maxItemSize: 1m maxMemory: 64 diff --git a/deploy/flux-deployment.yaml b/deploy/flux-deployment.yaml index c50665750..804f59ab6 100644 --- a/deploy/flux-deployment.yaml +++ b/deploy/flux-deployment.yaml @@ -17,7 +17,7 @@ spec: labels: name: flux spec: - serviceAccount: flux + serviceAccountName: flux volumes: - name: git-key secret: @@ -93,7 +93,10 @@ spec: # or with a different service name, you can supply these # following two arguments to tell fluxd how to connect to it. # - --memcached-hostname=memcached.default.svc.cluster.local - # - --memcached-service=memcached + + # use the memcached ClusterIP service name by setting the + # memcached-service to string empty + - --memcached-service= # this must be supplied, and be in the tmpfs (emptyDir) # mounted above, for K8s >= 1.10 From 23956ee37f908d4c4b9e17fdc650d117dd06eb78 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 27 Dec 2018 18:36:02 +0200 Subject: [PATCH 4/4] Add chart upgrade instruction for memcached changes --- chart/flux/CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/chart/flux/CHANGELOG.md b/chart/flux/CHANGELOG.md index 09c871ad7..a98f84318 100644 --- a/chart/flux/CHANGELOG.md +++ b/chart/flux/CHANGELOG.md @@ -1,3 +1,20 @@ +## 0.6.0 (TBA) + +**Note** To fix the connectivity problems between Flux and memcached we've changed the +memcached service from headless to ClusterIP. This change will make the Helm upgrade fail +with `ClusterIP field is immutable`. + +Before upgrading to 0.6.0 you have to delete the memcached headless service: + +```bash +kubectl -n flux delete svc flux-memcached +``` + +### Improvements + + - Use ClusterIP service name for connecting to memcached + [weaveworks/flux#1618](https://github.com/weaveworks/flux/pull/1618) + ## 0.5.2 (2018-12-20) ### Improvements