From 2632fe566baff7f844fbcc0eba6b380248daabc3 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 30 Mar 2016 20:12:37 -0300 Subject: [PATCH 1/4] Update Ingress status information in nginx controller --- controllers/nginx/controller.go | 110 +++++++++++++++++++++++++++++++- controllers/nginx/main.go | 22 ++++++- controllers/nginx/utils.go | 21 ++++++ 3 files changed, 148 insertions(+), 5 deletions(-) diff --git a/controllers/nginx/controller.go b/controllers/nginx/controller.go index 8f09eafea..08bd13776 100644 --- a/controllers/nginx/controller.go +++ b/controllers/nginx/controller.go @@ -30,6 +30,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/client/cache" + "k8s.io/kubernetes/pkg/client/record" client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/controller/framework" "k8s.io/kubernetes/pkg/runtime" @@ -64,6 +65,8 @@ type loadBalancerController struct { tcpConfigMap string udpConfigMap string + recorder record.EventRecorder + syncQueue *taskQueue // stopLock is used to enforce only a single call to Stop is active. @@ -77,6 +80,11 @@ type loadBalancerController struct { // newLoadBalancerController creates a controller for nginx loadbalancer func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Duration, defaultSvc, namespace, nxgConfigMapName, tcpConfigMapName, udpConfigMapName string, lbRuntimeInfo *lbInfo) (*loadBalancerController, error) { + + eventBroadcaster := record.NewBroadcaster() + eventBroadcaster.StartLogging(glog.Infof) + eventBroadcaster.StartRecordingToSink(kubeClient.Events("")) + lbc := loadBalancerController{ client: kubeClient, stopCh: make(chan struct{}), @@ -86,10 +94,33 @@ func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Dura tcpConfigMap: tcpConfigMapName, udpConfigMap: udpConfigMapName, defaultSvc: defaultSvc, + recorder: eventBroadcaster.NewRecorder(api.EventSource{Component: "loadbalancer-controller"}), } lbc.syncQueue = NewTaskQueue(lbc.sync) + ingEventHandler := framework.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + addIng := obj.(*extensions.Ingress) + lbc.recorder.Eventf(addIng, api.EventTypeNormal, "CREATE", fmt.Sprintf("%s/%s", addIng.Namespace, addIng.Name)) + lbc.updateIngressStatus(addIng) + lbc.syncQueue.enqueue(obj) + }, + DeleteFunc: func(obj interface{}) { + upIng := obj.(*extensions.Ingress) + lbc.recorder.Eventf(upIng, api.EventTypeNormal, "DELETE", fmt.Sprintf("%s/%s", upIng.Namespace, upIng.Name)) + lbc.syncQueue.enqueue(obj) + }, + UpdateFunc: func(old, cur interface{}) { + if !reflect.DeepEqual(old, cur) { + upIng := cur.(*extensions.Ingress) + lbc.recorder.Eventf(upIng, api.EventTypeNormal, "UPDATE", fmt.Sprintf("%s/%s", upIng.Namespace, upIng.Name)) + lbc.updateIngressStatus(upIng) + lbc.syncQueue.enqueue(cur) + } + }, + } + eventHandler := framework.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { lbc.syncQueue.enqueue(obj) @@ -109,7 +140,7 @@ func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Dura ListFunc: ingressListFunc(lbc.client, namespace), WatchFunc: ingressWatchFunc(lbc.client, namespace), }, - &extensions.Ingress{}, resyncPeriod, eventHandler) + &extensions.Ingress{}, resyncPeriod, ingEventHandler) lbc.endpLister.Store, lbc.endpController = framework.NewInformer( &cache.ListWatch{ @@ -206,6 +237,39 @@ func (lbc *loadBalancerController) sync(key string) { }) } +func (lbc *loadBalancerController) updateIngressStatus(ing *extensions.Ingress) { + ingClient := lbc.client.Extensions().Ingress(ing.Namespace) + currIng, err := ingClient.Get(ing.Name) + if err != nil { + glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) + return + } + + lbIPs := ing.Status.LoadBalancer.Ingress + if len(lbIPs) > 0 && !lbc.isStatusIPDefined(lbIPs) { + glog.Infof("Updating loadbalancer %v/%v with IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) + currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress, api.LoadBalancerIngress{ + IP: lbc.lbInfo.Address, + }) + if _, err := ingClient.UpdateStatus(currIng); err != nil { + lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) + return + } + + lbc.recorder.Eventf(currIng, api.EventTypeNormal, "CREATE", "ip: %v", lbc.lbInfo.Address) + } +} + +func (lbc *loadBalancerController) isStatusIPDefined(lbings []api.LoadBalancerIngress) bool { + for _, lbing := range lbings { + if lbing.IP == lbc.lbInfo.Address { + return true + } + } + + return false +} + func (lbc *loadBalancerController) getTCPServices() []*nginx.Location { if lbc.tcpConfigMap == "" { // no configmap for TCP services @@ -571,17 +635,59 @@ func (lbc *loadBalancerController) getEndpoints(s *api.Service, servicePort ints } // Stop stops the loadbalancer controller. -func (lbc *loadBalancerController) Stop() { +func (lbc *loadBalancerController) Stop() error { // Stop is invoked from the http endpoint. lbc.stopLock.Lock() defer lbc.stopLock.Unlock() // Only try draining the workqueue if we haven't already. if !lbc.shutdown { + + lbc.removeFromIngress() + close(lbc.stopCh) glog.Infof("shutting down controller queues") lbc.shutdown = true lbc.syncQueue.shutdown() + + return nil + } + + return fmt.Errorf("shutdown already in progress") +} + +func (lbc *loadBalancerController) removeFromIngress() { + ings := lbc.ingLister.Store.List() + glog.Infof("updating %v Ingress rule/s", len(ings)) + for _, cur := range ings { + ing := cur.(*extensions.Ingress) + + ingClient := lbc.client.Extensions().Ingress(ing.Namespace) + currIng, err := ingClient.Get(ing.Name) + if err != nil { + glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) + continue + } + + lbIPs := ing.Status.LoadBalancer.Ingress + if len(lbIPs) > 0 && lbc.isStatusIPDefined(lbIPs) { + glog.Infof("Updating loadbalancer %v/%v. Removing IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) + + for idx, lbStatus := range currIng.Status.LoadBalancer.Ingress { + if lbStatus.IP == lbc.lbInfo.Address { + currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress[:idx], + currIng.Status.LoadBalancer.Ingress[idx+1:]...) + break + } + } + + if _, err := ingClient.UpdateStatus(currIng); err != nil { + lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) + continue + } + + lbc.recorder.Eventf(currIng, api.EventTypeNormal, "DELETE", "ip: %v", lbc.lbInfo.Address) + } } } diff --git a/controllers/nginx/main.go b/controllers/nginx/main.go index 0511f49e5..60d94dc65 100644 --- a/controllers/nginx/main.go +++ b/controllers/nginx/main.go @@ -22,6 +22,8 @@ import ( "net/http" "net/http/pprof" "os" + "os/signal" + "syscall" "time" "github.com/golang/glog" @@ -32,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/healthz" - "k8s.io/kubernetes/pkg/runtime" ) const ( @@ -111,6 +112,7 @@ func main() { } go registerHandlers(lbc) + go handleSigterm(lbc) lbc.Run() @@ -122,11 +124,10 @@ func main() { // lbInfo contains runtime information about the pod type lbInfo struct { - ObjectName string - DeployType runtime.Object Podname string PodIP string PodNamespace string + Address string } func registerHandlers(lbc *loadBalancerController) { @@ -149,3 +150,18 @@ func registerHandlers(lbc *loadBalancerController) { } glog.Fatal(server.ListenAndServe()) } + +func handleSigterm(lbc *loadBalancerController) { + signalChan := make(chan os.Signal, 1) + signal.Notify(signalChan, syscall.SIGTERM) + <-signalChan + glog.Infof("Received SIGTERM, shutting down") + + exitCode := 0 + if err := lbc.Stop(); err != nil { + glog.Infof("Error during shutdown %v", err) + exitCode = 1 + } + glog.Infof("Exiting with %v", exitCode) + os.Exit(exitCode) +} diff --git a/controllers/nginx/utils.go b/controllers/nginx/utils.go index 122a3d810..eaf5b7423 100644 --- a/controllers/nginx/utils.go +++ b/controllers/nginx/utils.go @@ -24,6 +24,7 @@ import ( "github.com/golang/glog" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/cache" "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/util/wait" @@ -108,10 +109,30 @@ func getLBDetails(kubeClient *unversioned.Client) (*lbInfo, error) { return nil, fmt.Errorf("Unable to get POD information") } + node, err := kubeClient.Nodes().Get(pod.Spec.NodeName) + if err != nil { + return nil, err + } + + var externalIP string + for _, address := range node.Status.Addresses { + if address.Type == api.NodeExternalIP { + if address.Address != "" { + externalIP = address.Address + break + } + } + + if externalIP == "" && address.Type == api.NodeLegacyHostIP { + externalIP = address.Address + } + } + return &lbInfo{ PodIP: podIP, Podname: podName, PodNamespace: podNs, + Address: externalIP, }, nil } From 60e2e5f9ad8b842c87534085d37e79246b392413 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 31 Mar 2016 14:59:28 -0300 Subject: [PATCH 2/4] Avoid sync Ingress updates --- controllers/nginx/controller.go | 51 +++++++++++++------ .../rc-custom-configuration.yaml | 2 +- .../examples/daemonset/as-daemonset.yaml | 2 +- .../nginx/examples/default/rc-default.yaml | 2 +- controllers/nginx/examples/full/rc-full.yaml | 2 +- controllers/nginx/examples/tcp/rc-tcp.yaml | 2 +- controllers/nginx/examples/tls/rc-ssl.yaml | 2 +- controllers/nginx/examples/udp/rc-udp.yaml | 2 +- 8 files changed, 42 insertions(+), 23 deletions(-) diff --git a/controllers/nginx/controller.go b/controllers/nginx/controller.go index 08bd13776..55d696238 100644 --- a/controllers/nginx/controller.go +++ b/controllers/nginx/controller.go @@ -68,6 +68,7 @@ type loadBalancerController struct { recorder record.EventRecorder syncQueue *taskQueue + ingQueue *taskQueue // stopLock is used to enforce only a single call to Stop is active. // Needed because we allow stopping through an http endpoint and @@ -98,12 +99,13 @@ func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Dura } lbc.syncQueue = NewTaskQueue(lbc.sync) + lbc.ingQueue = NewTaskQueue(lbc.syncIngress) ingEventHandler := framework.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { addIng := obj.(*extensions.Ingress) lbc.recorder.Eventf(addIng, api.EventTypeNormal, "CREATE", fmt.Sprintf("%s/%s", addIng.Namespace, addIng.Name)) - lbc.updateIngressStatus(addIng) + lbc.ingQueue.enqueue(obj) lbc.syncQueue.enqueue(obj) }, DeleteFunc: func(obj interface{}) { @@ -115,7 +117,7 @@ func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Dura if !reflect.DeepEqual(old, cur) { upIng := cur.(*extensions.Ingress) lbc.recorder.Eventf(upIng, api.EventTypeNormal, "UPDATE", fmt.Sprintf("%s/%s", upIng.Namespace, upIng.Name)) - lbc.updateIngressStatus(upIng) + lbc.ingQueue.enqueue(cur) lbc.syncQueue.enqueue(cur) } }, @@ -237,26 +239,42 @@ func (lbc *loadBalancerController) sync(key string) { }) } -func (lbc *loadBalancerController) updateIngressStatus(ing *extensions.Ingress) { - ingClient := lbc.client.Extensions().Ingress(ing.Namespace) - currIng, err := ingClient.Get(ing.Name) - if err != nil { - glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) +func (lbc *loadBalancerController) syncIngress(key string) { + if !lbc.controllersInSync() { + lbc.ingQueue.requeue(key, fmt.Errorf("deferring sync till endpoints controller has synced")) return } - lbIPs := ing.Status.LoadBalancer.Ingress - if len(lbIPs) > 0 && !lbc.isStatusIPDefined(lbIPs) { - glog.Infof("Updating loadbalancer %v/%v with IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) - currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress, api.LoadBalancerIngress{ - IP: lbc.lbInfo.Address, - }) - if _, err := ingClient.UpdateStatus(currIng); err != nil { - lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) + obj, ingExists, err := lbc.ingLister.Store.GetByKey(key) + if err != nil { + lbc.ingQueue.requeue(key, err) + return + } + + if ingExists { + ing := obj.(*extensions.Ingress) + + ingClient := lbc.client.Extensions().Ingress(ing.Namespace) + + currIng, err := ingClient.Get(ing.Name) + if err != nil { + glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) return } - lbc.recorder.Eventf(currIng, api.EventTypeNormal, "CREATE", "ip: %v", lbc.lbInfo.Address) + lbIPs := ing.Status.LoadBalancer.Ingress + if len(lbIPs) > 0 && !lbc.isStatusIPDefined(lbIPs) { + glog.Infof("Updating loadbalancer %v/%v with IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) + currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress, api.LoadBalancerIngress{ + IP: lbc.lbInfo.Address, + }) + if _, err := ingClient.UpdateStatus(currIng); err != nil { + lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) + return + } + + lbc.recorder.Eventf(currIng, api.EventTypeNormal, "CREATE", "ip: %v", lbc.lbInfo.Address) + } } } @@ -701,6 +719,7 @@ func (lbc *loadBalancerController) Run() { go lbc.svcController.Run(lbc.stopCh) go lbc.syncQueue.run(time.Second, lbc.stopCh) + go lbc.ingQueue.run(time.Second, lbc.stopCh) <-lbc.stopCh glog.Infof("shutting down NGINX loadbalancer controller") diff --git a/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml b/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml index 8e5f92655..aef3d16f3 100644 --- a/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml +++ b/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml @@ -15,7 +15,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/daemonset/as-daemonset.yaml b/controllers/nginx/examples/daemonset/as-daemonset.yaml index e029c53bb..dc3b37f6b 100644 --- a/controllers/nginx/examples/daemonset/as-daemonset.yaml +++ b/controllers/nginx/examples/daemonset/as-daemonset.yaml @@ -9,7 +9,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/default/rc-default.yaml b/controllers/nginx/examples/default/rc-default.yaml index 73f377742..459865753 100644 --- a/controllers/nginx/examples/default/rc-default.yaml +++ b/controllers/nginx/examples/default/rc-default.yaml @@ -15,7 +15,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/full/rc-full.yaml b/controllers/nginx/examples/full/rc-full.yaml index 2e406f7f4..f6d6e9580 100644 --- a/controllers/nginx/examples/full/rc-full.yaml +++ b/controllers/nginx/examples/full/rc-full.yaml @@ -20,7 +20,7 @@ spec: secret: secretName: dhparam-example containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/tcp/rc-tcp.yaml b/controllers/nginx/examples/tcp/rc-tcp.yaml index 61e6b7a37..a38bf7145 100644 --- a/controllers/nginx/examples/tcp/rc-tcp.yaml +++ b/controllers/nginx/examples/tcp/rc-tcp.yaml @@ -15,7 +15,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/tls/rc-ssl.yaml b/controllers/nginx/examples/tls/rc-ssl.yaml index 9ef3b864d..798539860 100644 --- a/controllers/nginx/examples/tls/rc-ssl.yaml +++ b/controllers/nginx/examples/tls/rc-ssl.yaml @@ -15,7 +15,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: gcr.io/google_containers/nginx-ingress-controller:0.4 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: diff --git a/controllers/nginx/examples/udp/rc-udp.yaml b/controllers/nginx/examples/udp/rc-udp.yaml index 4a057fc5c..9e0920f07 100644 --- a/controllers/nginx/examples/udp/rc-udp.yaml +++ b/controllers/nginx/examples/udp/rc-udp.yaml @@ -15,7 +15,7 @@ spec: name: nginx-ingress-lb spec: containers: - - image: aledbf/nginx-third-party:0.9 + - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb imagePullPolicy: Always livenessProbe: From 2ca6c8256b9a498d82e070e1c3fce464a8451175 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 31 Mar 2016 15:00:02 -0300 Subject: [PATCH 3/4] Update terminationGracePeriodSeconds in examples --- .../examples/custom-configuration/rc-custom-configuration.yaml | 1 + controllers/nginx/examples/daemonset/as-daemonset.yaml | 1 + controllers/nginx/examples/default-backend.yaml | 2 +- controllers/nginx/examples/default/rc-default.yaml | 1 + controllers/nginx/examples/full/rc-full.yaml | 1 + controllers/nginx/examples/tcp/rc-tcp.yaml | 1 + controllers/nginx/examples/tls/rc-ssl.yaml | 1 + controllers/nginx/examples/udp/rc-udp.yaml | 1 + 8 files changed, 8 insertions(+), 1 deletion(-) diff --git a/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml b/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml index aef3d16f3..2b9614f88 100644 --- a/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml +++ b/controllers/nginx/examples/custom-configuration/rc-custom-configuration.yaml @@ -14,6 +14,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb diff --git a/controllers/nginx/examples/daemonset/as-daemonset.yaml b/controllers/nginx/examples/daemonset/as-daemonset.yaml index dc3b37f6b..28de460da 100644 --- a/controllers/nginx/examples/daemonset/as-daemonset.yaml +++ b/controllers/nginx/examples/daemonset/as-daemonset.yaml @@ -8,6 +8,7 @@ spec: labels: name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb diff --git a/controllers/nginx/examples/default-backend.yaml b/controllers/nginx/examples/default-backend.yaml index 41169aca2..a48d4b9bc 100644 --- a/controllers/nginx/examples/default-backend.yaml +++ b/controllers/nginx/examples/default-backend.yaml @@ -11,7 +11,7 @@ spec: labels: app: default-http-backend spec: - terminationGracePeriodSeconds: 600 + terminationGracePeriodSeconds: 60 containers: - name: default-http-backend # Any image is permissable as long as: diff --git a/controllers/nginx/examples/default/rc-default.yaml b/controllers/nginx/examples/default/rc-default.yaml index 459865753..ac7d25d86 100644 --- a/controllers/nginx/examples/default/rc-default.yaml +++ b/controllers/nginx/examples/default/rc-default.yaml @@ -14,6 +14,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb diff --git a/controllers/nginx/examples/full/rc-full.yaml b/controllers/nginx/examples/full/rc-full.yaml index f6d6e9580..5ef8de056 100644 --- a/controllers/nginx/examples/full/rc-full.yaml +++ b/controllers/nginx/examples/full/rc-full.yaml @@ -15,6 +15,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 volumes: - name: dhparam-example secret: diff --git a/controllers/nginx/examples/tcp/rc-tcp.yaml b/controllers/nginx/examples/tcp/rc-tcp.yaml index a38bf7145..2979be94d 100644 --- a/controllers/nginx/examples/tcp/rc-tcp.yaml +++ b/controllers/nginx/examples/tcp/rc-tcp.yaml @@ -14,6 +14,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb diff --git a/controllers/nginx/examples/tls/rc-ssl.yaml b/controllers/nginx/examples/tls/rc-ssl.yaml index 798539860..560477f7b 100644 --- a/controllers/nginx/examples/tls/rc-ssl.yaml +++ b/controllers/nginx/examples/tls/rc-ssl.yaml @@ -14,6 +14,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb diff --git a/controllers/nginx/examples/udp/rc-udp.yaml b/controllers/nginx/examples/udp/rc-udp.yaml index 9e0920f07..1e1bcd933 100644 --- a/controllers/nginx/examples/udp/rc-udp.yaml +++ b/controllers/nginx/examples/udp/rc-udp.yaml @@ -14,6 +14,7 @@ spec: k8s-app: nginx-ingress-lb name: nginx-ingress-lb spec: + terminationGracePeriodSeconds: 60 containers: - image: gcr.io/google_containers/nginx-ingress-controller:0.5 name: nginx-ingress-lb From 0a71f4911b57ac6fa33c75bc45b54d67fb2a4941 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 31 Mar 2016 21:29:36 -0300 Subject: [PATCH 4/4] Add test to verify SSL certificate creation --- controllers/nginx/README.md | 2 +- controllers/nginx/controller.go | 57 ++++++++++++++---------- controllers/nginx/nginx/main.go | 2 + controllers/nginx/nginx/ssl.go | 10 ++--- controllers/nginx/nginx/ssl_test.go | 68 +++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 30 deletions(-) create mode 100644 controllers/nginx/nginx/ssl_test.go diff --git a/controllers/nginx/README.md b/controllers/nginx/README.md index 0e1d0779a..380a63f7c 100644 --- a/controllers/nginx/README.md +++ b/controllers/nginx/README.md @@ -224,4 +224,4 @@ The previous behavior can be restored using `retry-non-idempotent=true` in the c ## Limitations -TODO +- Ingress rules for TLS require the definition of the field `host` diff --git a/controllers/nginx/controller.go b/controllers/nginx/controller.go index 55d696238..b7710a686 100644 --- a/controllers/nginx/controller.go +++ b/controllers/nginx/controller.go @@ -68,7 +68,10 @@ type loadBalancerController struct { recorder record.EventRecorder syncQueue *taskQueue - ingQueue *taskQueue + + // taskQueue used to update the status of the Ingress rules. + // this avoids a sync execution in the ResourceEventHandlerFuncs + ingQueue *taskQueue // stopLock is used to enforce only a single call to Stop is active. // Needed because we allow stopping through an http endpoint and @@ -99,7 +102,7 @@ func newLoadBalancerController(kubeClient *client.Client, resyncPeriod time.Dura } lbc.syncQueue = NewTaskQueue(lbc.sync) - lbc.ingQueue = NewTaskQueue(lbc.syncIngress) + lbc.ingQueue = NewTaskQueue(lbc.updateIngressStatus) ingEventHandler := framework.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { @@ -239,7 +242,7 @@ func (lbc *loadBalancerController) sync(key string) { }) } -func (lbc *loadBalancerController) syncIngress(key string) { +func (lbc *loadBalancerController) updateIngressStatus(key string) { if !lbc.controllersInSync() { lbc.ingQueue.requeue(key, fmt.Errorf("deferring sync till endpoints controller has synced")) return @@ -251,30 +254,32 @@ func (lbc *loadBalancerController) syncIngress(key string) { return } - if ingExists { - ing := obj.(*extensions.Ingress) + if !ingExists { + return + } - ingClient := lbc.client.Extensions().Ingress(ing.Namespace) + ing := obj.(*extensions.Ingress) - currIng, err := ingClient.Get(ing.Name) - if err != nil { - glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) + ingClient := lbc.client.Extensions().Ingress(ing.Namespace) + + currIng, err := ingClient.Get(ing.Name) + if err != nil { + glog.Errorf("unexpected error searching Ingress %v/%v: %v", ing.Namespace, ing.Name, err) + return + } + + lbIPs := ing.Status.LoadBalancer.Ingress + if !lbc.isStatusIPDefined(lbIPs) { + glog.Infof("Updating loadbalancer %v/%v with IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) + currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress, api.LoadBalancerIngress{ + IP: lbc.lbInfo.Address, + }) + if _, err := ingClient.UpdateStatus(currIng); err != nil { + lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) return } - lbIPs := ing.Status.LoadBalancer.Ingress - if len(lbIPs) > 0 && !lbc.isStatusIPDefined(lbIPs) { - glog.Infof("Updating loadbalancer %v/%v with IP %v", ing.Namespace, ing.Name, lbc.lbInfo.Address) - currIng.Status.LoadBalancer.Ingress = append(currIng.Status.LoadBalancer.Ingress, api.LoadBalancerIngress{ - IP: lbc.lbInfo.Address, - }) - if _, err := ingClient.UpdateStatus(currIng); err != nil { - lbc.recorder.Eventf(currIng, api.EventTypeWarning, "UPDATE", "error: %v", err) - return - } - - lbc.recorder.Eventf(currIng, api.EventTypeNormal, "CREATE", "ip: %v", lbc.lbInfo.Address) - } + lbc.recorder.Eventf(currIng, api.EventTypeNormal, "CREATE", "ip: %v", lbc.lbInfo.Address) } } @@ -587,10 +592,14 @@ func (lbc *loadBalancerController) getPemsFromIngress(data []interface{}) map[st continue } - pemFileName := lbc.nginx.AddOrUpdateCertAndKey(secretName, string(cert), string(key)) + pemFileName, err := lbc.nginx.AddOrUpdateCertAndKey(fmt.Sprintf("%v-%v", ing.Namespace, secretName), string(cert), string(key)) + if err != nil { + glog.Errorf("No valid SSL certificate found in secret %v: %v", secretName, err) + continue + } cn, err := lbc.nginx.CheckSSLCertificate(pemFileName) if err != nil { - glog.Warningf("No valid SSL certificate found in secret %v", secretName) + glog.Errorf("No valid SSL certificate found in secret %v: %v", secretName, err) continue } diff --git a/controllers/nginx/nginx/main.go b/controllers/nginx/nginx/main.go index b78a1e3a2..706fde77b 100644 --- a/controllers/nginx/nginx/main.go +++ b/controllers/nginx/nginx/main.go @@ -77,7 +77,9 @@ const ( // Size of the SSL shared cache between all worker processes. // http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_session_cache sslSessionCacheSize = "10m" +) +var ( // Base directory that contains the mounted secrets with SSL certificates, keys and sslDirectory = "/etc/nginx-ssl" ) diff --git a/controllers/nginx/nginx/ssl.go b/controllers/nginx/nginx/ssl.go index 3a5afb20c..65ad0608c 100644 --- a/controllers/nginx/nginx/ssl.go +++ b/controllers/nginx/nginx/ssl.go @@ -27,21 +27,21 @@ import ( ) // AddOrUpdateCertAndKey creates a .pem file wth the cert and the key with the specified name -func (nginx *Manager) AddOrUpdateCertAndKey(name string, cert string, key string) string { +func (nginx *Manager) AddOrUpdateCertAndKey(name string, cert string, key string) (string, error) { pemFileName := sslDirectory + "/" + name + ".pem" pem, err := os.Create(pemFileName) if err != nil { - glog.Fatalf("Couldn't create pem file %v: %v", pemFileName, err) + return "", fmt.Errorf("Couldn't create pem file %v: %v", pemFileName, err) } defer pem.Close() - _, err = pem.WriteString(fmt.Sprintf("%v\n%v", key, cert)) + _, err = pem.WriteString(fmt.Sprintf("%v\n%v", cert, key)) if err != nil { - glog.Fatalf("Couldn't write to pem file %v: %v", pemFileName, err) + return "", fmt.Errorf("Couldn't write to pem file %v: %v", pemFileName, err) } - return pemFileName + return pemFileName, nil } // CheckSSLCertificate checks if the certificate and key file are valid diff --git a/controllers/nginx/nginx/ssl_test.go b/controllers/nginx/nginx/ssl_test.go new file mode 100644 index 000000000..10a0c1b67 --- /dev/null +++ b/controllers/nginx/nginx/ssl_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nginx + +import ( + "encoding/base64" + "fmt" + "os" + "testing" + "time" +) + +func TestAddOrUpdateCertAndKey(t *testing.T) { + sslDirectory = os.TempDir() + // openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout /tmp/tls.key -out /tmp/tls.crt -subj "/CN=echoheaders/O=echoheaders" + tlsCrt := "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURhakNDQWxLZ0F3SUJBZ0lKQUxHUXR5VVBKTFhYTUEwR0NTcUdTSWIzRFFFQkJRVUFNQ3d4RkRBU0JnTlYKQkFNVEMyVmphRzlvWldGa1pYSnpNUlF3RWdZRFZRUUtFd3RsWTJodmFHVmhaR1Z5Y3pBZUZ3MHhOakF6TXpFeQpNekU1TkRoYUZ3MHhOekF6TXpFeU16RTVORGhhTUN3eEZEQVNCZ05WQkFNVEMyVmphRzlvWldGa1pYSnpNUlF3CkVnWURWUVFLRXd0bFkyaHZhR1ZoWkdWeWN6Q0NBU0l3RFFZSktvWklodmNOQVFFQkJRQURnZ0VQQURDQ0FRb0MKZ2dFQkFONzVmS0N5RWwxanFpMjUxTlNabDYzeGQweG5HMHZTVjdYL0xxTHJveVNraW5nbnI0NDZZWlE4UEJWOAo5TUZzdW5RRGt1QVoyZzA3NHM1YWhLSm9BRGJOMzhld053RXNsVDJkRzhRTUw0TktrTUNxL1hWbzRQMDFlWG1PCmkxR2txZFA1ZUExUHlPZCtHM3gzZmxPN2xOdmtJdHVHYXFyc0tvMEhtMHhqTDVtRUpwWUlOa0tGSVhsWWVLZS8KeHRDR25CU2tLVHFMTG0yeExKSGFFcnJpaDZRdkx4NXF5U2gzZTU2QVpEcTlkTERvcWdmVHV3Z2IzekhQekc2NwppZ0E0dkYrc2FRNHpZUE1NMHQyU1NiVkx1M2pScWNvL3lxZysrOVJBTTV4bjRubnorL0hUWFhHKzZ0RDBaeGI1CmVVRDNQakVhTnlXaUV2dTN6UFJmdysyNURMY0NBd0VBQWFPQmpqQ0JpekFkQmdOVkhRNEVGZ1FVcktMZFhHeUUKNUlEOGRvd2lZNkdzK3dNMHFKc3dYQVlEVlIwakJGVXdVNEFVcktMZFhHeUU1SUQ4ZG93aVk2R3Mrd00wcUp1aApNS1F1TUN3eEZEQVNCZ05WQkFNVEMyVmphRzlvWldGa1pYSnpNUlF3RWdZRFZRUUtFd3RsWTJodmFHVmhaR1Z5CmM0SUpBTEdRdHlVUEpMWFhNQXdHQTFVZEV3UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUZCUUFEZ2dFQkFNZVMKMHFia3VZa3Z1enlSWmtBeE1PdUFaSDJCK0Evb3N4ODhFRHB1ckV0ZWN5RXVxdnRvMmpCSVdCZ2RkR3VBYU5jVQorUUZDRm9NakJOUDVWVUxIWVhTQ3VaczN2Y25WRDU4N3NHNlBaLzhzbXJuYUhTUjg1ZVpZVS80bmFyNUErdWErClIvMHJrSkZnOTlQSmNJd3JmcWlYOHdRcWdJVVlLNE9nWEJZcUJRL0VZS2YvdXl6UFN3UVZYRnVJTTZTeDBXcTYKTUNML3d2RlhLS0FaWDBqb3J4cHRjcldkUXNCcmYzWVRnYmx4TE1sN20zL2VuR1drcEhDUHdYeVRCOC9rRkw3SApLL2ZHTU1NWGswUkVSbGFPM1hTSUhrZUQ2SXJiRnRNV3R1RlJwZms2ZFA2TXlMOHRmTmZ6a3VvUHVEWUFaWllWCnR1NnZ0c0FRS0xWb0pGaGV0b1k9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K" + tlsKey := "LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFb3dJQkFBS0NBUUVBM3ZsOG9MSVNYV09xTGJuVTFKbVhyZkYzVEdjYlM5Slh0Zjh1b3V1akpLU0tlQ2V2CmpqcGhsRHc4Rlh6MHdXeTZkQU9TNEJuYURUdml6bHFFb21nQU5zM2Z4N0EzQVN5VlBaMGJ4QXd2ZzBxUXdLcjkKZFdqZy9UVjVlWTZMVWFTcDAvbDREVS9JNTM0YmZIZCtVN3VVMitRaTI0WnFxdXdxalFlYlRHTXZtWVFtbGdnMgpRb1VoZVZoNHA3L0cwSWFjRktRcE9vc3ViYkVza2RvU3V1S0hwQzh2SG1ySktIZDdub0JrT3IxMHNPaXFCOU83CkNCdmZNYy9NYnJ1S0FEaThYNnhwRGpOZzh3elMzWkpKdFV1N2VOR3B5ai9LcUQ3NzFFQXpuR2ZpZWZQNzhkTmQKY2I3cTBQUm5Gdmw1UVBjK01SbzNKYUlTKzdmTTlGL0Q3YmtNdHdJREFRQUJBb0lCQUViNmFEL0hMNjFtMG45bgp6bVkyMWwvYW83MUFmU0h2dlZnRCtWYUhhQkY4QjFBa1lmQUdpWlZrYjBQdjJRSFJtTERoaWxtb0lROWhadHVGCldQOVIxKythTFlnbGdmenZzanBBenR2amZTUndFaEFpM2pnSHdNY1p4S2Q3UnNJZ2hxY2huS093S0NYNHNNczQKUnBCbEFBZlhZWGs4R3F4NkxUbGptSDRDZk42QzZHM1EwTTlLMUxBN2lsck1Na3hwcngxMnBlVTNkczZMVmNpOQptOFdBL21YZ2I0c3pEbVNaWVpYRmNZMEhYNTgyS3JKRHpQWEVJdGQwZk5wd3I0eFIybzdzMEwvK2RnZCtqWERjCkh2SDBKZ3NqODJJaTIxWGZGM2tST3FxR3BKNmhVcncxTUZzVWRyZ29GL3pFck0vNWZKMDdVNEhodGFlalVzWTIKMFJuNXdpRUNnWUVBKzVUTVRiV084Wkg5K2pIdVQwc0NhZFBYcW50WTZYdTZmYU04Tm5CZWNoeTFoWGdlQVN5agpSWERlZGFWM1c0SjU5eWxIQ3FoOVdseVh4cDVTWWtyQU41RnQ3elFGYi91YmorUFIyWWhMTWZpYlBSYlYvZW1MCm5YaGF6MmtlNUUxT1JLY0x6QUVwSmpuZGQwZlZMZjdmQzFHeStnS2YyK3hTY1hjMHJqRE5iNGtDZ1lFQTR1UVEKQk91TlJQS3FKcDZUZS9zUzZrZitHbEpjQSs3RmVOMVlxM0E2WEVZVm9ydXhnZXQ4a2E2ZEo1QjZDOWtITGtNcQpwdnFwMzkxeTN3YW5uWC9ONC9KQlU2M2RxZEcyd1BWRUQ0REduaE54Qm1oaWZpQ1I0R0c2ZnE4MUV6ZE1vcTZ4CklTNHA2RVJaQnZkb1RqNk9pTHl6aUJMckpxeUhIMWR6c0hGRlNqOENnWUVBOWlSSEgyQ2JVazU4SnVYak8wRXcKUTBvNG4xdS9TZkQ4TFNBZ01VTVBwS1hpRTR2S0Qyd1U4a1BUNDFiWXlIZUh6UUpkdDFmU0RTNjZjR0ZHU1ZUSgphNVNsOG5yN051ejg3bkwvUmMzTGhFQ3Y0YjBOOFRjbW1oSy9CbDdiRXBOd0dFczNoNGs3TVdNOEF4QU15c3VxCmZmQ1pJM0tkNVJYNk0zbGwyV2QyRjhFQ2dZQlQ5RU9oTG0vVmhWMUVjUVR0cVZlMGJQTXZWaTVLSGozZm5UZkUKS0FEUVIvYVZncElLR3RLN0xUdGxlbVpPbi8yeU5wUS91UnpHZ3pDUUtldzNzU1RFSmMzYVlzbFVudzdhazJhZAp2ZTdBYXowMU84YkdHTk1oamNmdVBIS05LN2Nsc3pKRHJzcys4SnRvb245c0JHWEZYdDJuaWlpTTVPWVN5TTg4CkNJMjFEUUtCZ0hEQVRZbE84UWlDVWFBQlVqOFBsb1BtMDhwa3cyc1VmQW0xMzJCY00wQk9BN1hqYjhtNm1ManQKOUlteU5kZ2ZiM080UjlKVUxTb1pZSTc1dUxIL3k2SDhQOVlpWHZOdzMrTXl6VFU2b2d1YU8xSTNya2pna29NeAo5cU5pYlJFeGswS1A5MVZkckVLSEdHZEFwT05ES1N4VzF3ektvbUxHdmtYSTVKV05KRXFkCi0tLS0tRU5EIFJTQSBQUklWQVRFIEtFWS0tLS0tCg==" + + dCrt, err := base64.StdEncoding.DecodeString(tlsCrt) + if err != nil { + t.Fatalf("Unexpected error: %+v", err) + return + } + + dKey, err := base64.StdEncoding.DecodeString(tlsKey) + if err != nil { + t.Fatalf("Unexpected error: %+v", err) + } + + ngx := &Manager{} + + name := fmt.Sprintf("test-%v", time.Now().UnixNano()) + pemPath, err := ngx.AddOrUpdateCertAndKey(name, string(dCrt), string(dKey)) + if err != nil { + t.Fatalf("unexpected error checking SSL certificate: %v", err) + } + + if pemPath == "" { + t.Fatalf("expected path to pem file but returned empty") + } + + cnames, err := ngx.CheckSSLCertificate(pemPath) + if err != nil { + t.Fatalf("unexpected error checking SSL certificate: %v", err) + } + + if len(cnames) == 0 { + t.Fatalf("expected at least one cname but none returned") + } + + if cnames[0] != "echoheaders" { + t.Fatalf("expected cname echoheaders but %v returned", cnames[0]) + } +}