From e117ace05f5a0d3e7973aeefd93073478ffc6353 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 10 Aug 2018 11:21:46 +0100 Subject: [PATCH 1/5] Fix uncheckedConfigurator errors --- nginx-controller/nginx/configurator.go | 36 +++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index 63ab37120a..48d6c10405 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -57,10 +57,12 @@ func (cnf *Configurator) AddOrUpdateDHParam(content string) (string, error) { // AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { - cnf.addOrUpdateIngress(ingEx) - - if err := cnf.nginx.Reload(); err != nil { - return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + err := cnf.addOrUpdateIngress(ingEx) + if err != nil { + return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + } + if err = cnf.nginx.Reload(); err != nil { + return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } return nil } @@ -81,11 +83,13 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { // AddOrUpdateMergableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types func (cnf *Configurator) AddOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error { - cnf.addOrUpdateMergableIngress(mergeableIngs) - - if err := cnf.nginx.Reload(); err != nil { + err := cnf.addOrUpdateMergableIngress(mergeableIngs) + if err != nil { return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } + if err = cnf.nginx.Reload(); err != nil { + return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) + } return nil } @@ -877,7 +881,10 @@ func upstreamMapToSlice(upstreams map[string]Upstream) []Upstream { func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) error { cnf.addOrUpdateSecret(secret) - kind, _ := GetSecretKind(secret) + kind, err := GetSecretKind(secret) + if err != nil { + return fmt.Errorf("Error secret is unknown") + } if cnf.isPlus() && kind == JWK { return nil } @@ -962,10 +969,13 @@ func (cnf *Configurator) DeleteIngress(key string) error { // UpdateEndpoints updates endpoints in NGINX configuration for the Ingress resource func (cnf *Configurator) UpdateEndpoints(ingEx *IngressEx) error { - cnf.addOrUpdateIngress(ingEx) + err := cnf.addOrUpdateIngress(ingEx) + if err != nil { + return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + } if cnf.isPlus() { - err := cnf.updatePlusEndpoints(ingEx) + err = cnf.updatePlusEndpoints(ingEx) if err == nil { return nil } @@ -980,10 +990,12 @@ func (cnf *Configurator) UpdateEndpoints(ingEx *IngressEx) error { // UpdateEndpointsMergeableIngress updates endpoints in NGINX configuration for a mergeable Ingress resource func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngs *MergeableIngresses) error { - cnf.addOrUpdateMergableIngress(mergeableIngs) + err := cnf.addOrUpdateMergableIngress(mergeableIngs) + if err != nil { + return fmt.Errorf("Error adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) + } if cnf.isPlus() { - var err error for _, ing := range mergeableIngs.Minions { err = cnf.updatePlusEndpoints(ing) if err != nil { From dd0ab261d669a1fec6953e3a94150c38bab31610 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 10 Aug 2018 13:24:46 +0100 Subject: [PATCH 2/5] Add Unit tests for Add/Update Ingress --- nginx-controller/nginx/configurator_test.go | 39 +++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/nginx-controller/nginx/configurator_test.go b/nginx-controller/nginx/configurator_test.go index df9641eecf..c453b04b9d 100644 --- a/nginx-controller/nginx/configurator_test.go +++ b/nginx-controller/nginx/configurator_test.go @@ -618,3 +618,42 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { t.Errorf("generateNginxCfgForMergeableIngresses returned \n%v, but expected \n%v", result.Servers[0].JWTRedirectLocations, expected.Servers[0].JWTRedirectLocations) } } + +func TestAddOrUpdateIngress(t *testing.T) { + cnf := createTestConfigurator() + validIngress := createCafeIngressEx() + err := cnf.AddOrUpdateIngress(&validIngress) + if err != nil { + t.Errorf("Adding ingress failed: %v", err) + } + + expectedIng := cnf.ingresses["default-cafe-ingress"] + if !reflect.DeepEqual(&validIngress, expectedIng) { + t.Errorf("TestAddOrUpdateIngress returned: \n%v, but expected: \n%v", validIngress, expectedIng) + } +} + +func TestAddOrUpdateMIngress(t *testing.T) { + cnf := createTestConfigurator() + validMergeableIngress := createMergeableCafeIngress() + err := cnf.AddOrUpdateMergableIngress(validMergeableIngress) + if err != nil { + t.Errorf("Adding mergable ingress failed: %v", err) + } + + expectedIng := cnf.ingresses["default-cafe-ingress-master"] + if !reflect.DeepEqual(validMergeableIngress.Master, expectedIng) { + t.Errorf("TestAddOrUpdateMergableIngress returned: \n%v, but expected: \n%v", validMergeableIngress.Master, expectedIng) + } +} + +func TestAddOrUpdateIngressFailsWithInvalidTemplate(t *testing.T) { + t.Skip() // Skip test until TODO is completed + cnf := createTestConfigurator() + validIngress := createCafeIngressEx() + // TODO: Alter template to be invalid + err := cnf.AddOrUpdateIngress(&validIngress) + if err == nil { + t.Errorf("TestAddOrUpdateIngressFailsWithInvalidTemplate returned: \n%v, but expected \n template execution error", err) + } +} From fe6bf813ddeb6ac96ae4fa667b8529c9503fd103 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Wed, 22 Aug 2018 15:03:37 +0100 Subject: [PATCH 3/5] Fix MergeableIngress unit test. Fix typos --- nginx-controller/nginx/configurator.go | 21 ++-- nginx-controller/nginx/configurator_test.go | 111 +++++++++++++++----- 2 files changed, 90 insertions(+), 42 deletions(-) diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index 48d6c10405..f31296cf8d 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -57,11 +57,10 @@ func (cnf *Configurator) AddOrUpdateDHParam(content string) (string, error) { // AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { - err := cnf.addOrUpdateIngress(ingEx) - if err != nil { + if err := cnf.addOrUpdateIngress(ingEx); err != nil { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } - if err = cnf.nginx.Reload(); err != nil { + if err := cnf.nginx.Reload(); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } return nil @@ -83,17 +82,16 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { // AddOrUpdateMergableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types func (cnf *Configurator) AddOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error { - err := cnf.addOrUpdateMergableIngress(mergeableIngs) - if err != nil { + if err := cnf.addOrUpdateMergeableIngress(mergeableIngs); err != nil { return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } - if err = cnf.nginx.Reload(); err != nil { + if err := cnf.nginx.Reload(); err != nil { return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } return nil } -func (cnf *Configurator) addOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error { +func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) error { nginxCfg := cnf.generateNginxCfgForMergeableIngresses(mergeableIngs) name := objectMetaToFileName(&mergeableIngs.Master.Ingress.ObjectMeta) content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg) @@ -881,10 +879,7 @@ func upstreamMapToSlice(upstreams map[string]Upstream) []Upstream { func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) error { cnf.addOrUpdateSecret(secret) - kind, err := GetSecretKind(secret) - if err != nil { - return fmt.Errorf("Error secret is unknown") - } + kind, _ := GetSecretKind(secret) if cnf.isPlus() && kind == JWK { return nil } @@ -990,7 +985,7 @@ func (cnf *Configurator) UpdateEndpoints(ingEx *IngressEx) error { // UpdateEndpointsMergeableIngress updates endpoints in NGINX configuration for a mergeable Ingress resource func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngs *MergeableIngresses) error { - err := cnf.addOrUpdateMergableIngress(mergeableIngs) + err := cnf.addOrUpdateMergeableIngress(mergeableIngs) if err != nil { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } @@ -1154,7 +1149,7 @@ func (cnf *Configurator) UpdateConfig(config *Config, ingExes []*IngressEx, merg } } for _, mergeableIng := range mergeableIngs { - if err := cnf.addOrUpdateMergableIngress(mergeableIng); err != nil { + if err := cnf.addOrUpdateMergeableIngress(mergeableIng); err != nil { return err } } diff --git a/nginx-controller/nginx/configurator_test.go b/nginx-controller/nginx/configurator_test.go index c453b04b9d..997362b47e 100644 --- a/nginx-controller/nginx/configurator_test.go +++ b/nginx-controller/nginx/configurator_test.go @@ -482,16 +482,39 @@ func createExpectedConfigForMergeableCafeIngress() IngressNginxConfig { } -func createTestConfigurator() *Configurator { - templateExecutor, _ := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080) +func createTestConfigurator() (*Configurator, error) { + templateExecutor, err := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080) + if err != nil { + return nil, err + } + ngxc := NewNginxController("/etc/nginx", true) + apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true) + if err != nil { + return nil, err + } + return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil +} + +func createTestConfiguratorInvalidIngressTemplate() (*Configurator, error) { + templateExecutor, err := NewTemplateExecutor("templates/nginx-plus.tmpl", "templates/nginx-plus.ingress.tmpl", true, true, 8080) + if err != nil { + return nil, err + } + invalidIngressTemplate := "{{.Upstreams.This.Field.Does.Not.Exist}}" + if err := templateExecutor.UpdateIngressTemplate(&invalidIngressTemplate); err != nil { + return nil, err + } ngxc := NewNginxController("/etc/nginx", true) apiCtrl, _ := plus.NewNginxAPIController(&http.Client{}, "", true) - return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor) + return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil } func TestGenerateNginxCfg(t *testing.T) { cafeIngressEx := createCafeIngressEx() - cnf := createTestConfigurator() + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } expected := createExpectedConfigForCafeIngressEx() pems := map[string]string{ @@ -514,7 +537,10 @@ func TestGenerateNginxCfgForJWT(t *testing.T) { cafeIngressEx.Ingress.Annotations["nginx.com/jwt-login-url"] = "https://login.example.com" cafeIngressEx.JWTKey = &api_v1.Secret{} - cnf := createTestConfigurator() + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } expected := createExpectedConfigForCafeIngressEx() expected.Servers[0].JWTAuth = &JWTAuth{ Key: "/etc/nginx/secrets/default-cafe-jwk", @@ -547,7 +573,10 @@ func TestGenerateNginxCfgForMergeableIngresses(t *testing.T) { mergeableIngresses := createMergeableCafeIngress() expected := createExpectedConfigForMergeableCafeIngress() - cnf := createTestConfigurator() + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } result := cnf.generateNginxCfgForMergeableIngresses(mergeableIngresses) @@ -604,7 +633,10 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { }, } - cnf := createTestConfigurator() + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } result := cnf.generateNginxCfgForMergeableIngresses(mergeableIngresses) @@ -620,40 +652,61 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { } func TestAddOrUpdateIngress(t *testing.T) { - cnf := createTestConfigurator() - validIngress := createCafeIngressEx() - err := cnf.AddOrUpdateIngress(&validIngress) + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + ingress := createCafeIngressEx() + err = cnf.AddOrUpdateIngress(&ingress) + if err != nil { + t.Errorf("AddOrUpdateIngress returned: \n%v, but expected: \n%v", err, nil) + } + + cnfHasIngress := cnf.HasIngress(ingress.Ingress) + if !cnfHasIngress { + t.Errorf("AddOrUpdateIngress didn't add ingress successfully. HasIngress returned %v, expected %v", cnfHasIngress, true) + } +} + +func TestAddOrUpdateMergableIngress(t *testing.T) { + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + mergeableIngess := createMergeableCafeIngress() + err = cnf.AddOrUpdateMergableIngress(mergeableIngess) if err != nil { - t.Errorf("Adding ingress failed: %v", err) + t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", err, nil) } - expectedIng := cnf.ingresses["default-cafe-ingress"] - if !reflect.DeepEqual(&validIngress, expectedIng) { - t.Errorf("TestAddOrUpdateIngress returned: \n%v, but expected: \n%v", validIngress, expectedIng) + cnfHasMergableIngress := cnf.HasIngress(mergeableIngess.Master.Ingress) + if !cnfHasMergableIngress { + t.Errorf("AddOrUpdateMergableIngress didn't add mergable ingress successfully. HasIngress returned %v, expected %v", cnfHasMergableIngress, true) } } -func TestAddOrUpdateMIngress(t *testing.T) { - cnf := createTestConfigurator() - validMergeableIngress := createMergeableCafeIngress() - err := cnf.AddOrUpdateMergableIngress(validMergeableIngress) +func TestAddOrUpdateIngressFailsWithInvalidIngressTemplate(t *testing.T) { + cnf, err := createTestConfiguratorInvalidIngressTemplate() if err != nil { - t.Errorf("Adding mergable ingress failed: %v", err) + t.Errorf("Failed to create a test configurator: %v", err) } - expectedIng := cnf.ingresses["default-cafe-ingress-master"] - if !reflect.DeepEqual(validMergeableIngress.Master, expectedIng) { - t.Errorf("TestAddOrUpdateMergableIngress returned: \n%v, but expected: \n%v", validMergeableIngress.Master, expectedIng) + ingress := createCafeIngressEx() + err = cnf.AddOrUpdateIngress(&ingress) + if err == nil { + t.Errorf("AddOrUpdateIngressFailsWithInvalidTemplate returned \n%v, but expected \n%v", nil, "template execution error") } } -func TestAddOrUpdateIngressFailsWithInvalidTemplate(t *testing.T) { - t.Skip() // Skip test until TODO is completed - cnf := createTestConfigurator() - validIngress := createCafeIngressEx() - // TODO: Alter template to be invalid - err := cnf.AddOrUpdateIngress(&validIngress) +func TestAddOrUpdateMergableIngressFailsWithInvalidIngressTemplate(t *testing.T) { + cnf, err := createTestConfiguratorInvalidIngressTemplate() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + + mergeableIngess := createMergeableCafeIngress() + err = cnf.AddOrUpdateMergableIngress(mergeableIngess) if err == nil { - t.Errorf("TestAddOrUpdateIngressFailsWithInvalidTemplate returned: \n%v, but expected \n template execution error", err) + t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", nil, "template execution error") } } From 6051d2bee11b53d75f0782638114f735f85ab5f9 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Thu, 23 Aug 2018 16:25:18 +0100 Subject: [PATCH 4/5] Add unit tests for updating endpoints * Added test for UpdateEndpoints * Added test for UpdateEndpointsMergeableIngress --- nginx-controller/nginx/configurator_test.go | 68 ++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/nginx-controller/nginx/configurator_test.go b/nginx-controller/nginx/configurator_test.go index 997362b47e..fbb8732d6e 100644 --- a/nginx-controller/nginx/configurator_test.go +++ b/nginx-controller/nginx/configurator_test.go @@ -707,6 +707,72 @@ func TestAddOrUpdateMergableIngressFailsWithInvalidIngressTemplate(t *testing.T) mergeableIngess := createMergeableCafeIngress() err = cnf.AddOrUpdateMergableIngress(mergeableIngess) if err == nil { - t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", nil, "template execution error") + t.Errorf("AddOrUpdateMergableIngress returned \n%v, but expected \n%v", nil, "template execution error") + } +} + +func TestUpdateEndpoints(t *testing.T) { + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + + ingress := createCafeIngressEx() + err = cnf.UpdateEndpoints(&ingress) + if err != nil { + t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", err, nil) + } + + // test with OSS Configurator + cnf.nginxAPI = nil + err = cnf.UpdateEndpoints(&ingress) + if err != nil { + t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", err, nil) + } +} + +func TestUpdateEndpointsMergeableIngress(t *testing.T) { + cnf, err := createTestConfigurator() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + + mergeableIngress := createMergeableCafeIngress() + err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress) + if err != nil { + t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", err, nil) + } + + // test with OSS Configurator + cnf.nginxAPI = nil + err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress) + if err != nil { + t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", err, nil) + } +} + +func TestUpdateEndpointsFailsWithInvalidTemplate(t *testing.T) { + cnf, err := createTestConfiguratorInvalidIngressTemplate() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + + ingress := createCafeIngressEx() + err = cnf.UpdateEndpoints(&ingress) + if err == nil { + t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", nil, "template execution error") + } +} + +func TestUpdateEndpointsMergeableIngressFailsWithInvalidTemplate(t *testing.T) { + cnf, err := createTestConfiguratorInvalidIngressTemplate() + if err != nil { + t.Errorf("Failed to create a test configurator: %v", err) + } + + mergeableIngress := createMergeableCafeIngress() + err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress) + if err == nil { + t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error") } } From 00d452fd274852a6cf3b4c828f4c97489ccb3969 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 24 Aug 2018 16:07:16 +0100 Subject: [PATCH 5/5] Fix mergeableIngress typo --- nginx-controller/controller/controller.go | 6 +++--- nginx-controller/nginx/configurator.go | 4 ++-- nginx-controller/nginx/configurator_test.go | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go index 563bf1b128..d10c4165f0 100644 --- a/nginx-controller/controller/controller.go +++ b/nginx-controller/controller/controller.go @@ -679,7 +679,7 @@ func (lbc *LoadBalancerController) syncIng(task Task) { } return } - err = lbc.cnf.AddOrUpdateMergableIngress(mergeableIngExs) + err = lbc.cnf.AddOrUpdateMergeableIngress(mergeableIngExs) if err != nil { lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "AddedOrUpdatedWithError", "Configuration for %v(Master) was added or updated, but not applied: %v", key, err) for _, minion := range mergeableIngExs.Minions { @@ -804,7 +804,7 @@ func (lbc *LoadBalancerController) syncSecret(task Task) { glog.Errorf("Ignoring Ingress %v(Minion): %v", minion.Name, err) continue } - err = lbc.cnf.AddOrUpdateMergableIngress(mergeableIngress) + err = lbc.cnf.AddOrUpdateMergeableIngress(mergeableIngress) if err != nil { glog.Errorf("Failed to update Ingress %v(Master) of %v(Minion): %v", master.Name, minion.Name, err) } @@ -864,7 +864,7 @@ func (lbc *LoadBalancerController) syncSecret(task Task) { glog.Errorf("Ignoring Ingress %v(Minion): %v", minion.Name, err) continue } - err = lbc.cnf.AddOrUpdateMergableIngress(mergeableIngress) + err = lbc.cnf.AddOrUpdateMergeableIngress(mergeableIngress) if err != nil { glog.Errorf("Failed to update Ingress %v(Master) of %v(Minion): %v", master.Name, minion.Name, err) } diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index f31296cf8d..1f002a7297 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -80,8 +80,8 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { return nil } -// AddOrUpdateMergableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types -func (cnf *Configurator) AddOrUpdateMergableIngress(mergeableIngs *MergeableIngresses) error { +// AddOrUpdateMergeableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types +func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) error { if err := cnf.addOrUpdateMergeableIngress(mergeableIngs); err != nil { return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } diff --git a/nginx-controller/nginx/configurator_test.go b/nginx-controller/nginx/configurator_test.go index fbb8732d6e..f63f4688f1 100644 --- a/nginx-controller/nginx/configurator_test.go +++ b/nginx-controller/nginx/configurator_test.go @@ -668,20 +668,20 @@ func TestAddOrUpdateIngress(t *testing.T) { } } -func TestAddOrUpdateMergableIngress(t *testing.T) { +func TestAddOrUpdateMergeableIngress(t *testing.T) { cnf, err := createTestConfigurator() if err != nil { t.Errorf("Failed to create a test configurator: %v", err) } mergeableIngess := createMergeableCafeIngress() - err = cnf.AddOrUpdateMergableIngress(mergeableIngess) + err = cnf.AddOrUpdateMergeableIngress(mergeableIngess) if err != nil { - t.Errorf("AddOrUpdateMergableIngress returned \n%v, expected \n%v", err, nil) + t.Errorf("AddOrUpdateMergeableIngress returned \n%v, expected \n%v", err, nil) } - cnfHasMergableIngress := cnf.HasIngress(mergeableIngess.Master.Ingress) - if !cnfHasMergableIngress { - t.Errorf("AddOrUpdateMergableIngress didn't add mergable ingress successfully. HasIngress returned %v, expected %v", cnfHasMergableIngress, true) + cnfHasMergeableIngress := cnf.HasIngress(mergeableIngess.Master.Ingress) + if !cnfHasMergeableIngress { + t.Errorf("AddOrUpdateMergeableIngress didn't add mergeable ingress successfully. HasIngress returned %v, expected %v", cnfHasMergeableIngress, true) } } @@ -698,16 +698,16 @@ func TestAddOrUpdateIngressFailsWithInvalidIngressTemplate(t *testing.T) { } } -func TestAddOrUpdateMergableIngressFailsWithInvalidIngressTemplate(t *testing.T) { +func TestAddOrUpdateMergeableIngressFailsWithInvalidIngressTemplate(t *testing.T) { cnf, err := createTestConfiguratorInvalidIngressTemplate() if err != nil { t.Errorf("Failed to create a test configurator: %v", err) } mergeableIngess := createMergeableCafeIngress() - err = cnf.AddOrUpdateMergableIngress(mergeableIngess) + err = cnf.AddOrUpdateMergeableIngress(mergeableIngess) if err == nil { - t.Errorf("AddOrUpdateMergableIngress returned \n%v, but expected \n%v", nil, "template execution error") + t.Errorf("AddOrUpdateMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error") } }