@@ -20,7 +20,7 @@ import (

 var (

 	// availableClusterDiagnostics contains the names of cluster diagnostics that can be executed

 	// during a single run of diagnostics. Add more diagnostics to the list as they are defined.

-	availableClusterDiagnostics = sets.NewString(clustdiags.NodeDefinitionsName, clustdiags.ClusterRegistryName, clustdiags.ClusterRouterName, clustdiags.ClusterRolesName, clustdiags.ClusterRoleBindingsName, clustdiags.MasterNodeName, clustdiags.MetricsApiProxyName)

+	availableClusterDiagnostics = sets.NewString(clustdiags.NodeDefinitionsName, clustdiags.ClusterRegistryName, clustdiags.ClusterRouterName, clustdiags.ClusterRolesName, clustdiags.ClusterRoleBindingsName, clustdiags.MasterNodeName, clustdiags.MetricsApiProxyName, clustdiags.ServiceExternalIPsName)

 )

 // buildClusterDiagnostics builds cluster Diagnostic objects if a cluster-admin client can be extracted from the rawConfig passed in.

@@ -44,24 +44,28 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf

 	diagnostics := []types.Diagnostic{}

 	for _, diagnosticName := range requestedDiagnostics {

+		var d types.Diagnostic

 		switch diagnosticName {

 		case clustdiags.NodeDefinitionsName:

-			diagnostics = append(diagnostics, &clustdiags.NodeDefinitions{KubeClient: kclusterClient, OsClient: clusterClient})

+			d = &clustdiags.NodeDefinitions{KubeClient: kclusterClient, OsClient: clusterClient}

 		case clustdiags.MasterNodeName:

-			diagnostics = append(diagnostics, &clustdiags.MasterNode{KubeClient: kclusterClient, OsClient: clusterClient, ServerUrl: serverUrl, MasterConfigFile: o.MasterConfigLocation})

+			d = &clustdiags.MasterNode{KubeClient: kclusterClient, OsClient: clusterClient, ServerUrl: serverUrl, MasterConfigFile: o.MasterConfigLocation}

 		case clustdiags.ClusterRegistryName:

-			diagnostics = append(diagnostics, &clustdiags.ClusterRegistry{KubeClient: kclusterClient, OsClient: clusterClient, PreventModification: o.PreventModification})

+			d = &clustdiags.ClusterRegistry{KubeClient: kclusterClient, OsClient: clusterClient, PreventModification: o.PreventModification}

 		case clustdiags.ClusterRouterName:

-			diagnostics = append(diagnostics, &clustdiags.ClusterRouter{KubeClient: kclusterClient, OsClient: clusterClient})

+			d = &clustdiags.ClusterRouter{KubeClient: kclusterClient, OsClient: clusterClient}

 		case clustdiags.ClusterRolesName:

-			diagnostics = append(diagnostics, &clustdiags.ClusterRoles{ClusterRolesClient: clusterClient, SARClient: clusterClient})

+			d = &clustdiags.ClusterRoles{ClusterRolesClient: clusterClient, SARClient: clusterClient}

 		case clustdiags.ClusterRoleBindingsName:

-			diagnostics = append(diagnostics, &clustdiags.ClusterRoleBindings{ClusterRoleBindingsClient: clusterClient, SARClient: clusterClient})

+			d = &clustdiags.ClusterRoleBindings{ClusterRoleBindingsClient: clusterClient, SARClient: clusterClient}

 		case clustdiags.MetricsApiProxyName:

-			diagnostics = append(diagnostics, &clustdiags.MetricsApiProxy{KubeClient: kclusterClient})

+			d = &clustdiags.MetricsApiProxy{KubeClient: kclusterClient}

+		case clustdiags.ServiceExternalIPsName:

+			d = &clustdiags.ServiceExternalIPs{MasterConfigFile: o.MasterConfigLocation, KclusterClient: kclusterClient}

 		default:

 			return nil, false, fmt.Errorf("unknown diagnostic: %v", diagnosticName)

 		}

+		diagnostics = append(diagnostics, d)

 	}

 	return diagnostics, true, nil

 }

