Skip to content

Commit

Permalink
Clarify log messages in controller pkg
Browse files Browse the repository at this point in the history
  • Loading branch information
@antoineco
antoineco committed Jul 2, 2018
1 parent 92474ed commit 472dcb3
Show file tree
Hide file tree
Showing 11 changed files with 168 additions and 138 deletions.
95 changes: 58 additions & 37 deletions internal/ingress/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr

configmap, err := n.store.GetConfigMap(configmapName)
if err != nil {
glog.Errorf("Error reading ConfigMap %q: %v", configmapName, err)
glog.Errorf("Error getting ConfigMap %q: %v", configmapName, err)
return []ingress.L4Service{}
}

Expand All @@ -236,7 +236,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
}
reserverdPorts := sets.NewInt(rp...)

// svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<(bool)decode>:<(bool)encode>]
// svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<("PROXY")decode>:<("PROXY")encode>]
for port, svcRef := range configmap.Data {
externalPort, err := strconv.Atoi(port)
if err != nil {
Expand Down Expand Up @@ -278,7 +278,7 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr

svc, err := n.store.GetService(nsName)
if err != nil {
glog.Warningf("Error getting Service %q from local store: %v", nsName, err)
glog.Warningf("Error getting Service %q: %v", nsName, err)
continue
}

Expand Down Expand Up @@ -339,7 +339,7 @@ func (n *NGINXController) getDefaultUpstream() *ingress.Backend {
svcKey := n.cfg.DefaultService
svc, err := n.store.GetService(svcKey)
if err != nil {
glog.Warningf("Unexpected error getting default backend %q from local store: %v", n.cfg.DefaultService, err)
glog.Warningf("Error getting default backend %q: %v", svcKey, err)
upstream.Endpoints = append(upstream.Endpoints, n.DefaultEndpoint())
return upstream
}
Expand All @@ -364,9 +364,11 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
servers := n.createServers(ingresses, upstreams, du)

for _, ing := range ingresses {
anns, err := n.store.GetIngressAnnotations(ing)
ingKey := k8s.MetaNamespaceKey(ing)

anns, err := n.store.GetIngressAnnotations(ingKey)
if err != nil {
glog.Errorf("Unexpected error reading annotations for Ingress %q from local store: %v", ing.Name, err)
glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err)
}

for _, rule := range ing.Spec.Rules {
Expand All @@ -381,7 +383,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]

if rule.HTTP == nil &&
host != defServerName {
glog.V(3).Infof("Ingress \"%v/%v\" does not contain any HTTP rule, using default backend.", ing.Namespace, ing.Name)
glog.V(3).Infof("Ingress %q does not contain any HTTP rule, using default backend", ingKey)
continue
}

Expand All @@ -392,10 +394,12 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
if server.CertificateAuth.CAFileName == "" {
server.CertificateAuth = anns.CertificateAuth
if server.CertificateAuth.Secret != "" && server.CertificateAuth.CAFileName == "" {
glog.V(3).Infof("Secret %q does not contain 'ca.crt' key, mutual authentication disabled for Ingress \"%v/%v\"", server.CertificateAuth.Secret, ing.Namespace, ing.Name)
glog.V(3).Infof("Secret %q has no 'ca.crt' key, mutual authentication disabled for Ingress %q",
server.CertificateAuth.Secret, ingKey)
}
} else {
glog.V(3).Infof("Server %v is already configured for mutual authentication (Ingress \"%v/%v\")", server.Hostname, ing.Namespace, ing.Name)
glog.V(3).Infof("Server %q is already configured for mutual authentication (Ingress %q)",
server.Hostname, ingKey)
}

for _, path := range rule.HTTP.Paths {
Expand All @@ -417,11 +421,14 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
addLoc = false

if !loc.IsDefBackend {
glog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress \"%v/%v\")", loc.Path, server.Hostname, loc.Backend, ing.Namespace, ing.Name)
glog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ingKey)
break
}

glog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress \"%v/%v\")", loc.Path, server.Hostname, loc.Backend, ups.Name, ing.Namespace, ing.Name)
glog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ups.Name, ingKey)

loc.Backend = ups.Name
loc.IsDefBackend = false
loc.Port = ups.Port
Expand Down Expand Up @@ -457,7 +464,9 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]

