Skip to content

Commit

Permalink
Vendor nginx-plus-go-sdk client
Browse files Browse the repository at this point in the history
* Change MaxFails datatype in IC. `int64`->`int` to match the type specified in the SDK
* Update Convert, Config and Configurator to use vendored sdk
* Add unit test for GetMapKeyAsInt64
* Update unit tests to pass
  • Loading branch information
Dean-Coakley committed Nov 5, 2018
1 parent 2608533 commit 563d62a
Show file tree
Hide file tree
Showing 11 changed files with 1,090 additions and 318 deletions.
9 changes: 9 additions & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@
[prune]
go-tests = true
unused-packages = true

[[constraint]]
name = "github.com/nginxinc/nginx-plus-go-sdk"
version = "0.2.0"
7 changes: 3 additions & 4 deletions internal/nginx/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"strings"

"github.com/golang/glog"

api_v1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -44,7 +43,7 @@ type Config struct {
MainWorkerConnections string
MainWorkerRlimitNofile string
Keepalive int64
MaxFails int64
MaxFails int
FailTimeout string
HealthCheckEnabled bool
HealthCheckMandatory bool
Expand Down Expand Up @@ -189,7 +188,7 @@ func ParseConfigMap(cfgm *api_v1.ConfigMap, nginxPlus bool) *Config {
} else {
parsingErrors := false

hstsMaxAge, existsMA, err := GetMapKeyAsInt(cfgm.Data, "hsts-max-age", cfgm)
hstsMaxAge, existsMA, err := GetMapKeyAsInt64(cfgm.Data, "hsts-max-age", cfgm)
if existsMA && err != nil {
glog.Error(err)
parsingErrors = true
Expand Down Expand Up @@ -332,7 +331,7 @@ func ParseConfigMap(cfgm *api_v1.ConfigMap, nginxPlus bool) *Config {
if workerRlimitNofile, exists := cfgm.Data["worker-rlimit-nofile"]; exists {
cfg.MainWorkerRlimitNofile = workerRlimitNofile
}
if keepalive, exists, err := GetMapKeyAsInt(cfgm.Data, "keepalive", cfgm); exists {
if keepalive, exists, err := GetMapKeyAsInt64(cfgm.Data, "keepalive", cfgm); exists {
if err != nil {
glog.Error(err)
} else {
Expand Down
6 changes: 3 additions & 3 deletions internal/nginx/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
}
}
if ingCfg.HealthCheckMandatory {
if healthCheckQueue, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
if healthCheckQueue, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
if err != nil {
glog.Error(err)
}
Expand Down Expand Up @@ -543,7 +543,7 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
} else {
parsingErrors := false

hstsMaxAge, existsMA, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.org/hsts-max-age", ingEx.Ingress)
hstsMaxAge, existsMA, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.org/hsts-max-age", ingEx.Ingress)
if existsMA && err != nil {
glog.Error(err)
parsingErrors = true
Expand Down Expand Up @@ -602,7 +602,7 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
ingCfg.SSLPorts = sslPorts
}

if keepalive, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.org/keepalive", ingEx.Ingress); exists {
if keepalive, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.org/keepalive", ingEx.Ingress); exists {
if err != nil {
glog.Error(err)
} else {
Expand Down
16 changes: 14 additions & 2 deletions internal/nginx/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,20 @@ func GetMapKeyAsBool(m map[string]string, key string, context apiObject) (bool,
return false, false, nil
}

// GetMapKeyAsInt tries to find and parse a key in a map as int64
func GetMapKeyAsInt(m map[string]string, key string, context apiObject) (int64, bool, error) {
// GetMapKeyAsInt tries to find and parse a key in a map as int
func GetMapKeyAsInt(m map[string]string, key string, context apiObject) (int, bool, error) {
if str, exists := m[key]; exists {
i, err := strconv.Atoi(str)
if err != nil {
return 0, exists, fmt.Errorf("%s %v/%v '%s' contains invalid integer: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
}
return i, exists, nil
}
return 0, false, nil
}

// GetMapKeyAsInt64 tries to find and parse a key in a map as int64
func GetMapKeyAsInt64(m map[string]string, key string, context apiObject) (int64, bool, error) {
if str, exists := m[key]; exists {
i, err := strconv.ParseInt(str, 10, 64)
if err != nil {
Expand Down
67 changes: 65 additions & 2 deletions internal/nginx/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestGetMapKeyAsInt(t *testing.T) {
if !exists {
t.Errorf("The key 'key' must exist in the configMap")
}
var expected int64 = 123456789
expected := 123456789
if i != expected {
t.Errorf("Unexpected return value:\nGot: %v\nExpected: %v", i, expected)
}
Expand All @@ -135,7 +135,7 @@ func TestGetMapKeyAsIntErrorMessage(t *testing.T) {
if err == nil {
t.Error("An error was expected")
}
expected := `ConfigMap default/test 'key' contains invalid integer: strconv.ParseInt: parsing "string": invalid syntax, ignoring`
expected := `ConfigMap default/test 'key' contains invalid integer: strconv.Atoi: parsing "string": invalid syntax, ignoring`
if err.Error() != expected {
t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
}
Expand All @@ -149,6 +149,69 @@ func TestGetMapKeyAsIntErrorMessage(t *testing.T) {
if err == nil {
t.Error("An error was expected")
}
expected = `Ingress kube-system/test 'key' contains invalid integer: strconv.Atoi: parsing "other_string": invalid syntax, ignoring`
if err.Error() != expected {
t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
}
}

//
// GetMapKeyAsInt64
//
func TestGetMapKeyAsInt64(t *testing.T) {
configMap := configMap
configMap.Data = map[string]string{
"key": "123456789",
}

i, exists, err := GetMapKeyAsInt64(configMap.Data, "key", &configMap)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if !exists {
t.Errorf("The key 'key' must exist in the configMap")
}
var expected int64 = 123456789
if i != expected {
t.Errorf("Unexpected return value:\nGot: %v\nExpected: %v", i, expected)
}
}

func TestGetMapKeyAsInt64NotFound(t *testing.T) {
configMap := configMap
configMap.Data = map[string]string{}

_, exists, _ := GetMapKeyAsInt64(configMap.Data, "key", &configMap)
if exists {
t.Errorf("The key 'key' must not exist in the configMap")
}
}

func TestGetMapKeyAsInt64ErrorMessage(t *testing.T) {
cfgm := configMap
cfgm.Data = map[string]string{
"key": "string",
}

// Test with configmap
_, _, err := GetMapKeyAsInt64(cfgm.Data, "key", &cfgm)
if err == nil {
t.Error("An error was expected")
}
expected := `ConfigMap default/test 'key' contains invalid integer: strconv.ParseInt: parsing "string": invalid syntax, ignoring`
if err.Error() != expected {
t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
}

// Test with ingress object
ingress := ingress
ingress.Annotations = map[string]string{
"key": "other_string",
}
_, _, err = GetMapKeyAsInt64(ingress.Annotations, "key", &ingress)
if err == nil {
t.Error("An error was expected")
}
expected = `Ingress kube-system/test 'key' contains invalid integer: strconv.ParseInt: parsing "other_string": invalid syntax, ignoring`
if err.Error() != expected {
t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type Upstream struct {
type UpstreamServer struct {
Address string
Port string
MaxFails int64
MaxFails int
FailTimeout string
SlowStart string
}
Expand Down
18 changes: 10 additions & 8 deletions internal/nginx/plus/nginx_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,30 @@ import (
"net/http"

"github.com/golang/glog"
plus "github.com/nginxinc/nginx-plus-go-sdk/client"
)

// NginxAPIController works with the NGINX API
type NginxAPIController struct {
client *NginxClient
local bool
client *plus.NginxClient
httpClient *http.Client
local bool
}

// ServerConfig holds the config data
type ServerConfig struct {
MaxFails int64
MaxFails int
FailTimeout string
SlowStart string
}

// NewNginxAPIController creates an instance of NginxAPIController
func NewNginxAPIController(httpClient *http.Client, endpoint string, local bool) (*NginxAPIController, error) {
client, err := NewNginxClient(httpClient, endpoint)
client, err := plus.NewNginxClient(httpClient, endpoint)
if !local && err != nil {
return nil, err
}
nginx := &NginxAPIController{client: client, local: local}
nginx := &NginxAPIController{client: client, httpClient: httpClient, local: local}
return nginx, nil
}

Expand Down Expand Up @@ -57,15 +59,15 @@ func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string
return nil
}

err := verifyConfigVersion(nginx.client.httpClient, configVersion)
err := verifyConfigVersion(nginx.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
var upsServers []plus.UpstreamServer
for _, s := range servers {
upsServers = append(upsServers, UpstreamServer{
upsServers = append(upsServers, plus.UpstreamServer{
Server: s,
MaxFails: config.MaxFails,
FailTimeout: config.FailTimeout,
Expand Down
Loading

0 comments on commit 563d62a

Please sign in to comment.