Merge pull request #18437 from wozniakjan/bz1538922/diagnostics/aggregatedlogging

Automatic merge from submit-queue (batch tested with PRs 18437, 18546, 18550, 18579).

Bug 1538922 - Fix diagnostics for AggregatedLogging

The `AssetConfig` section with the `loggingPublicURL` is being removed from the `masterConfig`, therefore, we need a different logic to obtain logging project name.

Currently, no location can provide logging project nor `loggingPublicURL` so we make an assumption that logging is deployed by default to 'openshift-logging' with legacy fallback to 'logging'. Optionally user can override the behaior with `--logging-project=[project]` flag.

Author: openshift-merge-robot

contrib/completions/bash/oc

@@ -94,7 +94,8 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf
 		var d types.Diagnostic
 		switch diagnosticName {
 		case agldiags.AggregatedLoggingName:
-			d = agldiags.NewAggregatedLogging(o.MasterConfigLocation, kclusterClient, oauthClient.Oauth(), projectClient.Project(), routeClient.Route(), oauthorizationClient.Authorization(), appsClient.Apps(), securityClient.Security())
+			p := o.ParameterizedDiagnostics[agldiags.AggregatedLoggingName].(*agldiags.AggregatedLogging).Project
+			d = agldiags.NewAggregatedLogging(p, kclusterClient, oauthClient.Oauth(), projectClient.Project(), routeClient.Route(), oauthorizationClient.Authorization(), appsClient.Apps(), securityClient.Security())
 		case clustdiags.NodeDefinitionsName:
 			d = &clustdiags.NodeDefinitions{KubeClient: kclusterClient}
 		case clustdiags.MasterNodeName:

contrib/completions/zsh/oc

@@ -3,7 +3,6 @@ package aggregated_logging
 import (
 	"errors"
 	"fmt"
-	"net/url"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -15,26 +14,24 @@ import (
 	appstypedclient "github.com/openshift/origin/pkg/apps/generated/internalclientset/typed/apps/internalversion"
 	authapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
 	oauthorizationtypedclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset/typed/authorization/internalversion"
-	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	oauthtypedclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
-	hostdiag "github.com/openshift/origin/pkg/oc/admin/diagnostics/diagnostics/host"
 	"github.com/openshift/origin/pkg/oc/admin/diagnostics/diagnostics/log"
 	"github.com/openshift/origin/pkg/oc/admin/diagnostics/diagnostics/types"
 	projecttypedclient "github.com/openshift/origin/pkg/project/generated/internalclientset/typed/project/internalversion"
 	routesapi "github.com/openshift/origin/pkg/route/apis/route"
 	routetypedclient "github.com/openshift/origin/pkg/route/generated/internalclientset/typed/route/internalversion"
 	securityapi "github.com/openshift/origin/pkg/security/apis/security"
 	securitytypedclient "github.com/openshift/origin/pkg/security/generated/internalclientset/typed/security/internalversion"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	"strings"
 )
 
 // AggregatedLogging is a Diagnostic to check the configurations
 // and general integration of the OpenShift stack
 // for aggregating container logs
 // https://github.com/openshift/origin-aggregated-logging
 type AggregatedLogging struct {
-	masterConfig      *configapi.MasterConfig
-	loggingURL        string
-	MasterConfigFile  string
+	Project           string
 	OAuthClientClient oauthtypedclient.OAuthClientsGetter
 	ProjectClient     projecttypedclient.ProjectsGetter
 	RouteClient       routetypedclient.RoutesGetter
@@ -54,13 +51,16 @@ const (
 	openshiftValue  = "openshift"
 
 	fluentdServiceAccountName = "aggregated-logging-fluentd"
+
+	flagLoggingProject = "logging-project"
 )
 
 var loggingSelector = labels.Set{loggingInfraKey: "support"}
+var defaultLoggingProjects = []string{"openshift-logging", "logging"}
 
 //NewAggregatedLogging returns the AggregatedLogging Diagnostic
 func NewAggregatedLogging(
-	masterConfigFile string,
+	project string,
 	kclient kclientset.Interface,
 	oauthClientClient oauthtypedclient.OAuthClientsGetter,
 	projectClient projecttypedclient.ProjectsGetter,
@@ -70,10 +70,7 @@ func NewAggregatedLogging(
 	sccClient securitytypedclient.SecurityContextConstraintsGetter,
 ) *AggregatedLogging {
 	return &AggregatedLogging{
-		masterConfig: nil,
-		// TODO this needs to be plumbed because the master-config no longer has it.
-		loggingURL:        "",
-		MasterConfigFile:  masterConfigFile,
+		Project:           project,
 		OAuthClientClient: oauthClientClient,
 		ProjectClient:     projectClient,
 		RouteClient:       routeClient,
@@ -153,20 +150,30 @@ func (d *AggregatedLogging) Requirements() (client bool, host bool) {
 }
 
 func (d *AggregatedLogging) Complete(logger *log.Logger) error {
-	if len(d.MasterConfigFile) > 0 {
-		var err error
-		d.masterConfig, err = hostdiag.GetMasterConfig(d.MasterConfigFile, logger)
+	if len(d.Project) > 0 {
+		return nil
+	}
+
+	// Check if any of the default logging projects are present in the cluster
+	for _, project := range defaultLoggingProjects {
+		d.Debug("AGL0031", fmt.Sprintf("Trying default logging project %q", project))
+		_, err := d.ProjectClient.Projects().Get(project, metav1.GetOptions{})
 		if err != nil {
-			return err
+			if kerrors.IsNotFound(err) {
+				d.Debug("AGL0032", fmt.Sprintf("Project %q not found", project))
+				continue
+			}
+			return fmt.Errorf("failed fetching one of the default logging projects %q: %v", project, err)
 		}
+
+		d.Debug("AGL0033", fmt.Sprintf("Found default logging project %q", project))
+		d.Project = project
+		return nil
 	}
-	return nil
+	return fmt.Errorf("default logging project not found, use '--%s' to specify logging project", flagLoggingProject)
 }
 
 func (d *AggregatedLogging) CanRun() (bool, error) {
-	if len(d.MasterConfigFile) == 0 || d.masterConfig == nil {
-		return false, errors.New("No master config file was provided")
-	}
 	if d.OAuthClientClient == nil || d.ProjectClient == nil || d.RouteClient == nil || d.CRBClient == nil || d.DCClient == nil {
 		return false, errors.New("Config must include a cluster-admin context to run this diagnostic")
 	}
@@ -177,24 +184,33 @@ func (d *AggregatedLogging) CanRun() (bool, error) {
 }
 
 func (d *AggregatedLogging) Check() types.DiagnosticResult {
-	if len(d.loggingURL) == 0 {
+	d.Debug("AGL0015", fmt.Sprintf("Trying diagnostics for project '%s'", d.Project))
+	p, err := d.ProjectClient.Projects().Get(d.Project, metav1.GetOptions{})
+	if err != nil {
+		d.Error("AGL0018", err, fmt.Sprintf("There was an error retrieving project '%s' which is most likely a transient error: %s", d.Project, err))
 		return d.result
 	}
-
-	project := retrieveLoggingProject(d.result, d.loggingURL, d.ProjectClient, d.RouteClient)
-	if len(project) != 0 {
-		checkServiceAccounts(d, d, project)
-		checkClusterRoleBindings(d, d, project)
-		checkSccs(d, d, project)
-		checkDeploymentConfigs(d, d, project)
-		checkDaemonSets(d, d, project)
-		checkServices(d, d, project)
-		checkRoutes(d, d, project)
-		checkKibana(d.result, d.RouteClient, d.OAuthClientClient, d.KubeClient, project)
+	nodeSelector, ok := p.ObjectMeta.Annotations["openshift.io/node-selector"]
+	if !ok || len(nodeSelector) != 0 {
+		d.Warn("AGL0030", nil, fmt.Sprintf(projectNodeSelectorWarning, d.Project))
 	}
+	checkServiceAccounts(d, d, d.Project)
+	checkClusterRoleBindings(d, d, d.Project)
+	checkSccs(d, d, d.Project)
+	checkDeploymentConfigs(d, d, d.Project)
+	checkDaemonSets(d, d, d.Project)
+	checkServices(d, d, d.Project)
+	checkRoutes(d, d, d.Project)
+	checkKibana(d, d.RouteClient, d.OAuthClientClient, d.KubeClient, d.Project)
 	return d.result
 }
 
+func (d *AggregatedLogging) AvailableParameters() []types.Parameter {
+	return []types.Parameter{
+		{flagLoggingProject, fmt.Sprintf("Project that has deployed aggregated logging. Default projects: %s", strings.Join(defaultLoggingProjects, " or ")), &d.Project, ""},
+	}
+}
+
 const projectNodeSelectorWarning = `
 The project '%[1]s' was found with either a missing or non-empty node selector annotation.
 This could keep Fluentd from running on certain nodes and collecting logs from the entire cluster.
@@ -207,51 +223,3 @@ and updating the annotation:
   'openshift.io/node-selector' : ""
 
 `
-
-func retrieveLoggingProject(r types.DiagnosticResult, loggingURL string, projectClient projecttypedclient.ProjectsGetter, routeClient routetypedclient.RoutesGetter) string {
-	r.Debug("AGL0010", fmt.Sprintf("masterConfig.AssetConfig.LoggingPublicURL: '%s'", loggingURL))
-	projectName := ""
-	if len(loggingURL) == 0 {
-		r.Debug("AGL0017", "masterConfig.AssetConfig.LoggingPublicURL is empty")
-		return projectName
-	}
-
-	loggingUrl, err := url.Parse(loggingURL)
-	if err != nil {
-		r.Error("AGL0011", err, fmt.Sprintf("Unable to parse the loggingPublicURL from the masterConfig '%s'", loggingURL))
-		return projectName
-	}
-
-	routeList, err := routeClient.Routes(metav1.NamespaceAll).List(metav1.ListOptions{LabelSelector: loggingSelector.AsSelector().String()})
-	if err != nil {
-		r.Error("AGL0012", err, fmt.Sprintf("There was an error while trying to find the route associated with '%s' which is probably transient: %s", loggingUrl, err))
-		return projectName
-	}
-
-	for _, route := range routeList.Items {
-		r.Debug("AGL0013", fmt.Sprintf("Comparing URL to route.Spec.Host: %s", route.Spec.Host))
-		if loggingUrl.Host == route.Spec.Host {
-			if len(projectName) == 0 {
-				projectName = route.ObjectMeta.Namespace
-				r.Info("AGL0015", fmt.Sprintf("Found route '%s' matching logging URL '%s' in project: '%s'", route.ObjectMeta.Name, loggingUrl.Host, projectName))
-			} else {
-				r.Warn("AGL0019", nil, fmt.Sprintf("Found additional route '%s' matching logging URL '%s' in project: '%s'.  This could mean you have multiple logging deployments.", route.ObjectMeta.Name, loggingUrl.Host, projectName))
-			}
-		}
-	}
-	if len(projectName) == 0 {
-		message := fmt.Sprintf("Unable to find a route matching the loggingPublicURL defined in the master config '%s'. Check that the URL is correct and aggregated logging is deployed.", loggingUrl)
-		r.Error("AGL0014", errors.New(message), message)
-		return ""
-	}
-	project, err := projectClient.Projects().Get(projectName, metav1.GetOptions{})
-	if err != nil {
-		r.Error("AGL0018", err, fmt.Sprintf("There was an error retrieving project '%s' which is most likely a transient error: %s", projectName, err))
-		return ""
-	}
-	nodeSelector, ok := project.ObjectMeta.Annotations["openshift.io/node-selector"]
-	if !ok || len(nodeSelector) != 0 {
-		r.Warn("AGL0030", nil, fmt.Sprintf(projectNodeSelectorWarning, projectName))
-	}
-	return projectName
-}

pkg/oc/admin/diagnostics/cluster.go

@@ -13,7 +13,6 @@ import (
 
 	oauthapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
 	oauthtypedclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
-	"github.com/openshift/origin/pkg/oc/admin/diagnostics/diagnostics/types"
 	routetypedclient "github.com/openshift/origin/pkg/route/generated/internalclientset/typed/route/internalversion"
 )
 
@@ -24,7 +23,7 @@ const (
 )
 
 //checkKibana verifies the various integration points between Kibana and logging
-func checkKibana(r types.DiagnosticResult, routeClient routetypedclient.RoutesGetter, oauthClientClient oauthtypedclient.OAuthClientsGetter, kClient kclientset.Interface, project string) {
+func checkKibana(r diagnosticReporter, routeClient routetypedclient.RoutesGetter, oauthClientClient oauthtypedclient.OAuthClientsGetter, kClient kclientset.Interface, project string) {
 	oauthclient, err := oauthClientClient.OAuthClients().Get(kibanaProxyOauthClientName, metav1.GetOptions{})
 	if err != nil {
 		r.Error("AGL0115", err, fmt.Sprintf("Error retrieving the OauthClient '%s': %s. Unable to check Kibana", kibanaProxyOauthClientName, err))
@@ -35,7 +34,7 @@ func checkKibana(r types.DiagnosticResult, routeClient routetypedclient.RoutesGe
 }
 
 //checkKibanaSecret confirms the secret used by kibana matches that configured in the oauth client
-func checkKibanaSecret(r types.DiagnosticResult, kClient kclientset.Interface, project string, oauthclient *oauthapi.OAuthClient) {
+func checkKibanaSecret(r diagnosticReporter, kClient kclientset.Interface, project string, oauthclient *oauthapi.OAuthClient) {
 	r.Debug("AGL0100", "Checking oauthclient secrets...")
 	secret, err := kClient.Core().Secrets(project).Get(kibanaProxySecretName, metav1.GetOptions{})
 	if err != nil {
@@ -56,7 +55,7 @@ func checkKibanaSecret(r types.DiagnosticResult, kClient kclientset.Interface, p
 }
 
 //checkKibanaRoutesInOauthClient verifies the client contains the correct redirect uris
-func checkKibanaRoutesInOauthClient(r types.DiagnosticResult, routeClient routetypedclient.RoutesGetter, project string, oauthclient *oauthapi.OAuthClient) {
+func checkKibanaRoutesInOauthClient(r diagnosticReporter, routeClient routetypedclient.RoutesGetter, project string, oauthclient *oauthapi.OAuthClient) {
 	r.Debug("AGL0141", "Checking oauthclient redirectURIs for the logging routes...")
 	routeList, err := routeClient.Routes(project).List(metav1.ListOptions{LabelSelector: loggingSelector.AsSelector().String()})
 	if err != nil {