From 42b58e957c1704b4c228ffe1f14e502f4d445b70 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 23 Nov 2016 21:14:14 -0300 Subject: [PATCH] Avoid nginx reloads --- controllers/nginx/pkg/cmd/controller/nginx.go | 16 ++++++---------- core/pkg/ingress/controller/backend_ssl.go | 4 +++- core/pkg/ingress/controller/controller.go | 16 ++++++++-------- core/pkg/ingress/types.go | 10 +++------- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index 757e6aaaf..92ff980c9 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/controllers/nginx/pkg/cmd/controller/nginx.go @@ -105,22 +105,18 @@ func (n NGINXController) Start() { // Reload checks if the running configuration file is different // to the specified and reload nginx if required -func (n NGINXController) Reload(data []byte) ([]byte, error) { +func (n NGINXController) Reload(data []byte) ([]byte, bool, error) { if !n.isReloadRequired(data) { - return nil, fmt.Errorf("Reload not required") + return []byte("Reload not required"), false, nil } err := ioutil.WriteFile(cfgPath, data, 0644) if err != nil { - return nil, err + return nil, false, err } - return exec.Command(n.binary, "-s", "reload").CombinedOutput() -} - -// Test checks is a file contains a valid NGINX configuration -func (n NGINXController) Test(file string) *exec.Cmd { - return exec.Command(n.binary, "-t", "-c", file) + o, e := exec.Command(n.binary, "-s", "reload").CombinedOutput() + return o, true, e } // BackendDefaults returns the nginx defaults @@ -188,7 +184,7 @@ func (n NGINXController) testTemplate(cfg []byte) error { } defer tmpfile.Close() ioutil.WriteFile(tmpfile.Name(), cfg, 0644) - out, err := n.Test(tmpfile.Name()).CombinedOutput() + out, err := exec.Command(n.binary, "-t", "-c", tmpfile.Name()).CombinedOutput() if err != nil { // this error is different from the rest because it must be clear why nginx is not working oe := fmt.Sprintf(` diff --git a/core/pkg/ingress/controller/backend_ssl.go b/core/pkg/ingress/controller/backend_ssl.go index d6f2f0c54..aa17c7807 100644 --- a/core/pkg/ingress/controller/backend_ssl.go +++ b/core/pkg/ingress/controller/backend_ssl.go @@ -89,9 +89,11 @@ func (ic *GenericController) syncSecret(k interface{}) error { // create certificates and add or update the item in the store _, exists = ic.sslCertTracker.Get(key) if exists { + glog.V(3).Infof("updating secret %v/%v in the store ", sec.Namespace, sec.Name) ic.sslCertTracker.Update(key, cert) return nil } + glog.V(3).Infof("adding secret %v/%v to the store ", sec.Namespace, sec.Name) ic.sslCertTracker.Add(key, cert) return nil } @@ -99,7 +101,7 @@ func (ic *GenericController) syncSecret(k interface{}) error { func (ic *GenericController) getPemCertificate(secretName string) (*ingress.SSLCert, error) { secretInterface, exists, err := ic.secrLister.Store.GetByKey(secretName) if err != nil { - return nil, fmt.Errorf("Error retriveing secret %v: %v", secretName, err) + return nil, fmt.Errorf("error retriveing secret %v: %v", secretName, err) } if !exists { return nil, fmt.Errorf("secret named %v does not exists", secretName) diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 0d349a3d9..655d7ea8a 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -386,14 +386,16 @@ func (ic *GenericController) sync(key interface{}) error { return err } - out, err := ic.cfg.Backend.Reload(data) + out, reloaded, err := ic.cfg.Backend.Reload(data) if err != nil { incReloadErrorCount() glog.Errorf("unexpected failure restarting the backend: \n%v", string(out)) return err } - glog.Infof("ingress backend successfully reloaded...") - incReloadCount() + if reloaded { + glog.Infof("ingress backend successfully reloaded...") + incReloadCount() + } return nil } @@ -686,7 +688,6 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress loc.BasicDigestAuth = *nginxAuth loc.RateLimit = *rl loc.Redirect = *locRew - //loc.SecureUpstream = secUpstream loc.Whitelist = *wl loc.Backend = ups.Name loc.EnableCORS = eCORS @@ -706,7 +707,6 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress BasicDigestAuth: *nginxAuth, RateLimit: *rl, Redirect: *locRew, - //SecureUpstream: secUpstream, Whitelist: *wl, EnableCORS: eCORS, ExternalAuth: ra, @@ -921,7 +921,7 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str host = defServerName } - // only add certificate if the server does not have one previously configured + // only add a certificate if the server does not have one previously configured if len(ing.Spec.TLS) > 0 && servers[host].SSLCertificate != "" { key := fmt.Sprintf("%v/%v", ing.Namespace, ing.Spec.TLS[0].SecretName) bc, exists := ic.sslCertTracker.Get(key) @@ -1048,8 +1048,8 @@ func (ic GenericController) Start() { go ic.secrController.Run(ic.stopCh) go ic.mapController.Run(ic.stopCh) - go ic.secretQueue.Run(time.Second, ic.stopCh) - go ic.syncQueue.Run(time.Second, ic.stopCh) + go ic.secretQueue.Run(5*time.Second, ic.stopCh) + go ic.syncQueue.Run(5*time.Second, ic.stopCh) go ic.syncStatus.Run(ic.stopCh) diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index dd490ad70..c1dfc0fa2 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -17,8 +17,6 @@ limitations under the License. package ingress import ( - "os/exec" - "k8s.io/kubernetes/pkg/api" "k8s.io/ingress/core/pkg/ingress/annotations/auth" @@ -43,15 +41,13 @@ var ( // TODO (#18): Make sure this is sufficiently supportive of other backends. type Controller interface { // Reload takes a byte array representing the new loadbalancer configuration, - // and returns a byte array containing any output/errors from the backend. + // and returns a byte array containing any output/errors from the backend and + // if a reload was required. // Before returning the backend must load the configuration in the given array // into the loadbalancer and restart it, or fail with an error and message string. // If reloading fails, there should be not change in the running configuration or // the given byte array. - Reload(data []byte) ([]byte, error) - // Tests returns the commands to execute that verifies if the configuration file is valid - // Example: nginx -t -c - Test(file string) *exec.Cmd + Reload(data []byte) ([]byte, bool, error) // OnUpdate callback invoked from the sync queue https://k8s.io/ingress/core/blob/master/pkg/ingress/controller/controller.go#L387 // when an update occurs. This is executed frequently because Ingress // controllers watches changes in: