Merge pull request #11295 from sosiouxme/20161010-diagnostics-fixes

Merged by openshift-bot

Author: OpenShift Bot

pkg/diagnostics/cluster/aggregated_logging/clusterrolebindings.go

@@ -17,7 +17,7 @@ is required to enable Fluentd to look up pod metadata for the logs it gathers.
 As a user with a cluster-admin role, you can grant the permissions by running
 the following:
 
-  oadm policy add-cluster-role-to-user cluster-reader system:serviceaccount:%[2]s:%[1]s
+  $ oadm policy add-cluster-role-to-user cluster-reader system:serviceaccount:%[2]s:%[1]s
 `
 
 func checkClusterRoleBindings(r diagnosticReporter, adapter clusterRoleBindingsAdapter, project string) {

pkg/diagnostics/cluster/aggregated_logging/daemonsets.go

@@ -13,18 +13,18 @@ There are no nodes that match the selector for DaemonSet '%[1]s'. This
 means Fluentd is not running and is not gathering logs from any nodes.
 An example of a command to target a specific node for this DaemonSet:
 
-  oc label node/node1.example.com %[2]s
+  $ oc label node/node1.example.com %[2]s
 
 or to label them all:
 
-  oc label node --all %[2]s
+  $ oc label node --all %[2]s
 `
 
 const daemonSetPartialNodesLabeled = `
 There are some nodes that match the selector for DaemonSet '%s'.  
 A list of matching nodes can be discovered by running:
 
-  oc get nodes -l %s
+  $ oc get nodes -l %s
 `
 const daemonSetNoPodsFound = `
 There were no pods found that match DaemonSet '%s' with matchLabels '%s'
@@ -36,9 +36,9 @@ Depending upon the state, this could mean there is an error running the image
 for one or more pod containers, the node could be pulling images, etc.  Try running
 the following commands to get additional information:
 
-  oc describe pod %[1]s -n %[5]s
-  oc logs %[1]s -n %[5]s
-  oc get events -n %[5]s
+  $ oc describe pod %[1]s -n %[5]s
+  $ oc logs %[1]s -n %[5]s
+  $ oc get events -n %[5]s
 `
 const daemonSetNotFound = `
 There were no DaemonSets in project '%s' that included label '%s'.  This implies

pkg/diagnostics/cluster/aggregated_logging/deploymentconfigs.go

@@ -33,15 +33,15 @@ const deploymentConfigZeroPodsFound = `
 There were no Pods found that support logging.  Try running
 the following commands for additional information:
 
-  oc describe dc -n %[1]s
-  oc get events -n %[1]s
+  $ oc describe dc -n %[1]s
+  $ oc get events -n %[1]s
 `
 const deploymentConfigNoPodsFound = `
 There were no Pods found for DeploymentConfig '%[1]s'.  Try running
 the following commands for additional information:
 
-  oc describe dc %[1]s -n %[2]s
-  oc get events -n %[2]s
+  $ oc describe dc %[1]s -n %[2]s
+  $ oc get events -n %[2]s
 `
 const deploymentConfigPodsNotRunning = `
 The Pod '%[1]s' matched by DeploymentConfig '%[2]s' is not in '%[3]s' status: %[4]s. 
@@ -50,9 +50,9 @@ Depending upon the state, this could mean there is an error running the image
 for one or more pod containers, the node could be pulling images, etc.  Try running
 the following commands for additional information:
 
