diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 408d918ac7..8c9ffea389 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -137,16 +137,17 @@ func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIng } // AddOrUpdateVirtualServer adds or updates NGINX configuration for the VirtualServer resource. -func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) error { - if err := cnf.addOrUpdateVirtualServer(virtualServerEx); err != nil { - return fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) +func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (Warnings, error) { + warnings, err := cnf.addOrUpdateVirtualServer(virtualServerEx) + if err != nil { + return warnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } if err := cnf.nginxManager.Reload(); err != nil { - return fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) + return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err) } - return nil + return warnings, nil } func (cnf *Configurator) addOrUpdateOpenTracingTracerConfig(content string) error { @@ -154,21 +155,22 @@ func (cnf *Configurator) addOrUpdateOpenTracingTracerConfig(content string) erro return err } -func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) error { +func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServerEx) (Warnings, error) { tlsPemFileName := "" if virtualServerEx.TLSSecret != nil { tlsPemFileName = cnf.addOrUpdateTLSSecret(virtualServerEx.TLSSecret) } - vsCfg := generateVirtualServerConfig(virtualServerEx, tlsPemFileName, cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured()) + vsc := newVirtualServerConfigurator(cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured()) + vsCfg, warnings := vsc.GenerateVirtualServerConfig(virtualServerEx, tlsPemFileName) name := getFileNameForVirtualServer(virtualServerEx.VirtualServer) content, err := cnf.templateExecutorV2.ExecuteVirtualServerTemplate(&vsCfg) if err != nil { - return fmt.Errorf("Error generating VirtualServer config: %v: %v", name, err) + return warnings, fmt.Errorf("Error generating VirtualServer config: %v: %v", name, err) } cnf.nginxManager.CreateConfig(name, content) - return nil + return warnings, nil } func (cnf *Configurator) updateTLSSecrets(ingEx *IngressEx) map[string]string { @@ -220,7 +222,6 @@ func (cnf *Configurator) AddOrUpdateJWKSecret(secret *api_v1.Secret) { // AddOrUpdateTLSSecret adds or updates a file with the content of the TLS secret. func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []IngressEx, mergeableIngresses []MergeableIngresses, virtualServerExes []*VirtualServerEx) error { cnf.addOrUpdateTLSSecret(secret) - for i := range ingExes { err := cnf.addOrUpdateIngress(&ingExes[i]) if err != nil { @@ -236,7 +237,8 @@ func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []I } for _, vsEx := range virtualServerExes { - err := cnf.addOrUpdateVirtualServer(vsEx) + // It is safe to ignore warnings here as no new warnings should appear when adding or updating a secret + _, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { return fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) } @@ -301,7 +303,8 @@ func (cnf *Configurator) DeleteSecret(key string, ingExes []IngressEx, mergeable } for _, vsEx := range virtualServerExes { - err := cnf.addOrUpdateVirtualServer(vsEx) + // It is safe to ignore warnings here as no new warnings should appear when deleting a secret + _, err := cnf.addOrUpdateVirtualServer(vsEx) if err != nil { return fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err) } @@ -412,7 +415,8 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V reloadPlus := false for _, vs := range virtualServerExes { - err := cnf.addOrUpdateVirtualServer(vs) + // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for VirtualServers + _, err := cnf.addOrUpdateVirtualServer(vs) if err != nil { return fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", vs.VirtualServer.Namespace, vs.VirtualServer.Name, err) } @@ -505,13 +509,14 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error { } // UpdateConfig updates NGINX configuration parameters. -func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*IngressEx, mergeableIngs map[string]*MergeableIngresses, virtualServerExes []*VirtualServerEx) error { +func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*IngressEx, mergeableIngs map[string]*MergeableIngresses, virtualServerExes []*VirtualServerEx) (Warnings, error) { cnf.cfgParams = cfgParams + allWarnings := newWarnings() if cnf.cfgParams.MainServerSSLDHParamFileContent != nil { fileName, err := cnf.nginxManager.CreateDHParam(*cnf.cfgParams.MainServerSSLDHParamFileContent) if err != nil { - return fmt.Errorf("Error when updating dhparams: %v", err) + return allWarnings, fmt.Errorf("Error when updating dhparams: %v", err) } cfgParams.MainServerSSLDHParam = fileName } @@ -519,52 +524,54 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres if cfgParams.MainTemplate != nil { err := cnf.templateExecutor.UpdateMainTemplate(cfgParams.MainTemplate) if err != nil { - return fmt.Errorf("Error when parsing the main template: %v", err) + return allWarnings, fmt.Errorf("Error when parsing the main template: %v", err) } } if cfgParams.IngressTemplate != nil { err := cnf.templateExecutor.UpdateIngressTemplate(cfgParams.IngressTemplate) if err != nil { - return fmt.Errorf("Error when parsing the ingress template: %v", err) + return allWarnings, fmt.Errorf("Error when parsing the ingress template: %v", err) } } mainCfg := GenerateNginxMainConfig(cnf.staticCfgParams, cfgParams) mainCfgContent, err := cnf.templateExecutor.ExecuteMainConfigTemplate(mainCfg) if err != nil { - return fmt.Errorf("Error when writing main Config") + return allWarnings, fmt.Errorf("Error when writing main Config") } cnf.nginxManager.CreateMainConfig(mainCfgContent) for _, ingEx := range ingExes { if err := cnf.addOrUpdateIngress(ingEx); err != nil { - return err + return allWarnings, err } } for _, mergeableIng := range mergeableIngs { if err := cnf.addOrUpdateMergeableIngress(mergeableIng); err != nil { - return err + return allWarnings, err } } for _, vsEx := range virtualServerExes { - if err := cnf.addOrUpdateVirtualServer(vsEx); err != nil { - return err + warnings, err := cnf.addOrUpdateVirtualServer(vsEx) + if err != nil { + return allWarnings, err } + allWarnings.Add(warnings) } if mainCfg.OpenTracingLoadModule { if err := cnf.addOrUpdateOpenTracingTracerConfig(mainCfg.OpenTracingTracerConfig); err != nil { - return fmt.Errorf("Error when updating OpenTracing tracer config: %v", err) + return allWarnings, fmt.Errorf("Error when updating OpenTracing tracer config: %v", err) } } cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule) if err := cnf.nginxManager.Reload(); err != nil { - return fmt.Errorf("Error when updating config from ConfigMap: %v", err) + return allWarnings, fmt.Errorf("Error when updating config from ConfigMap: %v", err) } - return nil + return allWarnings, nil } func keyToFileName(key string) string { diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 896c66b217..7c2eec57c9 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -7,6 +7,7 @@ import ( "github.com/golang/glog" "github.com/nginxinc/kubernetes-ingress/internal/nginx" api_v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" @@ -109,99 +110,54 @@ func newHealthCheckWithDefaults(upstream conf_v1alpha1.Upstream, upstreamName st } } -func generateHealthCheck(upstream conf_v1alpha1.Upstream, upstreamName string, cfgParams *ConfigParams) *version2.HealthCheck { - if upstream.HealthCheck == nil || !upstream.HealthCheck.Enable { - return nil - } - - hc := newHealthCheckWithDefaults(upstream, upstreamName, cfgParams) - - if upstream.HealthCheck.Path != "" { - hc.URI = upstream.HealthCheck.Path - } - - if upstream.HealthCheck.Interval != "" { - hc.Interval = upstream.HealthCheck.Interval - } - - if upstream.HealthCheck.Jitter != "" { - hc.Jitter = upstream.HealthCheck.Jitter - } - - if upstream.HealthCheck.Fails > 0 { - hc.Fails = upstream.HealthCheck.Fails - } - - if upstream.HealthCheck.Passes > 0 { - hc.Passes = upstream.HealthCheck.Passes - } - - if upstream.HealthCheck.Port > 0 { - hc.Port = upstream.HealthCheck.Port - } - - if upstream.HealthCheck.ConnectTimeout != "" { - hc.ProxyConnectTimeout = upstream.HealthCheck.ConnectTimeout - } - - if upstream.HealthCheck.ReadTimeout != "" { - hc.ProxyReadTimeout = upstream.HealthCheck.ReadTimeout - } - - if upstream.HealthCheck.SendTimeout != "" { - hc.ProxySendTimeout = upstream.HealthCheck.SendTimeout - } - - for _, h := range upstream.HealthCheck.Headers { - hc.Headers[h.Name] = h.Value - } - - if upstream.HealthCheck.TLS != nil { - hc.ProxyPass = fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream.HealthCheck.TLS.Enable), upstreamName) - } - - if upstream.HealthCheck.StatusMatch != "" { - hc.Match = generateStatusMatchName(upstreamName) - } - - return hc +// VirtualServerConfigurator generates a VirtualServer configuration +type virtualServerConfigurator struct { + cfgParams *ConfigParams + isPlus bool + isResolverConfigured bool + warnings Warnings } -func generateStatusMatchName(upstreamName string) string { - return fmt.Sprintf("%s_match", upstreamName) +func (vsc *virtualServerConfigurator) addWarningf(obj runtime.Object, msgFmt string, args ...interface{}) { + vsc.warnings[obj] = append(vsc.warnings[obj], fmt.Sprintf(msgFmt, args...)) } -func generateUpstreamStatusMatch(upstreamName string, status string) version2.StatusMatch { - return version2.StatusMatch{ - Name: generateStatusMatchName(upstreamName), - Code: status, - } +func (vsc *virtualServerConfigurator) clearWarnings() { + vsc.warnings = make(map[runtime.Object][]string) } -// GenerateExternalNameSvcKey returns the key to identify an ExternalName service. -func GenerateExternalNameSvcKey(namespace string, service string) string { - return fmt.Sprintf("%v/%v", namespace, service) +// newVirtualServerConfigurator creates a new VirtualServerConfigurator +func newVirtualServerConfigurator(cfgParams *ConfigParams, isPlus bool, isResolverConfigured bool) *virtualServerConfigurator { + return &virtualServerConfigurator{ + cfgParams: cfgParams, + isPlus: isPlus, + isResolverConfigured: isResolverConfigured, + warnings: make(map[runtime.Object][]string), + } } -func generateEndpointsForUpstream(namespace string, upstream conf_v1alpha1.Upstream, virtualServerEx *VirtualServerEx, isResolverConfigured bool, isPlus bool) []string { +func (vsc *virtualServerConfigurator) generateEndpointsForUpstream(owner runtime.Object, namespace string, upstream conf_v1alpha1.Upstream, virtualServerEx *VirtualServerEx) []string { endpointsKey := GenerateEndpointsKey(namespace, upstream.Service, upstream.Port) externalNameSvcKey := GenerateExternalNameSvcKey(namespace, upstream.Service) endpoints := virtualServerEx.Endpoints[endpointsKey] - if !isPlus && len(endpoints) == 0 { + if !vsc.isPlus && len(endpoints) == 0 { return []string{nginx502Server} } _, isExternalNameSvc := virtualServerEx.ExternalNameSvcs[externalNameSvcKey] - if isExternalNameSvc && !isResolverConfigured { - glog.Warningf("A resolver must be configured for Type ExternalName service %s, no upstream servers will be created", upstream.Service) + if isExternalNameSvc && !vsc.isResolverConfigured { + msgFmt := "Type ExternalName service %v in upstream %v will be ignored. To use ExternaName services, a resolver must be configured in the ConfigMap" + vsc.addWarningf(owner, msgFmt, upstream.Service, upstream.Name) endpoints = []string{} } return endpoints } -func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileName string, baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool) version2.VirtualServerConfig { - ssl := generateSSLConfig(virtualServerEx.VirtualServer.Spec.TLS, tlsPemFileName, baseCfgParams) +// GenerateVirtualServerConfig generates a full configuration for a VirtualServer +func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileName string) (version2.VirtualServerConfig, Warnings) { + vsc.clearWarnings() + ssl := generateSSLConfig(virtualServerEx.VirtualServer.Spec.TLS, tlsPemFileName, vsc.cfgParams) // crUpstreams maps an UpstreamName to its conf_v1alpha1.Upstream as they are generated // necessary for generateLocation to know what Upstream each Location references @@ -217,14 +173,15 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam for _, u := range virtualServerEx.VirtualServer.Spec.Upstreams { upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(u.Name) upstreamNamespace := virtualServerEx.VirtualServer.Namespace - endpoints := generateEndpointsForUpstream(upstreamNamespace, u, virtualServerEx, isResolverConfigured, isPlus) + endpoints := vsc.generateEndpointsForUpstream(virtualServerEx.VirtualServer, upstreamNamespace, u, virtualServerEx) + // isExternalNameSvc is always false for OSS _, isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) + ups := vsc.generateUpstream(virtualServerEx.VirtualServer, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) crUpstreams[upstreamName] = u - if hc := generateHealthCheck(u, upstreamName, baseCfgParams); hc != nil { + if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { healthChecks = append(healthChecks, *hc) if u.HealthCheck.StatusMatch != "" { statusMatches = append(statusMatches, generateUpstreamStatusMatch(upstreamName, u.HealthCheck.StatusMatch)) @@ -237,14 +194,15 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam for _, u := range vsr.Spec.Upstreams { upstreamName := upstreamNamer.GetNameForUpstream(u.Name) upstreamNamespace := vsr.Namespace - endpoints := generateEndpointsForUpstream(upstreamNamespace, u, virtualServerEx, isResolverConfigured, isPlus) + endpoints := vsc.generateEndpointsForUpstream(vsr, upstreamNamespace, u, virtualServerEx) + // isExternalNameSvc is always false for OSS _, isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) + ups := vsc.generateUpstream(vsr, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) crUpstreams[upstreamName] = u - if hc := generateHealthCheck(u, upstreamName, baseCfgParams); hc != nil { + if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { healthChecks = append(healthChecks, *hc) if u.HealthCheck.StatusMatch != "" { statusMatches = append(statusMatches, generateUpstreamStatusMatch(upstreamName, u.HealthCheck.StatusMatch)) @@ -270,13 +228,13 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam } if len(r.Splits) > 0 { - splitCfg := generateSplitRouteConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, len(splitClients), baseCfgParams) + splitCfg := generateSplitRouteConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams) splitClients = append(splitClients, splitCfg.SplitClient) locations = append(locations, splitCfg.Locations...) internalRedirectLocations = append(internalRedirectLocations, splitCfg.InternalRedirectLocation) } else if r.Rules != nil { - rulesRouteCfg := generateRulesRouteConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, rulesRoutes, baseCfgParams) + rulesRouteCfg := generateRulesRouteConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, rulesRoutes, vsc.cfgParams) maps = append(maps, rulesRouteCfg.Maps...) locations = append(locations, rulesRouteCfg.Locations...) @@ -286,7 +244,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam } else { upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(r.Upstream) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, baseCfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, vsc.cfgParams) locations = append(locations, loc) } @@ -297,13 +255,13 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam upstreamNamer := newUpstreamNamerForVirtualServerRoute(virtualServerEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { if len(r.Splits) > 0 { - splitCfg := generateSplitRouteConfig(r, upstreamNamer, crUpstreams, variableNamer, len(splitClients), baseCfgParams) + splitCfg := generateSplitRouteConfig(r, upstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams) splitClients = append(splitClients, splitCfg.SplitClient) locations = append(locations, splitCfg.Locations...) internalRedirectLocations = append(internalRedirectLocations, splitCfg.InternalRedirectLocation) } else if r.Rules != nil { - rulesRouteCfg := generateRulesRouteConfig(r, upstreamNamer, crUpstreams, variableNamer, rulesRoutes, baseCfgParams) + rulesRouteCfg := generateRulesRouteConfig(r, upstreamNamer, crUpstreams, variableNamer, rulesRoutes, vsc.cfgParams) maps = append(maps, rulesRouteCfg.Maps...) locations = append(locations, rulesRouteCfg.Locations...) @@ -313,13 +271,13 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam } else { upstreamName := upstreamNamer.GetNameForUpstream(r.Upstream) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, baseCfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, vsc.cfgParams) locations = append(locations, loc) } } } - return version2.VirtualServerConfig{ + vscfg := version2.VirtualServerConfig{ Upstreams: upstreams, SplitClients: splitClients, Maps: maps, @@ -327,24 +285,25 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam Server: version2.Server{ ServerName: virtualServerEx.VirtualServer.Spec.Host, StatusZone: virtualServerEx.VirtualServer.Spec.Host, - ProxyProtocol: baseCfgParams.ProxyProtocol, + ProxyProtocol: vsc.cfgParams.ProxyProtocol, SSL: ssl, - RedirectToHTTPSBasedOnXForwarderProto: baseCfgParams.RedirectToHTTPS, - ServerTokens: baseCfgParams.ServerTokens, - SetRealIPFrom: baseCfgParams.SetRealIPFrom, - RealIPHeader: baseCfgParams.RealIPHeader, - RealIPRecursive: baseCfgParams.RealIPRecursive, - Snippets: baseCfgParams.ServerSnippets, + RedirectToHTTPSBasedOnXForwarderProto: vsc.cfgParams.RedirectToHTTPS, + ServerTokens: vsc.cfgParams.ServerTokens, + SetRealIPFrom: vsc.cfgParams.SetRealIPFrom, + RealIPHeader: vsc.cfgParams.RealIPHeader, + RealIPRecursive: vsc.cfgParams.RealIPRecursive, + Snippets: vsc.cfgParams.ServerSnippets, InternalRedirectLocations: internalRedirectLocations, Locations: locations, HealthChecks: healthChecks, }, } + + return vscfg, vsc.warnings } -func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isExternalNameSvc bool, endpoints []string, cfgParams *ConfigParams, isPlus bool) version2.Upstream { +func (vsc *virtualServerConfigurator) generateUpstream(owner runtime.Object, upstreamName string, upstream conf_v1alpha1.Upstream, isExternalNameSvc bool, endpoints []string) version2.Upstream { var upsServers []version2.UpstreamServer - for _, e := range endpoints { s := version2.UpstreamServer{ Address: e, @@ -353,44 +312,116 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isEx upsServers = append(upsServers, s) } - lbMethod := generateLBMethod(upstream.LBMethod, cfgParams.LBMethod) + lbMethod := generateLBMethod(upstream.LBMethod, vsc.cfgParams.LBMethod) ups := version2.Upstream{ Name: upstreamName, Servers: upsServers, Resolve: isExternalNameSvc, LBMethod: lbMethod, - Keepalive: generateIntFromPointer(upstream.Keepalive, cfgParams.Keepalive), - MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), - FailTimeout: generateString(upstream.FailTimeout, cfgParams.FailTimeout), - MaxConns: generateIntFromPointer(upstream.MaxConns, cfgParams.MaxConns), - UpstreamZoneSize: cfgParams.UpstreamZoneSize, + Keepalive: generateIntFromPointer(upstream.Keepalive, vsc.cfgParams.Keepalive), + MaxFails: generateIntFromPointer(upstream.MaxFails, vsc.cfgParams.MaxFails), + FailTimeout: generateString(upstream.FailTimeout, vsc.cfgParams.FailTimeout), + MaxConns: generateIntFromPointer(upstream.MaxConns, vsc.cfgParams.MaxConns), + UpstreamZoneSize: vsc.cfgParams.UpstreamZoneSize, } - if isPlus { - ups.SlowStart = generateSlowStartForPlus(upstream, lbMethod) + if vsc.isPlus { + ups.SlowStart = vsc.generateSlowStartForPlus(owner, upstream, lbMethod) ups.Queue = generateQueueForPlus(upstream.Queue, "60s") } return ups } -func generateSlowStartForPlus(upstream conf_v1alpha1.Upstream, lbMethod string) string { +func (vsc *virtualServerConfigurator) generateSlowStartForPlus(owner runtime.Object, upstream conf_v1alpha1.Upstream, lbMethod string) string { if upstream.SlowStart == "" { return "" } - if strings.HasPrefix(lbMethod, "hash") { - glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name) + _, isIncompatible := incompatibleLBMethodsForSlowStart[lbMethod] + isHash := strings.HasPrefix(lbMethod, "hash") + if isIncompatible || isHash { + msgFmt := "Slow start will be disabled for upstream %v because lb method '%v' is incompatible with slow start" + vsc.addWarningf(owner, msgFmt, upstream.Name, lbMethod) return "" } - if _, exists := incompatibleLBMethodsForSlowStart[lbMethod]; exists { - glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name) - return "" + return upstream.SlowStart +} + +func generateHealthCheck(upstream conf_v1alpha1.Upstream, upstreamName string, cfgParams *ConfigParams) *version2.HealthCheck { + if upstream.HealthCheck == nil || !upstream.HealthCheck.Enable { + return nil } - return upstream.SlowStart + hc := newHealthCheckWithDefaults(upstream, upstreamName, cfgParams) + + if upstream.HealthCheck.Path != "" { + hc.URI = upstream.HealthCheck.Path + } + + if upstream.HealthCheck.Interval != "" { + hc.Interval = upstream.HealthCheck.Interval + } + + if upstream.HealthCheck.Jitter != "" { + hc.Jitter = upstream.HealthCheck.Jitter + } + + if upstream.HealthCheck.Fails > 0 { + hc.Fails = upstream.HealthCheck.Fails + } + + if upstream.HealthCheck.Passes > 0 { + hc.Passes = upstream.HealthCheck.Passes + } + + if upstream.HealthCheck.Port > 0 { + hc.Port = upstream.HealthCheck.Port + } + + if upstream.HealthCheck.ConnectTimeout != "" { + hc.ProxyConnectTimeout = upstream.HealthCheck.ConnectTimeout + } + + if upstream.HealthCheck.ReadTimeout != "" { + hc.ProxyReadTimeout = upstream.HealthCheck.ReadTimeout + } + + if upstream.HealthCheck.SendTimeout != "" { + hc.ProxySendTimeout = upstream.HealthCheck.SendTimeout + } + + for _, h := range upstream.HealthCheck.Headers { + hc.Headers[h.Name] = h.Value + } + + if upstream.HealthCheck.TLS != nil { + hc.ProxyPass = fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream.HealthCheck.TLS.Enable), upstreamName) + } + + if upstream.HealthCheck.StatusMatch != "" { + hc.Match = generateStatusMatchName(upstreamName) + } + + return hc +} + +func generateStatusMatchName(upstreamName string) string { + return fmt.Sprintf("%s_match", upstreamName) +} + +func generateUpstreamStatusMatch(upstreamName string, status string) version2.StatusMatch { + return version2.StatusMatch{ + Name: generateStatusMatchName(upstreamName), + Code: status, + } +} + +// GenerateExternalNameSvcKey returns the key to identify an ExternalName service. +func GenerateExternalNameSvcKey(namespace string, service string) string { + return fmt.Sprintf("%v/%v", namespace, service) } func generateLBMethod(method string, defaultMethod string) string { @@ -710,8 +741,10 @@ func createEndpointsFromUpstream(upstream version2.Upstream) []string { func createUpstreamsForPlus(virtualServerEx *VirtualServerEx, baseCfgParams *ConfigParams) []version2.Upstream { var upstreams []version2.Upstream + isPlus := true upstreamNamer := newUpstreamNamerForVirtualServer(virtualServerEx.VirtualServer) + vsc := newVirtualServerConfigurator(baseCfgParams, isPlus, false) for _, u := range virtualServerEx.VirtualServer.Spec.Upstreams { isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(virtualServerEx.VirtualServer.Namespace, u.Service)] @@ -726,7 +759,7 @@ func createUpstreamsForPlus(virtualServerEx *VirtualServerEx, baseCfgParams *Con endpointsKey := GenerateEndpointsKey(upstreamNamespace, u.Service, u.Port) endpoints := virtualServerEx.Endpoints[endpointsKey] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) + ups := vsc.generateUpstream(virtualServerEx.VirtualServer, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) } @@ -745,7 +778,7 @@ func createUpstreamsForPlus(virtualServerEx *VirtualServerEx, baseCfgParams *Con endpointsKey := GenerateEndpointsKey(upstreamNamespace, u.Service, u.Port) endpoints := virtualServerEx.Endpoints[endpointsKey] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) + ups := vsc.generateUpstream(vsr, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) } } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 75c58650d3..0be964f59b 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -287,9 +287,14 @@ func TestGenerateVirtualServerConfig(t *testing.T) { isPlus := false isResolverConfigured := false tlsPemFileName := "" - result := generateVirtualServerConfig(&virtualServerEx, tlsPemFileName, &baseCfgParams, isPlus, isResolverConfigured) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + } + + if len(warnings) != 0 { + t.Errorf("GenerateVirtualServerConfig returned warnings: %v", vsc.warnings) } } func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { @@ -504,9 +509,14 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { isPlus := false isResolverConfigured := false tlsPemFileName := "" - result := generateVirtualServerConfig(&virtualServerEx, tlsPemFileName, &baseCfgParams, isPlus, isResolverConfigured) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + } + + if len(warnings) != 0 { + t.Errorf("GenerateVirtualServerConfig returned warnings: %v", vsc.warnings) } } @@ -762,9 +772,14 @@ func TestGenerateVirtualServerConfigForVirtualServerWithRules(t *testing.T) { isPlus := false isResolverConfigured := false tlsPemFileName := "" - result := generateVirtualServerConfig(&virtualServerEx, tlsPemFileName, &baseCfgParams, isPlus, isResolverConfigured) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + } + + if len(warnings) != 0 { + t.Errorf("GenerateVirtualServerConfig returned warnings: %v", vsc.warnings) } } @@ -798,10 +813,15 @@ func TestGenerateUpstream(t *testing.T) { UpstreamZoneSize: "256k", } - result := generateUpstream(name, upstream, false, endpoints, &cfgParams, false) + vsc := newVirtualServerConfigurator(&cfgParams, false, false) + result := vsc.generateUpstream(&conf_v1alpha1.VirtualServer{}, name, upstream, false, endpoints) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } + + if len(vsc.warnings) != 0 { + t.Errorf("generateUpstream returned warnings for %v", upstream) + } } func TestGenerateUpstreamWithKeepalive(t *testing.T) { @@ -862,10 +882,15 @@ func TestGenerateUpstreamWithKeepalive(t *testing.T) { } for _, test := range tests { - result := generateUpstream(name, test.upstream, false, endpoints, test.cfgParams, false) + vsc := newVirtualServerConfigurator(test.cfgParams, false, false) + result := vsc.generateUpstream(&conf_v1alpha1.VirtualServer{}, name, test.upstream, false, endpoints) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } + + if len(vsc.warnings) != 0 { + t.Errorf("generateUpstream() returned warnings for %v", test.upstream) + } } } @@ -885,11 +910,15 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { Resolve: true, } - result := generateUpstream(name, upstream, true, endpoints, &cfgParams, false) + vsc := newVirtualServerConfigurator(&cfgParams, true, true) + result := vsc.generateUpstream(&conf_v1alpha1.VirtualServer{}, name, upstream, true, endpoints) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } + if len(vsc.warnings) != 0 { + t.Errorf("generateUpstream() returned warnings for %v", upstream) + } } func TestGenerateProxyPassProtocol(t *testing.T) { @@ -1899,6 +1928,7 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { isPlus bool isResolverConfigured bool expected []string + warningsExpected bool msg string }{ { @@ -1946,6 +1976,7 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { }, isPlus: true, isResolverConfigured: false, + warningsExpected: true, expected: []string{}, msg: "ExternalName service without resolver configured", }, @@ -2011,11 +2042,22 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { } for _, test := range tests { - result := generateEndpointsForUpstream(namespace, test.upstream, test.vsEx, test.isResolverConfigured, test.isPlus) + vsc := newVirtualServerConfigurator(&ConfigParams{}, test.isPlus, test.isResolverConfigured) + result := vsc.generateEndpointsForUpstream(test.vsEx.VirtualServer, namespace, test.upstream, test.vsEx) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateEndpointsForUpstream(isPlus=%v, isResolverConfigured=%v) returned %v, but expected %v for case: %v", test.isPlus, test.isResolverConfigured, result, test.expected, test.msg) } + + if len(vsc.warnings) == 0 && test.warningsExpected { + t.Errorf("generateEndpointsForUpstream(isPlus=%v, isResolverConfigured=%v) didn't return any warnings for %v but warnings expected", + test.isPlus, test.isResolverConfigured, test.upstream) + } + + if len(vsc.warnings) != 0 && !test.warningsExpected { + t.Errorf("generateEndpointsForUpstream(isPlus=%v, isResolverConfigured=%v) returned warnings for %v", + test.isPlus, test.isResolverConfigured, test.upstream) + } } } @@ -2035,11 +2077,16 @@ func TestGenerateSlowStartForPlusWithInCompatibleLBMethods(t *testing.T) { } for _, lbMethod := range tests { - result := generateSlowStartForPlus(upstream, lbMethod) + vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false) + result := vsc.generateSlowStartForPlus(&conf_v1alpha1.VirtualServer{}, upstream, lbMethod) if !reflect.DeepEqual(result, expected) { t.Errorf("generateSlowStartForPlus returned %v, but expected %v for lbMethod %v", result, expected, lbMethod) } + + if len(vsc.warnings) == 0 { + t.Errorf("generateSlowStartForPlus returned no warnings for %v but warnings expected", upstream) + } } } @@ -2065,10 +2112,15 @@ func TestGenerateSlowStartForPlus(t *testing.T) { } for _, test := range tests { - result := generateSlowStartForPlus(test.upstream, test.lbMethod) + vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false) + result := vsc.generateSlowStartForPlus(&conf_v1alpha1.VirtualServer{}, test.upstream, test.lbMethod) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) } + + if len(vsc.warnings) != 0 { + t.Errorf("generateSlowStartForPlus returned warnings for %v", test.upstream) + } } } @@ -2146,7 +2198,8 @@ func TestGenerateUpstreamWithQueue(t *testing.T) { } for _, test := range tests { - result := generateUpstream(test.name, test.upstream, false, []string{}, &ConfigParams{}, test.isPlus) + vsc := newVirtualServerConfigurator(&ConfigParams{}, test.isPlus, false) + result := vsc.generateUpstream(&conf_v1alpha1.VirtualServer{}, test.name, test.upstream, false, []string{}) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } diff --git a/internal/configs/warnings.go b/internal/configs/warnings.go new file mode 100644 index 0000000000..8e4e814070 --- /dev/null +++ b/internal/configs/warnings.go @@ -0,0 +1,17 @@ +package configs + +import "k8s.io/apimachinery/pkg/runtime" + +// Warnings stores a list of warnings for a given runtime k8s object in a map +type Warnings map[runtime.Object][]string + +func newWarnings() Warnings { + return make(map[runtime.Object][]string) +} + +// Add adds new Warnings to the map +func (w Warnings) Add(warnings Warnings) { + for k, v := range warnings { + w[k] = v + } +} diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 1ef29d253f..345794b166 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -402,7 +402,7 @@ func (lbc *LoadBalancerController) syncEndpoint(task task) { if len(virtualServersExes) > 0 { glog.V(3).Infof("Updating endpoints for %v", virtualServersExes) - err = lbc.configurator.UpdateEndpointsForVirtualServers(virtualServersExes) + err := lbc.configurator.UpdateEndpointsForVirtualServers(virtualServersExes) if err != nil { glog.Errorf("Error updating endpoints for %v: %v", virtualServersExes, err) } @@ -445,7 +445,7 @@ func (lbc *LoadBalancerController) syncConfig(task task) { virtualServerExes = lbc.virtualServersToVirtualServerExes(virtualServers) } - updateErr := lbc.configurator.UpdateConfig(cfgParams, ingExes, mergeableIngresses, virtualServerExes) + warnings, updateErr := lbc.configurator.UpdateConfig(cfgParams, ingExes, mergeableIngresses, virtualServerExes) eventTitle := "Updated" eventType := api_v1.EventTypeNormal @@ -456,9 +456,15 @@ func (lbc *LoadBalancerController) syncConfig(task task) { eventType = api_v1.EventTypeWarning eventWarningMessage = fmt.Sprintf("but was not applied: %v", updateErr) } + cmWarningMessage := eventWarningMessage + + if len(warnings) > 0 && updateErr == nil { + cmWarningMessage = "with warnings. Please check the logs" + } + if configExists { cfgm := obj.(*api_v1.ConfigMap) - lbc.recorder.Eventf(cfgm, eventType, eventTitle, "Configuration from %v was updated %s", key, eventWarningMessage) + lbc.recorder.Eventf(cfgm, eventType, eventTitle, "Configuration from %v was updated %s", key, cmWarningMessage) } for _, ingEx := range ingExes { lbc.recorder.Eventf(ingEx.Ingress, eventType, eventTitle, "Configuration for %v/%v was updated %s", @@ -473,11 +479,30 @@ func (lbc *LoadBalancerController) syncConfig(task task) { } } for _, vsEx := range virtualServerExes { - lbc.recorder.Eventf(vsEx.VirtualServer, eventType, eventTitle, "Configuration for %v/%v was updated %s", - vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, eventWarningMessage) + vsEventType := eventType + vsEventTitle := eventTitle + vsEventWarningMessage := eventWarningMessage + + if messages, ok := warnings[vsEx.VirtualServer]; ok && updateErr == nil { + vsEventType = api_v1.EventTypeWarning + vsEventTitle = "UpdatedWithWarning" + vsEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + } + + lbc.recorder.Eventf(vsEx.VirtualServer, vsEventType, vsEventTitle, "Configuration for %v/%v was updated %s", + vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, vsEventWarningMessage) + for _, vsr := range vsEx.VirtualServerRoutes { - lbc.recorder.Eventf(vsr, eventType, eventTitle, "Configuration for %v/%v was updated %s", - vsr.Namespace, vsr.Name, eventWarningMessage) + vsrEventType := eventType + vsrEventTitle := eventTitle + vsrEventWarningMessage := eventWarningMessage + if messages, ok := warnings[vsr]; ok && updateErr == nil { + vsrEventType = api_v1.EventTypeWarning + vsrEventTitle = "UpdatedWithWarning" + vsrEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + } + lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, "Configuration for %v/%v was updated %s", + vsr.Namespace, vsr.Name, vsrEventWarningMessage) } } } @@ -610,7 +635,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } - addErr := lbc.configurator.AddOrUpdateVirtualServer(vsEx) + warnings, addErr := lbc.configurator.AddOrUpdateVirtualServer(vsEx) eventTitle := "AddedOrUpdated" eventType := api_v1.EventTypeNormal @@ -622,10 +647,31 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { eventWarningMessage = fmt.Sprintf("but was not applied: %v", addErr) } - lbc.recorder.Eventf(vs, eventType, eventTitle, "Configuration for %v was added or updated %s", key, eventWarningMessage) + vsEventType := eventType + vsEventTitle := eventTitle + vsEventWarningMessage := eventWarningMessage + + if messages, ok := warnings[vsEx.VirtualServer]; ok && addErr == nil { + vsEventType = api_v1.EventTypeWarning + vsEventTitle = "AddedOrUpdatedWithWarning" + vsEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + } + + lbc.recorder.Eventf(vs, vsEventType, vsEventTitle, "Configuration for %v was added or updated %s", key, vsEventWarningMessage) + for _, vsr := range vsEx.VirtualServerRoutes { - lbc.recorder.Eventf(vsr, eventType, eventTitle, "Configuration for %v/%v was added or updated %s", vsr.Namespace, vsr.Name, eventWarningMessage) + vsrEventType := eventType + vsrEventTitle := eventTitle + vsrEventWarningMessage := eventWarningMessage + + if messages, ok := warnings[vsr]; ok && addErr == nil { + vsrEventType = api_v1.EventTypeWarning + vsrEventTitle = "AddedOrUpdatedWithWarning" + vsrEventWarningMessage = fmt.Sprintf("with warning(s): %v", formatWarningMessages(messages)) + } + lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, "Configuration for %v/%v was added or updated %s", vsr.Namespace, vsr.Name, vsrEventWarningMessage) } + } func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { @@ -941,7 +987,8 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, ing virtualServerExes := lbc.virtualServersToVirtualServerExes(virtualServers) - if err := lbc.configurator.AddOrUpdateTLSSecret(secret, regular, mergeable, virtualServerExes); err != nil { + err := lbc.configurator.AddOrUpdateTLSSecret(secret, regular, mergeable, virtualServerExes) + if err != nil { glog.Errorf("Error when updating Secret %v: %v", secretNsName, err) lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "%v was updated, but not applied: %v", secretNsName, err) @@ -1949,3 +1996,7 @@ func (lbc *LoadBalancerController) createMergableIngresses(master *extensions.In return &mergeableIngresses, nil } + +func formatWarningMessages(w []string) string { + return strings.Join(w, "; ") +} diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 1ebf811f9c..0039ad22a2 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1507,3 +1507,14 @@ func TestFindVirtualServersForVirtualServerRoute(t *testing.T) { t.Errorf("findVirtualServersForVirtualServerRoute returned %v but expected %v", result, expected) } } + +func TestFormatWarningsMessages(t *testing.T) { + warnings := []string{"Test warning", "Test warning 2"} + + expected := "Test warning; Test warning 2" + result := formatWarningMessages(warnings) + + if result != expected { + t.Errorf("formatWarningMessages(%v) returned %v but expected %v", warnings, result, expected) + } +}