Skip to content

Commit

Permalink
Verify after reloading
Browse files Browse the repository at this point in the history
- Write config version directly in nginx config
- Check config version by querying over socket
- Add config version check to API
- Atomic write (write and rename) for version config
  • Loading branch information
isaac authored and isaachawley committed Oct 1, 2018
1 parent 92730bc commit aa0f464
Show file tree
Hide file tree
Showing 10 changed files with 315 additions and 29 deletions.
1 change: 1 addition & 0 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ func main() {
glog.Fatalf("Error generating NGINX main config: %v", err)
}
ngxc.UpdateMainConfigFile(content)
ngxc.UpdateConfigVersionFile()

nginxDone := make(chan error, 1)
ngxc.Start(nginxDone)
Expand Down
20 changes: 10 additions & 10 deletions internal/nginx/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {
name := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend)
endps, exists := ingEx.Endpoints[ingEx.Ingress.Spec.Backend.ServiceName+ingEx.Ingress.Spec.Backend.ServicePort.String()]
if exists {
err := cnf.nginxAPI.UpdateServers(name, endps, cfg)
err := cnf.nginxAPI.UpdateServers(name, endps, cfg, cnf.nginx.configVersion)
if err != nil {
return fmt.Errorf("Couldn't update the endpoints for %v: %v", name, err)
}
Expand All @@ -1055,7 +1055,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {
name := getNameForUpstream(ingEx.Ingress, rule.Host, &path.Backend)
endps, exists := ingEx.Endpoints[path.Backend.ServiceName+path.Backend.ServicePort.String()]
if exists {
err := cnf.nginxAPI.UpdateServers(name, endps, cfg)
err := cnf.nginxAPI.UpdateServers(name, endps, cfg, cnf.nginx.configVersion)
if err != nil {
return fmt.Errorf("Couldn't update the endpoints for %v: %v", name, err)
}
Expand Down Expand Up @@ -1117,14 +1117,14 @@ func GenerateNginxMainConfig(config *Config) *MainConfig {
SSLCiphers: config.MainServerSSLCiphers,
SSLDHParam: config.MainServerSSLDHParam,
SSLPreferServerCiphers: config.MainServerSSLPreferServerCiphers,
HTTP2: config.HTTP2,
ServerTokens: config.ServerTokens,
ProxyProtocol: config.ProxyProtocol,
WorkerProcesses: config.MainWorkerProcesses,
WorkerCPUAffinity: config.MainWorkerCPUAffinity,
WorkerShutdownTimeout: config.MainWorkerShutdownTimeout,
WorkerConnections: config.MainWorkerConnections,
WorkerRlimitNofile: config.MainWorkerRlimitNofile,
HTTP2: config.HTTP2,
ServerTokens: config.ServerTokens,
ProxyProtocol: config.ProxyProtocol,
WorkerProcesses: config.MainWorkerProcesses,
WorkerCPUAffinity: config.MainWorkerCPUAffinity,
WorkerShutdownTimeout: config.MainWorkerShutdownTimeout,
WorkerConnections: config.MainWorkerConnections,
WorkerRlimitNofile: config.MainWorkerRlimitNofile,
}
return nginxCfg
}
Expand Down
84 changes: 68 additions & 16 deletions internal/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path"

"github.com/golang/glog"
"github.com/nginxinc/kubernetes-ingress/internal/nginx/verify"
)

const dhparamFilename = "dhparam.pem"
Expand All @@ -19,9 +20,12 @@ const jwkSecretFileMode = 0644

// Controller updates NGINX configuration, starts and reloads NGINX
type Controller struct {
nginxConfdPath string
nginxSecretsPath string
local bool
nginxConfdPath string
nginxSecretsPath string
local bool
verifyConfigGenerator *verify.ConfigGenerator
verifyClient *verify.Client
configVersion int
}

// IngressNginxConfig describes an NGINX configuration
Expand Down Expand Up @@ -187,10 +191,18 @@ func NewUpstreamWithDefaultServer(name string) Upstream {

// NewNginxController creates a NGINX controller
func NewNginxController(nginxConfPath string, local bool) *Controller {
verifyConfigGenerator, err := verify.NewConfigGenerator()
if err != nil {
glog.Fatalf("error instantiating a verify.ConfigGenerator: %v", err)
}

ngxc := Controller{
nginxConfdPath: path.Join(nginxConfPath, "conf.d"),
nginxSecretsPath: path.Join(nginxConfPath, "secrets"),
local: local,
nginxConfdPath: path.Join(nginxConfPath, "conf.d"),
nginxSecretsPath: path.Join(nginxConfPath, "secrets"),
local: local,
verifyConfigGenerator: verifyConfigGenerator,
configVersion: 0,
verifyClient: verify.NewClient(),
}

return &ngxc
Expand Down Expand Up @@ -271,7 +283,6 @@ func (nginx *Controller) DeleteSecretFile(name string) {
glog.Warningf("Failed to delete %v: %v", filename, err)
}
}

}

func (nginx *Controller) getIngressNginxConfigFileName(name string) string {
Expand All @@ -284,16 +295,23 @@ func (nginx *Controller) getSecretFileName(name string) string {

// Reload reloads NGINX
func (nginx *Controller) Reload() error {
if !nginx.local {
if err := shellOut("nginx -t"); err != nil {
return fmt.Errorf("Invalid nginx configuration detected, not reloading: %s", err)
}
if err := shellOut("nginx -s reload"); err != nil {
return fmt.Errorf("Reloading NGINX failed: %s", err)
}
} else {
glog.V(3).Info("Reloading nginx")
if nginx.local {
glog.V(3).Info("local - skipping nginx reload")
return nil
}
// write a new config version
nginx.configVersion++
nginx.UpdateConfigVersionFile()

glog.V(3).Infof("Reloading nginx. configVersion: %v", nginx.configVersion)
if err := shellOut("nginx -s reload"); err != nil {
return fmt.Errorf("nginx reload failed: %v", err)
}
err := nginx.verifyClient.WaitForCorrectVersion(nginx.configVersion)
if err != nil {
return fmt.Errorf("could not get newest config version: %v", err)
}

return nil
}

Expand Down Expand Up @@ -399,3 +417,37 @@ func (nginx *Controller) UpdateIngressConfigFile(name string, cfg []byte) {
}
glog.V(3).Infof("The Ingress config file has been updated")
}

// UpdateConfigVersionFile writes the config version file.
func (nginx *Controller) UpdateConfigVersionFile() {
cfg, err := nginx.verifyConfigGenerator.GenerateVersionConfig(nginx.configVersion)
if err != nil {
glog.Fatalf("Error generating config version content: %v", err)
}

filename := "/etc/nginx/config-version.conf"
tempname := "/etc/nginx/config-version.conf.tmp"
glog.V(3).Infof("Writing config version to %v", filename)

if bool(glog.V(3)) || nginx.local {
glog.Info(string(cfg))
}

if !nginx.local {
w, err := os.Create(tempname)
if err != nil {
glog.Fatalf("Failed to open %v: %v", filename, err)
}
_, err = w.Write(cfg)
if err != nil {
glog.Fatalf("Failed to write to %v: %v", filename, err)
}
w.Close()

err = os.Rename(tempname, filename)
if err != nil {
glog.Fatalf("failed to rename version config file: %v", err)
}
}
glog.V(3).Infof("The config version file has been updated.")
}
31 changes: 29 additions & 2 deletions internal/nginx/plus/nginx_api.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package plus

import (
"fmt"
"net/http"

"github.com/golang/glog"
Expand Down Expand Up @@ -29,13 +30,39 @@ func NewNginxAPIController(httpClient *http.Client, endpoint string, local bool)
return nginx, nil
}

// verifyConfigVersion is used to check if the worker process that the API client is connected
// to is using the latest version of nginx config. This way we avoid making changes on
// a worker processes that is being shut down.
func verifyConfigVersion(httpClient *http.Client, configVersion int) error {
req, err := http.NewRequest("GET", "http://nginx-plus-api/configVersionCheck", nil)
if err != nil {
return fmt.Errorf("error creating request: %v", err)
}
req.Header.Set("x-expected-config-version", fmt.Sprintf("%v", configVersion))
resp, err := httpClient.Do(req)
if err != nil {
return fmt.Errorf("error doing request: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != 200 {
return fmt.Errorf("API returned non-success status: %v", resp.StatusCode)
}
return nil
}

// UpdateServers updates upstream servers
func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string, config ServerConfig) error {
func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string, config ServerConfig, configVersion int) error {
if nginx.local {
glog.V(3).Infof("Updating endpoints of %v: %v\n", upstream, servers)
return nil
}

err := verifyConfigVersion(nginx.client.httpClient, configVersion)
if err != nil {
return fmt.Errorf("error verifying config version: %v", err)
}
glog.V(3).Infof("API has the correct config version: %v.", configVersion)

var upsServers []UpstreamServer
for _, s := range servers {
upsServers = append(upsServers, UpstreamServer{
Expand All @@ -49,7 +76,7 @@ func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string
added, removed, err := nginx.client.UpdateHTTPServers(upstream, upsServers)
if err != nil {
glog.V(3).Infof("Couldn't update servers of %v upstream: %v", upstream, err)
return err
return fmt.Errorf("error updating servers of %v upstream: %v", upstream, err)
}

glog.V(3).Infof("Updated servers of %v; Added: %v, Removed: %v", upstream, added, removed)
Expand Down
9 changes: 9 additions & 0 deletions internal/nginx/templates/nginx-plus.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,20 @@ http {
listen unix:/var/run/nginx-plus-api.sock;
access_log off;

# $config_version_mismatch is defined in /etc/nginx/config-version.conf
location /configVersionCheck {
if ($config_version_mismatch) {
return 503;
}
return 200;
}

location /api {
api write=on;
}
}

include /etc/nginx/config-version.conf;
include /etc/nginx/conf.d/*.conf;
}

Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/templates/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ http {
}
{{- end }}


include /etc/nginx/config-version.conf;
include /etc/nginx/conf.d/*.conf;
}

Expand Down
75 changes: 75 additions & 0 deletions internal/nginx/verify/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package verify

import (
"context"
"fmt"
"io/ioutil"
"net"
"net/http"
"strconv"
"time"

"github.com/golang/glog"
)

// Client is a client for verifying the config version.
type Client struct {
client *http.Client
}

// NewClient returns a new client pointed at the config version socket.
func NewClient() *Client {
return &Client{
client: &http.Client{
Transport: &http.Transport{
DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
return net.Dial("unix", "/var/run/nginx-config-version.sock")
},
},
},
}
}

// GetConfigVersion get version number that we put in the nginx config to verify that we're using
// the correct config.
func (c *Client) GetConfigVersion() (int, error) {
resp, err := c.client.Get("http://config-version/configVersion")
if err != nil {
return 0, fmt.Errorf("error getting client: %v", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return 0, fmt.Errorf("non-200 response: %v", resp.StatusCode)
}

body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return 0, fmt.Errorf("failed to read the response body: %v", err)
}
v, err := strconv.Atoi(string(body))
if err != nil {
return 0, fmt.Errorf("error converting string to int: %v", err)
}
return v, nil
}

// WaitForCorrectVersion calls the config version endpoint until it gets the expectedVersion,
// which ensures that a new worker process has been started for that config version.
func (c *Client) WaitForCorrectVersion(expectedVersion int) error {
// This value needs tuning.
maxRetries := 160
sleep := 25 * time.Millisecond
for i := 0; i < maxRetries; i++ {
version, err := c.GetConfigVersion()
if err != nil {
return fmt.Errorf("unable to fetch version: %v", err)
}
if version == expectedVersion {
glog.V(3).Infof("success, version %v ensured. iterations: %v. took: %v", expectedVersion, i, time.Duration(i)*sleep)
return nil
}
time.Sleep(sleep)
}
return fmt.Errorf("could not get expected version: %v", expectedVersion)
}
51 changes: 51 additions & 0 deletions internal/nginx/verify/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package verify

import (
"bytes"
"io/ioutil"
"net/http"
"testing"
)

type Transport struct {
}

func (c Transport) RoundTrip(req *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: 200,
Body: ioutil.NopCloser(bytes.NewBufferString("42")),
Header: make(http.Header),
}, nil
}

func getTestHTTPClient() *http.Client {
ts := Transport{}
tClient := &http.Client{
Transport: ts,
}
return tClient
}

func TestVerifyClient(t *testing.T) {

c := Client{
client: getTestHTTPClient(),
}

configVersion, err := c.GetConfigVersion()
if err != nil {
t.Errorf("error getting config version: %v", err)
}
if configVersion != 42 {
t.Errorf("got bad config version, expected 42 got %v", configVersion)
}

err = c.WaitForCorrectVersion(43)
if err == nil {
t.Error("expected error from WaitForCorrectVersion ")
}
err = c.WaitForCorrectVersion(42)
if err != nil {
t.Errorf("error waiting for config version: %v", err)
}
}
Loading

0 comments on commit aa0f464

Please sign in to comment.