// new location
if addLoc {
glog.V(3).Infof("Adding location %q for server %q with upstream %q (Ingress \"%v/%v\")", nginxPath, server.Hostname, ups.Name, ing.Namespace, ing.Name)
glog.V(3).Infof("Adding location %q for server %q with upstream %q (Ingress %q)",
nginxPath, server.Hostname, ups.Name, ingKey)

loc := &ingress.Location{
Path: nginxPath,
Backend: ups.Name,
Expand Down Expand Up @@ -520,7 +529,8 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
for _, location := range server.Locations {
if upstream.Name == location.Backend {
if len(upstream.Endpoints) == 0 {
glog.V(3).Infof("Upstream %q does not have any active endpoints.", upstream.Name)
glog.V(3).Infof("Upstream %q has no active Endpoint", upstream.Name)

location.Backend = "" // for nginx.tmpl checking

// check if the location contains endpoints and a custom default backend
Expand All @@ -530,6 +540,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
if len(endps) > 0 {
glog.V(3).Infof("Using custom default backend for location %q in server %q (Service \"%v/%v\")",
location.Path, server.Hostname, location.DefaultBackend.Namespace, location.DefaultBackend.Name)

nb := upstream.DeepCopy()
name := fmt.Sprintf("custom-default-backend-%v", upstream.Name)
nb.Name = name
Expand Down Expand Up @@ -592,9 +603,11 @@ func (n *NGINXController) createUpstreams(data []*extensions.Ingress, du *ingres
upstreams[defUpstreamName] = du

for _, ing := range data {
anns, err := n.store.GetIngressAnnotations(ing)
ingKey := k8s.MetaNamespaceKey(ing)

anns, err := n.store.GetIngressAnnotations(ingKey)
if err != nil {
glog.Errorf("Error reading Ingress annotations: %v", err)
glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err)
}

var defBackend string
Expand Down Expand Up @@ -736,7 +749,7 @@ func (n *NGINXController) getServiceClusterEndpoint(svcKey string, backend *exte
}
}
if port == -1 {
return endpoint, fmt.Errorf("service %q does not have a port named %q", svc.Name, backend.ServicePort)
return endpoint, fmt.Errorf("Service %q does not have a port named %q", svc.Name, backend.ServicePort)
}
endpoint.Port = fmt.Sprintf("%d", port)
} else {
Expand All @@ -754,7 +767,7 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string,

var upstreams []ingress.Endpoint
if err != nil {
return upstreams, fmt.Errorf("error getting Service %q from local store: %v", svcKey, err)
return upstreams, err
}

glog.V(3).Infof("Obtaining ports information for Service %q", svcKey)
Expand Down Expand Up @@ -875,9 +888,11 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,

// initialize all other servers
for _, ing := range data {
anns, err := n.store.GetIngressAnnotations(ing)
ingKey := k8s.MetaNamespaceKey(ing)

anns, err := n.store.GetIngressAnnotations(ingKey)
if err != nil {
glog.Errorf("Error reading Ingress %q annotations from local store: %v", ing.Name, err)
glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err)
}

// default upstream name
Expand All @@ -893,7 +908,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
// special "catch all" case, Ingress with a backend but no rule
defLoc := servers[defServerName].Locations[0]
if defLoc.IsDefBackend && len(ing.Spec.Rules) == 0 {
glog.Infof("Ingress \"%v/%v\" defines a backend but no rule. Using it to configure the catch-all server %q", ing.Namespace, ing.Name, defServerName)
glog.Infof("Ingress %q defines a backend but no rule. Using it to configure the catch-all server %q",
ingKey, defServerName)

defLoc.IsDefBackend = false
defLoc.Backend = backendUpstream.Name
Expand All @@ -919,7 +935,8 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
defLoc.LuaRestyWAF = anns.LuaRestyWAF
defLoc.InfluxDB = anns.InfluxDB
} else {
glog.V(3).Infof("Ingress \"%v/%v\" defines both a backend and rules. Using its backend as default upstream for all its rules.", ing.Namespace, ing.Name)
glog.V(3).Infof("Ingress %q defines both a backend and rules. Using its backend as default upstream for all its rules.",
ingKey)
}
}
}
Expand Down Expand Up @@ -953,9 +970,11 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,

// configure default location, alias, and SSL
for _, ing := range data {
anns, err := n.store.GetIngressAnnotations(ing)
ingKey := k8s.MetaNamespaceKey(ing)

anns, err := n.store.GetIngressAnnotations(ingKey)
if err != nil {
glog.Errorf("Error reading Ingress %q annotations from local store: %v", ing.Name, err)
glog.Errorf("Error getting Ingress annotations %q: %v", ingKey, err)
}

for _, rule := range ing.Spec.Rules {
Expand All @@ -971,17 +990,17 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
aliases["Alias"] = host
}
} else {
glog.Warningf("Aliases already configured for server %q, skipping (Ingress \"%v/%v\")",
host, ing.Namespace, ing.Name)
glog.Warningf("Aliases already configured for server %q, skipping (Ingress %q)",
host, ingKey)
}
}

if anns.ServerSnippet != "" {
if servers[host].ServerSnippet == "" {
servers[host].ServerSnippet = anns.ServerSnippet
} else {
glog.Warningf("Server snippet already configured for server %q, skipping (Ingress \"%v/%v\")",
host, ing.Namespace, ing.Name)
glog.Warningf("Server snippet already configured for server %q, skipping (Ingress %q)",
host, ingKey)
}
}

Expand All @@ -996,7 +1015,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
}

if len(ing.Spec.TLS) == 0 {
glog.V(3).Infof("Ingress \"%v/%v\" does not contains a TLS section.", ing.Namespace, ing.Name)
glog.V(3).Infof("Ingress %q does not contains a TLS section.", ingKey)
continue
}

Expand All @@ -1009,22 +1028,23 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
continue
}

key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName)
cert, err := n.store.GetLocalSSLCert(key)
secrKey := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName)
cert, err := n.store.GetLocalSSLCert(secrKey)
if err != nil {
glog.Warningf("SSL certificate %q does not exist in local store.", key)
glog.Warningf("Error getting SSL certificate %q: %v", secrKey, err)
continue
}

