diff --git a/collectors/metrics/pkg/metricfamily/hypershift_transformer.go b/collectors/metrics/pkg/metricfamily/hypershift_transformer.go index d3c9f73ea..9bb5f4f53 100644 --- a/collectors/metrics/pkg/metricfamily/hypershift_transformer.go +++ b/collectors/metrics/pkg/metricfamily/hypershift_transformer.go @@ -44,7 +44,7 @@ func NewHypershiftTransformer(l log.Logger, c client.Client, labels map[string]s //clusters := map[string]string{} hClient := c if hClient == nil { - if os.Getenv("UNIT_TEST") != "true" { + if _, ok := os.LookupEnv("UNIT_TEST"); !ok { config, err := clientcmd.BuildConfigFromFlags("", "") if err != nil { return nil, errors.New("failed to create the kube config") diff --git a/loaders/dashboards/pkg/util/grafana_util.go b/loaders/dashboards/pkg/util/grafana_util.go index 6b81a80b8..05d6577d9 100644 --- a/loaders/dashboards/pkg/util/grafana_util.go +++ b/loaders/dashboards/pkg/util/grafana_util.go @@ -61,14 +61,21 @@ func SetRequest(method string, url string, body io.Reader, retry int) ([]byte, i resp, err = getHTTPClient().Do(req) } - if resp != nil { - defer resp.Body.Close() - respBody, err := io.ReadAll(resp.Body) + if resp == nil { + return nil, http.StatusNotFound + } + + defer func() { + err := resp.Body.Close() if err != nil { - klog.Info("failed to parse response body ", "error ", err) + klog.Info("failed to close response body ", "error ", err) } - return respBody, resp.StatusCode - } else { - return nil, http.StatusNotFound + }() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + klog.Info("failed to parse response body ", "error ", err) + return nil, resp.StatusCode } + return respBody, resp.StatusCode } diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go index 7400c6b6a..4503e3a09 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/multiclusterobservability_controller.go @@ -20,7 +20,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" storev1 "k8s.io/api/storage/v1" - "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" @@ -72,6 +71,7 @@ var ( isReceiveStorageSizeChanged = false isStoreStorageSizeChanged = false isLegacyResourceRemoved = false + lastLogTime = time.Now() ) // MultiClusterObservabilityReconciler reconciles a MultiClusterObservability object @@ -170,6 +170,14 @@ func (r *MultiClusterObservabilityReconciler) Reconcile(ctx context.Context, req mchCrdExists := r.CRDMap[config.MCHCrdName] // requeue after 10 seconds if the mch crd exists and image image manifests map is empty if mchCrdExists && len(config.GetImageManifests()) == 0 { + currentTime := time.Now() + + // Log the message if it has been longer than 5 minutes since the last log + if currentTime.Sub(lastLogTime) > 5*time.Minute { + reqLogger.Info("Waiting for the mch CR to be ready", "mchCrdExists", mchCrdExists, "imageManifests", len(config.GetImageManifests())) + lastLogTime = currentTime + } + // if the mch CR is not ready, then requeue the request after 10s return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -321,8 +329,7 @@ func (r *MultiClusterObservabilityReconciler) Reconcile(ctx context.Context, req } } - if os.Getenv("UNIT_TEST") != "true" && !isLegacyResourceRemoved { - isLegacyResourceRemoved = true + if _, ok := os.LookupEnv("UNIT_TEST"); !ok && !isLegacyResourceRemoved { // Delete PrometheusRule from openshift-monitoring namespace if err := r.deleteSpecificPrometheusRule(ctx); err != nil { reqLogger.Error(err, "Failed to delete the specific PrometheusRule in the openshift-monitoring namespace") @@ -333,6 +340,7 @@ func (r *MultiClusterObservabilityReconciler) Reconcile(ctx context.Context, req reqLogger.Error(err, "Failed to delete service monitor in the openshift-monitoring namespace") return ctrl.Result{}, err } + isLegacyResourceRemoved = true } //update status @@ -626,19 +634,19 @@ func GenerateAlertmanagerRoute( log.Info("BYO CA/Certificate found for the Route of Alertmanager, will using BYO CA/certificate for the Route of Alertmanager") amRouteCA, ok := amRouteBYOCaSrt.Data["tls.crt"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO CA for the Route of Alertmanager") + return &ctrl.Result{}, cerr.New("invalid BYO CA for the Route of Alertmanager") } amGateway.Spec.TLS.CACertificate = string(amRouteCA) amRouteCert, ok := amRouteBYOCertSrt.Data["tls.crt"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO Certificate for the Route of Alertmanager") + return &ctrl.Result{}, cerr.New("invalid BYO Certificate for the Route of Alertmanager") } amGateway.Spec.TLS.Certificate = string(amRouteCert) amRouteCertKey, ok := amRouteBYOCertSrt.Data["tls.key"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO Certificate Key for the Route of Alertmanager") + return &ctrl.Result{}, cerr.New("invalid BYO Certificate Key for the Route of Alertmanager") } amGateway.Spec.TLS.Key = string(amRouteCertKey) } @@ -720,19 +728,19 @@ func GenerateProxyRoute( log.Info("BYO CA/Certificate found for the Route of Proxy, will using BYO CA/certificate for the Route of Proxy") proxyRouteCA, ok := proxyRouteBYOCaSrt.Data["tls.crt"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO CA for the Route of Proxy") + return &ctrl.Result{}, cerr.New("invalid BYO CA for the Route of Proxy") } proxyGateway.Spec.TLS.CACertificate = string(proxyRouteCA) proxyRouteCert, ok := proxyRouteBYOCertSrt.Data["tls.crt"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO Certificate for the Route of Proxy") + return &ctrl.Result{}, cerr.New("invalid BYO Certificate for the Route of Proxy") } proxyGateway.Spec.TLS.Certificate = string(proxyRouteCert) proxyRouteCertKey, ok := proxyRouteBYOCertSrt.Data["tls.key"] if !ok { - return &ctrl.Result{}, cerr.New("Invalid BYO Certificate Key for the Route of Proxy") + return &ctrl.Result{}, cerr.New("invalid BYO Certificate Key for the Route of Proxy") } proxyGateway.Spec.TLS.Key = string(proxyRouteCertKey) } @@ -826,7 +834,7 @@ func (r *MultiClusterObservabilityReconciler) ensureOpenShiftNamespaceLabel(ctx } err := r.Client.Get(ctx, types.NamespacedName{Name: resNS}, existingNs) - if err != nil || errors.IsNotFound(err) { + if err != nil || apierrors.IsNotFound(err) { log.Error(err, fmt.Sprintf("Failed to find namespace for Multicluster Operator: %s", resNS)) return reconcile.Result{Requeue: true}, err } diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go index 7ffdad336..29b1188e6 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium.go @@ -468,7 +468,7 @@ func newAPISpec(c client.Client, mco *mcov1beta2.MultiClusterObservability) (obs data, ok := storageSecret.Data[storageConfig.Key] if !ok { log.Error(err, "Invalid key in secret", "name", storageConfig.Name, "key", storageConfig.Key) - return apiSpec, fmt.Errorf("Invalid key %s in secret %s", storageConfig.Key, storageConfig.Name) + return apiSpec, fmt.Errorf("invalid key %s in secret %s", storageConfig.Key, storageConfig.Name) } ep := &mcoutil.RemoteWriteEndpointWithSecret{} err = yaml.Unmarshal(data, ep) diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go index d7a49acd4..cd995b95c 100644 --- a/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go +++ b/operators/multiclusterobservability/controllers/multiclusterobservability/observatorium_test.go @@ -11,7 +11,6 @@ import ( "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -20,9 +19,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" mcoshared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" - oashared "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/shared" mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2" - "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" + mcoconfig "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" mcoutil "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/util" observatoriumv1alpha1 "github.com/stolostron/observatorium-operator/api/v1alpha1" @@ -34,8 +32,8 @@ var ( func TestNewVolumeClaimTemplate(t *testing.T) { vct := newVolumeClaimTemplate("10Gi", "test") - if vct.Spec.AccessModes[0] != v1.ReadWriteOnce || - vct.Spec.Resources.Requests[v1.ResourceStorage] != resource.MustParse("10Gi") { + if vct.Spec.AccessModes[0] != corev1.ReadWriteOnce || + vct.Spec.Resources.Requests[corev1.ResourceStorage] != resource.MustParse("10Gi") { t.Errorf("Failed to newVolumeClaimTemplate") } } @@ -81,7 +79,7 @@ func TestNewDefaultObservatoriumSpec(t *testing.T) { writeStorageS := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "write_name", - Namespace: config.GetDefaultNamespace(), + Namespace: mcoconfig.GetDefaultNamespace(), }, Type: "Opaque", Data: map[string][]byte{ @@ -120,7 +118,7 @@ func TestNewDefaultObservatoriumSpec(t *testing.T) { endpointS := &corev1.Secret{} err := cl.Get(context.TODO(), types.NamespacedName{ Name: endpointsConfigName, - Namespace: config.GetDefaultNamespace(), + Namespace: mcoconfig.GetDefaultNamespace(), }, endpointS) if err != nil { t.Errorf("Failed to get endpoint config secret due to %v", err) @@ -214,7 +212,7 @@ func TestGetTLSSecretMountPath(t *testing.T) { testCaseList := []struct { name string secret *corev1.Secret - storeConfig *oashared.PreConfiguredStorage + storeConfig *mcoshared.PreConfiguredStorage expected string }{ @@ -223,7 +221,7 @@ func TestGetTLSSecretMountPath(t *testing.T) { &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test", - Namespace: config.GetDefaultNamespace(), + Namespace: mcoconfig.GetDefaultNamespace(), }, Type: "Opaque", Data: map[string][]byte{ @@ -234,7 +232,7 @@ config: `), }, }, - &oashared.PreConfiguredStorage{ + &mcoshared.PreConfiguredStorage{ Key: "thanos.yaml", Name: "test", }, @@ -245,7 +243,7 @@ config: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-1", - Namespace: config.GetDefaultNamespace(), + Namespace: mcoconfig.GetDefaultNamespace(), }, Type: "Opaque", Data: map[string][]byte{ @@ -263,7 +261,7 @@ config: `), }, }, - &oashared.PreConfiguredStorage{ + &mcoshared.PreConfiguredStorage{ Key: "thanos.yaml", Name: "test-1", }, @@ -274,7 +272,7 @@ config: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-2", - Namespace: config.GetDefaultNamespace(), + Namespace: mcoconfig.GetDefaultNamespace(), }, Type: "Opaque", Data: map[string][]byte{ @@ -292,7 +290,7 @@ config: `), }, }, - &oashared.PreConfiguredStorage{ + &mcoshared.PreConfiguredStorage{ Key: "thanos.yaml", Name: "test-2", }, @@ -307,6 +305,9 @@ config: t.Errorf("failed to create object storage secret, due to %v", err) } path, err := getTLSSecretMountPath(client, c.storeConfig) + if err != nil { + t.Errorf("failed to get tls secret mount path, due to %v", err) + } if path != c.expected { t.Errorf("case (%v) output: (%v) is not the expected: (%v)", c.name, path, c.expected) } diff --git a/proxy/pkg/util/util.go b/proxy/pkg/util/util.go index 757832b48..37a29190d 100644 --- a/proxy/pkg/util/util.go +++ b/proxy/pkg/util/util.go @@ -427,7 +427,13 @@ func FetchUserProjectList(token string, url string) []string { writeError(fmt.Sprintf("failed to send http request: %v", err)) return []string{} } - defer resp.Body.Close() + + defer func() { + err := resp.Body.Close() + if err != nil { + klog.Errorf("failed to close response body: %v", err) + } + }() var projects projectv1.ProjectList err = json.NewDecoder(resp.Body).Decode(&projects) @@ -451,9 +457,15 @@ func GetUserName(token string, url string) string { writeError(fmt.Sprintf("failed to send http request: %v", err)) return "" } - defer resp.Body.Close() user := userv1.User{} + defer func() { + err := resp.Body.Close() + if err != nil { + klog.Errorf("failed to close response body: %v", err) + } + }() + err = json.NewDecoder(resp.Body).Decode(&user) if err != nil { klog.Errorf("failed to decode response json body: %v", err) diff --git a/tests/pkg/utils/mco_pods.go b/tests/pkg/utils/mco_pods.go index 4a3d567f7..0e87fa40a 100644 --- a/tests/pkg/utils/mco_pods.go +++ b/tests/pkg/utils/mco_pods.go @@ -65,7 +65,14 @@ func GetPodLogs( klog.Errorf("Failed to get logs for %s/%s in namespace %s due to %v", podName, containerName, namespace, err) return "", err } - defer podLogs.Close() + + defer func() { + err := podLogs.Close() + if err != nil { + klog.Errorf("Failed to close pod logs due to %v", err) + } + }() + buf := new(bytes.Buffer) _, err = io.Copy(buf, podLogs) if err != nil { diff --git a/tests/pkg/utils/utils.go b/tests/pkg/utils/utils.go index 9dade9afa..d8f8b183b 100644 --- a/tests/pkg/utils/utils.go +++ b/tests/pkg/utils/utils.go @@ -191,7 +191,7 @@ func FetchBearerToken(opt TestOptions) (string, error) { } for _, secret := range secretList.Items { // nolint:staticcheck - if secret.GetObjectMeta() != nil && len(secret.GetObjectMeta().GetAnnotations()) > 0 { + if len(secret.GetObjectMeta().GetAnnotations()) > 0 { annos := secret.GetObjectMeta().GetAnnotations() sa, saExists := annos["kubernetes.io/service-account.name"] //_, createByExists := annos["kubernetes.io/created-by"]