diff --git a/api/v1beta1/hostedcluster_types.go b/api/v1beta1/hostedcluster_types.go index 738d063cf553..5516c7b9706e 100644 --- a/api/v1beta1/hostedcluster_types.go +++ b/api/v1beta1/hostedcluster_types.go @@ -719,6 +719,19 @@ const ( PowerVSPlatform PlatformType = "PowerVS" ) +// List all PlatformType instances +func PlatformTypes() []PlatformType { + return []PlatformType{ + AWSPlatform, + NonePlatform, + IBMCloudPlatform, + AgentPlatform, + KubevirtPlatform, + AzurePlatform, + PowerVSPlatform, + } +} + // PlatformSpec specifies the underlying infrastructure provider for the cluster // and is used to configure platform specific behavior. type PlatformSpec struct { diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 6292075bd55e..760c767714ae 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -102,11 +102,8 @@ import ( ) const ( - finalizer = "hypershift.openshift.io/finalizer" - HostedClusterAnnotation = "hypershift.openshift.io/cluster" - // HasBeenAvailableAnnotation is an implementation detail to check if HC has ever been available. - // This is useful for e.g. monitor duration from creation to available and avoid noise from condition flipping. - HasBeenAvailableAnnotation = "hypershift.openshift.io/HasBeenAvailable" + finalizer = "hypershift.openshift.io/finalizer" + HostedClusterAnnotation = "hypershift.openshift.io/cluster" clusterDeletionRequeueDuration = 5 * time.Second ReportingGracePeriodRequeueDuration = 25 * time.Second @@ -397,10 +394,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } - reportLimitedSuportEnabled(hcluster) - reportSilecedAlerts(hcluster) - reportProxyConfig(hcluster) - var hcDestroyGracePeriod time.Duration if gracePeriodString := hcluster.Annotations[hyperv1.HCDestroyGracePeriodAnnotation]; len(gracePeriodString) > 0 { @@ -412,8 +405,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques // If deleted, clean up and return early. if !hcluster.DeletionTimestamp.IsZero() { - reportHostedClusterGuestCloudResourcesDeletionDuration(hcluster) - // This new condition is necessary for OCM personnel to report any cloud dangling objects to the user. // The grace period is customizable using an annotation called HCDestroyGracePeriodAnnotation. It's a time.Duration annotation. // This annotation will create a new condition called HostedClusterDestroyed which in conjuntion with CloudResourcesDestroyed @@ -432,9 +423,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } - // Metric for deletion duration should not include grace period which is only used to report status - reportHostedClusterDeletionDuration(hcluster, r.Clock) - if hcDestroyGracePeriod > 0 { if hostedClusterDestroyedCondition == nil { hostedClusterDestroyedCondition = &metav1.Condition{ @@ -542,9 +530,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques // Set version status hcluster.Status.Version = computeClusterVersionStatus(r.Clock, hcluster, hcp) - // compute ClusterUpgradeDuration and record metric if there were any upgrades - hcmetrics.ReportClusterUpgradeDuration(hcluster) - // Copy the CVO conditions from the HCP. hcpCVOConditions := map[hyperv1.ConditionType]*metav1.Condition{ hyperv1.ClusterVersionSucceeding: nil, @@ -721,13 +706,25 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques // conditions (so that it could incorporate e.g. HostedControlPlane and IgnitionServer // availability in the ultimate HostedCluster Available condition) { - meta.SetStatusCondition(&hcluster.Status.Conditions, computeHostedClusterAvailability(hcluster, hcp)) + availableCondition := computeHostedClusterAvailability(hcluster, hcp) + _, isHasBeenAvailableAnnotationSet := hcluster.Annotations[hcmetrics.HasBeenAvailableAnnotation] - // SLI: Hosted Cluster available duration. - reportAvailableTime(hcluster) - } + meta.SetStatusCondition(&hcluster.Status.Conditions, availableCondition) + + if availableCondition.Status == metav1.ConditionTrue && !isHasBeenAvailableAnnotationSet { + original := hcluster.DeepCopy() + + if hcluster.Annotations == nil { + hcluster.Annotations = make(map[string]string) + } - reportClusterVersionRolloutTime(hcluster) + hcluster.Annotations[hcmetrics.HasBeenAvailableAnnotation] = "true" + + if err := r.Patch(ctx, hcluster, client.MergeFromWithOptions(original)); err != nil { + return ctrl.Result{}, fmt.Errorf("cannot patch hosted cluster with has been available annotation: %w", err) + } + } + } // Copy AWSEndpointAvailable and AWSEndpointServiceAvailable conditions from the AWSEndpointServices. if hcluster.Spec.Platform.Type == hyperv1.AWSPlatform { @@ -3189,6 +3186,7 @@ func computeHostedClusterAvailability(hcluster *hyperv1.HostedCluster, hcp *hype hcpAvailableMessage = "The hosted control plane is available" } } + return metav1.Condition{ Type: string(hyperv1.HostedClusterAvailable), Status: hcpAvailableStatus, @@ -3342,7 +3340,6 @@ func deleteAWSEndpointServices(ctx context.Context, c client.Client, hc *hyperv1 return false, fmt.Errorf("failed to remove finalizer from awsendpointservice: %w", err) } } - hcmetrics.SkippedCloudResourcesDeletion.WithLabelValues(hc.Namespace, hc.Name, hc.Spec.ClusterID).Set(float64(1)) log.Info("Removed CPO finalizer for awsendpointservice because the HC has no valid aws credentials", "name", ep.Name, "endpoint-id", ep.Status.EndpointID) continue } @@ -3375,19 +3372,6 @@ func (r *HostedClusterReconciler) delete(ctx context.Context, hc *hyperv1.Hosted controlPlaneNamespace := manifests.HostedControlPlaneNamespace(hc.Namespace, hc.Name).Name log := ctrl.LoggerFrom(ctx) - // add `hyperv1.SilenceClusterAlertsLabel` label to the HostedCluster when it is deleting - // this will ensure that alerts triggered because of the deletion process aren't forwarded - if _, ok := hc.Labels[hyperv1.SilenceClusterAlertsLabel]; !ok { - original := hc.DeepCopy() - if hc.Labels == nil { - hc.Labels = make(map[string]string) - } - hc.Labels[hyperv1.SilenceClusterAlertsLabel] = "clusterDeleting" - if err := r.Patch(ctx, hc, client.MergeFromWithOptions(original)); err != nil { - return false, fmt.Errorf("cannot patch hosted cluster with silence label: %w", err) - } - } - // ensure that the cleanup annotation has been propagated to the hcp if it is set if hc.Annotations[hyperv1.CleanupCloudResourcesAnnotation] == "true" { hcp := controlplaneoperator.HostedControlPlane(controlPlaneNamespace, hc.Name) @@ -4817,111 +4801,6 @@ func (r *HostedClusterReconciler) reconcileSREMetricsConfig(ctx context.Context, return nil } -func hasBeenAvailable(hc *hyperv1.HostedCluster) bool { - _, ok := hc.Annotations[HasBeenAvailableAnnotation] - return ok -} - -// clusterAvailableTime returns the time in seconds from cluster creation to first available transition. -// If the condition is nil, false or the cluster has already been available it returns nil. -func clusterAvailableTime(hc *hyperv1.HostedCluster) *float64 { - if hasBeenAvailable(hc) { - return nil - } - condition := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.HostedClusterAvailable)) - if condition == nil { - return nil - } - if condition.Status == metav1.ConditionFalse { - return nil - } - transitionTime := condition.LastTransitionTime - return k8sutilspointer.Float64(transitionTime.Sub(hc.CreationTimestamp.Time).Seconds()) -} - -func clusterVersionRolloutTime(hc *hyperv1.HostedCluster) *float64 { - if hc.Status.Version == nil || len(hc.Status.Version.History) == 0 { - return nil - } - completionTime := hc.Status.Version.History[len(hc.Status.Version.History)-1].CompletionTime - if completionTime == nil { - return nil - } - return k8sutilspointer.Float64(completionTime.Sub(hc.CreationTimestamp.Time).Seconds()) -} - -func reportAvailableTime(hcluster *hyperv1.HostedCluster) { - // SLI: Hosted Cluster available duration. - availableTime := clusterAvailableTime(hcluster) - if availableTime != nil { - hcmetrics.HostedClusterAvailableDuration.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(*availableTime) - if hcluster.Annotations == nil { - hcluster.Annotations = make(map[string]string) - } - hcluster.Annotations[HasBeenAvailableAnnotation] = "true" - } -} - -func reportClusterVersionRolloutTime(hcluster *hyperv1.HostedCluster) { - // SLI: Hosted Cluster roll out duration. - versionRolloutTime := clusterVersionRolloutTime(hcluster) - if versionRolloutTime != nil { - hcmetrics.HostedClusterInitialRolloutDuration.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(*versionRolloutTime) - } -} - -func reportLimitedSuportEnabled(hcluster *hyperv1.HostedCluster) { - // Collect limited support metric. - if _, ok := hcluster.Labels[hyperv1.LimitedSupportLabel]; ok { - hcmetrics.LimitedSupportEnabled.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(1) - } else { - hcmetrics.LimitedSupportEnabled.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(0) - } -} - -func reportSilecedAlerts(hcluster *hyperv1.HostedCluster) { - // Collect silence alerts metric - if _, ok := hcluster.Labels[hyperv1.SilenceClusterAlertsLabel]; ok { - hcmetrics.SilenceAlerts.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(1) - } else { - hcmetrics.SilenceAlerts.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(0) - } -} - -func reportProxyConfig(hcluster *hyperv1.HostedCluster) { - // Collect proxy metric. - var proxyHTTP, proxyHTTPS, proxyTrustedCA string - if hcluster.Spec.Configuration != nil && hcluster.Spec.Configuration.Proxy != nil { - if hcluster.Spec.Configuration.Proxy.HTTPProxy != "" { - proxyHTTP = "1" - } - if hcluster.Spec.Configuration.Proxy.HTTPSProxy != "" { - proxyHTTPS = "1" - } - if hcluster.Spec.Configuration.Proxy.TrustedCA.Name != "" { - proxyTrustedCA = "1" - } - hcmetrics.ProxyConfig.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID, proxyHTTP, proxyHTTPS, proxyTrustedCA).Set(1) - } else { - hcmetrics.ProxyConfig.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID, proxyHTTP, proxyHTTPS, proxyTrustedCA).Set(0) - } -} - -func reportHostedClusterGuestCloudResourcesDeletionDuration(hcluster *hyperv1.HostedCluster) { - // SLI: Guest cluster resources deletion duration. - condition := meta.FindStatusCondition(hcluster.Status.Conditions, string(hyperv1.CloudResourcesDestroyed)) - if condition != nil && condition.Status == metav1.ConditionTrue { - guestClusterResourceDeletionDuration := condition.LastTransitionTime.Sub(hcluster.DeletionTimestamp.Time).Seconds() - hcmetrics.HostedClusterGuestCloudResourcesDeletionDuration.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(guestClusterResourceDeletionDuration) - } -} - -func reportHostedClusterDeletionDuration(hcluster *hyperv1.HostedCluster, funcClock clock.WithTickerAndDelayedExecution) { - // SLI: HostedCluster deletion duration. - deletionDuration := funcClock.Since(hcluster.DeletionTimestamp.Time).Seconds() - hcmetrics.HostedClusterDeletionDuration.WithLabelValues(hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID).Set(deletionDuration) -} - func getNodePortIP(hcluster *hyperv1.HostedCluster) net.IP { for _, svc := range hcluster.Spec.Services { if svc.Service == hyperv1.APIServer && svc.Type == hyperv1.NodePort { diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go index 500841503066..51b8af5c50fe 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/clusterapi" "github.com/go-logr/logr" @@ -31,8 +30,6 @@ import ( "github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client" "github.com/openshift/hypershift/support/upsert" "github.com/openshift/hypershift/support/util/fakeimagemetadataprovider" - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" "go.uber.org/zap/zapcore" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -60,60 +57,112 @@ var Now = metav1.NewTime(time.Now()) var Later = metav1.NewTime(Now.Add(5 * time.Minute)) func TestHasBeenAvailable(t *testing.T) { + now := time.Now().Truncate(time.Second) + reconcilerNow := metav1.Time{Time: now.Add(time.Second)} + testCases := []struct { - name string - hc *hyperv1.HostedCluster - expected bool + name string + timestamp time.Time + hcAnnotationsBeforeReconciliation map[string]string + hcpConditions []metav1.Condition + isExpectingAnnotationToBeSet bool }{ { - name: "When it has HasBeenAvailable Annotation it should return true", - hc: &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - HasBeenAvailableAnnotation: "true", - }, + name: "When cluster just got created, annotation is not yet set", + timestamp: now, + }, + { + name: "When available condition is false, annotation is not set", + timestamp: now.Add(5 * time.Minute), + hcpConditions: []metav1.Condition{ + { + Type: string(hyperv1.HostedControlPlaneAvailable), + Status: metav1.ConditionFalse, }, }, - expected: true, }, { - name: "When it has no HasBeenAvailable Annotation it should return false", - hc: &hyperv1.HostedCluster{}, - expected: false, + name: "When available condition is true, annotation is set", + timestamp: now.Add(5 * time.Minute), + hcpConditions: []metav1.Condition{ + { + Type: string(hyperv1.HostedControlPlaneAvailable), + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + }, + }, + isExpectingAnnotationToBeSet: true, }, - } - - g := NewGomegaWithT(t) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - g.Expect(hasBeenAvailable(tc.hc)).To(Equal(tc.expected)) - }) - } -} - -func TestClusterAvailableTime(t *testing.T) { - testCases := []struct { - name string - hc *hyperv1.HostedCluster - expected *float64 - }{ { - name: "When HostedCluster has been available it should return nil", - hc: &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - HasBeenAvailableAnnotation: "true", - }, + name: "When available condition is false again, annotation is not unset if already set", + timestamp: now.Add(10 * time.Minute), + hcAnnotationsBeforeReconciliation: map[string]string{ + hcmetrics.HasBeenAvailableAnnotation: "true", + }, + hcpConditions: []metav1.Condition{ + { + Type: string(hyperv1.HostedClusterAvailable), + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Time{Time: now.Add(10 * time.Minute)}, }, }, - expected: nil, + isExpectingAnnotationToBeSet: true, }, } - g := NewGomegaWithT(t) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - g.Expect(clusterAvailableTime(tc.hc)).To(BeEquivalentTo(tc.expected)) + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + CreationTimestamp: metav1.Time{Time: now}, + Annotations: tc.hcAnnotationsBeforeReconciliation, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + Networking: hyperv1.ClusterNetworking{ + ClusterNetwork: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}}, // Needed or some reconcile checks will fail + }, + }, + } + + hcpNs := hcpmanifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name).Name + hcp := controlplaneoperator.HostedControlPlane(hcpNs, hcluster.Name) + + hcp.Status = hyperv1.HostedControlPlaneStatus{ + Conditions: tc.hcpConditions, + } + + client := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster, hcp).WithStatusSubresource(hcluster).Build() + clock := clocktesting.NewFakeClock(tc.timestamp) + r := &HostedClusterReconciler{ + Client: client, + Clock: clock, + createOrUpdate: func(reconcile.Request) upsert.CreateOrUpdateFN { return ctrl.CreateOrUpdate }, + ManagementClusterCapabilities: &fakecapabilities.FakeSupportNoCapabilities{}, + now: func() metav1.Time { return reconcilerNow }, + } + + ctx := context.Background() + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: crclient.ObjectKeyFromObject(hcluster)}) + if err != nil { + t.Fatalf("error on %s reconciliation: %v", hcluster.Name, err) + } + + if err := client.Get(ctx, crclient.ObjectKeyFromObject(hcluster), hcluster); err != nil { + t.Fatalf("failed to get cluster after reconciliation: %v", err) + } + + _, isAnnotationSet := hcluster.Annotations[hcmetrics.HasBeenAvailableAnnotation] + + if isAnnotationSet != tc.isExpectingAnnotationToBeSet { + if tc.isExpectingAnnotationToBeSet { + t.Errorf("expected annotation %s to be set, but annotation is not set", hcmetrics.HasBeenAvailableAnnotation) + } else { + t.Errorf("expected annotation %s not to be set, but annotation is set", hcmetrics.HasBeenAvailableAnnotation) + } + } }) } } @@ -262,6 +311,9 @@ func TestComputeHostedClusterAvailability(t *testing.T) { }, "should be available": { Cluster: hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, Spec: hyperv1.HostedClusterSpec{ Etcd: hyperv1.EtcdSpec{ManagementType: hyperv1.Managed}, }, @@ -2569,868 +2621,90 @@ func TestComputeAWSEndpointServiceCondition(t *testing.T) { } } -func TestSkipCloudResourceDeletionMetric(t *testing.T) { - now := metav1.Time{Time: time.Now().Round(time.Second)} - reconcilerNow := metav1.Time{Time: now.Add(time.Second)} - - testCases := []struct { - name string - reconcileResult error - conditions []metav1.Condition - expected []*dto.MetricFamily +func TestValidateSliceNetworkCIDRs(t *testing.T) { + tests := []struct { + name string + mn []hyperv1.MachineNetworkEntry + cn []hyperv1.ClusterNetworkEntry + sn []hyperv1.ServiceNetworkEntry + wantErr bool }{ { - name: "Force skipping deletion aws cloud resources by broken OIDC, SkippedCloudResourcesDeletion should be reported", - conditions: []metav1.Condition{ - { - Type: string(hyperv1.ValidAWSIdentityProvider), - Status: metav1.ConditionFalse, - }, - }, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.SkippedCloudResourcesDeletionName), - Help: pointer.String("Indicates the operator will skip the aws resources deletion"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hcluster"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(1)}, - }}, - }}, + name: "given a conflicting IPv6 clusterNetwork overlapped with machineNetwork, it should fail", + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/64")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, + wantErr: true, }, { - name: "In a usual cluster teardown, SkippedCloudResourcesDeletion should not be reported", - conditions: []metav1.Condition{ - { - Type: string(hyperv1.ValidAWSIdentityProvider), - Status: metav1.ConditionTrue, - }, - }, - expected: []*dto.MetricFamily{}, + name: "given different IPv6 network CIDRs, it should success", + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, + wantErr: false, }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - hcluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hcluster", - Namespace: "any", - Labels: map[string]string{hyperv1.SilenceClusterAlertsLabel: "clusterDeleting"}, - DeletionTimestamp: &metav1.Time{Time: time.Now().Round(time.Second)}, - Finalizers: []string{"necessary"}, // fake client needs finalizers when a deletionTimestamp is set - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", - InfraID: "fakeInfraID", - Platform: hyperv1.PlatformSpec{ - Type: hyperv1.PlatformType(hyperv1.AWSPlatform), - AWS: &hyperv1.AWSPlatformSpec{ - Region: "fake", - CloudProviderConfig: &hyperv1.AWSCloudProviderConfig{ - VPC: "fake", - }, - }, - }, - }, - Status: hyperv1.HostedClusterStatus{ - Conditions: tc.conditions, - }, - } - - hcpNs := hcpmanifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name).Name - hcp := controlplaneoperator.HostedControlPlane(hcpNs, hcluster.Name) - // This is necessary because, the fakeClient fails if the hcp does not have an Status - hcp.Status = hyperv1.HostedControlPlaneStatus{ - Conditions: tc.conditions, - } - orphanMachine := &capiaws.AWSMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "fakeMachine", - Namespace: hcpNs, - DeletionTimestamp: &metav1.Time{Time: time.Now().Round(time.Second)}, - Finalizers: []string{"necessary"}, // fake client needs finalizers when a deletionTimestamp is set - }, - } - awsep := &hyperv1.AWSEndpointService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "fakeAWSEp", - Namespace: hcpNs, - DeletionTimestamp: &metav1.Time{Time: time.Now().Round(time.Second)}, - Finalizers: []string{"necessary"}, // fake client needs finalizers when a deletionTimestamp is set - }, - } - - hcmetrics.SkippedCloudResourcesDeletion.Reset() - reg := prometheus.NewPedanticRegistry() - - if err := reg.Register(hcmetrics.SkippedCloudResourcesDeletion); err != nil { - t.Fatalf("registering SkippedCloudResourcesDeletion collector failed: %v", err) - } - - c := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster, hcp, awsep, orphanMachine).WithStatusSubresource(hcluster).Build() - r := &HostedClusterReconciler{ - Client: c, - Clock: clock.RealClock{}, - ManagementClusterCapabilities: &fakecapabilities.FakeSupportNoCapabilities{}, - now: func() metav1.Time { return reconcilerNow }, - } - - ctx := context.Background() - ctrl.SetLogger(zap.New(zap.UseDevMode(true))) - ctrl.LoggerInto(ctx, ctrl.Log) - - _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: crclient.ObjectKeyFromObject(hcluster)}) - if err != nil { - t.Fatalf("error on %s reconciliation: %v", hcluster.Name, err) - } - - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportAvailableTime(t *testing.T) { - testCases := []struct { - name string - conditions []metav1.Condition - expected []*dto.MetricFamily - }{ { - name: "When HostedClusterAvailable is false, Cluster available duration is not reported", - conditions: []metav1.Condition{ - { - Type: string(hyperv1.HostedClusterAvailable), - Status: metav1.ConditionFalse, - }, - }, - expected: []*dto.MetricFamily{}, + name: "given a conflicting IPv4 clusterNetwork overlapped with serviceNetwork, it should fail", + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/16")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}}, + wantErr: true, }, { - name: "When HostedClusterAvailable is true, Cluster available duration is reported", - conditions: []metav1.Condition{ - { - Type: string(hyperv1.HostedClusterAvailable), - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Time{Time: time.Time{}.Add(2 * time.Hour)}, - }, - }, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.AvailableDurationName), - Help: pointer.String("Time in seconds it took from initial cluster creation to HostedClusterAvailable condition becoming true"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(3600)}, - }}, - }}, + name: "given different IPv4 network CIDRs, it should success", + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}}, + wantErr: false, }, } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hc := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, - }, - Status: hyperv1.HostedClusterStatus{ - Conditions: tc.conditions, + Name: "hc", + Namespace: "any", }, Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", + Networking: hyperv1.ClusterNetworking{ + MachineNetwork: tt.mn, + ClusterNetwork: tt.cn, + ServiceNetwork: tt.sn, + }, }, } - - hcmetrics.HostedClusterAvailableDuration.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture metrics. - if err := reg.Register(hcmetrics.HostedClusterAvailableDuration); err != nil { - t.Fatalf("registering HostedClusterAvailableDuration collector failed: %v", err) - } - reportAvailableTime(cluster) - - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) + err := validateSliceNetworkCIDRs(hc) + if (err != nil) != tt.wantErr { + t.Errorf("validateSliceNetworkCIDRs() wantErr %v, err %v", tt.wantErr, err) } }) } } -func TestReportClusterVersionRolloutTime(t *testing.T) { - testCases := []struct { - name string - updateHistory []configv1.UpdateHistory - expected []*dto.MetricFamily +func TestCheckAdvertiseAddressOverlapping(t *testing.T) { + tests := []struct { + name string + mn []hyperv1.MachineNetworkEntry + cn []hyperv1.ClusterNetworkEntry + sn []hyperv1.ServiceNetworkEntry + aa *hyperv1.APIServerNetworking + wantErr bool }{ { - name: "When HostedCluster provision is finished, Cluster rollout duration is reported", - updateHistory: []configv1.UpdateHistory{{ - CompletionTime: &metav1.Time{Time: time.Time{}.Add(2 * time.Hour)}, - }}, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.InitialRolloutDurationName), - Help: pointer.String("Time in seconds it took from initial cluster creation and rollout of initial version"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(3600)}, - }}, - }}, + name: "given an IPv6 defined AdvertiseAddress overlapped with ClusterNetwork, it should fail", + aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1")}, + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, + wantErr: true, }, { - name: "When HostedCluster didn't finish updating, Cluster rollout duration is not reported", - expected: []*dto.MetricFamily{}, - }, - { - name: "When HostedCluster has Multiple versions, the oldest one is used in metrics report", - updateHistory: []configv1.UpdateHistory{ - { - CompletionTime: &metav1.Time{Time: time.Time{}.Add(3 * time.Hour)}, - }, - { - CompletionTime: &metav1.Time{Time: time.Time{}.Add(2 * time.Hour)}, - }, - }, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.InitialRolloutDurationName), - Help: pointer.String("Time in seconds it took from initial cluster creation and rollout of initial version"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(3600)}, - }}, - }}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, - }, - Status: hyperv1.HostedClusterStatus{ - Version: &hyperv1.ClusterVersionStatus{ - History: tc.updateHistory, - }, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", - }, - } - - hcmetrics.HostedClusterInitialRolloutDuration.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture metric. - if err := reg.Register(hcmetrics.HostedClusterInitialRolloutDuration); err != nil { - t.Fatalf("registering HostedClusterInitialRolloutDuration collector failed: %v", err) - } - - // Report metric. - reportClusterVersionRolloutTime(cluster) - - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportLimitedSuportEnabled(t *testing.T) { - testCases := []struct { - name string - labels map[string]string - expected []*dto.MetricFamily - }{ - { - name: "When LimitedSupport label is set, metric is reported as one", - labels: map[string]string{hyperv1.LimitedSupportLabel: "true"}, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.LimitedSupportEnabledName), - Help: pointer.String("Indicates if limited support is enabled for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("fakeClusterID"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(1)}, - }}, - }}, - }, - { - name: "When LimitedSupport label is not set, metric is reported as zero", - labels: map[string]string{}, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.LimitedSupportEnabledName), - Help: pointer.String("Indicates if limited support is enabled for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("fakeClusterID"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(0)}, - }}, - }}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - Labels: tc.labels, - CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "fakeClusterID", - }, - } - - hcmetrics.LimitedSupportEnabled.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture. - if err := reg.Register(hcmetrics.LimitedSupportEnabled); err != nil { - t.Fatalf("registering LimitedSupportEnabled collector failed: %v", err) - } - - // Report. - reportLimitedSuportEnabled(cluster) - - // Gather. - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - // Validate. - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportSilencedAlerts(t *testing.T) { - testCases := []struct { - name string - labels map[string]string - expected []*dto.MetricFamily - }{ - { - name: "When SilencedAlerts label is set, metric is reported as one", - labels: map[string]string{hyperv1.SilenceClusterAlertsLabel: "true"}, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.SilenceAlertsName), - Help: pointer.String("Indicates if alerts are silenced for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("fakeClusterID"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(1)}, - }}, - }}, - }, - { - name: "When SilencedAlerts label is not set, metric is reported as zero", - labels: map[string]string{}, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.SilenceAlertsName), - Help: pointer.String("Indicates if alerts are silenced for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("fakeClusterID"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(0)}, - }}, - }}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - Labels: tc.labels, - CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "fakeClusterID", - }, - } - - hcmetrics.SilenceAlerts.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture. - if err := reg.Register(hcmetrics.SilenceAlerts); err != nil { - t.Fatalf("registering SilenceAlertsName collector failed: %v", err) - } - - // Report. - reportSilecedAlerts(cluster) - - // Gather. - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - // Validate. - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportProxyConfig(t *testing.T) { - testCases := []struct { - name string - clusterConf hyperv1.ClusterConfiguration - expected []*dto.MetricFamily - }{ - { - name: "When Proxy configuration is set in the HostedCluster, proxy fields are reported as one", - clusterConf: hyperv1.ClusterConfiguration{ - Proxy: &configv1.ProxySpec{ - HTTPProxy: "fakeProxyServer", - HTTPSProxy: "fakeProxySecureServer", - TrustedCA: configv1.ConfigMapNameReference{ - Name: "fakeProxyTrustedCA", - }, - }, - }, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.ProxyName), - Help: pointer.String("Indicates cluster proxy state for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - { - Name: pointer.String("proxy_http"), Value: pointer.String("1"), - }, - { - Name: pointer.String("proxy_https"), Value: pointer.String("1"), - }, - { - Name: pointer.String("proxy_trusted_ca"), Value: pointer.String("1"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(1)}, - }}, - }}, - }, - { - name: "When Proxy configuration is not set in the HostedCluster, proxy fields are reported as empty", - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.ProxyName), - Help: pointer.String("Indicates cluster proxy state for each cluster"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - { - Name: pointer.String("proxy_http"), Value: pointer.String(""), - }, - { - Name: pointer.String("proxy_https"), Value: pointer.String(""), - }, - { - Name: pointer.String("proxy_trusted_ca"), Value: pointer.String(""), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(0)}, - }}, - }}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", - Configuration: &tc.clusterConf, - }, - } - - hcmetrics.ProxyConfig.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture. - if err := reg.Register(hcmetrics.ProxyConfig); err != nil { - t.Fatalf("registering reportProxyConfig collector failed: %v", err) - } - - // Report. - reportProxyConfig(cluster) - - // Gather. - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - // Validate. - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportHostedClusterGuestCloudResourcesDeletionDuration(t *testing.T) { - testCases := []struct { - name string - conditions []metav1.Condition - expected []*dto.MetricFamily - }{ - { - name: "When HostedCluster cloud resources are deleted, it should report GuestCloudResourcesDeletionDuration metric", - conditions: []metav1.Condition{ - { - Type: string(hyperv1.CloudResourcesDestroyed), - Status: metav1.ConditionTrue, - LastTransitionTime: Later, - }, - }, - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.GuestCloudResourcesDeletionDurationMetricName), - Help: pointer.String("Time in seconds it took from HostedCluster having a deletion timestamp to the CloudResourcesDestroyed being true"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(300)}, - }}, - }}, - }, - { - name: "When Cluster is not deleted, it should not report GuestCloudResourcesDeletionDuration metric", - conditions: []metav1.Condition{}, - expected: []*dto.MetricFamily{}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - DeletionTimestamp: &Now, - }, - Status: hyperv1.HostedClusterStatus{ - Conditions: tc.conditions, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", - }, - } - - hcmetrics.HostedClusterGuestCloudResourcesDeletionDuration.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture metrics. - if err := reg.Register(hcmetrics.HostedClusterGuestCloudResourcesDeletionDuration); err != nil { - t.Fatalf("registering TestReportHostedClusterGuestCloudResourcesDeletionDuration collector failed: %v", err) - } - reportHostedClusterGuestCloudResourcesDeletionDuration(cluster) - - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestReportHostedClusterDeletionDuration(t *testing.T) { - testCases := []struct { - name string - expected []*dto.MetricFamily - }{ - { - name: "When HostedCluster has deletionTimestamp set, it should report hostedClusterDeletionDuration time metric", - expected: []*dto.MetricFamily{{ - Name: pointer.String(hcmetrics.DeletionDurationMetricName), - Help: pointer.String("Time in seconds it took from HostedCluster having a deletion timestamp to all hypershift finalizers being removed"), - Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), - Metric: []*dto.Metric{{ - Label: []*dto.LabelPair{ - { - Name: pointer.String("_id"), Value: pointer.String("id"), - }, - { - Name: pointer.String("name"), Value: pointer.String("hc"), - }, - { - Name: pointer.String("namespace"), Value: pointer.String("any"), - }, - }, - Gauge: &dto.Gauge{Value: pointer.Float64(300)}, - }}, - }}, - }, - } - - for _, tc := range testCases { - now := time.Now() - fakeClock := clocktesting.NewFakeClock(now) - deletionTime := metav1.NewTime(now.Add(-5 * time.Minute)) - - t.Run(tc.name, func(t *testing.T) { - cluster := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - DeletionTimestamp: &deletionTime, - }, - Spec: hyperv1.HostedClusterSpec{ - ClusterID: "id", - }, - } - - hcmetrics.HostedClusterDeletionDuration.Reset() - reg := prometheus.NewPedanticRegistry() - - // Capture metrics. - if err := reg.Register(hcmetrics.HostedClusterDeletionDuration); err != nil { - t.Fatalf("registering HostedClusterDeletionDuration collector failed: %v", err) - } - reportHostedClusterDeletionDuration(cluster, fakeClock) - - result, err := reg.Gather() - if err != nil { - t.Fatalf("gathering metrics failed: %v", err) - } - - if diff := cmp.Diff(result, tc.expected, ignoreUnexportedDto()); diff != "" { - t.Errorf("result differs from actual: %s", diff) - } - }) - } -} - -func TestValidateSliceNetworkCIDRs(t *testing.T) { - tests := []struct { - name string - mn []hyperv1.MachineNetworkEntry - cn []hyperv1.ClusterNetworkEntry - sn []hyperv1.ServiceNetworkEntry - wantErr bool - }{ - { - name: "given a conflicting IPv6 clusterNetwork overlapped with machineNetwork, it should fail", - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/64")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, - wantErr: true, - }, - { - name: "given different IPv6 network CIDRs, it should success", - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, - wantErr: false, - }, - { - name: "given a conflicting IPv4 clusterNetwork overlapped with serviceNetwork, it should fail", - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/16")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}}, - wantErr: true, - }, - { - name: "given different IPv4 network CIDRs, it should success", - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("192.168.1.0/24")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.1.0/24")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.16.0.0/24")}}, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - hc := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "hc", - Namespace: "any", - }, - Spec: hyperv1.HostedClusterSpec{ - Networking: hyperv1.ClusterNetworking{ - MachineNetwork: tt.mn, - ClusterNetwork: tt.cn, - ServiceNetwork: tt.sn, - }, - }, - } - err := validateSliceNetworkCIDRs(hc) - if (err != nil) != tt.wantErr { - t.Errorf("validateSliceNetworkCIDRs() wantErr %v, err %v", tt.wantErr, err) - } - }) - } -} - -func TestCheckAdvertiseAddressOverlapping(t *testing.T) { - tests := []struct { - name string - mn []hyperv1.MachineNetworkEntry - cn []hyperv1.ClusterNetworkEntry - sn []hyperv1.ServiceNetworkEntry - aa *hyperv1.APIServerNetworking - wantErr bool - }{ - { - name: "given an IPv6 defined AdvertiseAddress overlapped with ClusterNetwork, it should fail", - aa: &hyperv1.APIServerNetworking{AdvertiseAddress: pointer.String("fd03::1")}, - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, - wantErr: true, - }, - { - name: "given not overlapped IPv6 networks CIDRs and not defined AdvertiseAddress, it should success", - mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, - cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}}, - sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, - wantErr: false, + name: "given not overlapped IPv6 networks CIDRs and not defined AdvertiseAddress, it should success", + mn: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/48")}}, + cn: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd01::/64")}}, + sn: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("2620:52:0:1306::1/64")}}, + wantErr: false, }, { name: "given an IPv4 defined AdvertiseAddress overlapped with MachineNetwork, it should fail", @@ -3674,7 +2948,7 @@ func TestReconcileCAPIProviderDeployment(t *testing.T) { Name: clusterapi.CAPIProviderDeployment("test").Name, Namespace: clusterapi.CAPIProviderDeployment("test").Namespace, Annotations: map[string]string{ - HasBeenAvailableAnnotation: "true", + hcmetrics.HasBeenAvailableAnnotation: "true", }, }, Spec: appsv1.DeploymentSpec{ @@ -3698,7 +2972,7 @@ func TestReconcileCAPIProviderDeployment(t *testing.T) { Name: clusterapi.CAPIProviderDeployment("test").Name, Namespace: clusterapi.CAPIProviderDeployment("test").Namespace, Annotations: map[string]string{ - HasBeenAvailableAnnotation: "true", + hcmetrics.HasBeenAvailableAnnotation: "true", }, }, Spec: appsv1.DeploymentSpec{}, @@ -4090,7 +3364,3 @@ func TestKubevirtETCDEncKey(t *testing.T) { ) } } - -func ignoreUnexportedDto() cmp.Option { - return cmpopts.IgnoreUnexported(dto.MetricFamily{}, dto.Metric{}, dto.LabelPair{}, dto.Gauge{}) -} diff --git a/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go b/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go index 4948eef90508..81073708c82a 100644 --- a/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go +++ b/hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go @@ -10,7 +10,6 @@ import ( "github.com/blang/semver" hyperv1 "github.com/openshift/hypershift/api/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas" - hcmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/metrics" "github.com/openshift/hypershift/support/images" "github.com/openshift/hypershift/support/upsert" "github.com/openshift/hypershift/support/util" @@ -320,7 +319,6 @@ func (AWS) DeleteOrphanedMachines(ctx context.Context, c client.Client, hc *hype errs = append(errs, fmt.Errorf("failed to delete machine %s/%s: %w", awsMachine.Namespace, awsMachine.Name, err)) continue } - hcmetrics.SkippedCloudResourcesDeletion.WithLabelValues(hc.Namespace, hc.Name, hc.Spec.ClusterID).Set(float64(1)) logger.Info("skipping cleanup of awsmachine because of invalid AWS identity provider", "machine", client.ObjectKeyFromObject(awsMachine)) } } diff --git a/hypershift-operator/controllers/hostedcluster/metrics/metrics.go b/hypershift-operator/controllers/hostedcluster/metrics/metrics.go index 843b22b246e7..d5d551c2b0b7 100644 --- a/hypershift-operator/controllers/hostedcluster/metrics/metrics.go +++ b/hypershift-operator/controllers/hostedcluster/metrics/metrics.go @@ -1,102 +1,473 @@ package metrics import ( + "context" + "time" + + configv1 "github.com/openshift/api/config/v1" hyperv1 "github.com/openshift/hypershift/api/v1beta1" + platformaws "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/aws" + "github.com/openshift/hypershift/support/conditions" "github.com/prometheus/client_golang/prometheus" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/clock" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/metrics" ) +const ( + HasBeenAvailableAnnotation = "hypershift.openshift.io/HasBeenAvailable" + + // Aggregating metrics - name & help + + CountByIdentityProviderMetricName = "hypershift_cluster_identity_providers" // What about renaming it to hypershift_clusters_by_identity_provider_type ? + countByIdentityProviderMetricHelp = "Number of HostedClusters for a given identity provider." + + CountByPlatformMetricName = "hypershift_hostedclusters" // What about renaming it to hypershift_clusters_by_platform ? + countByPlatformMetricHelp = "Number of HostedClusters for a given platform." + + CountByPlatformAndFailureConditionMetricName = "hypershift_hostedclusters_failure_conditions" // What about renaming it to hypershift_clusters_by_platform_and_failure_condition ? + countByPlatformAndFailureConditionMetricHelp = "Number of HostedClusters for a given platform and failure condition." + + TransitionDurationMetricName = "hypershift_hosted_cluster_transition_seconds" // What about renaming it to hypershift_hosted_clusters_transition_duration_seconds ? + transitionDurationMetricHelp = "Time in seconds it took for conditions to become true since the creation of the HostedCluster." + + // Per hosted cluster metrics - name & help + + WaitingInitialAvailabilityDurationMetricName = "hypershift_cluster_waiting_initial_avaibility_duration_seconds" + waitingInitialAvailabilityDurationMetricHelp = "Time in seconds it is taking to get the HostedClusterAvailable condition becoming true since the creation of the HostedCluster. " + + "Undefined if the condition has already become true once or if the cluster no longer exists." + + InitialRollingOutDurationMetricName = "hypershift_cluster_initial_rolling_out_duration_seconds" + initialRollingOutDurationMetricHelp = "Time in seconds it is taking to roll out the initial version since the creation of the HostedCluster. " + + "Version is rolled out when its state is set to 'Completed' in the history. " + + "Undefined if this state has already been reached in the past or if the cluster no longer exists." + + UpgradingDurationMetricName = "hypershift_cluster_upgrading_duration_seconds" + upgradingDurationMetricHelp = "Time in seconds it is taking to upgrade the HostedCluster / to roll out subsequent versions since the beginning of the update. " + + "Version is rolled out when its state is set to 'Completed' in the history. " + + "Undefined if the cluster is not upgrading or if the upgrade is finished or if the cluster no longer exists." + + LimitedSupportEnabledMetricName = "hypershift_cluster_limited_support_enabled" + limitedSupportEnabledMetricHelp = "Indicates if the given HostedCluster is in limited support or not" + + SilenceAlertsMetricName = "hypershift_cluster_silence_alerts" + silenceAlertsMetricHelp = "Indicates if the given HostedCluster is silenced or not" + + ProxyMetricName = "hypershift_cluster_proxy" + proxyMetricHelp = "Indicates if the given HostedCluster is available through a proxy or not" + + InvalidAwsCredsMetricName = "hypershift_cluster_invalid_aws_creds" + invalidAwsCredsMetricHelp = "Indicates if the given HostedCluster has valid AWS credentials or not" + + DeletingDurationMetricName = "hypershift_cluster_deleting_duration_seconds" + deletingDurationMetricHelp = "Time in seconds it is taking to delete the HostedCluster since the beginning of the delete. " + + "Undefined if the cluster is not deleting or no longer exists." + + GuestCloudResourcesDeletingDurationMetricName = "hypershift_cluster_guest_cloud_resources_deleting_duration_seconds" + guestCloudResourcesDeletingDurationMetricHelp = "Time in seconds it is taking to get the CloudResourcesDestroyed condition become true since the beginning of the delete of the HostedCluster. " + + "Undefined if the cluster is not deleting/no longer exists or if the condition has already become true." +) + +// semantically constant - not suposed to be changed at runtime var ( - DeletionDurationMetricName = "hypershift_cluster_deletion_duration_seconds" - HostedClusterDeletionDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took from HostedCluster having a deletion timestamp to all hypershift finalizers being removed", - Name: DeletionDurationMetricName, - }, []string{"namespace", "name", "_id"}) - - GuestCloudResourcesDeletionDurationMetricName = "hypershift_cluster_guest_cloud_resources_deletion_duration_seconds" - HostedClusterGuestCloudResourcesDeletionDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took from HostedCluster having a deletion timestamp to the CloudResourcesDestroyed being true", - Name: GuestCloudResourcesDeletionDurationMetricName, - }, []string{"namespace", "name", "_id"}) - - AvailableDurationName = "hypershift_cluster_available_duration_seconds" - HostedClusterAvailableDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took from initial cluster creation to HostedClusterAvailable condition becoming true", - Name: AvailableDurationName, - }, []string{"namespace", "name", "_id"}) - - InitialRolloutDurationName = "hypershift_cluster_initial_rollout_duration_seconds" - HostedClusterInitialRolloutDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took from initial cluster creation and rollout of initial version", - Name: InitialRolloutDurationName, - }, []string{"namespace", "name", "_id"}) - - ClusterUpgradeDurationMetricName = "hypershift_cluster_upgrade_duration_seconds" - ClusterUpgradeDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took a cluster to upgrade and rollout a given version", - Name: ClusterUpgradeDurationMetricName, - }, []string{"namespace", "name", "_id", "previous_version", "new_version"}) - - LimitedSupportEnabledName = "hypershift_cluster_limited_support_enabled" - LimitedSupportEnabled = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Indicates if limited support is enabled for each cluster", - Name: LimitedSupportEnabledName, - }, []string{"namespace", "name", "_id"}) - - SilenceAlertsName = "hypershift_cluster_silence_alerts" - SilenceAlerts = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Indicates if alerts are silenced for each cluster", - Name: SilenceAlertsName, - }, []string{"namespace", "name", "_id"}) - - ProxyName = "hypershift_cluster_proxy" - ProxyConfig = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Indicates cluster proxy state for each cluster", - Name: ProxyName, - }, []string{"namespace", "name", "_id", "proxy_http", "proxy_https", "proxy_trusted_ca"}) - - SkippedCloudResourcesDeletionName = "hypershift_cluster_skipped_cloud_resources_deletion" - SkippedCloudResourcesDeletion = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Indicates the operator will skip the aws resources deletion", - Name: SkippedCloudResourcesDeletionName, - }, []string{"namespace", "name", "_id"}) + // List of known identidy providers + // To be updated when a new identity provider is added; failure to do so is not a big deal it is just that + // countByIdentityProviderMetric metric will be undefined rather than initialized to 0 for the new identity provider + knownIdentityProviders = []configv1.IdentityProviderType{ + configv1.IdentityProviderTypeBasicAuth, + configv1.IdentityProviderTypeGitHub, + configv1.IdentityProviderTypeGitLab, + configv1.IdentityProviderTypeGoogle, + configv1.IdentityProviderTypeHTPasswd, + configv1.IdentityProviderTypeKeystone, + configv1.IdentityProviderTypeLDAP, + configv1.IdentityProviderTypeOpenID, + configv1.IdentityProviderTypeRequestHeader, + } + + knownPlatforms = hyperv1.PlatformTypes() + + knownConditionToExpectedStatus = conditions.ExpectedHCConditions() + + // Metrics descriptions + countByIdentityProviderMetricDesc = prometheus.NewDesc( + CountByIdentityProviderMetricName, + countByIdentityProviderMetricHelp, + []string{"identity_provider"}, nil) + + countByPlatformMetricDesc = prometheus.NewDesc( + CountByPlatformMetricName, + countByPlatformMetricHelp, + []string{"platform"}, nil) + + countByPlatformAndFailureConditionMetricDesc = prometheus.NewDesc( + CountByPlatformAndFailureConditionMetricName, + countByPlatformAndFailureConditionMetricHelp, + []string{"platform", "condition"}, nil) + + // One time series per hosted cluster for below metrics + hclusterLabels = []string{"namespace", "name", "_id"} + + waitingInitialAvailabilityDurationMetricDesc = prometheus.NewDesc( + WaitingInitialAvailabilityDurationMetricName, + waitingInitialAvailabilityDurationMetricHelp, + hclusterLabels, nil) + + initialRollingOutDurationMetricDesc = prometheus.NewDesc( + InitialRollingOutDurationMetricName, + initialRollingOutDurationMetricHelp, + hclusterLabels, nil) + + upgradingDurationMetricDesc = prometheus.NewDesc( + UpgradingDurationMetricName, upgradingDurationMetricHelp, + append(hclusterLabels, "previous_version", "new_version"), nil) + + limitedSupportEnabledMetricDesc = prometheus.NewDesc( + LimitedSupportEnabledMetricName, limitedSupportEnabledMetricHelp, + hclusterLabels, nil) + + silenceAlertsMetricDesc = prometheus.NewDesc( + SilenceAlertsMetricName, silenceAlertsMetricHelp, + hclusterLabels, nil) + + proxyMetricDesc = prometheus.NewDesc( + ProxyMetricName, proxyMetricHelp, + append(hclusterLabels, "proxy_http", "proxy_https", "proxy_trusted_ca"), nil) + + invalidAwsCredsMetricDesc = prometheus.NewDesc( + InvalidAwsCredsMetricName, invalidAwsCredsMetricHelp, + hclusterLabels, nil) + + deletingDurationMetricDesc = prometheus.NewDesc( + DeletingDurationMetricName, deletingDurationMetricHelp, + hclusterLabels, nil) + + guestCloudResourcesDeletingDurationMetricDesc = prometheus.NewDesc( + GuestCloudResourcesDeletingDurationMetricName, guestCloudResourcesDeletingDurationMetricHelp, + hclusterLabels, nil) ) -func init() { - metrics.Registry.MustRegister( - HostedClusterDeletionDuration, - HostedClusterGuestCloudResourcesDeletionDuration, - HostedClusterAvailableDuration, - HostedClusterInitialRolloutDuration, - ClusterUpgradeDuration, - LimitedSupportEnabled, - SilenceAlerts, - ProxyConfig, - SkippedCloudResourcesDeletion, - ) +type hostedClustersMetricsCollector struct { + client.Client + clock clock.Clock + + transitionDurationMetric *prometheus.HistogramVec + + lastCollectTime time.Time } -func ReportClusterUpgradeDuration(hc *hyperv1.HostedCluster) { - // if history has less than 2 entries, then there were no upgrades. - if hc.Status.Version == nil || len(hc.Status.Version.History) < 2 { - return +func createHostedClustersMetricsCollector(client client.Client, clock clock.Clock) prometheus.Collector { + return &hostedClustersMetricsCollector{ + Client: client, + clock: clock, + transitionDurationMetric: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Name: TransitionDurationMetricName, + Help: transitionDurationMetricHelp, + Buckets: []float64{5, 10, 20, 30, 60, 90, 120, 180, 240, 300, 360, 480, 600}, + }, []string{"condition"}), + lastCollectTime: time.UnixMilli(0), } +} + +func CreateAndRegisterHostedClustersMetricsCollector(client client.Client) prometheus.Collector { + collector := createHostedClustersMetricsCollector(client, clock.RealClock{}) + + metrics.Registry.MustRegister(collector) + + return collector +} - newVersion := hc.Status.Version.History[0] - for i := 1; i < len(hc.Status.Version.History); i++ { - prevVersion := hc.Status.Version.History[i] - if newVersion.CompletionTime != nil { - upgradeDuration := newVersion.CompletionTime.Time.Sub(newVersion.StartedTime.Time).Seconds() - - ClusterUpgradeDuration.With(prometheus.Labels{ - "namespace": hc.Namespace, - "name": hc.Name, - "_id": hc.Spec.ClusterID, - "previous_version": prevVersion.Version, - "new_version": newVersion.Version, - }).Set(upgradeDuration) +func (c *hostedClustersMetricsCollector) Describe(ch chan<- *prometheus.Desc) { + prometheus.DescribeByCollect(c, ch) +} + +func createFailureConditionToHClustersCountMap() *map[string]int { + res := make(map[string]int) + + for conditionType, expectedStatus := range knownConditionToExpectedStatus { + failureCondPrefix := "" + + if expectedStatus == metav1.ConditionTrue { + failureCondPrefix = "not_" } - newVersion = prevVersion + res[failureCondPrefix+string(conditionType)] = 0 + } + + return &res +} + +func (c *hostedClustersMetricsCollector) Collect(ch chan<- prometheus.Metric) { + currentCollectTime := c.clock.Now() + log := ctrllog.Log + + // countByIdentityProviderMetric - init + identityProviderToHClustersCount := make(map[configv1.IdentityProviderType]int) + + for k := range knownIdentityProviders { + identityProviderToHClustersCount[knownIdentityProviders[k]] = 0 + } + + // countByPlatformMetric - init + platformToHClustersCount := make(map[hyperv1.PlatformType]int) + + for k := range knownPlatforms { + platformToHClustersCount[knownPlatforms[k]] = 0 } + + // countByPlatformAndFailureConditionMetric - init + platformToFailureConditionToHClustersCount := make(map[hyperv1.PlatformType]*map[string]int) + + for k := range knownPlatforms { + platformToFailureConditionToHClustersCount[knownPlatforms[k]] = createFailureConditionToHClustersCountMap() + } + + // MAIN LOOP - Hosted clusters loop + { + hclusters := &hyperv1.HostedClusterList{} + + if err := c.List(context.Background(), hclusters); err != nil { + log.Error(err, "failed to list hosted clusters while collecting metrics") + } + + for k := range hclusters.Items { + hcluster := &hclusters.Items[k] + + // countByIdentityProviderMetric - aggregation + if hcluster.Spec.Configuration != nil && hcluster.Spec.Configuration.OAuth != nil { + for _, identityProvider := range hcluster.Spec.Configuration.OAuth.IdentityProviders { + identityProviderToHClustersCount[identityProvider.Type] = identityProviderToHClustersCount[identityProvider.Type] + 1 + } + } + + // countByPlatformMetric - aggregation + platform := hcluster.Spec.Platform.Type + platformToHClustersCount[platform] = platformToHClustersCount[platform] + 1 + + // countByPlatformAndFailureConditionMetric - aggregation + { + _, isKnownPlatform := platformToFailureConditionToHClustersCount[platform] + + if !isKnownPlatform { + platformToFailureConditionToHClustersCount[platform] = createFailureConditionToHClustersCountMap() + } + + failureConditionToHClustersCount := platformToFailureConditionToHClustersCount[platform] + + for _, condition := range hcluster.Status.Conditions { + expectedStatus, isKnownCondition := knownConditionToExpectedStatus[hyperv1.ConditionType(condition.Type)] + + if isKnownCondition && condition.Status != expectedStatus { + failureCondPrefix := "" + + if expectedStatus == metav1.ConditionTrue { + failureCondPrefix = "not_" + } + + failureCondition := failureCondPrefix + condition.Type + + (*failureConditionToHClustersCount)[failureCondition] = (*failureConditionToHClustersCount)[failureCondition] + 1 + } + } + } + + // transitionDurationMetric - aggregation + for _, conditionType := range []hyperv1.ConditionType{hyperv1.EtcdAvailable, hyperv1.InfrastructureReady, hyperv1.ExternalDNSReachable} { + condition := meta.FindStatusCondition(hcluster.Status.Conditions, string(conditionType)) + + if condition != nil && condition.Status == metav1.ConditionTrue { + t := condition.LastTransitionTime.Time + + if c.lastCollectTime.Before(t) && (t.Before(currentCollectTime) || t.Equal(currentCollectTime)) { + c.transitionDurationMetric.With(map[string]string{"condition": string(conditionType)}).Observe(t.Sub(hcluster.CreationTimestamp.Time).Seconds()) + } + } + } + + hclusterLabelValues := []string{hcluster.Namespace, hcluster.Name, hcluster.Spec.ClusterID} + + // waitingInitialAvailabilityDurationMetric + { + _, hasBeenAvailable := hcluster.Annotations[HasBeenAvailableAnnotation] + + if !hasBeenAvailable { + initializingDuration := c.clock.Since(hcluster.CreationTimestamp.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + waitingInitialAvailabilityDurationMetricDesc, + prometheus.GaugeValue, + initializingDuration, + hclusterLabelValues..., + ) + } + } + + // initialRollingOutDurationMetric + if hcluster.Status.Version == nil || len(hcluster.Status.Version.History) == 0 || hcluster.Status.Version.History[0].CompletionTime == nil { + initializingDuration := c.clock.Since(hcluster.CreationTimestamp.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + initialRollingOutDurationMetricDesc, + prometheus.GaugeValue, + initializingDuration, + hclusterLabelValues..., + ) + } + + // upgradingDurationMetric + // The upgrade is adding a new entry in the history on top of the initial rollout. + if hcluster.Status.Version != nil && len(hcluster.Status.Version.History) > 1 { + newVersionEntry := hcluster.Status.Version.History[len(hcluster.Status.Version.History)-1] + + if newVersionEntry.CompletionTime == nil { + previousVersionEntry := hcluster.Status.Version.History[len(hcluster.Status.Version.History)-2] + upgradingDuration := c.clock.Since(newVersionEntry.StartedTime.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + upgradingDurationMetricDesc, + prometheus.GaugeValue, + upgradingDuration, + append(hclusterLabelValues, previousVersionEntry.Version, newVersionEntry.Version)..., + ) + } + } + + // limitedSupportEnabledMetric + { + limitedSupportValue := 0.0 + if _, ok := hcluster.Labels[hyperv1.LimitedSupportLabel]; ok { + limitedSupportValue = 1.0 + } + + ch <- prometheus.MustNewConstMetric( + limitedSupportEnabledMetricDesc, + prometheus.GaugeValue, + limitedSupportValue, + hclusterLabelValues..., + ) + } + + // silenceAlertsMetric + { + silenceAlertsValue := 0.0 + if _, ok := hcluster.Labels[hyperv1.SilenceClusterAlertsLabel]; ok { + silenceAlertsValue = 1.0 + } + + ch <- prometheus.MustNewConstMetric( + silenceAlertsMetricDesc, + prometheus.GaugeValue, + silenceAlertsValue, + hclusterLabelValues..., + ) + } + + // proxyMetric + { + var proxyHTTP, proxyHTTPS, proxyTrustedCA string + proxyValue := 0.0 + if hcluster.Spec.Configuration != nil && hcluster.Spec.Configuration.Proxy != nil { + if hcluster.Spec.Configuration.Proxy.HTTPProxy != "" { + proxyHTTP = "1" + } + if hcluster.Spec.Configuration.Proxy.HTTPSProxy != "" { + proxyHTTPS = "1" + } + if hcluster.Spec.Configuration.Proxy.TrustedCA.Name != "" { + proxyTrustedCA = "1" + } + proxyValue = 1.0 + } + + ch <- prometheus.MustNewConstMetric( + proxyMetricDesc, + prometheus.GaugeValue, + proxyValue, + append(hclusterLabelValues, proxyHTTP, proxyHTTPS, proxyTrustedCA)..., + ) + } + + // invalidAwsCredsMetric + { + invalidAwsCredsValue := 0.0 + if !platformaws.ValidCredentials(hcluster) { + invalidAwsCredsValue = 1.0 + } + + ch <- prometheus.MustNewConstMetric( + invalidAwsCredsMetricDesc, + prometheus.GaugeValue, + invalidAwsCredsValue, + hclusterLabelValues..., + ) + } + + if !hcluster.DeletionTimestamp.IsZero() { + // deletingDurationMetric + deletingDuration := c.clock.Since(hcluster.DeletionTimestamp.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + deletingDurationMetricDesc, + prometheus.GaugeValue, + deletingDuration, + hclusterLabelValues..., + ) + + // guestCloudResourcesDeletingDurationMetric + condition := meta.FindStatusCondition(hcluster.Status.Conditions, string(hyperv1.CloudResourcesDestroyed)) + + if condition == nil || condition.Status != metav1.ConditionTrue { + ch <- prometheus.MustNewConstMetric( + guestCloudResourcesDeletingDurationMetricDesc, + prometheus.GaugeValue, + deletingDuration, + hclusterLabelValues..., + ) + } + } + } + } + + // AGGREGATED METRICS + + // countByIdentityProviderMetric + for identityProvider, hclustersCount := range identityProviderToHClustersCount { + ch <- prometheus.MustNewConstMetric( + countByIdentityProviderMetricDesc, + prometheus.GaugeValue, + float64(hclustersCount), + string(identityProvider), + ) + } + + // countByPlatformMetric + for platform, hclustersCount := range platformToHClustersCount { + ch <- prometheus.MustNewConstMetric( + countByPlatformMetricDesc, + prometheus.GaugeValue, + float64(hclustersCount), + string(platform), + ) + } + + // countByPlatformAndFailureConditionMetric + for platform, failureConditionToHClustersCount := range platformToFailureConditionToHClustersCount { + for failureCondition, hclustersCount := range *failureConditionToHClustersCount { + ch <- prometheus.MustNewConstMetric( + countByPlatformAndFailureConditionMetricDesc, + prometheus.GaugeValue, + float64(hclustersCount), + string(platform), + failureCondition, + ) + } + } + + // transitionDurationMetric + c.transitionDurationMetric.Collect(ch) + + c.lastCollectTime = currentCollectTime } diff --git a/hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go b/hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go new file mode 100644 index 000000000000..5b2949338a6f --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/metrics/metrics_test.go @@ -0,0 +1,758 @@ +package metrics + +import ( + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/hypershift/api" + hyperv1 "github.com/openshift/hypershift/api/v1beta1" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/clock" + clocktesting "k8s.io/utils/clock/testing" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var ( + now = time.Now().Truncate(time.Second) + + ignoreUnexportedDto = cmpopts.IgnoreUnexported(dto.MetricFamily{}, dto.Metric{}, dto.LabelPair{}, dto.Gauge{}) +) + +func createMetricValue(metricName, metricDesc string, value float64) *dto.MetricFamily { + return &dto.MetricFamily{ + Name: pointer.String(metricName), + Help: pointer.String(metricDesc), + Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), + Metric: []*dto.Metric{{ + Label: []*dto.LabelPair{ + { + Name: pointer.String("_id"), Value: pointer.String("id"), + }, + { + Name: pointer.String("name"), Value: pointer.String("hc"), + }, + { + Name: pointer.String("namespace"), Value: pointer.String("any"), + }, + }, + Gauge: &dto.Gauge{Value: pointer.Float64(value)}, + }}, + } +} + +func findMetricValue(allMetricsValues *[]*dto.MetricFamily, metricName string) *dto.MetricFamily { + if allMetricsValues != nil { + for _, timeSeries := range *allMetricsValues { + if timeSeries != nil && timeSeries.Name != nil && *timeSeries.Name == metricName { + return timeSeries + } + } + } + + return nil +} + +func checkMetric(t *testing.T, client client.Client, clock clock.Clock, metricName string, expectedMetricValue *dto.MetricFamily) { + reg := prometheus.NewPedanticRegistry() + reg.MustRegister(createHostedClustersMetricsCollector(client, clock)) + + result, err := reg.Gather() + if err != nil { + t.Fatalf("gathering metrics failed: %v", err) + } + + if diff := cmp.Diff(findMetricValue(&result, metricName), expectedMetricValue, ignoreUnexportedDto); diff != "" { + t.Errorf("result differs from actual: %s", diff) + } +} + +func TestReportWaitingInitialAvailabilityDuration(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + WaitingInitialAvailabilityDurationMetricName, + waitingInitialAvailabilityDurationMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + timestamp time.Time + annotations map[string]string + expected *dto.MetricFamily + }{ + { + name: "When cluster just got created, metric is reported with a value set to 0", + timestamp: now, + expected: wrapExpectedValueAsMetric(0), + }, + { + name: "When annotation is not set, metric reports the elapsed time since the cluster has been created", + timestamp: now.Add(5 * time.Minute), + expected: wrapExpectedValueAsMetric(300), + }, + { + name: "When annotation is set, metric is not reported anymore", + timestamp: now.Add(5 * time.Minute), + annotations: map[string]string{ + HasBeenAvailableAnnotation: "true", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + annotations := map[string]string{ + "some.key": "some.value", // We have to make sure that the map is not empty... or the map unserialized by the fake client will be the nil map which cannot be modified. + } + + for key, value := range tc.annotations { + annotations[key] = value + } + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + CreationTimestamp: metav1.Time{Time: now}, + Annotations: annotations, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clocktesting.NewFakeClock(tc.timestamp), + WaitingInitialAvailabilityDurationMetricName, tc.expected) + }) + } +} + +func TestReportInitialRollingOutDuration(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + InitialRollingOutDurationMetricName, + initialRollingOutDurationMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + timestamp time.Time + updateHistory []configv1.UpdateHistory + expected *dto.MetricFamily + }{ + { + name: "When cluster just got created, metric is reported with a value set to 0", + timestamp: now, + expected: wrapExpectedValueAsMetric(0), + }, + { + name: "When cluster is not yet provisioned, metric reports the elapsed time since the cluster has been created", + timestamp: now.Add(30 * time.Minute), + updateHistory: []configv1.UpdateHistory{{ + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + Version: "1.0", + }}, + expected: wrapExpectedValueAsMetric(1800), + }, + { + name: "When cluster is provisioned, metric is not reported anymore", + timestamp: now.Add(30 * time.Minute), + updateHistory: []configv1.UpdateHistory{{ + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(30 * time.Minute)}, + Version: "1.0", + }}, + }, + { + name: "When cluster is upgrading, metric is not reported", + timestamp: now.Add(5*time.Hour + 30*time.Minute), + updateHistory: []configv1.UpdateHistory{ + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(1 * time.Hour)}, + Version: "1.0", + }, + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Hour)}, + Version: "1.1", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + CreationTimestamp: metav1.Time{Time: now}, + }, + Status: hyperv1.HostedClusterStatus{ + Version: &hyperv1.ClusterVersionStatus{ + History: tc.updateHistory, + }, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clocktesting.NewFakeClock(tc.timestamp), + InitialRollingOutDurationMetricName, + tc.expected) + }) + } +} + +func TestReportUpgradingDuration(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64, previousVersion, newVersion string) *dto.MetricFamily { + return &dto.MetricFamily{ + Name: pointer.String(UpgradingDurationMetricName), + Help: pointer.String(upgradingDurationMetricHelp), + Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), + Metric: []*dto.Metric{{ + Label: []*dto.LabelPair{ + { + Name: pointer.String("_id"), Value: pointer.String("id"), + }, + { + Name: pointer.String("name"), Value: pointer.String("hc"), + }, + { + Name: pointer.String("namespace"), Value: pointer.String("any"), + }, + { + Name: pointer.String("new_version"), Value: pointer.String(newVersion), + }, + { + Name: pointer.String("previous_version"), Value: pointer.String(previousVersion), + }, + }, + Gauge: &dto.Gauge{Value: pointer.Float64(expectedValue)}, + }}, + } + } + + testCases := []struct { + name string + timestamp time.Time + updateHistory []configv1.UpdateHistory + expected *dto.MetricFamily + }{ + { + name: "When cluster just got created, metric is not reported", + timestamp: now, + }, + { + name: "When cluster is not yet provisioned, metric is not reported", + timestamp: now.Add(30 * time.Minute), + updateHistory: []configv1.UpdateHistory{{ + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + Version: "1.0", + }}, + }, + { + name: "When cluster is provisioned, metric is not reported", + timestamp: now.Add(30 * time.Minute), + updateHistory: []configv1.UpdateHistory{{ + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(30 * time.Minute)}, + Version: "1.0", + }}, + }, + { + name: "When cluster is upgrading, metric reports the time since the beginning of the upgrade", + timestamp: now.Add(5*time.Hour + 30*time.Minute), + updateHistory: []configv1.UpdateHistory{ + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(1 * time.Hour)}, + Version: "1.0", + }, + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Hour)}, + Version: "1.1", + }, + }, + expected: wrapExpectedValueAsMetric(1800, "1.0", "1.1"), + }, + { + name: "When cluster has upgraded, metric is not reported again", + timestamp: now.Add(5*time.Hour + 30*time.Minute), + updateHistory: []configv1.UpdateHistory{ + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(1 * time.Hour)}, + Version: "1.0", + }, + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Hour)}, + CompletionTime: &metav1.Time{Time: now.Add(5*time.Hour + 30*time.Minute)}, + Version: "1.1", + }, + }, + }, + { + name: "When cluster is upgrading again, metric reports the time since the beginning of the upgrade again", + timestamp: now.Add(12*time.Hour + 20*time.Minute), + updateHistory: []configv1.UpdateHistory{ + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + CompletionTime: &metav1.Time{Time: now.Add(1 * time.Hour)}, + Version: "1.0", + }, + { + StartedTime: metav1.Time{Time: now.Add(5 * time.Hour)}, + CompletionTime: &metav1.Time{Time: now.Add(5*time.Hour + 30*time.Minute)}, + Version: "1.1", + }, + { + StartedTime: metav1.Time{Time: now.Add(12 * time.Hour)}, + Version: "1.2", + }, + }, + expected: wrapExpectedValueAsMetric(1200, "1.1", "1.2"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + CreationTimestamp: metav1.Time{Time: now}, + }, + Status: hyperv1.HostedClusterStatus{ + Version: &hyperv1.ClusterVersionStatus{ + History: tc.updateHistory, + }, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clocktesting.NewFakeClock(tc.timestamp), + UpgradingDurationMetricName, + tc.expected) + }) + } +} + +func TestReportLimitedSuportEnabled(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + LimitedSupportEnabledMetricName, + limitedSupportEnabledMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + labels map[string]string + expected *dto.MetricFamily + }{ + { + name: "When limited support label is set, metric is reported as one", + labels: map[string]string{hyperv1.LimitedSupportLabel: "true"}, + expected: wrapExpectedValueAsMetric(1), + }, + { + name: "When limited support label is not set, metric is reported as zero", + labels: map[string]string{}, + expected: wrapExpectedValueAsMetric(0), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + Labels: tc.labels, + CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clock.RealClock{}, + LimitedSupportEnabledMetricName, + tc.expected) + }) + } +} + +func TestReportSilenceAlerts(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + SilenceAlertsMetricName, + silenceAlertsMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + labels map[string]string + expected *dto.MetricFamily + }{ + { + name: "When silenced alerts label is set, metric is reported as one", + labels: map[string]string{hyperv1.SilenceClusterAlertsLabel: "true"}, + expected: wrapExpectedValueAsMetric(1), + }, + { + name: "When silenced alerts label is not set, metric is reported as zero", + labels: map[string]string{}, + expected: wrapExpectedValueAsMetric(0), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + Labels: tc.labels, + CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clock.RealClock{}, + SilenceAlertsMetricName, + tc.expected) + }) + } +} + +func TestReportProxy(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + var labelValue string + + if expectedValue != 0.0 { + labelValue = "1" + } + + return &dto.MetricFamily{ + Name: pointer.String(ProxyMetricName), + Help: pointer.String(proxyMetricHelp), + Type: func() *dto.MetricType { v := dto.MetricType(1); return &v }(), + Metric: []*dto.Metric{{ + Label: []*dto.LabelPair{ + { + Name: pointer.String("_id"), Value: pointer.String("id"), + }, + { + Name: pointer.String("name"), Value: pointer.String("hc"), + }, + { + Name: pointer.String("namespace"), Value: pointer.String("any"), + }, + { + Name: pointer.String("proxy_http"), Value: pointer.String(labelValue), + }, + { + Name: pointer.String("proxy_https"), Value: pointer.String(labelValue), + }, + { + Name: pointer.String("proxy_trusted_ca"), Value: pointer.String(labelValue), + }, + }, + Gauge: &dto.Gauge{Value: pointer.Float64(expectedValue)}, + }}, + } + } + + testCases := []struct { + name string + clusterConf hyperv1.ClusterConfiguration + expected *dto.MetricFamily + }{ + { + name: "When proxy configuration is set, metric is reported with a value set to 1, same for the metric labels", + clusterConf: hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{ + HTTPProxy: "fakeProxyServer", + HTTPSProxy: "fakeProxySecureServer", + TrustedCA: configv1.ConfigMapNameReference{ + Name: "fakeProxyTrustedCA", + }, + }, + }, + expected: wrapExpectedValueAsMetric(1), + }, + { + name: "When Proxy configuration is not set, metric is reported with a value set to 0, metric labels are empty", + expected: wrapExpectedValueAsMetric(0), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + CreationTimestamp: metav1.Time{Time: time.Time{}.Add(time.Hour)}, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + Configuration: &tc.clusterConf, + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clock.RealClock{}, + ProxyMetricName, + tc.expected) + }) + } +} + +func TestReportInvalidAwsCreds(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + InvalidAwsCredsMetricName, + invalidAwsCredsMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + ValidOIDCConfigurationConditionStatus metav1.ConditionStatus + ValidAWSIdentityProviderConditionStatus metav1.ConditionStatus + expected *dto.MetricFamily + }{ + { + name: "When ValidOIDCConfigurationCondition status is false, metric is reported with a value set to 1", + ValidOIDCConfigurationConditionStatus: metav1.ConditionFalse, + ValidAWSIdentityProviderConditionStatus: metav1.ConditionTrue, + expected: wrapExpectedValueAsMetric(1), + }, + { + name: "When ValidAWSIdentityProviderCondition status is false, metric is reported with a value set to 1", + ValidOIDCConfigurationConditionStatus: metav1.ConditionTrue, + ValidAWSIdentityProviderConditionStatus: metav1.ConditionFalse, + expected: wrapExpectedValueAsMetric(1), + }, + { + name: "When both ValidAWSIdentityProviderCondition and ValidOIDCConfigurationCondition statuses is true, metric is reported with a value set to 0", + ValidOIDCConfigurationConditionStatus: metav1.ConditionTrue, + ValidAWSIdentityProviderConditionStatus: metav1.ConditionTrue, + expected: wrapExpectedValueAsMetric(0), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + meta.SetStatusCondition(&hcluster.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.ValidOIDCConfiguration), + Status: tc.ValidOIDCConfigurationConditionStatus, + }) + meta.SetStatusCondition(&hcluster.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.ValidAWSIdentityProvider), + Status: tc.ValidAWSIdentityProviderConditionStatus, + }) + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clock.RealClock{}, + InvalidAwsCredsMetricName, + tc.expected) + }) + } +} + +func TestReportGuestCloudResourcesDeletionDuration(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + GuestCloudResourcesDeletingDurationMetricName, + guestCloudResourcesDeletingDurationMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + timestamp time.Time + isDeleting bool + conditions []metav1.Condition + expected *dto.MetricFamily + }{ + { + name: "When cluster is not yet deleting, metric is not reported", + timestamp: now, + }, + { + name: "When cluster just started to be deleted, metric is reported with a value set to 0", + timestamp: now, + isDeleting: true, + conditions: []metav1.Condition{}, + expected: wrapExpectedValueAsMetric(0), + }, + { + name: "When destroyed condition is false, metric reports the elapsed time since the beginning of the delete", + timestamp: now.Add(5 * time.Minute), + isDeleting: true, + conditions: []metav1.Condition{ + { + Type: string(hyperv1.CloudResourcesDestroyed), + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + }, + }, + expected: wrapExpectedValueAsMetric(300), + }, + { + name: "When destroyed condition is true, metric is not reported anymore", + timestamp: now.Add(5 * time.Minute), + isDeleting: true, + conditions: []metav1.Condition{ + { + Type: string(hyperv1.CloudResourcesDestroyed), + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{Time: now.Add(5 * time.Minute)}, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var deletionTimestamp *metav1.Time + + if tc.isDeleting { + deletionTimestamp = &metav1.Time{Time: now} + } + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + DeletionTimestamp: deletionTimestamp, + Finalizers: []string{"necessary"}, // fake client needs finalizers when a deletionTimestamp is set + }, + Status: hyperv1.HostedClusterStatus{ + Conditions: tc.conditions, + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + } + + checkMetric(t, + fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).Build(), + clocktesting.NewFakeClock(tc.timestamp), + GuestCloudResourcesDeletingDurationMetricName, + tc.expected) + }) + } +} + +func TestReportDeletingDuration(t *testing.T) { + wrapExpectedValueAsMetric := func(expectedValue float64) *dto.MetricFamily { + return createMetricValue( + DeletingDurationMetricName, + deletingDurationMetricHelp, + expectedValue) + } + + testCases := []struct { + name string + timestamp time.Time + isDeleting bool + isDeleted bool + expected *dto.MetricFamily + }{ + { + name: "When cluster is not yet deleting, metric is not reported", + timestamp: now, + }, + { + name: "When cluster just started to be deleted, metric is reported with a value set to 0", + timestamp: now, + isDeleting: true, + expected: wrapExpectedValueAsMetric(0), + }, + { + name: "When cluster is not yet deleted, metric reports the elapsed time since the beginning of the delete", + timestamp: now.Add(10 * time.Minute), + isDeleting: true, + expected: wrapExpectedValueAsMetric(600), + }, + { + name: "When cluster is deleted, metric is not reported anymore", + timestamp: now, + isDeleted: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + clientBuilder := fake.NewClientBuilder().WithScheme(api.Scheme) + + if !tc.isDeleted { + var deletionTimestamp *metav1.Time + + if tc.isDeleting { + deletionTimestamp = &metav1.Time{Time: now} + } + + clientBuilder = clientBuilder.WithObjects(&hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hc", + Namespace: "any", + DeletionTimestamp: deletionTimestamp, + Finalizers: []string{"necessary"}, // fake client needs finalizers when a deletionTimestamp is set + }, + Spec: hyperv1.HostedClusterSpec{ + ClusterID: "id", + }, + }) + } + + checkMetric(t, + clientBuilder.Build(), + clocktesting.NewFakeClock(tc.timestamp), + DeletingDurationMetricName, + tc.expected) + }) + } +} diff --git a/hypershift-operator/controllers/nodepool/inplace.go b/hypershift-operator/controllers/nodepool/inplace.go index ce03b16f42ed..7d67381d7b14 100644 --- a/hypershift-operator/controllers/nodepool/inplace.go +++ b/hypershift-operator/controllers/nodepool/inplace.go @@ -7,7 +7,6 @@ import ( "github.com/openshift/hypershift/api" hyperv1 "github.com/openshift/hypershift/api/v1beta1" - "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/metrics" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" k8sutilspointer "k8s.io/utils/pointer" @@ -158,10 +157,6 @@ func (r *NodePoolReconciler) reconcileMachineSet(ctx context.Context, if machineSetInPlaceRolloutIsComplete(machineSet) { if nodePool.Status.Version != targetVersion { - if nodePool.Status.Version == "" { - metrics.RecordNodePoolInitialRolloutDuration(nodePool) - } - log.Info("Version upgrade complete", "previous", nodePool.Status.Version, "new", targetVersion) nodePool.Status.Version = targetVersion diff --git a/hypershift-operator/controllers/nodepool/metrics/metrics.go b/hypershift-operator/controllers/nodepool/metrics/metrics.go index 98ac9a2f0dbd..729d9e3d7011 100644 --- a/hypershift-operator/controllers/nodepool/metrics/metrics.go +++ b/hypershift-operator/controllers/nodepool/metrics/metrics.go @@ -1,91 +1,408 @@ package metrics import ( + "context" "time" hyperv1 "github.com/openshift/hypershift/api/v1beta1" + "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" + "github.com/openshift/hypershift/support/conditions" "github.com/prometheus/client_golang/prometheus" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/clock" + capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrllog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/metrics" ) +const ( + // Aggregating metrics - name & help + + CountByPlatformMetricName = "hypershift_nodepools" // What about renaming it to hypershift_nodepools_by_platform ? + countByPlatformMetricHelp = "Number of NodePools for a given platform." + + CountByPlatformAndFailureConditionMetricName = "hypershift_nodepools_failure_conditions" // What about renaming it to hypershift_nodepools_by_platform_and_failure_condition ? + countByPlatformAndFailureConditionMetricHelp = "Number of NodePools for a given platform and failure condition." + + CountByHClusterMetricName = "hypershift_hostedcluster_nodepools" // What about renaming it to hypershift_cluster_nodepools ? + countByHClusterMetricHelp = "Number of NodePools for a given HostedCluster" + + TransitionDurationMetricName = "hypershift_nodepools_transition_seconds" // What about renaming it to hypershift_nodepools_transition_duration_seconds ? + transitionDurationMetricHelp = "Time in seconds it took for conditions to become true since the creation of the NodePool." + + // Per node pool metrics - name + + InitialRollingOutDurationMetricName = "hypershift_nodepools_initial_rolling_out_duration_seconds" // What about renaming it to hypershift_nodepool_initial_rolling_out_duration_seconds ? + initialRollingOutDurationMetricHelp = "Time in seconds it is taking to roll out the initial version since the creation of the NodePool" + + "Version is rolled out when the corresponding MachineDeployment has its number of available replicas matches the number of wished replicas. " + + "Undefined if the number of available replicas is already reached or if the node pool no longer exists." + + SizeMetricName = "hypershift_nodepools_size" // What about renaming it to hypershift_nodepool_size ? + sizeMetricHelp = "Number of desired replicas associated with a given NodePool" + + AvailableReplicasMetricName = "hypershift_nodepools_available_replicas" // What about renaming it to hypershift_nodepool_available_replicas ? + availableReplicasMetricHelp = "Number of available replicas associated with a given NodePool" + + DeletingDurationMetricName = "hypershift_nodepools_deleting_duration_seconds" // What about renaming it to hypershift_nodepool_deleting_duration_seconds ? + deletingDurationMetricHelp = "Time in seconds it is taking to delete the NodePool since the beginning of the delete. " + + "Undefined if the node pool is not deleting or no longer exists." +) + +type void struct{} + +// semantically constant - not suposed to be changed at runtime var ( - labelNames = []string{"namespace", "name", "cluster_name", "platform"} - - NodePoolSizeMetricName = "hypershift_nodepools_size" - nodePoolSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Number of desired replicas associated with a given NodePool", - Name: NodePoolSizeMetricName, - }, labelNames) - - NodePoolAvailableReplicasMetricName = "hypershift_nodepools_available_replicas" - nodePoolAvailableReplicas = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Number of available replicas associated with a given NodePool", - Name: NodePoolAvailableReplicasMetricName, - }, labelNames) - - NodePoolDeletionDurationMetricName = "hypershift_nodepools_deletion_duration_seconds" - nodePoolDeletionDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took for NodePool to be deleted", - Name: NodePoolDeletionDurationMetricName, - }, labelNames) - - NodePoolInitialRolloutDurationMetricName = "hypershift_nodepools_initial_rollout_duration_seconds" - nodePoolInitialRolloutDuration = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Time in seconds it took from initial NodePool creation and rollout of initial version", - Name: NodePoolInitialRolloutDurationMetricName, - }, labelNames) - - NodePoolTransitionSecondsMetricName = "hypershift_nodepools_transition_seconds" - nodePoolTransitionSeconds = prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Name: NodePoolTransitionSecondsMetricName, - Help: "Time in seconds it took from NodePool creation until a given condition transitions to true", - Buckets: []float64{5, 10, 20, 30, 60, 90, 120, 180, 240, 300, 360, 480, 600}, - }, []string{"condition"}) + transitionDurationMetricConditions = map[string]void{ + hyperv1.NodePoolReachedIgnitionEndpoint: void{}, + hyperv1.NodePoolAllMachinesReadyConditionType: void{}, + hyperv1.NodePoolAllNodesHealthyConditionType: void{}, + } + + knownPlatforms = hyperv1.PlatformTypes() + + knownConditionToExpectedStatus = conditions.ExpectedNodePoolConditions() + + // Metrics descriptions + countByPlatformMetricDesc = prometheus.NewDesc( + CountByPlatformMetricName, + countByPlatformMetricHelp, + []string{"platform"}, nil) + + countByPlatformAndFailureConditionMetricDesc = prometheus.NewDesc( + CountByPlatformAndFailureConditionMetricName, + countByPlatformAndFailureConditionMetricHelp, + []string{"platform", "condition"}, nil) + + countByHClusterMetricDesc = prometheus.NewDesc( + CountByHClusterMetricName, + countByHClusterMetricHelp, + []string{"namespace", "name", "_id", "platform"}, nil) + + // One time series per node pool for below metrics + nodePoolLabels = []string{"namespace", "name", "_id", "cluster_name", "platform"} + + initialRollingOutDurationMetricDesc = prometheus.NewDesc( + InitialRollingOutDurationMetricName, + initialRollingOutDurationMetricHelp, + nodePoolLabels, nil) + + sizeMetricDesc = prometheus.NewDesc( + SizeMetricName, + sizeMetricHelp, + nodePoolLabels, nil) + + availableReplicasMetricDesc = prometheus.NewDesc( + AvailableReplicasMetricName, + availableReplicasMetricHelp, + nodePoolLabels, nil) + + deletingDurationMetricDesc = prometheus.NewDesc( + DeletingDurationMetricName, + deletingDurationMetricHelp, + nodePoolLabels, nil) ) -func init() { - metrics.Registry.MustRegister( - nodePoolSize, - nodePoolAvailableReplicas, - nodePoolDeletionDuration, - nodePoolInitialRolloutDuration, - nodePoolTransitionSeconds, - ) +type nodePoolsMetricsCollector struct { + client.Client + clock clock.Clock + + transitionDurationMetric *prometheus.HistogramVec + + lastCollectTime time.Time } -func labels(nodePool *hyperv1.NodePool) prometheus.Labels { - return prometheus.Labels{ - "namespace": nodePool.Namespace, - "name": nodePool.Name, - "cluster_name": nodePool.Spec.ClusterName, - "platform": string(nodePool.Spec.Platform.Type), +func createNodePoolsMetricsCollector(client client.Client, clock clock.Clock) prometheus.Collector { + return &nodePoolsMetricsCollector{ + Client: client, + clock: clock, + transitionDurationMetric: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Name: TransitionDurationMetricName, + Help: transitionDurationMetricHelp, + Buckets: []float64{5, 10, 20, 30, 60, 90, 120, 180, 240, 300, 360, 480, 600}, + }, []string{"condition"}), + lastCollectTime: time.UnixMilli(0), } } -func RecordNodePoolSize(nodePool *hyperv1.NodePool, size float64) { - nodePoolSize.With(labels(nodePool)).Set(size) +func CreateAndRegisterNodePoolsMetricsCollector(client client.Client) prometheus.Collector { + collector := createNodePoolsMetricsCollector(client, clock.RealClock{}) + + metrics.Registry.MustRegister(collector) + + return collector } -func RecordNodePoolAvailableReplicas(nodePool *hyperv1.NodePool) { - nodePoolAvailableReplicas.With(labels(nodePool)).Set(float64(nodePool.Status.Replicas)) +func (c *nodePoolsMetricsCollector) Describe(ch chan<- *prometheus.Desc) { + prometheus.DescribeByCollect(c, ch) } -func RecordNodePoolDeletionDuration(nodePool *hyperv1.NodePool) { - duration := time.Since(nodePool.DeletionTimestamp.Time).Seconds() - nodePoolDeletionDuration.With(labels(nodePool)).Set(duration) +type hclusterData struct { + id string + name string + platform hyperv1.PlatformType + nodePoolsCount int } -func RecordNodePoolInitialRolloutDuration(nodePool *hyperv1.NodePool) { - duration := time.Since(nodePool.CreationTimestamp.Time).Seconds() - nodePoolInitialRolloutDuration.With(labels(nodePool)).Set(duration) +func createFailureConditionToNodePoolsCountMap() *map[string]int { + res := make(map[string]int) + + for conditionType, expectedStatus := range knownConditionToExpectedStatus { + failureCondPrefix := "" + + if expectedStatus == corev1.ConditionTrue { + failureCondPrefix = "not_" + } + + res[failureCondPrefix+conditionType] = 0 + } + + return &res } -func ObserveConditionTransitionDuration(nodePool *hyperv1.NodePool, newCondition, oldCondition *hyperv1.NodePoolCondition) { - if (oldCondition != nil && oldCondition.Status == newCondition.Status) || newCondition.Status != corev1.ConditionTrue { - return +func (c *nodePoolsMetricsCollector) Collect(ch chan<- prometheus.Metric) { + ctx := context.Background() + currentCollectTime := c.clock.Now() + log := ctrllog.Log + + // Data retrieved from objects other than node pools in below loops + hclusterNsToData := make(map[string]*hclusterData) + machineSetPathToReplicasCount := make(map[string]int32) + machineDeploymentPathToReplicasCount := make(map[string]int32) + + // Hosted clusters loop + { + hclusters := &hyperv1.HostedClusterList{} + + if err := c.List(ctx, hclusters); err != nil { + log.Error(err, "failed to list hosted clusters while collecting metrics") + } + + for k := range hclusters.Items { + hcluster := &hclusters.Items[k] + + hclusterNsToData[hcluster.Namespace] = &hclusterData{ + id: hcluster.Spec.ClusterID, + name: hcluster.Name, + platform: hcluster.Spec.Platform.Type, + } + } + } + + // Machine sets loop + { + machineSets := &capiv1.MachineSetList{} + + if err := c.List(ctx, machineSets); err != nil { + log.Error(err, "failed to list machine sets while collecting metrics") + } + + for k := range machineSets.Items { + machineSet := &machineSets.Items[k] + msPath := machineSet.Namespace + "/" + machineSet.Name + + machineSetPathToReplicasCount[msPath] = *machineSet.Spec.Replicas + } + } + + // Machine deployments loop + { + machineDeployments := &capiv1.MachineDeploymentList{} + + if err := c.List(ctx, machineDeployments); err != nil { + log.Error(err, "failed to list machine deployments while collecting metrics") + } + + for k := range machineDeployments.Items { + machineDeployment := &machineDeployments.Items[k] + mdPath := machineDeployment.Namespace + "/" + machineDeployment.Name + + machineDeploymentPathToReplicasCount[mdPath] = *machineDeployment.Spec.Replicas + } + } + + // countByPlatformMetric - init + platformToNodePoolsCount := make(map[hyperv1.PlatformType]int) + + for k := range knownPlatforms { + platformToNodePoolsCount[knownPlatforms[k]] = 0 + } + + // countByPlatformAndFailureConditionMetric - init + platformToFailureConditionToNodePoolsCount := make(map[hyperv1.PlatformType]*map[string]int) + + for k := range knownPlatforms { + platformToFailureConditionToNodePoolsCount[knownPlatforms[k]] = createFailureConditionToNodePoolsCountMap() + } + + // MAIN LOOP - node pools loop + { + npList := &hyperv1.NodePoolList{} + + if err := c.List(ctx, npList); err != nil { + log.Error(err, "failed to list node pools while collecting metrics") + } + + for k := range npList.Items { + nodePool := &npList.Items[k] + hclusterId := "" + + // countByPlatformMetric - aggregation + platform := nodePool.Spec.Platform.Type + platformToNodePoolsCount[platform] = platformToNodePoolsCount[platform] + 1 + + // countByPlatformAndFailureConditionMetric - aggregation + { + _, isKnownPlatform := platformToFailureConditionToNodePoolsCount[platform] + + if !isKnownPlatform { + platformToFailureConditionToNodePoolsCount[platform] = createFailureConditionToNodePoolsCountMap() + } + + failureConditionToNodePoolsCount := platformToFailureConditionToNodePoolsCount[platform] + + for _, condition := range nodePool.Status.Conditions { + expectedStatus, isKnownCondition := knownConditionToExpectedStatus[condition.Type] + + if isKnownCondition && condition.Status != expectedStatus { + failureCondPrefix := "" + + if expectedStatus == corev1.ConditionTrue { + failureCondPrefix = "not_" + } + + failureCondition := failureCondPrefix + condition.Type + + (*failureConditionToNodePoolsCount)[failureCondition] = (*failureConditionToNodePoolsCount)[failureCondition] + 1 + } + } + } + + // countByHClusterMetric - aggregation + if hclusterData := hclusterNsToData[nodePool.Namespace]; hclusterData != nil { + hclusterId = hclusterData.id + hclusterData.nodePoolsCount = hclusterData.nodePoolsCount + 1 + } + + // transitionDurationMetric - aggregation + for i := range nodePool.Status.Conditions { + condition := &nodePool.Status.Conditions[i] + if _, isOk := transitionDurationMetricConditions[condition.Type]; isOk { + if condition.Status == corev1.ConditionTrue { + t := condition.LastTransitionTime.Time + + if c.lastCollectTime.Before(t) && (t.Before(currentCollectTime) || t.Equal(currentCollectTime)) { + c.transitionDurationMetric.With(map[string]string{"condition": condition.Type}).Observe(t.Sub(nodePool.CreationTimestamp.Time).Seconds()) + } + } + } + } + + nodePoolLabelValues := []string{nodePool.Namespace, nodePool.Name, hclusterId, nodePool.Spec.ClusterName, string(nodePool.Spec.Platform.Type)} + + // initialRollingOutDurationMetric + if nodePool.Status.Version == "" { + initializingDuration := c.clock.Since(nodePool.CreationTimestamp.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + initialRollingOutDurationMetricDesc, + prometheus.GaugeValue, + initializingDuration, + nodePoolLabelValues..., + ) + } + + // sizeMetric + { + var pathToReplicasCount *map[string]int32 + + if nodePool.Spec.Management.UpgradeType == hyperv1.UpgradeTypeInPlace { + // we use machineSet.Spec.Replicas because .Spec.Replicas will not be set if autoscaling is enabled + pathToReplicasCount = &machineSetPathToReplicasCount + } else if nodePool.Spec.Management.UpgradeType == hyperv1.UpgradeTypeReplace { + // we use machineDeployment.Spec.Replicas because .Spec.Replicas will not be set if autoscaling is enabled + pathToReplicasCount = &machineDeploymentPathToReplicasCount + } + + if pathToReplicasCount != nil { + hcpNs := manifests.HostedControlPlaneNamespace(nodePool.Namespace, nodePool.Spec.ClusterName).Name + wishedReplicas := float64((*pathToReplicasCount)[hcpNs+"/"+nodePool.Name]) + + ch <- prometheus.MustNewConstMetric( + sizeMetricDesc, + prometheus.GaugeValue, + wishedReplicas, + nodePoolLabelValues..., + ) + } + } + + // availableReplicasMetric + { + availableReplicas := float64(nodePool.Status.Replicas) + + ch <- prometheus.MustNewConstMetric( + availableReplicasMetricDesc, + prometheus.GaugeValue, + availableReplicas, + nodePoolLabelValues..., + ) + } + + // deletingDurationMetric + if !nodePool.DeletionTimestamp.IsZero() { + deletingDuration := c.clock.Since(nodePool.DeletionTimestamp.Time).Seconds() + + ch <- prometheus.MustNewConstMetric( + deletingDurationMetricDesc, + prometheus.GaugeValue, + deletingDuration, + nodePoolLabelValues..., + ) + } + } + } + + // AGGREGATED METRICS + + // countByPlatformMetric + for platform, nodePoolsCount := range platformToNodePoolsCount { + ch <- prometheus.MustNewConstMetric( + countByPlatformMetricDesc, + prometheus.GaugeValue, + float64(nodePoolsCount), + string(platform), + ) } - duration := newCondition.LastTransitionTime.Sub(nodePool.CreationTimestamp.Time).Seconds() - nodePoolTransitionSeconds.With(prometheus.Labels{"condition": newCondition.Type}).Observe(duration) + // countByPlatformAndFailureConditionMetric + for platform, failureConditionToNodePoolsCount := range platformToFailureConditionToNodePoolsCount { + for failureCondition, nodePoolsCount := range *failureConditionToNodePoolsCount { + ch <- prometheus.MustNewConstMetric( + countByPlatformAndFailureConditionMetricDesc, + prometheus.GaugeValue, + float64(nodePoolsCount), + string(platform), + failureCondition, + ) + } + } + + // countByHClusterMetric + for namespace, hclusterData := range hclusterNsToData { + ch <- prometheus.MustNewConstMetric( + countByHClusterMetricDesc, + prometheus.GaugeValue, + float64(hclusterData.nodePoolsCount), + namespace, + hclusterData.name, + hclusterData.id, + string(hclusterData.platform), + ) + } + + // transitionDurationMetric + c.transitionDurationMetric.Collect(ch) + + c.lastCollectTime = currentCollectTime } diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go index 66a90d83130c..b5418ae62658 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller.go @@ -27,7 +27,6 @@ import ( "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/ignitionserver" "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/kubevirt" - "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/metrics" ignserver "github.com/openshift/hypershift/ignition-server/controllers" kvinfra "github.com/openshift/hypershift/kubevirtexternalinfra" "github.com/openshift/hypershift/support/globalconfig" @@ -185,8 +184,6 @@ func (r *NodePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } } - metrics.RecordNodePoolDeletionDuration(nodePool) - log.Info("Deleted nodepool", "name", req.NamespacedName) return ctrl.Result{}, nil } @@ -649,8 +646,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho } SetStatusCondition(&nodePool.Status.Conditions, *reachedIgnitionEndpointCondition) - - metrics.ObserveConditionTransitionDuration(nodePool, reachedIgnitionEndpointCondition, oldReachedIgnitionEndpointCondition) } // Validate tuningConfig input. @@ -766,7 +761,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho message = hyperv1.AllIsWellMessage } - oldAllMachinesReadyCondition := FindStatusCondition(nodePool.Status.Conditions, hyperv1.NodePoolAllMachinesReadyConditionType) allMachinesReadyCondition := &hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolAllMachinesReadyConditionType, Status: status, @@ -776,8 +770,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho } SetStatusCondition(&nodePool.Status.Conditions, *allMachinesReadyCondition) - metrics.ObserveConditionTransitionDuration(nodePool, allMachinesReadyCondition, oldAllMachinesReadyCondition) - // Set AllNodesHealthyCondition. status = corev1.ConditionTrue reason = hyperv1.AsExpectedReason @@ -802,7 +794,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho message = hyperv1.AllIsWellMessage } - oldAllMachinesHealthyCondition := FindStatusCondition(nodePool.Status.Conditions, hyperv1.NodePoolAllNodesHealthyConditionType) allMachinesHealthyCondition := &hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolAllNodesHealthyConditionType, Status: status, @@ -812,8 +803,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho } SetStatusCondition(&nodePool.Status.Conditions, *allMachinesHealthyCondition) - metrics.ObserveConditionTransitionDuration(nodePool, allMachinesHealthyCondition, oldAllMachinesHealthyCondition) - // 2. - Reconcile towards expected state of the world. compressedConfig, err := supportutil.CompressAndEncode([]byte(config)) if err != nil { @@ -978,8 +967,6 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho } else { log.Info("Reconciled MachineSet", "result", result) } - // we use machineSet.Spec.Replicas because .Spec.Replicas will not be set if autoscaling is enabled - metrics.RecordNodePoolSize(nodePool, float64(*ms.Spec.Replicas)) } if nodePool.Spec.Management.UpgradeType == hyperv1.UpgradeTypeReplace { @@ -998,13 +985,8 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho } else { log.Info("Reconciled MachineDeployment", "result", result) } - // we use machineDeployment.Spec.Replicas because .Spec.Replicas will not be set if autoscaling is enabled - metrics.RecordNodePoolSize(nodePool, float64(*md.Spec.Replicas)) } - // .Status.Replicas is updated at this point, record metric - metrics.RecordNodePoolAvailableReplicas(nodePool) - mhc := machineHealthCheck(nodePool, controlPlaneNamespace) if nodePool.Spec.Management.AutoRepair { if c := FindStatusCondition(nodePool.Status.Conditions, hyperv1.NodePoolReachedIgnitionEndpoint); c == nil || c.Status != corev1.ConditionTrue { @@ -1663,10 +1645,6 @@ func (r *NodePoolReconciler) reconcileMachineDeployment(log logr.Logger, // is at the expected version (spec.version) and config (userData Secret) so we reconcile status and annotation. if MachineDeploymentComplete(machineDeployment) { if nodePool.Status.Version != targetVersion { - if nodePool.Status.Version == "" { - metrics.RecordNodePoolInitialRolloutDuration(nodePool) - } - log.Info("Version update complete", "previous", nodePool.Status.Version, "new", targetVersion) nodePool.Status.Version = targetVersion diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index 4b0fc5155c15..dedf3bb5721f 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -34,7 +34,9 @@ import ( hyperv1 "github.com/openshift/hypershift/api/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster" + hcmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/metrics" "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool" + npmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/metrics" "github.com/openshift/hypershift/hypershift-operator/controllers/platform/aws" "github.com/openshift/hypershift/hypershift-operator/controllers/proxy" "github.com/openshift/hypershift/hypershift-operator/controllers/scheduler" @@ -332,6 +334,7 @@ func run(ctx context.Context, opts *StartOptions, log logr.Logger) error { return fmt.Errorf("unable to create webhook: %w", err) } } + hcmetrics.CreateAndRegisterHostedClustersMetricsCollector(mgr.GetClient()) // Since we dropped the validation webhook server we need to ensure this resource doesn't exist // otherwise it will intercept kas requests and fail. @@ -364,6 +367,7 @@ func run(ctx context.Context, opts *StartOptions, log logr.Logger) error { }).SetupWithManager(mgr); err != nil { return fmt.Errorf("unable to create controller: %w", err) } + npmetrics.CreateAndRegisterNodePoolsMetricsCollector(mgr.GetClient()) if mgmtClusterCaps.Has(capabilities.CapabilityProxy) { if err := proxy.Setup(mgr, opts.Namespace, opts.DeploymentName); err != nil { @@ -450,7 +454,7 @@ func run(ctx context.Context, opts *StartOptions, log logr.Logger) error { return fmt.Errorf("failed to get ingress controller: %w", err) } - if err := setupMetrics(mgr); err != nil { + if err := setupOperatorInfoMetric(mgr); err != nil { return fmt.Errorf("failed to setup metrics: %w", err) } diff --git a/hypershift-operator/metrics.go b/hypershift-operator/metrics.go index 0c1e5d90898d..3c21c6f33f9a 100644 --- a/hypershift-operator/metrics.go +++ b/hypershift-operator/metrics.go @@ -5,157 +5,64 @@ import ( "fmt" "os" "runtime" - "strings" - "time" - "github.com/go-logr/logr" - configv1 "github.com/openshift/api/config/v1" hyperapi "github.com/openshift/hypershift/api" - hyperv1 "github.com/openshift/hypershift/api/v1beta1" "github.com/openshift/hypershift/cmd/install/assets" "github.com/openshift/hypershift/pkg/version" - "github.com/openshift/hypershift/support/conditions" "github.com/openshift/hypershift/support/supportedversion" "github.com/prometheus/client_golang/prometheus" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" crclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" crmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" ) const ( - tickerTime = 30 + OperatorInfoMetricName = "hypershift_operator_info" ) +// semantically constant - not suposed to be changed at runtime var ( latestSupportedVersion = supportedversion.LatestSupportedVersion.String() hypershiftVersion = version.GetRevision() goVersion = runtime.Version() goArch = runtime.GOARCH - - // Metrics - HypershiftOperatorInfoName = "hypershift_operator_info" ) -type hypershiftMetrics struct { - clusterIdentityProviders *prometheus.GaugeVec - hostedClusters *prometheus.GaugeVec - hostedClustersWithFailureCondition *prometheus.GaugeVec - hostedClustersNodePools *prometheus.GaugeVec - nodePools *prometheus.GaugeVec - nodePoolsWithFailureCondition *prometheus.GaugeVec - hypershiftOperatorInfo prometheus.GaugeFunc - hostedClusterTransitionSeconds *prometheus.HistogramVec - - client crclient.Client - - log logr.Logger -} - -type ImageInfo struct { - image string - imageId string -} - -func newMetrics(client crclient.Client, log logr.Logger, hypershiftImage ImageInfo) *hypershiftMetrics { - image := hypershiftImage.image - imageId := hypershiftImage.imageId - - return &hypershiftMetrics{ - clusterIdentityProviders: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Help: "Indicates the number any identity provider in the fleet", - Name: "hypershift_cluster_identity_providers", - }, []string{"identity_provider"}), - hostedClusters: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "hypershift_hostedclusters", - Help: "Number of HostedClusters by platform", - }, []string{"platform"}), - hostedClustersWithFailureCondition: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "hypershift_hostedclusters_failure_conditions", - Help: "Total number of HostedClusters by platform with conditions in undesired state", - }, []string{"condition"}), - hostedClustersNodePools: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "hypershift_hostedcluster_nodepools", - Help: "Number of NodePools associated with a given HostedCluster", - }, []string{"cluster_name", "platform"}), - nodePools: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "hypershift_nodepools", - Help: "Number of NodePools by platform", - }, []string{"platform"}), - nodePoolsWithFailureCondition: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "hypershift_nodepools_failure_conditions", - Help: "Total number of NodePools by platform with conditions in undesired state", - }, []string{"platform", "condition"}), - // hypershiftOperatorInfo is a metric to capture the current operator details of the management cluster - hypershiftOperatorInfo: prometheus.NewGaugeFunc(prometheus.GaugeOpts{ - Name: HypershiftOperatorInfoName, - Help: "Metric to capture the current operator details of the management cluster", - ConstLabels: prometheus.Labels{ - "version": hypershiftVersion, - "image": image, - "imageId": imageId, - "latestSupportedVersion": latestSupportedVersion, - "goVersion": goVersion, - "goArch": goArch, - }, - }, func() float64 { return float64(1) }), - // hostedClusterTransitionSeconds is a metric to capture the time between a HostedCluster being created and entering a particular condition. - hostedClusterTransitionSeconds: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Name: "hypershift_hosted_cluster_transition_seconds", - Help: "Number of seconds between HostedCluster creation and HostedCluster transition to a condition.", - Buckets: []float64{5, 10, 20, 30, 60, 90, 120, 180, 240, 300, 360, 480, 600}, - }, []string{"condition"}), - client: client, - log: log, - } -} - -func (m *hypershiftMetrics) Start(ctx context.Context) error { - ticker := time.NewTicker(tickerTime * time.Second) +func getOperatorImage(client crclient.Client) (string, string, error) { + ctx := context.TODO() + hypershiftNamespace := os.Getenv("MY_NAMESPACE") + hypershiftPodName := os.Getenv("MY_NAME") + hypershiftPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: hypershiftPodName, Namespace: hypershiftNamespace}} + var err error + image := "not found" + imageId := "not found" - for { - select { - case <-ctx.Done(): - return nil - case <-ticker.C: - if err := m.collect(ctx); err != nil { - m.log.Error(err, "failed to collect metrics") + if err = client.Get(ctx, crclient.ObjectKeyFromObject(hypershiftPod), hypershiftPod); err == nil { + for _, c := range hypershiftPod.Status.ContainerStatuses { + if c.Name == assets.HypershiftOperatorName { + image = c.Image + imageId = c.ImageID } } } -} - -func (m *hypershiftMetrics) collect(ctx context.Context) error { - var clusters hyperv1.HostedClusterList - if err := m.client.List(ctx, &clusters); err != nil { - return fmt.Errorf("failed to list hostedclusters: %w", err) - } - m.observeHostedClusters(&clusters) - var nodePools hyperv1.NodePoolList - if err := m.client.List(ctx, &nodePools); err != nil { - return fmt.Errorf("failed to list nodepools: %w", err) - } - if err := m.observeNodePools(ctx, &nodePools); err != nil { - return err - } - return nil + return image, imageId, err } -func setupMetrics(mgr manager.Manager) error { - var hypershiftImage ImageInfo +func setupOperatorInfoMetric(mgr manager.Manager) error { + var image, imageId string // We need to create a new client because the manager one still does not have the cache started tmpClient, err := crclient.New(mgr.GetConfig(), crclient.Options{Scheme: hyperapi.Scheme}) if err != nil { return fmt.Errorf("error creating a temporary client: %w", err) } + // Grabbing the Image and ImageID from Operator - if hypershiftImage.image, hypershiftImage.imageId, err = getOperatorImage(tmpClient); err != nil { + if image, imageId, err = getOperatorImage(tmpClient); err != nil { if apierrors.IsNotFound(err) { log := mgr.GetLogger() log.Error(err, "pod not found, reporting empty image") @@ -164,247 +71,19 @@ func setupMetrics(mgr manager.Manager) error { } } - metrics := newMetrics(mgr.GetClient(), mgr.GetLogger().WithName("metrics"), hypershiftImage) - if err := crmetrics.Registry.Register(metrics.clusterIdentityProviders); err != nil { - return fmt.Errorf("failed to register clusterIdentityProviders metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.hostedClusters); err != nil { - return fmt.Errorf("failed to register hostedClusters metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.hostedClustersWithFailureCondition); err != nil { - return fmt.Errorf("failed to register hostedClustersWithCondition metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.hostedClustersNodePools); err != nil { - return fmt.Errorf("failed to register hostedClustersNodePools metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.nodePools); err != nil { - return fmt.Errorf("failed to register nodePools metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.hostedClusterTransitionSeconds); err != nil { - return fmt.Errorf("failed to register hostedClusterTransitionSeconds metric: %w", err) - } - if err := crmetrics.Registry.Register(metrics.hypershiftOperatorInfo); err != nil { - return fmt.Errorf("failed to register hypershiftOperatorInfo metric: %w", err) - } - if err := mgr.Add(metrics); err != nil { - return fmt.Errorf("failed to add metrics runnable to manager: %w", err) - } - - return nil -} - -var expectedHCConditionStates = conditions.ExpectedHCConditions() - -func (m *hypershiftMetrics) observeHostedClusters(hostedClusters *hyperv1.HostedClusterList) { - hcCount := newLabelCounter() - // We init to 0, this is needed so the metrics reports 0 in cases there's no HostedClusters. - // Otherwise the time series would show the last reported value by the counter. - hcCount.Init(string(hyperv1.AWSPlatform)) - hcCount.Init(string(hyperv1.NonePlatform)) - hcCount.Init(string(hyperv1.IBMCloudPlatform)) - hcCount.Init(string(hyperv1.AgentPlatform)) - hcCount.Init(string(hyperv1.KubevirtPlatform)) - hcCount.Init(string(hyperv1.AzurePlatform)) - hcCount.Init(string(hyperv1.PowerVSPlatform)) - - // Init hcByConditions counter. - hcByConditions := make(map[string]float64) - for condition, expectedState := range expectedHCConditionStates { - if expectedState == metav1.ConditionTrue { - hcByConditions[string("not_"+condition)] = 0 - } else { - hcByConditions[string(condition)] = 0 - } - } - - // Init identityProvidersCounter counter. - identityProvidersCounter := map[configv1.IdentityProviderType]float64{ - configv1.IdentityProviderTypeBasicAuth: 0, - configv1.IdentityProviderTypeGitHub: 0, - configv1.IdentityProviderTypeGitLab: 0, - configv1.IdentityProviderTypeGoogle: 0, - configv1.IdentityProviderTypeHTPasswd: 0, - configv1.IdentityProviderTypeKeystone: 0, - configv1.IdentityProviderTypeLDAP: 0, - configv1.IdentityProviderTypeOpenID: 0, - configv1.IdentityProviderTypeRequestHeader: 0, - } - - now := time.Now() - for _, hc := range hostedClusters.Items { - // Collect transition metrics. - // Every time a condition has a transition from false -> true, that would increase observation in the "time from creation to last transition" bucket. - if transitionTime := transitionTime(&hc, hyperv1.EtcdAvailable); transitionTime != nil { - if now.Sub(transitionTime.Time).Seconds() <= tickerTime { - conditionTimeToTrueSinceCreation := pointer.Float64(transitionTime.Sub(hc.CreationTimestamp.Time).Seconds()) - m.hostedClusterTransitionSeconds.With(map[string]string{"condition": string(hyperv1.EtcdAvailable)}).Observe(*conditionTimeToTrueSinceCreation) - } - } - if transitionTime := transitionTime(&hc, hyperv1.InfrastructureReady); transitionTime != nil { - if now.Sub(transitionTime.Time).Seconds() <= tickerTime { - conditionTimeToTrueSinceCreation := pointer.Float64(transitionTime.Sub(hc.CreationTimestamp.Time).Seconds()) - m.hostedClusterTransitionSeconds.With(map[string]string{"condition": string(hyperv1.InfrastructureReady)}).Observe(*conditionTimeToTrueSinceCreation) - } - } - if transitionTime := transitionTime(&hc, hyperv1.ExternalDNSReachable); transitionTime != nil { - if now.Sub(transitionTime.Time).Seconds() <= tickerTime { - conditionTimeToTrueSinceCreation := pointer.Float64(transitionTime.Sub(hc.CreationTimestamp.Time).Seconds()) - m.hostedClusterTransitionSeconds.With(map[string]string{"condition": string(hyperv1.ExternalDNSReachable)}).Observe(*conditionTimeToTrueSinceCreation) - } - } - - // Group identityProviders by type. - if hc.Spec.Configuration != nil && hc.Spec.Configuration.OAuth != nil { - for _, identityProvider := range hc.Spec.Configuration.OAuth.IdentityProviders { - identityProvidersCounter[identityProvider.Type] = identityProvidersCounter[identityProvider.Type] + 1 - } - } - - platform := string(hc.Spec.Platform.Type) - hcCount.Add(platform) - - for _, cond := range hc.Status.Conditions { - expectedState, known := expectedHCConditionStates[hyperv1.ConditionType(cond.Type)] - if !known { - continue - } - if expectedState == metav1.ConditionTrue { - if cond.Status == metav1.ConditionFalse { - hcByConditions["not_"+cond.Type] = hcByConditions["not_"+cond.Type] + 1 - } - } - if expectedState == metav1.ConditionFalse { - if cond.Status == metav1.ConditionTrue { - hcByConditions[cond.Type] = hcByConditions[cond.Type] + 1 - } - } - } - } - - // Collect identityProvider metric. - for identityProvider, count := range identityProvidersCounter { - m.clusterIdentityProviders.WithLabelValues(string(identityProvider)).Set(count) - } - - for key, count := range hcCount.Counts() { - labels := counterKeyToLabels(key) - m.hostedClusters.WithLabelValues(labels...).Set(float64(count)) - } - - // Collect hcByConditions metric. - for condition, count := range hcByConditions { - m.hostedClustersWithFailureCondition.WithLabelValues(condition).Set(count) - } -} - -func transitionTime(hc *hyperv1.HostedCluster, conditionType hyperv1.ConditionType) *metav1.Time { - condition := meta.FindStatusCondition(hc.Status.Conditions, string(conditionType)) - if condition == nil { - return nil - } - if condition.Status == metav1.ConditionFalse { - return nil - } - return &condition.LastTransitionTime -} - -var expectedNPConditionStates = conditions.ExpectedNodePoolConditions() - -func (m *hypershiftMetrics) observeNodePools(ctx context.Context, nodePools *hyperv1.NodePoolList) error { - npByCluster := newLabelCounter() - npCount := newLabelCounter() - npByCondition := newLabelCounter() - for _, np := range nodePools.Items { - hc := &hyperv1.HostedCluster{} - hc.Namespace = np.Namespace - hc.Name = np.Spec.ClusterName - hcPlatform := "" - if err := m.client.Get(ctx, crclient.ObjectKeyFromObject(hc), hc); err == nil { - hcPlatform = string(hc.Spec.Platform.Type) - npByCluster.Add(crclient.ObjectKeyFromObject(hc).String(), hcPlatform) - } else { - m.log.Error(err, "cannot get hosted cluster for nodepool", "nodepool", crclient.ObjectKeyFromObject(&np).String()) - } - platform := string(np.Spec.Platform.Type) - npCount.Add(platform) + crmetrics.Registry.MustRegister( + prometheus.NewGaugeFunc(prometheus.GaugeOpts{ + Name: OperatorInfoMetricName, + Help: "Metric to capture the current operator details of the management cluster", + ConstLabels: prometheus.Labels{ + "version": hypershiftVersion, + "image": image, + "imageId": imageId, + "latestSupportedVersion": latestSupportedVersion, + "goVersion": goVersion, + "goArch": goArch, + }, + }, func() float64 { return float64(1) })) - for _, cond := range np.Status.Conditions { - expectedState, known := expectedNPConditionStates[cond.Type] - if !known { - continue - } - if expectedState == corev1.ConditionTrue { - if cond.Status == corev1.ConditionFalse { - npByCondition.Add(platform, "not_"+cond.Type) - } - } else { - if cond.Status == corev1.ConditionTrue { - npByCondition.Add(platform, cond.Type) - } - } - } - } - for key, count := range npByCluster.Counts() { - labels := counterKeyToLabels(key) - m.hostedClustersNodePools.WithLabelValues(labels...).Set(float64(count)) - } - for key, count := range npCount.Counts() { - labels := counterKeyToLabels(key) - m.nodePools.WithLabelValues(labels...).Set(float64(count)) - } - for key, count := range npByCondition.Counts() { - labels := counterKeyToLabels(key) - m.nodePoolsWithFailureCondition.WithLabelValues(labels...).Set(float64(count)) - } return nil } - -func getOperatorImage(client crclient.Client) (string, string, error) { - ctx := context.TODO() - var image, imageId string - hypershiftNamespace := os.Getenv("MY_NAMESPACE") - hypershiftPodName := os.Getenv("MY_NAME") - - hypershiftPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: hypershiftPodName, Namespace: hypershiftNamespace}} - if err := client.Get(ctx, crclient.ObjectKeyFromObject(hypershiftPod), hypershiftPod); err != nil { - image = "not found" - imageId = "not found" - return image, imageId, err - } else { - for _, c := range hypershiftPod.Status.ContainerStatuses { - if c.Name == assets.HypershiftOperatorName { - image = c.Image - imageId = c.ImageID - } - } - } - return image, imageId, nil -} - -type labelCounter struct { - counts map[string]int -} - -func newLabelCounter() *labelCounter { - return &labelCounter{ - counts: map[string]int{}, - } -} - -func (c *labelCounter) Init(values ...string) { - key := strings.Join(values, "|") - c.counts[key] = 0 -} - -func (c *labelCounter) Add(values ...string) { - key := strings.Join(values, "|") - c.counts[key]++ -} - -func (c *labelCounter) Counts() map[string]int { - return c.counts -} - -func counterKeyToLabels(key string) []string { - return strings.Split(key, "|") -} diff --git a/test/e2e/util/hypershift_framework.go b/test/e2e/util/hypershift_framework.go index dc127418f24a..da3a80752686 100644 --- a/test/e2e/util/hypershift_framework.go +++ b/test/e2e/util/hypershift_framework.go @@ -9,7 +9,9 @@ import ( hyperv1 "github.com/openshift/hypershift/api/v1beta1" "github.com/openshift/hypershift/cmd/cluster/core" + hcmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/metrics" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" + npmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/metrics" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -148,7 +150,21 @@ func (h *hypershiftTest) postTeardown(hostedCluster *hyperv1.HostedCluster, opts EnsureAPIBudget(t, h.ctx, h.client, hostedCluster) } - ValidateMetrics(t, h.ctx, hostedCluster) + ValidateMetrics(t, h.ctx, hostedCluster, []string{ + hcmetrics.WaitingInitialAvailabilityDurationMetricName, + hcmetrics.InitialRollingOutDurationMetricName, + hcmetrics.UpgradingDurationMetricName, + hcmetrics.SilenceAlertsMetricName, + hcmetrics.LimitedSupportEnabledMetricName, + hcmetrics.ProxyMetricName, + hcmetrics.InvalidAwsCredsMetricName, + hcmetrics.DeletingDurationMetricName, + hcmetrics.GuestCloudResourcesDeletingDurationMetricName, + npmetrics.InitialRollingOutDurationMetricName, + npmetrics.SizeMetricName, + npmetrics.AvailableReplicasMetricName, + npmetrics.DeletingDurationMetricName, + }, false) }) } @@ -249,6 +265,16 @@ func (h *hypershiftTest) teardownHostedCluster(ctx context.Context, hc *hyperv1. h.Errorf("Failed to dump cluster: %v", err) } + ValidateMetrics(h.T, ctx, hc, []string{ + hcmetrics.SilenceAlertsMetricName, + hcmetrics.LimitedSupportEnabledMetricName, + hcmetrics.ProxyMetricName, + hcmetrics.InvalidAwsCredsMetricName, + HypershiftOperatorInfoName, + npmetrics.SizeMetricName, + npmetrics.AvailableReplicasMetricName, + }, true) + // Try repeatedly to destroy the cluster gracefully. For each failure, dump // the current cluster to help debug teardown lifecycle issues. destroyAttempt := 1 diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index 2db9a90d5ad3..e73ea15f109b 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -20,7 +20,6 @@ import ( "github.com/openshift/hypershift/cmd/cluster/core" hcmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/metrics" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" - nodepoolmetrics "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/metrics" "github.com/openshift/hypershift/support/conditions" suppconfig "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/util" @@ -1310,7 +1309,9 @@ const ( HypershiftOperatorInfoName = "hypershift_operator_info" ) -func ValidateMetrics(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluster) { +// Verifies that the given metrics are defined for the given hosted cluster if areMetricsExpectedToBePresent is set to true. +// Verifies that the given metrics are not defined otherwise. +func ValidateMetrics(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluster, metricsNames []string, areMetricsExpectedToBePresent bool) { t.Run("ValidateMetricsAreExposed", func(t *testing.T) { // TODO (alberto) this test should pass in None. // https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_hypershift/2459/pull-ci-openshift-hypershift-main-e2e-aws/1650438383060652032/artifacts/e2e-aws/run-e2e/artifacts/TestNoneCreateCluster_PreTeardownClusterDump/ @@ -1327,21 +1328,7 @@ func ValidateMetrics(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluste // Polling to prevent races with prometheus scrape interval. err = wait.PollImmediate(10*time.Second, 5*time.Minute, func() (bool, error) { - for _, metricName := range []string{ - hcmetrics.DeletionDurationMetricName, - hcmetrics.GuestCloudResourcesDeletionDurationMetricName, - hcmetrics.AvailableDurationName, - hcmetrics.InitialRolloutDurationName, - hcmetrics.ClusterUpgradeDurationMetricName, - hcmetrics.ProxyName, - hcmetrics.SilenceAlertsName, - hcmetrics.LimitedSupportEnabledName, - HypershiftOperatorInfoName, - nodepoolmetrics.NodePoolSizeMetricName, - nodepoolmetrics.NodePoolAvailableReplicasMetricName, - nodepoolmetrics.NodePoolDeletionDurationMetricName, - nodepoolmetrics.NodePoolInitialRolloutDurationMetricName, - } { + for _, metricName := range metricsNames { // Query fo HC specific metrics by hc.name. query := fmt.Sprintf("%v{name=\"%s\"}", metricName, hc.Name) if metricName == HypershiftOperatorInfoName { @@ -1352,7 +1339,7 @@ func ValidateMetrics(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluste query = fmt.Sprintf("%v{cluster_name=\"%s\"}", metricName, hc.Name) } // upgrade metric is only available for TestUpgradeControlPlane - if metricName == hcmetrics.ClusterUpgradeDurationMetricName && !strings.HasPrefix("TestUpgradeControlPlane", t.Name()) { + if metricName == hcmetrics.UpgradingDurationMetricName && !strings.HasPrefix("TestUpgradeControlPlane", t.Name()) { continue } @@ -1361,20 +1348,25 @@ func ValidateMetrics(t *testing.T, ctx context.Context, hc *hyperv1.HostedCluste return false, err } - if len(result.Data.Result) < 1 { - t.Logf("Metric not found: %q", metricName) - return false, nil - } - for _, series := range result.Data.Result { - t.Logf("Found metric: %v", series.String()) + if areMetricsExpectedToBePresent { + if len(result.Data.Result) < 1 { + t.Logf("Metric not found: %q", metricName) + return false, nil + } + for _, series := range result.Data.Result { + t.Logf("Time series found: %v", series.String()) + } + } else { + if len(result.Data.Result) > 0 { + t.Logf("Metric found: %q", metricName) + return false, nil + } } } return true, nil }) if err != nil { - t.Errorf("Failed to validate that all metrics are exposed") - } else { - t.Logf("Destroyed cluster. Namespace: %s, name: %s", hc.Namespace, hc.Name) + t.Errorf("Failed to validate all metrics") } }) }