Make updating status on the router optional

Also unify a number of host setup paths. Makes F5 and template more
consistent.

Author: smarterclayton

pkg/cmd/infra/router/f5.go

@@ -18,6 +18,7 @@ import (
 	projectinternalclientset "github.com/openshift/origin/pkg/project/generated/internalclientset"
 	routeapi "github.com/openshift/origin/pkg/route/apis/route"
 	routeinternalclientset "github.com/openshift/origin/pkg/route/generated/internalclientset"
+	"github.com/openshift/origin/pkg/router"
 	"github.com/openshift/origin/pkg/router/controller"
 	f5plugin "github.com/openshift/origin/pkg/router/f5"
 	"github.com/openshift/origin/pkg/version"
@@ -46,8 +47,6 @@ type F5RouterOptions struct {
 
 // F5Router is the config necessary to start an F5 router plugin.
 type F5Router struct {
-	RouterName string
-
 	// Host specifies the hostname or IP address of the F5 BIG-IP host.
 	Host string
 
@@ -95,7 +94,6 @@ type F5Router struct {
 
 // Bind binds F5Router arguments to flags
 func (o *F5Router) Bind(flag *pflag.FlagSet) {
-	flag.StringVar(&o.RouterName, "name", util.Env("ROUTER_SERVICE_NAME", "public"), "The name the router will identify itself with in the route status")
 	flag.StringVar(&o.Host, "f5-host", util.Env("ROUTER_EXTERNAL_HOST_HOSTNAME", ""), "The host of F5 BIG-IP's management interface")
 	flag.StringVar(&o.Username, "f5-username", util.Env("ROUTER_EXTERNAL_HOST_USERNAME", ""), "The username for F5 BIG-IP's management utility")
 	flag.StringVar(&o.Password, "f5-password", util.Env("ROUTER_EXTERNAL_HOST_PASSWORD", ""), "The password for F5 BIG-IP's management utility")
@@ -230,9 +228,18 @@ func (o *F5RouterOptions) Run() error {
 		return err
 	}
 
-	statusPlugin := controller.NewStatusAdmitter(f5Plugin, routeclient.Route(), o.RouterName, "")
-	uniqueHostPlugin := controller.NewUniqueHost(statusPlugin, o.RouteSelectionFunc(), o.RouterSelection.DisableNamespaceOwnershipCheck, statusPlugin)
-	plugin := controller.NewHostAdmitter(uniqueHostPlugin, o.F5RouteAdmitterFunc(), false, o.RouterSelection.DisableNamespaceOwnershipCheck, statusPlugin)
+	var recorder controller.RejectionRecorder = controller.LogRejections
+	var plugin router.Plugin = f5Plugin
+	if o.UpdateStatus {
+		status := controller.NewStatusAdmitter(plugin, routeclient.Route(), o.RouterName, o.RouterCanonicalHostname)
+		recorder = status
+		plugin = status
+	}
+	if o.ExtendedValidation {
+		plugin = controller.NewExtendedValidator(plugin, recorder)
+	}
+	plugin = controller.NewUniqueHost(plugin, o.RouteSelectionFunc(), o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)
+	plugin = controller.NewHostAdmitter(plugin, o.F5RouteAdmitterFunc(), o.AllowWildcardRoutes, o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)
 
 	factory := o.RouterSelection.NewFactory(routeclient, projectclient.Project().Projects(), kc)
 	watchNodes := (len(o.InternalAddress) != 0 && len(o.VxlanGateway) != 0)

pkg/cmd/infra/router/router.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation"
 	kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
@@ -25,8 +26,13 @@ import (
 // RouterSelection controls what routes and resources on the server are considered
 // part of this router.
 type RouterSelection struct {
+	RouterName              string
+	RouterCanonicalHostname string
+
 	ResyncInterval time.Duration
 
+	UpdateStatus bool
+
 	HostnameTemplate string
 	OverrideHostname bool
 
@@ -52,13 +58,18 @@ type RouterSelection struct {
 
 	DisableNamespaceOwnershipCheck bool
 
+	ExtendedValidation bool
+
 	EnableIngress bool
 
 	ListenAddr string
 }
 
 // Bind sets the appropriate labels
 func (o *RouterSelection) Bind(flag *pflag.FlagSet) {
+	flag.StringVar(&o.RouterName, "name", cmdutil.Env("ROUTER_SERVICE_NAME", "public"), "The name the router will identify itself with in the route status")
+	flag.StringVar(&o.RouterCanonicalHostname, "router-canonical-hostname", cmdutil.Env("ROUTER_CANONICAL_HOSTNAME", ""), "CanonicalHostname is the external host name for the router that can be used as a CNAME for the host requested for this route. This value is optional and may not be set in all cases.")
+	flag.BoolVar(&o.UpdateStatus, "update-status", isTrue(cmdutil.Env("ROUTER_UPDATE_STATUS", "true")), "If true, the router will update admitted route status.")
 	flag.DurationVar(&o.ResyncInterval, "resync-interval", controllerfactory.DefaultResyncInterval, "The interval at which the route list should be fully refreshed")
 	flag.StringVar(&o.HostnameTemplate, "hostname-template", cmdutil.Env("ROUTER_SUBDOMAIN", ""), "If specified, a template that should be used to generate the hostname for a route without spec.host (e.g. '${name}-${namespace}.myapps.mycompany.com')")
 	flag.BoolVar(&o.OverrideHostname, "override-hostname", isTrue(cmdutil.Env("ROUTER_OVERRIDE_HOSTNAME", "")), "Override the spec.host value for a route with --hostname-template")
@@ -72,6 +83,7 @@ func (o *RouterSelection) Bind(flag *pflag.FlagSet) {
 	flag.BoolVar(&o.AllowWildcardRoutes, "allow-wildcard-routes", isTrue(cmdutil.Env("ROUTER_ALLOW_WILDCARD_ROUTES", "")), "Allow wildcard host names for routes")
 	flag.BoolVar(&o.DisableNamespaceOwnershipCheck, "disable-namespace-ownership-check", isTrue(cmdutil.Env("ROUTER_DISABLE_NAMESPACE_OWNERSHIP_CHECK", "")), "Disables the namespace ownership checks for a route host with different paths or for overlapping host names in the case of wildcard routes. Please be aware that if namespace ownership checks are disabled, routes in a different namespace can use this mechanism to 'steal' sub-paths for existing domains. This is only safe if route creation privileges are restricted, or if all the users can be trusted.")
 	flag.BoolVar(&o.EnableIngress, "enable-ingress", isTrue(cmdutil.Env("ROUTER_ENABLE_INGRESS", "")), "Enable configuration via ingress resources")
+	flag.BoolVar(&o.ExtendedValidation, "extended-validation", isTrue(cmdutil.Env("EXTENDED_VALIDATION", "true")), "If set, then an additional extended validation step is performed on all routes admitted in by this router. Defaults to true and enables the extended validation checks.")
 	flag.StringVar(&o.ListenAddr, "listen-addr", cmdutil.Env("ROUTER_LISTEN_ADDR", ""), "The name of an interface to listen on to expose metrics and health checking. If not specified, will not listen. Overrides stats port.")
 }
 
@@ -205,6 +217,15 @@ func (o *RouterSelection) Complete() error {
 	o.BlacklistedDomains = sets.NewString(o.DeniedDomains...)
 	o.WhitelistedDomains = sets.NewString(o.AllowedDomains...)
 
+	if routerCanonicalHostname := o.RouterCanonicalHostname; len(routerCanonicalHostname) > 0 {
+		if errs := validation.IsDNS1123Subdomain(routerCanonicalHostname); len(errs) != 0 {
+			return fmt.Errorf("invalid canonical hostname: %s", routerCanonicalHostname)
+		}
+		if errs := validation.IsValidIP(routerCanonicalHostname); len(errs) == 0 {
+			return fmt.Errorf("canonical hostname must not be an IP address: %s", routerCanonicalHostname)
+		}
+	}
+
 	return nil
 }
 

pkg/cmd/infra/router/template.go

@@ -17,7 +17,6 @@ import (
 
 	ktypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/authorization/authorizerfactory"
@@ -70,8 +69,6 @@ type TemplateRouterOptions struct {
 }
 
 type TemplateRouter struct {
-	RouterName               string
-	RouterCanonicalHostname  string
 	WorkingDir               string
 	TemplateFile             string
 	ReloadScript             string
@@ -80,7 +77,6 @@ type TemplateRouter struct {
 	DefaultCertificatePath   string
 	DefaultCertificateDir    string
 	DefaultDestinationCAPath string
-	ExtendedValidation       bool
 	RouterService            *ktypes.NamespacedName
 	BindPortsAfterSync       bool
 	MaxConnections           string
@@ -108,8 +104,6 @@ func reloadInterval() time.Duration {
 }
 
 func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
-	flag.StringVar(&o.RouterName, "name", util.Env("ROUTER_SERVICE_NAME", "public"), "The name the router will identify itself with in the route status")
-	flag.StringVar(&o.RouterCanonicalHostname, "router-canonical-hostname", util.Env("ROUTER_CANONICAL_HOSTNAME", ""), "CanonicalHostname is the external host name for the router that can be used as a CNAME for the host requested for this route. This value is optional and may not be set in all cases.")
 	flag.StringVar(&o.WorkingDir, "working-dir", "/var/lib/haproxy/router", "The working directory for the router plugin")
 	flag.StringVar(&o.DefaultCertificate, "default-certificate", util.Env("DEFAULT_CERTIFICATE", ""), "The contents of a default certificate to use for routes that don't expose a TLS server cert; in PEM format")
 	flag.StringVar(&o.DefaultCertificatePath, "default-certificate-path", util.Env("DEFAULT_CERTIFICATE_PATH", ""), "A path to default certificate to use for routes that don't expose a TLS server cert; in PEM format")
@@ -118,8 +112,7 @@ func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
 	flag.StringVar(&o.TemplateFile, "template", util.Env("TEMPLATE_FILE", ""), "The path to the template file to use")
 	flag.StringVar(&o.ReloadScript, "reload", util.Env("RELOAD_SCRIPT", ""), "The path to the reload script to use")
 	flag.DurationVar(&o.ReloadInterval, "interval", reloadInterval(), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the last reload time.")
-	flag.BoolVar(&o.ExtendedValidation, "extended-validation", isTrue(util.Env("EXTENDED_VALIDATION", "true")), "If set, then an additional extended validation step is performed on all routes admitted in by this router. Defaults to true and enables the extended validation checks.")
-	flag.BoolVar(&o.BindPortsAfterSync, "bind-ports-after-sync", isTrue(util.Env("ROUTER_BIND_PORTS_AFTER_SYNC", "")), "Bind ports only after route state has been synchronized")
+	flag.BoolVar(&o.BindPortsAfterSync, "bind-ports-after-sync", util.Env("ROUTER_BIND_PORTS_AFTER_SYNC", "") == "true", "Bind ports only after route state has been synchronized")
 	flag.StringVar(&o.MaxConnections, "max-connections", util.Env("ROUTER_MAX_CONNECTIONS", ""), "Specifies the maximum number of concurrent connections.")
 	flag.StringVar(&o.Ciphers, "ciphers", util.Env("ROUTER_CIPHERS", ""), "Specifies the cipher suites to use. You can choose a predefined cipher set ('modern', 'intermediate', or 'old') or specify exact cipher suites by passing a : separated list.")
 	flag.BoolVar(&o.StrictSNI, "strict-sni", isTrue(util.Env("ROUTER_STRICT_SNI", "")), "Use strict-sni bind processing (do not use default cert).")
@@ -180,7 +173,6 @@ func NewCommandTemplateRouter(name string) *cobra.Command {
 func (o *TemplateRouterOptions) Complete() error {
 	routerSvcName := util.Env("ROUTER_SERVICE_NAME", "")
 	routerSvcNamespace := util.Env("ROUTER_SERVICE_NAMESPACE", "")
-	routerCanonicalHostname := util.Env("ROUTER_CANONICAL_HOSTNAME", "")
 	if len(routerSvcName) > 0 {
 		if len(routerSvcNamespace) == 0 {
 			return fmt.Errorf("ROUTER_SERVICE_NAMESPACE is required when ROUTER_SERVICE_NAME is specified")
@@ -219,15 +211,6 @@ func (o *TemplateRouterOptions) Complete() error {
 		return fmt.Errorf("invalid reload interval: %v - must be a positive duration", nsecs)
 	}
 
-	if len(routerCanonicalHostname) > 0 {
-		if errs := validation.IsDNS1123Subdomain(routerCanonicalHostname); len(errs) != 0 {
-			return fmt.Errorf("invalid canonical hostname: %s", routerCanonicalHostname)
-		}
-		if errs := validation.IsValidIP(routerCanonicalHostname); len(errs) == 0 {
-			return fmt.Errorf("canonical hostname must not be an IP address: %s", routerCanonicalHostname)
-		}
-	}
-
 	return o.RouterSelection.Complete()
 }
 
@@ -238,7 +221,7 @@ func (o *TemplateRouterOptions) Validate() error {
 	if len(o.MetricsType) > 0 && !supportedMetricsTypes.Has(o.MetricsType) {
 		return fmt.Errorf("supported metrics types are: %s", strings.Join(supportedMetricsTypes.List(), ", "))
 	}
-	if len(o.RouterName) == 0 {
+	if len(o.RouterName) == 0 && o.UpdateStatus {
 		return errors.New("router must have a name to identify itself in route status")
 	}
 	if len(o.TemplateFile) == 0 {
@@ -421,13 +404,18 @@ func (o *TemplateRouterOptions) Run() error {
 		return err
 	}
 
-	statusPlugin := controller.NewStatusAdmitter(templatePlugin, routeclient.Route(), o.RouterName, o.RouterCanonicalHostname)
-	var nextPlugin router.Plugin = statusPlugin
+	var recorder controller.RejectionRecorder = controller.LogRejections
+	var plugin router.Plugin = templatePlugin
+	if o.UpdateStatus {
+		status := controller.NewStatusAdmitter(plugin, routeclient.Route(), o.RouterName, o.RouterCanonicalHostname)
+		recorder = status
+		plugin = status
+	}
 	if o.ExtendedValidation {
-		nextPlugin = controller.NewExtendedValidator(nextPlugin, controller.RejectionRecorder(statusPlugin))
+		plugin = controller.NewExtendedValidator(plugin, recorder)
 	}
-	uniqueHostPlugin := controller.NewUniqueHost(nextPlugin, o.RouteSelectionFunc(), o.RouterSelection.DisableNamespaceOwnershipCheck, controller.RejectionRecorder(statusPlugin))
-	plugin := controller.NewHostAdmitter(uniqueHostPlugin, o.RouteAdmissionFunc(), o.AllowWildcardRoutes, o.RouterSelection.DisableNamespaceOwnershipCheck, controller.RejectionRecorder(statusPlugin))
+	plugin = controller.NewUniqueHost(plugin, o.RouteSelectionFunc(), o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)
+	plugin = controller.NewHostAdmitter(plugin, o.RouteAdmissionFunc(), o.AllowWildcardRoutes, o.RouterSelection.DisableNamespaceOwnershipCheck, recorder)
 
 	factory := o.RouterSelection.NewFactory(routeclient, projectclient.Project().Projects(), kc)
 	controller := factory.Create(plugin, false, o.EnableIngress)

pkg/router/controller/status.go

@@ -24,6 +24,15 @@ type RejectionRecorder interface {
 	RecordRouteRejection(route *routeapi.Route, reason, message string)
 }
 
+// LogRejections writes rejection messages to the log.
+var LogRejections = logRecorder{}
+
+type logRecorder struct{}
+
+func (logRecorder) RecordRouteRejection(route *routeapi.Route, reason, message string) {
+	glog.V(3).Infof("Rejected route %s in namespace %s: %s: %s", route.Name, route.Namespace, reason, message)
+}
+
 // StatusAdmitter ensures routes added to the plugin have status set.
 type StatusAdmitter struct {
 	plugin                  router.Plugin

pkg/router/controller/unique_host.go

@@ -25,14 +25,6 @@ func HostForRoute(route *routeapi.Route) string {
 type HostToRouteMap map[string][]*routeapi.Route
 type RouteToHostMap map[string]string
 
-var LogRejections = logRecorder{}
-
-type logRecorder struct{}
-
-func (logRecorder) RecordRouteRejection(route *routeapi.Route, reason, message string) {
-	glog.V(4).Infof("Rejected route %s: %s: %s", route.Name, reason, message)
-}
-
 // UniqueHost implements the router.Plugin interface to provide
 // a template based, backend-agnostic router.
 type UniqueHost struct {