pkg/gather/clusterconfig: Collapse GatherClusterID into GatherCluster…

…Version There's no reason to fetch the ClusterVersion twice, even if we are creating two Records based on its content. This also sets the stage for gathering additional items like sad cluster-version operator pods.
openshift · Mar 24, 2021 · bc39708 · bc39708
1 parent bbf2b8f
commit bc39708
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 39 deletions.
diff --git a/docs/insights-archive-sample/insights-operator/gathers.json b/docs/insights-archive-sample/insights-operator/gathers.json
@@ -82,12 +82,6 @@
             "records_count": 3,
             "errors": null
         },
-        {
-            "name": "clusterconfig.GatherClusterID",
-            "duration_in_ms": 1002,
-            "records_count": 1,
-            "errors": null
-        },
         {
             "name": "clusterconfig.GatherClusterOAuth",
             "duration_in_ms": 1152,
@@ -217,4 +211,4 @@
     ],
     "memory_alloc_bytes": 20949008,
     "uptime_seconds": 58.282
-}
+}
diff --git a/pkg/gather/clusterconfig/0_gatherer.go b/pkg/gather/clusterconfig/0_gatherer.go
@@ -66,7 +66,6 @@ var gatherFunctions = map[string]gathering{
 	"nodes":                             important(GatherNodes),
 	"config_maps":                       failable(GatherConfigMaps),
 	"version":                           important(GatherClusterVersion),
-	"id":                                important(GatherClusterID),
 	"infrastructures":                   important(GatherClusterInfrastructure),
 	"networks":                          important(GatherClusterNetwork),
 	"authentication":                    important(GatherClusterAuthentication),

diff --git a/pkg/gather/clusterconfig/version.go b/pkg/gather/clusterconfig/version.go
@@ -5,7 +5,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/rest"
 
 	configv1 "github.com/openshift/api/config/v1"
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
@@ -15,59 +14,44 @@ import (
 	"github.com/openshift/insights-operator/pkg/utils/anonymize"
 )
 
-// GatherClusterVersion fetches the ClusterVersion - the ClusterVersion with name version.
+// GatherClusterVersion fetches the ClusterVersion with the name 'version' and its resources.
 //
 // The Kubernetes api https://github.com/openshift/client-go/blob/master/config/clientset/versioned/typed/config/v1/clusterversion.go#L50
 // Response see https://docs.openshift.com/container-platform/4.3/rest_api/index.html#clusterversion-v1config-openshift-io
 //
 // Location in archive: config/version/
 // See: docs/insights-archive-sample/config/version
+// Location of cluster ID: config/id/
 // Id in config: version
 func GatherClusterVersion(g *Gatherer, c chan<- gatherResult) {
 	defer close(c)
-	config, err := getClusterVersion(g.ctx, g.gatherKubeConfig)
+	gatherConfigClient, err := configv1client.NewForConfig(g.gatherKubeConfig)
 	if err != nil {
 		c <- gatherResult{nil, []error{err}}
 		return
 	}
-	c <- gatherResult{[]record.Record{{Name: "config/version", Item: record.JSONMarshaller{Object: anonymizeClusterVersion(config)}}}, nil}
+	records, errors := getClusterVersion(g.ctx, gatherConfigClient)
+	c <- gatherResult{records, errors}
 }
 
-func getClusterVersion(ctx context.Context, kubeConfig *rest.Config) (*configv1.ClusterVersion, error) {
-	gatherConfigClient, err := configv1client.NewForConfig(kubeConfig)
-	if err != nil {
-		return nil, err
-	}
-	config, err := gatherConfigClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
+func getClusterVersion(ctx context.Context, configClient configv1client.ConfigV1Interface) ([]record.Record, []error) {
+	config, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
 	if errors.IsNotFound(err) {
 		return nil, nil
 	}
 	if err != nil {
-		return nil, err
+		return nil, []error{err}
 	}
-	return config, nil
-}
 
-// GatherClusterID stores ClusterID from ClusterVersion version
-// This method uses data already collected by Get ClusterVersion. In particular field .Spec.ClusterID
-// The Kubernetes api https://github.com/openshift/client-go/blob/master/config/clientset/versioned/typed/config/v1/clusterversion.go#L50
-// Response see https://github.com/openshift/api/blob/master/config/v1/types_cluster_version.go#L38
-//
-// Location in archive: config/id/
-// See: docs/insights-archive-sample/config/id
-// Id in config: id
-func GatherClusterID(g *Gatherer, c chan<- gatherResult) {
-	defer close(c)
-	version, err := getClusterVersion(g.ctx, g.gatherKubeConfig)
-	if err != nil {
-		c <- gatherResult{nil, []error{err}}
-		return
+	records := []record.Record{
+		{Name: "config/version", Item: record.JSONMarshaller{Object: anonymizeClusterVersion(config)}},
 	}
-	if version == nil {
-		c <- gatherResult{nil, nil}
-		return
+
+	if config.Spec.ClusterID != "" {
+		records = append(records, record.Record{Name: "config/id", Item: marshal.Raw{Str: string(config.Spec.ClusterID)}})
 	}
-	c <- gatherResult{[]record.Record{{Name: "config/id", Item: marshal.Raw{Str: string(version.Spec.ClusterID)}}}, nil}
+
+	return records, nil
 }
 
 func anonymizeClusterVersion(version *configv1.ClusterVersion) *configv1.ClusterVersion {