NO-JIRA: gather only in-use MachineConfigs (openshift#992)

* gather only in-use MachineConfigs

* describe the change in docs

Author: tremes

docs/gathered-data.md

@@ -1077,7 +1077,10 @@ None
 
 ## MachineConfigs
 
-Collects `MachineConfigs` definitions. Following data is intentionally removed from the definitions:
+Collects definitions of in-use 'MachineConfigs'. MachineConfig is used when it's referenced in
+a MachineConfigPool or in Node `machineconfiguration.openshift.io/desiredConfig` and `machineconfiguration.openshift.io/currentConfig`
+annotations
+Following data is intentionally removed from the definitions:
 - `spec.config.storage.files`
 - `spec.config.passwd.users`
 
@@ -1100,7 +1103,7 @@ Collects `MachineConfigs` definitions. Following data is intentionally removed f
 - 4.8.5
 
 ### Changes
-None
+- gathers only in-use MachineConfigs since 4.18+
 
 
 ## MachineHealthCheck

pkg/gatherers/clusterconfig/gather_machine_configs.go

@@ -7,13 +7,20 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 
+	mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
 	"github.com/openshift/insights-operator/pkg/record"
 )
 
-// GatherMachineConfigs Collects `MachineConfigs` definitions. Following data is intentionally removed from the definitions:
+// GatherMachineConfigs Collects definitions of in-use 'MachineConfigs'. MachineConfig is used when it's referenced in
+// a MachineConfigPool or in Node `machineconfiguration.openshift.io/desiredConfig` and `machineconfiguration.openshift.io/currentConfig`
+// annotations
+// Following data is intentionally removed from the definitions:
 // - `spec.config.storage.files`
 // - `spec.config.passwd.users`
 //