err = cert.Certificate.VerifyHostname(host)
if err != nil {
glog.Warningf("Unexpected error validating SSL certificate %q for server %q: %v", key, host, err)
glog.Warningf("Unexpected error validating SSL certificate %q for server %q: %v", secrKey, host, err)
glog.Warning("Validating certificate against DNS names. This will be deprecated in a future version.")
// check the Common Name field
// https://github.com/golang/go/issues/22922
err := verifyHostname(host, cert.Certificate)
if err != nil {
glog.Warningf("SSL certificate %q does not contain a Common Name or Subject Alternative Name for server %q: %v", key, host, err)
glog.Warningf("SSL certificate %q does not contain a Common Name or Subject Alternative Name for server %q: %v",
secrKey, host, err)
continue
}
}
Expand Down Expand Up @@ -1065,10 +1085,11 @@ func extractTLSSecretName(host string, ing *extensions.Ingress,

// no TLS host matching host name, try each TLS host for matching SAN or CN
for _, tls := range ing.Spec.TLS {
key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName)
cert, err := getLocalSSLCert(key)
secrKey := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName)

cert, err := getLocalSSLCert(secrKey)
if err != nil {
glog.Warningf("SSL certificate %q does not exist in local store.", key)
glog.Warningf("Error getting SSL certificate %q: %v", secrKey, err)
continue
}

Expand All @@ -1080,7 +1101,7 @@ func extractTLSSecretName(host string, ing *extensions.Ingress,
if err != nil {
continue
}
glog.V(3).Infof("Found SSL certificate matching host %q: %q", host, key)
glog.V(3).Infof("Found SSL certificate matching host %q: %q", host, secrKey)
return tls.SecretName
}

Expand Down
17 changes: 10 additions & 7 deletions internal/ingress/controller/endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ import (

"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/healthcheck"
"k8s.io/ingress-nginx/internal/k8s"
)

// getEndpoints returns a list of Endpoint structs for a given service/target port combination.
func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Protocol, hz *healthcheck.Config,
getServiceEndpoints func(*corev1.Service) (*corev1.Endpoints, error)) []ingress.Endpoint {
getServiceEndpoints func(string) (*corev1.Endpoints, error)) []ingress.Endpoint {

upsServers := []ingress.Endpoint{}

Expand All @@ -43,13 +44,15 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot
// contains multiple port definitions sharing the same targetport
processedUpstreamServers := make(map[string]struct{})

svcKey := k8s.MetaNamespaceKey(s)

// ExternalName services
if s.Spec.Type == corev1.ServiceTypeExternalName {
glog.V(3).Infof("Ingress using Service %q of type ExternalName.", s.Name)
glog.V(3).Infof("Ingress using Service %q of type ExternalName.", svcKey)

targetPort := port.TargetPort.IntValue()
if targetPort <= 0 {
glog.Errorf("ExternalName Service %q has an invalid port (%v)", s.Name, targetPort)
glog.Errorf("ExternalName Service %q has an invalid port (%v)", svcKey, targetPort)
return upsServers
}

Expand All @@ -69,10 +72,10 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot
})
}

glog.V(3).Infof("Getting Endpoints for Service \"%v/%v\" and port %v", s.Namespace, s.Name, port.String())
ep, err := getServiceEndpoints(s)
glog.V(3).Infof("Getting Endpoints for Service %q and port %v", svcKey, port.String())
ep, err := getServiceEndpoints(svcKey)
if err != nil {
glog.Warningf("Error obtaining Endpoints for Service \"%v/%v\": %v", s.Namespace, s.Name, err)
glog.Warningf("Error obtaining Endpoints for Service %q: %v", svcKey, err)
return upsServers
}

Expand Down Expand Up @@ -114,6 +117,6 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot
}
}

glog.V(3).Infof("Endpoints found for Service \"%v/%v\": %v", s.Namespace, s.Name, upsServers)
glog.V(3).Infof("Endpoints found for Service %q: %v", svcKey, upsServers)
return upsServers
}
Loading

0 comments on commit 472dcb3

Please sign in to comment.