Clean up apiserver and federation defaulting and validation

cmd/kube-apiserver/app/options/validation.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 The Kubernetes Authors.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -18,45 +18,49 @@ package options
 
 import (
 	"fmt"
-
-	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 )
 
 // TODO: Longer term we should read this from some config store, rather than a flag.
-func verifyClusterIPFlags(options *ServerRunOptions) []error {
+func validateClusterIPFlags(options *ServerRunOptions) []error {
 	errors := []error{}
 	if options.ServiceClusterIPRange.IP == nil {
-		errors = append(errors, fmt.Errorf("No --service-cluster-ip-range specified"))
+		errors = append(errors, fmt.Errorf("no --service-cluster-ip-range specified"))
 	}
 	var ones, bits = options.ServiceClusterIPRange.Mask.Size()
 	if bits-ones > 20 {
-		errors = append(errors, fmt.Errorf("Specified --service-cluster-ip-range is too large"))
+		errors = append(errors, fmt.Errorf("specified --service-cluster-ip-range is too large"))
 	}
 	return errors
 }
 
-func verifyServiceNodePort(options *ServerRunOptions) []error {
+func validateServiceNodePort(options *ServerRunOptions) []error {
 	errors := []error{}
 	if options.KubernetesServiceNodePort < 0 || options.KubernetesServiceNodePort > 65535 {
-		errors = append(errors, fmt.Errorf("--kubernetes-service-node-port %v must be between 0 and 65535, inclusive. If 0, the Kubernetes master service will be of type ClusterIP.", options.KubernetesServiceNodePort))
+		errors = append(errors, fmt.Errorf("--kubernetes-service-node-port %v must be between 0 and 65535, inclusive. If 0, the Kubernetes master service will be of type ClusterIP", options.KubernetesServiceNodePort))
 	}
 
 	if options.KubernetesServiceNodePort > 0 && !options.ServiceNodePortRange.Contains(options.KubernetesServiceNodePort) {
-		errors = append(errors, fmt.Errorf("Kubernetes service port range %v doesn't contain %v", options.ServiceNodePortRange, (options.KubernetesServiceNodePort)))
+		errors = append(errors, fmt.Errorf("kubernetes service port range %v doesn't contain %v", options.ServiceNodePortRange, (options.KubernetesServiceNodePort)))
 	}
 	return errors
 }
 
-func ValidateRunOptions(options *ServerRunOptions) error {
-	errors := []error{}
-	if errs := verifyClusterIPFlags(options); len(errs) > 0 {
+func (options *ServerRunOptions) Validate() []error {
+	var errors []error
+	if errs := options.Etcd.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := verifyServiceNodePort(options); len(errs) > 0 {
+	if errs := validateClusterIPFlags(options); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if err := utilerrors.NewAggregate(errors); err != nil {
-		return fmt.Errorf("validate server run options failed: %v", err)
+	if errs := validateServiceNodePort(options); len(errs) > 0 {
+		errors = append(errors, errs...)
 	}
-	return nil
+	if errs := options.SecureServing.Validate(); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+	if errs := options.InsecureServing.Validate("insecure-port"); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+	return errors
 }

cmd/kube-apiserver/app/server.go

@@ -81,32 +81,29 @@ cluster's shared state through which all other components interact.`,
 
 // Run runs the specified APIServer.  This should never exit.
 func Run(s *options.ServerRunOptions) error {
-	if errs := s.Etcd.Validate(); len(errs) > 0 {
-		return utilerrors.NewAggregate(errs)
-	}
-	if err := s.GenericServerRunOptions.DefaultExternalAddress(s.SecureServing, s.InsecureServing); err != nil {
+	// set defaults
+	if err := s.GenericServerRunOptions.DefaultAdvertiseAddress(s.SecureServing, s.InsecureServing); err != nil {
 		return err
 	}
-
 	serviceIPRange, apiServerServiceIP, err := master.DefaultServiceIPRange(s.ServiceClusterIPRange)
 	if err != nil {
 		return fmt.Errorf("error determining service IP ranges: %v", err)
 	}
-
 	if err := s.SecureServing.MaybeDefaultWithSelfSignedCerts(s.GenericServerRunOptions.AdvertiseAddress.String(), apiServerServiceIP); err != nil {
 		return fmt.Errorf("error creating self-signed certificates: %v", err)
 	}
-
-	// TODO(sttts): change signature of DefaultAndValidateRunOptions to aggregate errors
-	genericapiserver.DefaultAndValidateRunOptions(s.GenericServerRunOptions)
-
-	// TODO(sttts): move all defaulting and validation above into cmd/kube-apiserver/app/options.DefaultAndValidateRunOptions()
-	if err != options.ValidateRunOptions(s) {
-		return err
+	if err := s.GenericServerRunOptions.DefaultExternalHost(); err != nil {
+		return fmt.Errorf("error setting the external host value: %v", err)
 	}
 
-	genericConfig := genericapiserver.NewConfig(). // create the new config
-							ApplyOptions(s.GenericServerRunOptions). // apply the options selected
+	// validate options
+	if errs := s.Validate(); len(errs) != 0 {
+		return utilerrors.NewAggregate(errs)
+	}
+
+	// create config from options
+	genericConfig := genericapiserver.NewConfig().
+		ApplyOptions(s.GenericServerRunOptions).
 		ApplyInsecureServingOptions(s.InsecureServing)
 
 	if _, err := genericConfig.ApplySecureServingOptions(s.SecureServing); err != nil {

examples/apiserver/apiserver.go

@@ -18,7 +18,6 @@ package apiserver
 
 import (
 	"fmt"
-	"net"
 
 	"k8s.io/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/v1"
 	testgroupetcd "k8s.io/kubernetes/examples/apiserver/rest"
@@ -28,7 +27,6 @@ import (
 	"k8s.io/kubernetes/pkg/genericapiserver"
 	"k8s.io/kubernetes/pkg/genericapiserver/authorizer"
 	genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options"
-	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
 	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/runtime/schema"
 	"k8s.io/kubernetes/pkg/storage/storagebackend"
@@ -82,8 +80,15 @@ func NewServerRunOptions() *ServerRunOptions {
 func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error {
 	serverOptions.Etcd.StorageConfig.ServerList = []string{"http://127.0.0.1:2379"}
 
-	// TODO(sttts): unify signature of DefaultAndValidateRunOptions with the others
-	genericapiserver.DefaultAndValidateRunOptions(serverOptions.GenericServerRunOptions)
+	// set defaults
+	if err := serverOptions.GenericServerRunOptions.DefaultExternalHost(); err != nil {
+		return err
+	}
+	if err := serverOptions.SecureServing.MaybeDefaultWithSelfSignedCerts(serverOptions.GenericServerRunOptions.AdvertiseAddress.String()); err != nil {
+		glog.Fatalf("Error creating self-signed certificates: %v", err)
+	}
+
+	// validate options
 	if errs := serverOptions.Etcd.Validate(); len(errs) > 0 {
 		return utilerrors.NewAggregate(errs)
 	}
@@ -93,10 +98,8 @@ func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error {
 	if errs := serverOptions.InsecureServing.Validate("insecure-port"); len(errs) > 0 {
 		return utilerrors.NewAggregate(errs)
 	}
-	if err := serverOptions.SecureServing.MaybeDefaultWithSelfSignedCerts(serverOptions.GenericServerRunOptions.AdvertiseAddress.String()); err != nil {
-		glog.Fatalf("Error creating self-signed certificates: %v", err)
-	}
 
+	// create config from options
 	config := genericapiserver.NewConfig().
 		ApplyOptions(serverOptions.GenericServerRunOptions).
 		ApplyInsecureServingOptions(serverOptions.InsecureServing)

federation/cmd/federation-apiserver/app/options/validation.go

@@ -0,0 +1,32 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+func (options *ServerRunOptions) Validate() []error {
+	var errors []error
+	if errs := options.Etcd.Validate(); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+	if errs := options.SecureServing.Validate(); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+	if errs := options.InsecureServing.Validate("insecure-port"); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+	// TODO: add more checks
+	return errors
+}

federation/cmd/federation-apiserver/app/server.go

@@ -68,18 +68,23 @@ cluster's shared state through which all other components interact.`,
 
 // Run runs the specified APIServer.  This should never exit.
 func Run(s *options.ServerRunOptions) error {
-	if errs := s.Etcd.Validate(); len(errs) > 0 {
-		utilerrors.NewAggregate(errs)
-	}
-	if err := s.GenericServerRunOptions.DefaultExternalAddress(s.SecureServing, s.InsecureServing); err != nil {
+	// set defaults
+	if err := s.GenericServerRunOptions.DefaultAdvertiseAddress(s.SecureServing, s.InsecureServing); err != nil {
 		return err
 	}
-
 	if err := s.SecureServing.MaybeDefaultWithSelfSignedCerts(s.GenericServerRunOptions.AdvertiseAddress.String()); err != nil {
 		return fmt.Errorf("error creating self-signed certificates: %v", err)
 	}
+	if err := s.GenericServerRunOptions.DefaultExternalHost(); err != nil {
+		return fmt.Errorf("error setting the external host value: %v", err)
+	}
 
-	genericapiserver.DefaultAndValidateRunOptions(s.GenericServerRunOptions)
+	// validate options
+	if errs := s.Validate(); len(errs) != 0 {
+		return utilerrors.NewAggregate(errs)
+	}
+
+	// create config from options
 	genericConfig := genericapiserver.NewConfig(). // create the new config
 							ApplyOptions(s.GenericServerRunOptions). // apply the options selected
 							ApplyInsecureServingOptions(s.InsecureServing)

pkg/genericapiserver/config.go

@@ -25,7 +25,6 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
-	"os"
 	goruntime "runtime"
 	"sort"
 	"strconv"
@@ -34,13 +33,11 @@ import (
 
 	"github.com/emicklei/go-restful/swagger"
 	"github.com/go-openapi/spec"
-	"github.com/golang/glog"
 	"github.com/pborman/uuid"
 	"gopkg.in/natefinch/lumberjack.v2"
 
 	"k8s.io/kubernetes/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/v1"
 	metav1 "k8s.io/kubernetes/pkg/apis/meta/v1"
 	apiserverauthenticator "k8s.io/kubernetes/pkg/apiserver/authenticator"
 	apiserverfilters "k8s.io/kubernetes/pkg/apiserver/filters"
@@ -52,7 +49,6 @@ import (
 	authhandlers "k8s.io/kubernetes/pkg/auth/handlers"
 	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/client/restclient"
-	"k8s.io/kubernetes/pkg/cloudprovider"
 	apiserverauthorizer "k8s.io/kubernetes/pkg/genericapiserver/authorizer"
 	genericfilters "k8s.io/kubernetes/pkg/genericapiserver/filters"
 	"k8s.io/kubernetes/pkg/genericapiserver/mux"
@@ -629,43 +625,6 @@ func (s *GenericAPIServer) installAPI(c *Config) {
 	s.HandlerContainer.Add(s.DynamicApisDiscovery())
 }
 
-func DefaultAndValidateRunOptions(options *options.ServerRunOptions) {
-	glog.Infof("Will report %v as public IP address.", options.AdvertiseAddress)
-
-	// Set default value for ExternalAddress if not specified.
-	if len(options.ExternalHost) == 0 {
-		// TODO: extend for other providers
-		if options.CloudProvider == "gce" || options.CloudProvider == "aws" {
-			cloud, err := cloudprovider.InitCloudProvider(options.CloudProvider, options.CloudConfigFile)
-			if err != nil {
-				glog.Fatalf("Cloud provider could not be initialized: %v", err)
-			}
-			instances, supported := cloud.Instances()
-			if !supported {
-				glog.Fatalf("%q cloud provider has no instances.  this shouldn't happen. exiting.", options.CloudProvider)
-			}
-			hostname, err := os.Hostname()
-			if err != nil {
-				glog.Fatalf("Failed to get hostname: %v", err)
-			}
-			nodeName, err := instances.CurrentNodeName(hostname)
-			if err != nil {
-				glog.Fatalf("Failed to get NodeName: %v", err)
-			}
-			addrs, err := instances.NodeAddresses(nodeName)
-			if err != nil {
-				glog.Warningf("Unable to obtain external host address from cloud provider: %v", err)
-			} else {
-				for _, addr := range addrs {
-					if addr.Type == v1.NodeExternalIP {
-						options.ExternalHost = addr.Address
-					}
-				}
-			}
-		}
-	}
-}
-
 func NewRequestInfoResolver(c *Config) *request.RequestInfoFactory {
 	apiPrefixes := sets.NewString(strings.Trim(APIGroupPrefix, "/")) // all possible API prefixes
 	legacyAPIPrefixes := sets.String{}                               // APIPrefixes that won't have groups (legacy)

pkg/genericapiserver/options/server_run_options.go

@@ -19,11 +19,14 @@ package options
 import (
 	"fmt"
 	"net"
+	"os"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
+	"k8s.io/kubernetes/pkg/cloudprovider"
 	"k8s.io/kubernetes/pkg/runtime/schema"
 	"k8s.io/kubernetes/pkg/util/config"
 
@@ -82,7 +85,7 @@ func NewServerRunOptions() *ServerRunOptions {
 	}
 }
 
-func (s *ServerRunOptions) DefaultExternalAddress(secure *SecureServingOptions, insecure *ServingOptions) error {
+func (s *ServerRunOptions) DefaultAdvertiseAddress(secure *SecureServingOptions, insecure *ServingOptions) error {
 	if s.AdvertiseAddress == nil || s.AdvertiseAddress.IsUnspecified() {
 		switch {
 		case secure != nil:
@@ -106,6 +109,44 @@ func (s *ServerRunOptions) DefaultExternalAddress(secure *SecureServingOptions,
 	return nil
 }
 
+func (options *ServerRunOptions) DefaultExternalHost() error {
+	if len(options.ExternalHost) != 0 {
+		return nil
+	}
+
+	// TODO: extend for other providers
+	if options.CloudProvider == "gce" || options.CloudProvider == "aws" {
+		cloud, err := cloudprovider.InitCloudProvider(options.CloudProvider, options.CloudConfigFile)
+		if err != nil {
+			return fmt.Errorf("%q cloud provider could not be initialized: %v", options.CloudProvider, err)
+		}
+		instances, supported := cloud.Instances()
+		if !supported {
+			return fmt.Errorf("%q cloud provider has no instances", options.CloudProvider)
+		}
+		hostname, err := os.Hostname()
+		if err != nil {
+			return fmt.Errorf("failed to get hostname: %v", err)
+		}
+		nodeName, err := instances.CurrentNodeName(hostname)
+		if err != nil {
+			return fmt.Errorf("failed to get NodeName from %q cloud provider: %v", options.CloudProvider, err)
+		}
+		addrs, err := instances.NodeAddresses(nodeName)
+		if err != nil {
+			return fmt.Errorf("failed to get external host address from %q cloud provider: %v", options.CloudProvider, err)
+		} else {
+			for _, addr := range addrs {
+				if addr.Type == v1.NodeExternalIP {
+					options.ExternalHost = addr.Address
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 // StorageGroupsToEncodingVersion returns a map from group name to group version,
 // computed from s.StorageVersions flag.
 func (s *ServerRunOptions) StorageGroupsToEncodingVersion() (map[string]schema.GroupVersion, error) {

pkg/genericapiserver/validation/BUILD

@@ -1,19 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-licenses(["notice"])
-
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
-
-go_library(
-    name = "go_default_library",
-    srcs = ["universal_validation.go"],
-    tags = ["automanaged"],
-    deps = [
-        "//pkg/genericapiserver/options:go_default_library",
-        "//pkg/util/errors:go_default_library",
-        "//vendor:github.com/golang/glog",
-    ],
-)