@@ -36,28 +43,40 @@ import (
 // - 4.8.5
 //
 // ### Changes
-// None
+// - gathers only in-use MachineConfigs since 4.18+
 func (g *Gatherer) GatherMachineConfigs(ctx context.Context) ([]record.Record, []error) {
 	gatherDynamicClient, err := dynamic.NewForConfig(g.gatherKubeConfig)
 	if err != nil {
 		return nil, []error{err}
 	}
-
-	return gatherMachineConfigs(ctx, gatherDynamicClient)
+	var errs []error
+	inUseMachineConfigs, err := getInUseMachineConfigs(ctx, g.gatherKubeConfig)
+	if err != nil {
+		errs = append(errs, err)
+	}
+	records, gatherErrs := gatherMachineConfigs(ctx, gatherDynamicClient, inUseMachineConfigs)
+	errs = append(errs, gatherErrs...)
+	return records, errs
 }
 
-func gatherMachineConfigs(ctx context.Context, dynamicClient dynamic.Interface) ([]record.Record, []error) {
+func gatherMachineConfigs(ctx context.Context, dynamicClient dynamic.Interface,
+	inUseMachineConfigs sets.Set[string]) ([]record.Record, []error) {
 	mcList, err := dynamicClient.Resource(machineConfigGroupVersionResource).List(ctx, metav1.ListOptions{})
 	if errors.IsNotFound(err) {
 		return nil, nil
 	}
 	if err != nil {
 		return nil, []error{err}
 	}
+
 	records := []record.Record{}
 	var errs []error
 	for i := range mcList.Items {
 		mc := mcList.Items[i]
+		// skip machine configs which are not in use
+		if len(inUseMachineConfigs) != 0 && !inUseMachineConfigs.Has(mc.GetName()) {
+			continue
+		}
 		// remove the sensitive content by overwriting the values
 		err := unstructured.SetNestedField(mc.Object, nil, "spec", "config", "storage", "files")
 		if err != nil {
@@ -79,3 +98,48 @@ func gatherMachineConfigs(ctx context.Context, dynamicClient dynamic.Interface)
 	}
 	return records, nil
 }
+
+// GetInUseMachineConfigs filters in-use MachineConfig resources and returns set of their names.
+func getInUseMachineConfigs(ctx context.Context, clientConfig *rest.Config) (sets.Set[string], error) {
+	// Create a set to store in-use configs
+	inuseConfigs := sets.New[string]()
+
+	machineConfigClient, err := mcfgclientset.NewForConfig(clientConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	poolList, err := machineConfigClient.MachineconfigurationV1().MachineConfigPools().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("getting MachineConfigPools failed: %w", err)
+	}
+
+	for i := range poolList.Items {
+		pool := poolList.Items[i]
+		// Get the rendered config name from the status section
+		inuseConfigs.Insert(pool.Status.Configuration.Name)
+		inuseConfigs.Insert(pool.Spec.Configuration.Name)
+	}
+
+	kubeClient, err := kubernetes.NewForConfig(clientConfig)
+	if err != nil {
+		return nil, err
+	}
+	nodeList, err := kubeClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+	for i := range nodeList.Items {
+		node := nodeList.Items[i]
+		current, ok := node.Annotations["machineconfiguration.openshift.io/currentConfig"]
+		if ok {
+			inuseConfigs.Insert(current)
+		}
+		desired, ok := node.Annotations["machineconfiguration.openshift.io/desiredConfig"]
+		if ok {
+			inuseConfigs.Insert(desired)
+		}
+	}
+
+	return inuseConfigs, nil
+}

pkg/gatherers/clusterconfig/gather_machine_configs_test.go

@@ -4,113 +4,118 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer/yaml"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/dynamic"
 	dynamicfake "k8s.io/client-go/dynamic/fake"
 )
 
-func createMockConfigMachine(t *testing.T, c dynamic.Interface, data string) {
+func createMockConfigMachine(ctx context.Context, c dynamic.Interface, data string) error {
 	decUnstructured1 := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
 	testMachineConfig := &unstructured.Unstructured{}
 	_, _, err := decUnstructured1.Decode([]byte(data), nil, testMachineConfig)
 	if err != nil {
-		t.Fatal("unable to decode MachineConfig YAML", err)
+		return err
 	}
 
-	_, _ = c.
+	_, err = c.
 		Resource(machineConfigGroupVersionResource).
-		Create(context.Background(), testMachineConfig, metav1.CreateOptions{})
-}
-
-func Test_MachineConfigs(t *testing.T) {
-	// Initialize the fake dynamic client.
-	machineConfigClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
-		machineConfigGroupVersionResource: "MachineConfigsList",
-	})
-
-	records, errs := gatherMachineConfigs(context.Background(), machineConfigClient)
-	if len(errs) > 0 {
-		t.Fatalf("unexpected errors: %#v", errs)
-	}
-	// 0 records because there is no MachineConfigs yet.
-	if len(records) != 0 {
-		t.Fatalf("unexpected number or records in the first run: %d", len(records))
+		Create(ctx, testMachineConfig, metav1.CreateOptions{})
+	if err != nil {
+		return err
 	}
+	return nil
+}
 
-	// Create first MachineConfig resource.
-	machineConfigYAML1 := `apiVersion: machineconfiguration.openshift.io/v1
+func TestGatherMachineConfigs(t *testing.T) {
+	tests := []struct {
+		name                    string
+		machineConfigYAMLs      []string
+		inUseMachineConfigs     sets.Set[string]
+		expectedNumberOfRecords int
+	}{
+		{
+			name:                    "no machine configs exists",
+			machineConfigYAMLs:      []string{},
+			inUseMachineConfigs:     sets.Set[string]{},
+			expectedNumberOfRecords: 0,
+		},
+		{
+			name: "one machine config which is in use",
+			machineConfigYAMLs: []string{
+				`apiVersion: machineconfiguration.openshift.io/v1 
+kind: MachineConfig 
+metadata: 
+  name: 75-worker-sap-data-intelligence`},
+			inUseMachineConfigs:     sets.Set[string]{"75-worker-sap-data-intelligence": {}},
+			expectedNumberOfRecords: 1,
+		},
+		{
+			name:                    "two machine configs but only one in use",
+			expectedNumberOfRecords: 1,
+			machineConfigYAMLs: []string{
+				`apiVersion: machineconfiguration.openshift.io/v1
 kind: MachineConfig
 metadata:
-    name: 75-worker-sap-data-intelligence
-`
-
-	createMockConfigMachine(t, machineConfigClient, machineConfigYAML1)
-	records, errs = gatherMachineConfigs(context.Background(), machineConfigClient)
-	if len(errs) > 0 {
-		t.Fatalf("unexpected errors: %#v", errs)
-	}
-	// 1 record because there is now 1 MachineConfig resource.
-	if len(records) != 1 {
-		t.Fatalf("unexpected number or records in the second run: %d", len(records))
-	}
-
-	// Create second MachineConfig resource.
-	machineConfigYAML2 := `apiVersion: machineconfiguration.openshift.io/v1
+  name: test-not-in-use`,
+				`apiVersion: machineconfiguration.openshift.io/v1
 kind: MachineConfig
 metadata:
-    name: 75-master-sap-data-intelligence
-`
-
-	createMockConfigMachine(t, machineConfigClient, machineConfigYAML2)
-	records, errs = gatherMachineConfigs(context.Background(), machineConfigClient)
-	if len(errs) > 0 {
-		t.Fatalf("unexpected errors: %#v", errs)
-	}
-	// 2 record because there are now 2 MachineConfig resource.
-	if len(records) != 2 {
-		t.Fatalf("unexpected number or records in the third run: %d", len(records))
-	}
-
-	// Create third MachineConfig resource.
-	machineConfigYAML3 := `apiVersion: machineconfiguration.openshift.io/v1
+  name: in-use`,
+			},
+			inUseMachineConfigs: sets.Set[string]{"in-use": {}},
+		},
+		{
+			name:                    "no machine config in use",
+			expectedNumberOfRecords: 0,
+			machineConfigYAMLs: []string{
+				`apiVersion: machineconfiguration.openshift.io/v1
 kind: MachineConfig
 metadata:
-    name: 99-sdi-generated-containerruntime
-    ownerReferences:
-        - kind: ContainerRuntimeConfig
-          name: sdi-pids-limit
-`
-
-	createMockConfigMachine(t, machineConfigClient, machineConfigYAML3)
-	records, errs = gatherMachineConfigs(context.Background(), machineConfigClient)
-	if len(errs) > 0 {
-		t.Fatalf("unexpected errors: %#v", errs)
-	}
-	// 3 record because there are now 3 MachineConfig resource.
-	if len(records) != 3 {
-		t.Fatalf("unexpected number or records in the fourth run: %d", len(records))
-	}
-
-	// Create fourth MachineConfig resource.
-	machineConfigYAML4 := `apiVersion: machineconfiguration.openshift.io/v1
+  name: test-not-in-use-1`,
+				`apiVersion: machineconfiguration.openshift.io/v1
 kind: MachineConfig
 metadata:
-    name: 00-example-machine-config
-    labels:
-        workload: sap-data-intelligence
-`
-
-	createMockConfigMachine(t, machineConfigClient, machineConfigYAML4)
-	records, errs = gatherMachineConfigs(context.Background(), machineConfigClient)
-	if len(errs) > 0 {
-		t.Fatalf("unexpected errors: %#v", errs)
+  name: test-not-in-use-2`,
+			},
+			inUseMachineConfigs: sets.Set[string]{"in-use": {}, "in-use-2": {}},
+		},
+		{
+			name:                    "two machine configs in use",
+			expectedNumberOfRecords: 2,
+			machineConfigYAMLs: []string{
+				`apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  name: in-use-1`,
+				`apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  name: in-use-2`,
+			},
+			inUseMachineConfigs: sets.Set[string]{"in-use-1": {}, "in-use-2": {}},
+		},
 	}
-	// 4 record because there are now 4 MachineConfig resource.
-	if len(records) != 4 {
-		t.Fatalf("unexpected number or records in the fifth run: %d", len(records))
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			// Initialize the fake dynamic client.
+			machineConfigClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
+				machineConfigGroupVersionResource: "MachineConfigsList",
+			})
+			for _, mcYAML := range tt.machineConfigYAMLs {
+				err := createMockConfigMachine(ctx, machineConfigClient, mcYAML)
+				assert.NoError(t, err)
+			}
+			records, errs := gatherMachineConfigs(ctx, machineConfigClient, tt.inUseMachineConfigs)
+			assert.Len(t, errs, 0)
+			assert.Len(t, records, tt.expectedNumberOfRecords)
+		})
 	}
 }