-  oc describe pod %[1]s -n %[5]s
-  oc logs %[1]s -n %[5]s
-  oc get events -n %[5]s
+  $ oc describe pod %[1]s -n %[5]s
+  $ oc logs %[1]s -n %[5]s
+  $ oc get events -n %[5]s
 `
 
 func checkDeploymentConfigs(r diagnosticReporter, adapter deploymentConfigAdapter, project string) {

pkg/diagnostics/cluster/aggregated_logging/diagnostic.go

@@ -125,7 +125,10 @@ func (d *AggregatedLogging) CanRun() (bool, error) {
 	var err error
 	d.masterConfig, err = hostdiag.GetMasterConfig(d.result, d.MasterConfigFile)
 	if err != nil {
-		return false, errors.New("Unreadable master config; skipping this diagnostic.")
+		return false, errors.New("Master configuration is unreadable")
+	}
+	if d.masterConfig.AssetConfig.LoggingPublicURL == "" {
+		return false, errors.New("No LoggingPublicURL is defined in the master configuration")
 	}
 	return true, nil
 }
@@ -150,7 +153,7 @@ The project '%[1]s' was found with either a missing or non-empty node selector a
 This could keep Fluentd from running on certain nodes and collecting logs from the entire cluster.  
 You can correct it by editing the project:
 
-  oc edit namespace %[1]s
+  $ oc edit namespace %[1]s
 
 and updating the annotation:
 

pkg/diagnostics/cluster/aggregated_logging/routes.go

@@ -21,7 +21,7 @@ An unaccepted route is most likely due to one of the following reasons:
 If a router has been deployed, look for duplicate matching routes by
 running the following:
 
-  oc get --all-namespaces routes --template='{{range .items}}{{if eq .spec.host "%[2]s"}}{{println .metadata.name "in" .metadata.namespace}}{{end}}{{end}}'
+  $ oc get --all-namespaces routes --template='{{range .items}}{{if eq .spec.host "%[2]s"}}{{println .metadata.name "in" .metadata.namespace}}{{end}}{{end}}'
 
 `
 const routeCertMissingHostName = `

pkg/diagnostics/cluster/aggregated_logging/scc.go

@@ -15,7 +15,7 @@ The ServiceAccount '%[1]s' does not have a privileged SecurityContextConstraint
 user with a cluster-admin role, you can grant the permissions by running
 the following:
 
-  oadm policy add-scc-to-user privileged system:serviceaccount:%[2]s:%[1]s
+  $ oadm policy add-scc-to-user privileged system:serviceaccount:%[2]s:%[1]s
 `
 
 func checkSccs(r diagnosticReporter, adapter sccAdapter, project string) {

pkg/diagnostics/cluster/roles.go

@@ -20,7 +20,34 @@ type ClusterRoles struct {
 }
 
 const (
-	ClusterRolesName = "ClusterRoles"
+	ClusterRolesName   = "ClusterRoles"
+	clusterRoleMissing = `
+clusterrole/%s is missing.
+
+Use the 'oadm policy reconcile-cluster-roles' command to create the role. For example,
+
+  $ oadm policy reconcile-cluster-roles \
+         --additive-only=true --confirm
+`
+	clusterRoleReduced = `
+clusterrole/%s has changed, but the existing role has more permissions than the new role.
+
+If you can confirm that the extra permissions are not required, you may use the
+'oadm policy reconcile-cluster-roles' command to update the role to reduce permissions.
+For example,
+
+  $ oadm policy reconcile-cluster-roles \
+         --additive-only=false --confirm
+`
+	clusterRoleChanged = `
+clusterrole/%s has changed and the existing role does not have enough permissions.
+
+Use the 'oadm policy reconcile-cluster-roles' command to update the role.
+For example,
+
+  $ oadm policy reconcile-cluster-roles \
+         --additive-only=true --confirm
+`
 )
 
 func (d *ClusterRoles) Name() string {
@@ -70,7 +97,7 @@ func (d *ClusterRoles) Check() types.DiagnosticResult {
 	for _, changedClusterRole := range changedClusterRoles {
 		actualClusterRole, err := d.ClusterRolesClient.ClusterRoles().Get(changedClusterRole.Name)
 		if kerrs.IsNotFound(err) {
-			r.Error("CRD1002", nil, fmt.Sprintf("clusterrole/%s is missing.\n\nUse the `oadm policy reconcile-cluster-roles` command to create the role.", changedClusterRole.Name))
+			r.Error("CRD1002", nil, fmt.Sprintf(clusterRoleMissing, changedClusterRole.Name))
 			continue
 		}
 		if err != nil {
@@ -79,15 +106,15 @@ func (d *ClusterRoles) Check() types.DiagnosticResult {
 
 		_, missingRules := rulevalidation.Covers(actualClusterRole.Rules, changedClusterRole.Rules)
 		if len(missingRules) == 0 {
-			r.Warn("CRD1003", nil, fmt.Sprintf("clusterrole/%s has changed, but the existing role has more permissions than the new role.\n\nUse the `oadm policy reconcile-cluster-roles` command to update the role to reduce permissions.", changedClusterRole.Name))
+			r.Info("CRD1003", fmt.Sprintf(clusterRoleReduced, changedClusterRole.Name))
 			_, extraRules := rulevalidation.Covers(changedClusterRole.Rules, actualClusterRole.Rules)
 			for _, extraRule := range extraRules {
 				r.Info("CRD1008", fmt.Sprintf("clusterrole/%s has extra permission %v.", changedClusterRole.Name, extraRule))
 			}
 			continue
 		}
 
-		r.Error("CRD1005", nil, fmt.Sprintf("clusterrole/%s has changed and the existing role does not have enough permissions.\n\nUse the `oadm policy reconcile-cluster-roles` command to update the role.", changedClusterRole.Name))
+		r.Error("CRD1005", nil, fmt.Sprintf(clusterRoleChanged, changedClusterRole.Name))
 		for _, missingRule := range missingRules {
 			r.Info("CRD1007", fmt.Sprintf("clusterrole/%s is missing permission %v.", changedClusterRole.Name, missingRule))
 		}