@@ -36,26 +36,20 @@ func (o DiagnosticsOptions) buildHostDiagnostics() ([]types.Diagnostic, bool, er

 	diagnostics := []types.Diagnostic{}

 	systemdUnits := systemddiags.GetSystemdUnits(o.Logger)

 	for _, diagnosticName := range requestedDiagnostics {

+		var d types.Diagnostic

 		switch diagnosticName {

 		case systemddiags.AnalyzeLogsName:

-			diagnostics = append(diagnostics, systemddiags.AnalyzeLogs{SystemdUnits: systemdUnits})

-

+			d = systemddiags.AnalyzeLogs{SystemdUnits: systemdUnits}

 		case systemddiags.UnitStatusName:

-			diagnostics = append(diagnostics, systemddiags.UnitStatus{SystemdUnits: systemdUnits})

-

+			d = systemddiags.UnitStatus{SystemdUnits: systemdUnits}

 		case hostdiags.MasterConfigCheckName:

-			if len(o.MasterConfigLocation) > 0 {

-				diagnostics = append(diagnostics, hostdiags.MasterConfigCheck{MasterConfigFile: o.MasterConfigLocation})

-			}

-

+			d = hostdiags.MasterConfigCheck{MasterConfigFile: o.MasterConfigLocation}

 		case hostdiags.NodeConfigCheckName:

-			if len(o.NodeConfigLocation) > 0 {

-				diagnostics = append(diagnostics, hostdiags.NodeConfigCheck{NodeConfigFile: o.NodeConfigLocation})

-			}

-

+			d = hostdiags.NodeConfigCheck{NodeConfigFile: o.NodeConfigLocation}

 		default:

 			return diagnostics, false, fmt.Errorf("unknown diagnostic: %v", diagnosticName)

 		}

+		diagnostics = append(diagnostics, d)

 	}

 	return diagnostics, true, nil

new file mode 100644

@@ -0,0 +1,95 @@

+package cluster

+

+import (

+	"errors"

+	"fmt"

+	"net"

+	"strings"

+

+	kapi "k8s.io/kubernetes/pkg/api"

+	kclient "k8s.io/kubernetes/pkg/client/unversioned"

+

+	hostdiag "github.com/openshift/origin/pkg/diagnostics/host"

+	"github.com/openshift/origin/pkg/diagnostics/types"

+	"github.com/openshift/origin/pkg/service/admission"

+)

+

+// ServiceExternalIPs is a Diagnostic to check for the services in the cluster

+// that do not comply with an updated master ExternalIPNetworkCIDR setting.

+// Background: https://github.com/openshift/origin/issues/7808

+type ServiceExternalIPs struct {

+	MasterConfigFile string

+	KclusterClient   *kclient.Client

+}

+

+const ServiceExternalIPsName = "ServiceExternalIPs"

+

+func (d *ServiceExternalIPs) Name() string {

+	return ServiceExternalIPsName

+}

+

+func (d *ServiceExternalIPs) Description() string {

+	return "Check for existing services with ExternalIPs that are disallowed by master config"

+}

+

+func (d *ServiceExternalIPs) CanRun() (bool, error) {

+	if len(d.MasterConfigFile) == 0 {

+		return false, errors.New("No master config file was detected")

+	}

+	if d.KclusterClient == nil {

+		return false, errors.New("Client config must include a cluster-admin context to run this diagnostic")

+	}

+

+	return true, nil

+}

+

+func (d *ServiceExternalIPs) Check() types.DiagnosticResult {

+	r := types.NewDiagnosticResult(ServiceExternalIPsName)

+	masterConfig, err := hostdiag.GetMasterConfig(r, d.MasterConfigFile)

+	if err != nil {

+		r.Info("DH2004", "Unreadable master config; skipping this diagnostic.")

+		return r

+	}

+

+	admit, reject := []*net.IPNet{}, []*net.IPNet{}

+	if cidrs := masterConfig.NetworkConfig.ExternalIPNetworkCIDRs; cidrs != nil {

+		reject, admit, err = admission.ParseCIDRRules(cidrs)

+		if err != nil {

+			r.Error("DH2007", err, fmt.Sprintf("Could not parse master config NetworkConfig.ExternalIPNetworkCIDRs: (%[1]T) %[1]v", err))

+			return r

+		}

+	}

+	services, err := d.KclusterClient.Services("").List(kapi.ListOptions{})

+	if err != nil {

+		r.Error("DH2005", err, fmt.Sprintf("Error while listing cluster services: (%[1]T) %[1]v", err))

+		return r

+	}

+

+	errList := []string{}

+	for _, service := range services.Items {

+		if len(service.Spec.ExternalIPs) == 0 {

+			continue

+		}

+		if len(admit) == 0 {

+			errList = append(errList, fmt.Sprintf("Service %s.%s specifies ExternalIPs %v, but none are permitted.", service.Namespace, service.Name, service.Spec.ExternalIPs))

+			continue

+		}

+		for _, ipString := range service.Spec.ExternalIPs {

+			ip := net.ParseIP(ipString)

+			if ip == nil {

+				continue // we don't really care for the purposes of this diagnostic

+			}

+			if admission.NetworkSlice(reject).Contains(ip) || !admission.NetworkSlice(admit).Contains(ip) {

+				errList = append(errList, fmt.Sprintf("Service %s.%s specifies ExternalIP %s that is not permitted by the master ExternalIPNetworkCIDRs setting.", service.Namespace, service.Name, ipString))

+			}

+		}

+	}

+	if len(errList) > 0 {

+		r.Error("DH2006", nil, `The following problems were found with service ExternalIPs in the cluster.

+These services were created before the master ExternalIPNetworkCIDRs setting changed to exclude them.

+The default ExternalIPNetworkCIDRs now excludes all ExternalIPs on services.

+`+strings.Join(errList, "\n"))

+	}

+

+	return r

+}

@@ -4,7 +4,6 @@ import (

 	"errors"

 	"fmt"

-	configapilatest "github.com/openshift/origin/pkg/cmd/server/api/latest"

 	configvalidation "github.com/openshift/origin/pkg/cmd/server/api/validation"

 	"github.com/openshift/origin/pkg/diagnostics/types"

 )

@@ -25,23 +24,18 @@ func (d MasterConfigCheck) Description() string {

 }

 func (d MasterConfigCheck) CanRun() (bool, error) {

 	if len(d.MasterConfigFile) == 0 {

-		return false, errors.New("must have master config file")

+		return false, errors.New("No master config file was detected")

 	}

 	return true, nil

 }

 func (d MasterConfigCheck) Check() types.DiagnosticResult {

 	r := types.NewDiagnosticResult(MasterConfigCheckName)

-

-	r.Debug("DH0001", fmt.Sprintf("Looking for master config file at '%s'", d.MasterConfigFile))

-	masterConfig, err := configapilatest.ReadAndResolveMasterConfig(d.MasterConfigFile)

+	masterConfig, err := GetMasterConfig(r, d.MasterConfigFile)

 	if err != nil {

-		r.Error("DH0002", err, fmt.Sprintf("Could not read master config file '%s':\n(%T) %[2]v", d.MasterConfigFile, err))

 		return r

 	}

-	r.Info("DH0003", fmt.Sprintf("Found a master config file: %[1]s", d.MasterConfigFile))

-

 	results := configvalidation.ValidateMasterConfig(masterConfig, nil)

 	if len(results.Errors) > 0 {

 		errText := fmt.Sprintf("Validation of master config file '%s' failed:\n", d.MasterConfigFile)

@@ -25,7 +25,7 @@ func (d NodeConfigCheck) Description() string {

 }

 func (d NodeConfigCheck) CanRun() (bool, error) {

 	if len(d.NodeConfigFile) == 0 {

-		return false, errors.New("must have node config file")

+		return false, errors.New("No node config file was detected")

 	}

 	return true, nil

new file mode 100644

@@ -0,0 +1,32 @@

+package host

+

+import (

+	"fmt"

+

+	configapi "github.com/openshift/origin/pkg/cmd/server/api"

+	configapilatest "github.com/openshift/origin/pkg/cmd/server/api/latest"

+	"github.com/openshift/origin/pkg/diagnostics/types"

+)

+

+// this would be bad practice if there were ever a need to load more than one master config for diagnostics.

+// however we will proceed with the assumption that will never be necessary.

+var (

+	masterConfigLoaded    = false

+	masterConfig          *configapi.MasterConfig

+	masterConfigLoadError error

+)

+

+func GetMasterConfig(r types.DiagnosticResult, masterConfigFile string) (*configapi.MasterConfig, error) {

+	if masterConfigLoaded { // no need to do this more than once

+		return masterConfig, masterConfigLoadError

+	}

+	r.Debug("DH0001", fmt.Sprintf("Looking for master config file at '%s'", masterConfigFile))

+	masterConfigLoaded = true

+	masterConfig, masterConfigLoadError = configapilatest.ReadAndResolveMasterConfig(masterConfigFile)

+	if masterConfigLoadError != nil {

+		r.Error("DH0002", masterConfigLoadError, fmt.Sprintf("Could not read master config file '%s':\n(%T) %[2]v", masterConfigFile, masterConfigLoadError))

+	} else {

+		r.Debug("DH0003", fmt.Sprintf("Found a master config file: %[1]s", masterConfigFile))

+	}

+	return masterConfig, masterConfigLoadError

+}

@@ -59,11 +59,11 @@ func NewExternalIPRanger(reject, admit []*net.IPNet) *externalIPRanger {

 	}

 }

-// networkSlice is a helper for checking whether an IP is contained in a range

+// NetworkSlice is a helper for checking whether an IP is contained in a range

 // of networks.

-type networkSlice []*net.IPNet

+type NetworkSlice []*net.IPNet

-func (s networkSlice) Contains(ip net.IP) bool {

+func (s NetworkSlice) Contains(ip net.IP) bool {

 	for _, cidr := range s {

 		if cidr.Contains(ip) {

 			return true

@@ -97,7 +97,7 @@ func (r *externalIPRanger) Admit(a kadmission.Attributes) error {

 				errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs").Index(i), "externalIPs must be a valid address"))

 				continue

 			}

-			if networkSlice(r.reject).Contains(ip) || !networkSlice(r.admit).Contains(ip) {

+			if NetworkSlice(r.reject).Contains(ip) || !NetworkSlice(r.admit).Contains(ip) {

 				errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs").Index(i), "externalIP is not allowed"))

 				continue

 			}