DevFW-CICD/ingress-nginx-helm
16
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix review comments

Browse source
This commit is contained in:
Nick Sardo 2017-04-18 10:53:18 -07:00
parent 6af52e7195
commit 9b381641b3
4 changed files with 25 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
controllers/gce/backends/backends.go
View file

@ -165,7 +165,7 @@ func (b *Backends) ensureHealthCheck(sp ServicePort) (string, error) {
return "", err return "", err
} }
if probe != nil { if probe != nil {
glog.V(1).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp) glog.V(2).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp)
applyProbeSettingsToHC(probe, hc) applyProbeSettingsToHC(probe, hc)
} }
} }
@ -245,7 +245,7 @@ func (b *Backends) Add(p ServicePort) error {
pName := b.namer.BeName(p.Port) pName := b.namer.BeName(p.Port)
be, _ = b.Get(p.Port) be, _ = b.Get(p.Port)
if be == nil { if be == nil {
glog.V(1).Infof("Creating backend for %d instance groups, port %v named port %v", len(igs), p.Port, namedPort) glog.V(2).Infof("Creating backend for %d instance groups, port %v named port %v", len(igs), p.Port, namedPort)
be, err = b.create(igs, namedPort, hcLink, p.Protocol, pName) be, err = b.create(igs, namedPort, hcLink, p.Protocol, pName)
if err != nil { if err != nil {
return err return err
@ -258,7 +258,7 @@ func (b *Backends) Add(p ServicePort) error {
} }
if be.Protocol != string(p.Protocol) || existingHCLink != hcLink { if be.Protocol != string(p.Protocol) || existingHCLink != hcLink {
glog.Infof("Updating backend protocol %v (%v) for change in protocol (%v) or health check", pName, be.Protocol, string(p.Protocol)) glog.V(2).Infof("Updating backend protocol %v (%v) for change in protocol (%v) or health check", pName, be.Protocol, string(p.Protocol))
be.Protocol = string(p.Protocol) be.Protocol = string(p.Protocol)
be.HealthChecks = []string{hcLink} be.HealthChecks = []string{hcLink}
if err = b.cloud.UpdateBackendService(be); err != nil { if err = b.cloud.UpdateBackendService(be); err != nil {
@ -286,7 +286,7 @@ func (b *Backends) Add(p ServicePort) error {
// Delete deletes the Backend for the given port. // Delete deletes the Backend for the given port.
func (b *Backends) Delete(port int64) (err error) { func (b *Backends) Delete(port int64) (err error) {
name := b.namer.BeName(port) name := b.namer.BeName(port)
glog.Infof("Deleting backend service %v", name) glog.V(2).Infof("Deleting backend service %v", name)
defer func() { defer func() {
if utils.IsHTTPErrorCode(err, http.StatusNotFound) { if utils.IsHTTPErrorCode(err, http.StatusNotFound) {
err = nil err = nil
@ -377,7 +377,7 @@ func (b *Backends) GC(svcNodePorts []ServicePort) error {
} }
pool := b.snapshotter.Snapshot() pool := b.snapshotter.Snapshot()
for port := range pool { for port := range pool {
p, err := strconv.ParseInt(port, 10, 64) p, err := strconv.ParseUint(port, 10, 16)
if err != nil { if err != nil {
return err return err
} }

11
controllers/gce/controller/utils.go
View file

@ -141,7 +141,7 @@ func (svc svcAnnotations) ApplicationProtocols() (map[string]utils.AppProtocol,
switch proto { switch proto {
case utils.ProtocolHTTP, utils.ProtocolHTTPS: case utils.ProtocolHTTP, utils.ProtocolHTTPS:
default: default:
return nil, fmt.Errorf("unexpected port application protocol: %v", proto) return nil, fmt.Errorf("invalid port application protocol: %v", proto)
} }
} }
@ -415,7 +415,6 @@ func (t *GCETranslator) toGCEBackend(be *extensions.IngressBackend, ns string) (
// getServiceNodePort looks in the svc store for a matching service:port, // getServiceNodePort looks in the svc store for a matching service:port,
// and returns the nodeport. // and returns the nodeport.
func (t *GCETranslator) getServiceNodePort(be extensions.IngressBackend, namespace string) (backends.ServicePort, error) { func (t *GCETranslator) getServiceNodePort(be extensions.IngressBackend, namespace string) (backends.ServicePort, error) {
invalidPort := backends.ServicePort{}
obj, exists, err := t.svcLister.Indexer.Get( obj, exists, err := t.svcLister.Indexer.Get(
&api_v1.Service{ &api_v1.Service{
ObjectMeta: meta_v1.ObjectMeta{ ObjectMeta: meta_v1.ObjectMeta{
@ -424,15 +423,15 @@ func (t *GCETranslator) getServiceNodePort(be extensions.IngressBackend, namespa
}, },
}) })
if !exists { if !exists {
return invalidPort, errorNodePortNotFound{be, fmt.Errorf("service %v/%v not found in store", namespace, be.ServiceName)} return backends.ServicePort{}, errorNodePortNotFound{be, fmt.Errorf("service %v/%v not found in store", namespace, be.ServiceName)}
} }
if err != nil { if err != nil {
return invalidPort, errorNodePortNotFound{be, err} return backends.ServicePort{}, errorNodePortNotFound{be, err}
} }
svc := obj.(*api_v1.Service) svc := obj.(*api_v1.Service)
appProtocols, err := svcAnnotations(svc.GetAnnotations()).ApplicationProtocols() appProtocols, err := svcAnnotations(svc.GetAnnotations()).ApplicationProtocols()
if err != nil { if err != nil {
return invalidPort, errorSvcAppProtosParsing{svc, err} return backends.ServicePort{}, errorSvcAppProtosParsing{svc, err}
} }
var port *api_v1.ServicePort var port *api_v1.ServicePort
@ -454,7 +453,7 @@ PortLoop:
} }
if port == nil { if port == nil {
return invalidPort, errorNodePortNotFound{be, fmt.Errorf("could not find matching nodeport from service")} return backends.ServicePort{}, errorNodePortNotFound{be, fmt.Errorf("could not find matching nodeport from service")}
} }
proto := utils.ProtocolHTTP proto := utils.ProtocolHTTP

20
controllers/gce/healthchecks/healthchecks.go
View file

@ -76,8 +76,8 @@ func (h *HealthChecks) Sync(hc *HealthCheck) (string, error) {
return "", err return "", err
} }
glog.Infof("Creating health check for port %v with protocol %v", hc.Port, hc.Type) glog.V(2).Infof("Creating health check for port %v with protocol %v", hc.Port, hc.Type)
if err = h.cloud.CreateHealthCheck(hc.Out()); err != nil { if err = h.cloud.CreateHealthCheck(hc.ToComputeHealthCheck()); err != nil {
return "", err return "", err
} }
@ -85,8 +85,8 @@ func (h *HealthChecks) Sync(hc *HealthCheck) (string, error) {
} }
if existingHC.Protocol() != hc.Protocol() { if existingHC.Protocol() != hc.Protocol() {
glog.Infof("Updating health check %v because it has protocol %v but need %v", existingHC.Name, existingHC.Type, hc.Type) glog.V(2).Infof("Updating health check %v because it has protocol %v but need %v", existingHC.Name, existingHC.Type, hc.Type)
err = h.cloud.UpdateHealthCheck(hc.Out()) err = h.cloud.UpdateHealthCheck(hc.ToComputeHealthCheck())
return existingHC.SelfLink, err return existingHC.SelfLink, err
} }
@ -94,9 +94,9 @@ func (h *HealthChecks) Sync(hc *HealthCheck) (string, error) {
// TODO: reconcile health checks, and compare headers interval etc. // TODO: reconcile health checks, and compare headers interval etc.
// Currently Ingress doesn't expose all the health check params // Currently Ingress doesn't expose all the health check params
// natively, so some users prefer to hand modify the check. // natively, so some users prefer to hand modify the check.
glog.Infof("Unexpected request path on health check %v, has %v want %v, NOT reconciling", hc.Name, existingHC.RequestPath, hc.RequestPath) glog.V(2).Infof("Unexpected request path on health check %v, has %v want %v, NOT reconciling", hc.Name, existingHC.RequestPath, hc.RequestPath)
} else { } else {
glog.Infof("Health check %v already exists and has the expected path %v", hc.Name, hc.RequestPath) glog.V(2).Infof("Health check %v already exists and has the expected path %v", hc.Name, hc.RequestPath)
} }
return existingHC.SelfLink, nil return existingHC.SelfLink, nil
@ -113,7 +113,7 @@ func (h *HealthChecks) getHealthCheckLink(port int64) (string, error) {
// Delete deletes the health check by port. // Delete deletes the health check by port.
func (h *HealthChecks) Delete(port int64) error { func (h *HealthChecks) Delete(port int64) error {
name := h.namer.BeName(port) name := h.namer.BeName(port)
glog.Infof("Deleting health check %v", name) glog.V(2).Infof("Deleting health check %v", name)
return h.cloud.DeleteHealthCheck(name) return h.cloud.DeleteHealthCheck(name)
} }
@ -127,7 +127,7 @@ func (h *HealthChecks) Get(port int64) (*HealthCheck, error) {
// DeleteLegacy deletes legacy HTTP health checks // DeleteLegacy deletes legacy HTTP health checks
func (h *HealthChecks) DeleteLegacy(port int64) error { func (h *HealthChecks) DeleteLegacy(port int64) error {
name := h.namer.BeName(port) name := h.namer.BeName(port)
glog.Infof("Deleting legacy HTTP health check %v", name) glog.V(2).Infof("Deleting legacy HTTP health check %v", name)
return h.cloud.DeleteHttpHealthCheck(name) return h.cloud.DeleteHttpHealthCheck(name)
} }
@ -197,8 +197,8 @@ func (hc *HealthCheck) Protocol() utils.AppProtocol {
return utils.AppProtocol(hc.Type) return utils.AppProtocol(hc.Type)
} }
// Out returns a valid compute.HealthCheck object // ToComputeHealthCheck returns a valid compute.HealthCheck object
func (hc *HealthCheck) Out() *compute.HealthCheck { func (hc *HealthCheck) ToComputeHealthCheck() *compute.HealthCheck {
// Zeroing out child settings as a precaution. GoogleAPI throws an error // Zeroing out child settings as a precaution. GoogleAPI throws an error
// if the wrong child struct is set. // if the wrong child struct is set.
hc.HealthCheck.HttpsHealthCheck = nil hc.HealthCheck.HttpsHealthCheck = nil

10
controllers/gce/healthchecks/healthchecks_test.go
View file

@ -63,7 +63,7 @@ func TestHealthCheckAddExisting(t *testing.T) {
httpHC := DefaultHealthCheck(3000, utils.ProtocolHTTP) httpHC := DefaultHealthCheck(3000, utils.ProtocolHTTP)
httpHC.Name = namer.BeName(3000) httpHC.Name = namer.BeName(3000)
httpHC.RequestPath = "/my-probes-health" httpHC.RequestPath = "/my-probes-health"
hcp.CreateHealthCheck(httpHC.Out()) hcp.CreateHealthCheck(httpHC.ToComputeHealthCheck())
// Should not fail adding the same type of health check // Should not fail adding the same type of health check
hc := healthChecks.New(3000, utils.ProtocolHTTP) hc := healthChecks.New(3000, utils.ProtocolHTTP)
@ -82,7 +82,7 @@ func TestHealthCheckAddExisting(t *testing.T) {
httpsHC := DefaultHealthCheck(4000, utils.ProtocolHTTPS) httpsHC := DefaultHealthCheck(4000, utils.ProtocolHTTPS)
httpsHC.Name = namer.BeName(4000) httpsHC.Name = namer.BeName(4000)
httpsHC.RequestPath = "/my-probes-health" httpsHC.RequestPath = "/my-probes-health"
hcp.CreateHealthCheck(httpsHC.Out()) hcp.CreateHealthCheck(httpsHC.ToComputeHealthCheck())
hc = healthChecks.New(4000, utils.ProtocolHTTPS) hc = healthChecks.New(4000, utils.ProtocolHTTPS)
_, err = healthChecks.Sync(hc) _, err = healthChecks.Sync(hc)
@ -104,11 +104,11 @@ func TestHealthCheckDelete(t *testing.T) {
// Create HTTP HC for 1234 // Create HTTP HC for 1234
hc := DefaultHealthCheck(1234, utils.ProtocolHTTP) hc := DefaultHealthCheck(1234, utils.ProtocolHTTP)
hc.Name = namer.BeName(1234) hc.Name = namer.BeName(1234)
hcp.CreateHealthCheck(hc.Out()) hcp.CreateHealthCheck(hc.ToComputeHealthCheck())
// Create HTTPS HC for 1234) // Create HTTPS HC for 1234)
hc.Type = string(utils.ProtocolHTTPS) hc.Type = string(utils.ProtocolHTTPS)
hcp.CreateHealthCheck(hc.Out()) hcp.CreateHealthCheck(hc.ToComputeHealthCheck())
// Delete only HTTP 1234 // Delete only HTTP 1234
err := healthChecks.Delete(1234) err := healthChecks.Delete(1234)
@ -139,7 +139,7 @@ func TestHealthCheckUpdate(t *testing.T) {
hc := DefaultHealthCheck(3000, utils.ProtocolHTTP) hc := DefaultHealthCheck(3000, utils.ProtocolHTTP)
hc.Name = namer.BeName(3000) hc.Name = namer.BeName(3000)
hc.RequestPath = "/my-probes-health" hc.RequestPath = "/my-probes-health"
hcp.CreateHealthCheck(hc.Out()) hcp.CreateHealthCheck(hc.ToComputeHealthCheck())
// Verify the health check exists // Verify the health check exists
_, err := healthChecks.Get(3000) _, err := healthChecks.Get(3000)