diff --git a/internal/addon/addon.go b/internal/addon/addon.go index 8a9f9db..3f19232 100644 --- a/internal/addon/addon.go +++ b/internal/addon/addon.go @@ -26,8 +26,8 @@ func NewRegistrationOption(agentName string) *agent.RegistrationOption { } } -func GetObjectKey(configRef []addonapiv1alpha1.ConfigReference, group, resource string) client.ObjectKey { - var key client.ObjectKey +func GetObjectKeys(configRef []addonapiv1alpha1.ConfigReference, group, resource string) []client.ObjectKey { + var keys []client.ObjectKey for _, config := range configRef { if config.ConfigGroupResource.Group != group { continue @@ -36,11 +36,12 @@ func GetObjectKey(configRef []addonapiv1alpha1.ConfigReference, group, resource continue } - key.Name = config.Name - key.Namespace = config.Namespace - break + keys = append(keys, client.ObjectKey{ + Name: config.Name, + Namespace: config.Namespace, + }) } - return key + return keys } // AgentHealthProber returns a HealthProber struct that contains the necessary diff --git a/internal/addon/helm/values.go b/internal/addon/helm/values.go index 76bc1cc..a3e1e05 100644 --- a/internal/addon/helm/values.go +++ b/internal/addon/helm/values.go @@ -2,6 +2,7 @@ package helm import ( "context" + "errors" "github.com/rhobs/multicluster-observability-addon/internal/addon" lhandlers "github.com/rhobs/multicluster-observability-addon/internal/logging/handlers" @@ -17,6 +18,11 @@ import ( const annotationLocalCluster = "local-cluster" +var ( + errMissingAODCRef = errors.New("missing AddOnDeploymentConfig reference on addon installation") + errMultipleAODCRef = errors.New("multiple AddOnDeploymentConfig references on addon installation") +) + type HelmChartValues struct { Enabled bool `json:"enabled"` Logging lmanifests.LoggingValues `json:"logging"` @@ -81,13 +87,19 @@ func GetValuesFunc(ctx context.Context, k8s client.Client) addonfactory.GetValue } func getAddOnDeploymentConfig(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn) (*addonapiv1alpha1.AddOnDeploymentConfig, error) { - key := addon.GetObjectKey(mcAddon.Status.ConfigReferences, addonutils.AddOnDeploymentConfigGVR.Group, addon.AddonDeploymentConfigResource) - addOnDeployment := &addonapiv1alpha1.AddOnDeploymentConfig{} - if err := k8s.Get(ctx, key, addOnDeployment, &client.GetOptions{}); err != nil { + aodc := &addonapiv1alpha1.AddOnDeploymentConfig{} + keys := addon.GetObjectKeys(mcAddon.Status.ConfigReferences, addonutils.AddOnDeploymentConfigGVR.Group, addon.AddonDeploymentConfigResource) + switch { + case len(keys) == 0: + return aodc, errMissingAODCRef + case len(keys) > 1: + return aodc, errMultipleAODCRef + } + if err := k8s.Get(ctx, keys[0], aodc, &client.GetOptions{}); err != nil { // TODO(JoaoBraveCoding) Add proper error handling - return addOnDeployment, err + return aodc, err } - return addOnDeployment, nil + return aodc, nil } func isHubCluster(cluster *clusterv1.ManagedCluster) bool { diff --git a/internal/addon/helm/values_test.go b/internal/addon/helm/values_test.go index f1d62be..b819ec1 100644 --- a/internal/addon/helm/values_test.go +++ b/internal/addon/helm/values_test.go @@ -16,8 +16,10 @@ import ( "open-cluster-management.io/addon-framework/pkg/addonfactory" "open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting" "open-cluster-management.io/addon-framework/pkg/agent" + addonutils "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" clusterv1 "open-cluster-management.io/api/cluster/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -139,3 +141,101 @@ func Test_Mcoa_Disable_Chart_Hub(t *testing.T) { require.NoError(t, err) require.Empty(t, objects) } + +func TestGetAddOnDeploymentConfig(t *testing.T) { + tests := []struct { + name string + mcAddon *addonapiv1alpha1.ManagedClusterAddOn + existingAODC *addonapiv1alpha1.AddOnDeploymentConfig + expectedErr error + }{ + { + name: "No AODC reference", + mcAddon: &addonapiv1alpha1.ManagedClusterAddOn{ + Status: addonapiv1alpha1.ManagedClusterAddOnStatus{ + ConfigReferences: nil, + }, + }, + expectedErr: errMissingAODCRef, + }, + { + name: "Multiple AODC references", + mcAddon: &addonapiv1alpha1.ManagedClusterAddOn{ + Status: addonapiv1alpha1.ManagedClusterAddOnStatus{ + ConfigReferences: []addonapiv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: addonutils.AddOnDeploymentConfigGVR.Group, + Resource: addon.AddonDeploymentConfigResource, + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "foo", + Namespace: "foo", + }, + }, + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: addonutils.AddOnDeploymentConfigGVR.Group, + Resource: addon.AddonDeploymentConfigResource, + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "bar", + Namespace: "bar", + }, + }, + }, + }, + }, + expectedErr: errMultipleAODCRef, + }, + { + name: "AODC reference found", + mcAddon: &addonapiv1alpha1.ManagedClusterAddOn{ + Status: addonapiv1alpha1.ManagedClusterAddOnStatus{ + ConfigReferences: []addonapiv1alpha1.ConfigReference{ + { + ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ + Group: addonutils.AddOnDeploymentConfigGVR.Group, + Resource: addon.AddonDeploymentConfigResource, + }, + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Name: "foo", + Namespace: "foo", + }, + }, + }, + }, + }, + existingAODC: &addonapiv1alpha1.AddOnDeploymentConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "foo", + }, + }, + expectedErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a fake client with the existing AODC if provided + objs := []client.Object{} + if tt.existingAODC != nil { + objs = append(objs, tt.existingAODC) + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(objs...).Build() + + // Call the function + ctx := context.TODO() + _, err := getAddOnDeploymentConfig(ctx, fakeClient, tt.mcAddon) + + // require the results + if tt.expectedErr != nil { + require.Error(t, err) + require.Equal(t, tt.expectedErr, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/logging/handlers/handler.go b/internal/logging/handlers/handler.go index e50aa8d..2bfbf6c 100644 --- a/internal/logging/handlers/handler.go +++ b/internal/logging/handlers/handler.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "errors" loggingv1 "github.com/openshift/cluster-logging-operator/apis/logging/v1" "github.com/rhobs/multicluster-observability-addon/internal/addon" @@ -10,6 +11,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +var ( + errMissingCLFRef = errors.New("missing ClusterLogForwarder reference on addon installation") + errMultipleCLFRef = errors.New("multiple ClusterLogForwarder references on addon installation") +) + func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, platform, userWorkloads addon.LogsOptions) (manifests.Options, error) { opts := manifests.Options{ Platform: platform, @@ -22,9 +28,15 @@ func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alp opts.SubscriptionChannel = userWorkloads.SubscriptionChannel } - key := addon.GetObjectKey(mcAddon.Status.ConfigReferences, loggingv1.GroupVersion.Group, addon.ClusterLogForwardersResource) + keys := addon.GetObjectKeys(mcAddon.Status.ConfigReferences, loggingv1.GroupVersion.Group, addon.ClusterLogForwardersResource) + switch { + case len(keys) == 0: + return opts, errMissingCLFRef + case len(keys) > 1: + return opts, errMultipleCLFRef + } clf := &loggingv1.ClusterLogForwarder{} - if err := k8s.Get(ctx, key, clf, &client.GetOptions{}); err != nil { + if err := k8s.Get(ctx, keys[0], clf, &client.GetOptions{}); err != nil { return opts, err } opts.ClusterLogForwarder = clf diff --git a/internal/tracing/handlers/handler.go b/internal/tracing/handlers/handler.go index 43929c9..4f8c4d9 100644 --- a/internal/tracing/handlers/handler.go +++ b/internal/tracing/handlers/handler.go @@ -2,7 +2,7 @@ package handlers import ( "context" - "fmt" + "errors" "strings" otelv1beta1 "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" @@ -15,9 +15,11 @@ import ( ) var ( - errNoExportersFound = fmt.Errorf("no exporters found") - errNoMountPathFound = fmt.Errorf("mountpath not found in any secret") - errNoVolumeMountForSecret = fmt.Errorf("no volumemount found for secret") + errNoExportersFound = errors.New("no exporters found") + errNoMountPathFound = errors.New("mountpath not found in any secret") + errNoVolumeMountForSecret = errors.New("no volumemount found for secret") + errMissingOTELColRef = errors.New("missing OpenTelemetryCollector reference on addon installation") + errMultipleOTELColRef = errors.New("multiple OpenTelemetryCollector references on addon installation") ) func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alpha1.ManagedClusterAddOn, userWorkloads addon.TracesOptions) (manifests.Options, error) { @@ -27,9 +29,15 @@ func BuildOptions(ctx context.Context, k8s client.Client, mcAddon *addonapiv1alp } klog.Info("Retrieving OpenTelemetry Collector template") - key := addon.GetObjectKey(mcAddon.Status.ConfigReferences, otelv1beta1.GroupVersion.Group, addon.OpenTelemetryCollectorsResource) + keys := addon.GetObjectKeys(mcAddon.Status.ConfigReferences, otelv1beta1.GroupVersion.Group, addon.OpenTelemetryCollectorsResource) + switch { + case len(keys) == 0: + return opts, errMissingOTELColRef + case len(keys) > 1: + return opts, errMultipleOTELColRef + } otelCol := &otelv1beta1.OpenTelemetryCollector{} - if err := k8s.Get(ctx, key, otelCol, &client.GetOptions{}); err != nil { + if err := k8s.Get(ctx, keys[0], otelCol, &client.GetOptions{}); err != nil { return opts, err } opts.OpenTelemetryCollector = otelCol