Merge pull request #499 from rolandmkunkel/SREP-215-finetune-the-hibernation-notification-in-CAD

SREP-215: always add comment to CHGM comment if the cluster was resum…

Author: openshift-merge-bot[bot]

pkg/investigations/chgm/chgm.go

@@ -61,15 +61,15 @@ func (c *Investiation) Run(r *investigation.Resources) (investigation.Investigat
 	notes.AppendSuccess("Customer did not stop nodes.")
 	logging.Info("The customer has not stopped/terminated any nodes.")
 
-	// 2. Check if the cluster is fresh out of a long hibernation
-	// TODO(Claudio): OSD-18775 - add the note regardless of how long the cluster was hibernated, as long as it came just out of hibernation.
-	longHibernation, err := investigateHibernation(r.Cluster, r.OcmClient)
+	// 2. Check if the cluster was hibernated and has recently resumed.
+	hibernationPeriods, err := getHibernationStatusForCluster(r.OcmClient, r.Cluster)
 	if err != nil {
 		logging.Warnf("could not check hibernation status of cluster: %w", err)
 	}
-	if longHibernation {
-		logging.Info("The cluster was hibernated for too long.")
-		notes.AppendWarning("Cluster was hibernated more than %.0f days - investigate CSRs and kubelet certificates: see https://github.com/openshift/ops-sop/blob/master/v4/alerts/cluster_has_gone_missing.md#24-hibernation", hibernationTooLong.Hours()/24)
+
+	if hasRecentlyResumed(hibernationPeriods, time.Now()) {
+		logging.Info("The cluster has recently resumed from hibernation.")
+		notes.AppendWarning("Cluster has resumed from hibernation within the last %.0f hours - investigate CSRs and kubelet certificates: see https://github.com/openshift/ops-sop/blob/master/v4/alerts/cluster_has_gone_missing.md#24-hibernation", recentWakeupTime.Hours())
 	} else {
 		logging.Info("The cluster was not hibernated for too long.")
 	}
@@ -132,20 +132,17 @@ func (c *Investiation) IsExperimental() bool {
 	return false
 }
 
-// investigateHibernation checks if the cluster was recently woken up from
-// hibernation. If clusters are hibernated for more than 30 days, the internal
-// certificates of the kubelets can expire and CSRs need to be approved
+// hasRecentlyResumed checks if the cluster was woken up from
+// hibernation within the last 2h. In that case, the internal
+// certificates of the kubelets could have expired and CSRs need to be approved
 // manually:
 // - https://github.com/openshift/hive/blob/master/docs/hibernating-clusters.md
-func investigateHibernation(cluster *cmv1.Cluster, client ocm.Client) (bool, error) {
-	hibernations, err := getHibernationStatusForCluster(client, cluster)
-	if err != nil {
-		return false, err
-	}
-	if len(hibernations) == 0 {
-		return false, nil
+func hasRecentlyResumed(hibernationPeriods []*hibernationPeriod, now time.Time) bool {
+	if len(hibernationPeriods) == 0 {
+		return false
 	}
-	return hibernatedTooLong(hibernations, time.Now()), nil
+	latestHibernation := hibernationPeriods[len(hibernationPeriods)-1]
+	return now.Sub(latestHibernation.DehibernationTime) <= recentWakeupTime
 }
 
 // isUserAllowedToStop verifies if a user is allowed to stop/terminate instances

pkg/investigations/chgm/chgm_hibernation_check.go

@@ -11,9 +11,6 @@ import (
 
 const recentWakeupTime = 2 * time.Hour
 
-// 30 Days is always a problem as kubelet certificates will be expired
-const hibernationTooLong = 30 * 24 * time.Hour
-
 const (
 	hibernationStartEvent = "cluster_state_hibernating"
 	hibernationEndEvent   = "cluster_state_ready"
@@ -27,24 +24,6 @@ type hibernationPeriod struct {
 	DehibernationTime   time.Time
 }
 
-func hibernatedTooLong(hibernations []*hibernationPeriod, now time.Time) bool {
-	if len(hibernations) == 0 {
-		return false
-	}
-	latestHibernation := hibernations[len(hibernations)-1]
-	// The cluster was woken up within the RECENT_WAKEUP_TIME which might
-	// indicate a CSR problem.
-	if now.Sub(latestHibernation.DehibernationTime) >= recentWakeupTime {
-		return false
-	}
-	// Only clusters that have hibernated for a long time are susceptible to
-	// have cert issues.
-	if latestHibernation.HibernationDuration >= hibernationTooLong {
-		return true
-	}
-	return false
-}
-
 func getHibernationStatusForCluster(ocmClient ocm.Client, cluster *cmv1.Cluster) ([]*hibernationPeriod, error) {
 	filter := "log_type='cluster-state-updates'"
 	clusterStateUpdates, err := ocmClient.GetServiceLog(cluster, filter)

pkg/investigations/chgm/chgm_hibernation_check_test.go

@@ -71,15 +71,10 @@ func TestHibernatedTooLong(t *testing.T) {
 		now          time.Time
 	}
 	hibernationStartTime := time.Date(2023, 0o1, 0o1, 0o0, 0o0, 0o0, 0o0, time.Local)
-	hibernationShortStopTime := time.Date(2023, 0o1, 11, 0o0, 0o0, 0o0, 0o0, time.Local)
-	hibernationLongStopTime := time.Date(2023, 0o2, 11, 0o0, 0o0, 0o0, 0o0, time.Local)
-	shortHibernation := &hibernationPeriod{
-		HibernationDuration: hibernationShortStopTime.Sub(hibernationStartTime),
-		DehibernationTime:   hibernationShortStopTime,
-	}
-	longHibernation := &hibernationPeriod{
-		HibernationDuration: hibernationLongStopTime.Sub(hibernationStartTime),
-		DehibernationTime:   hibernationLongStopTime,
+	hibernationStopTime := time.Date(2023, 0o2, 11, 0o0, 0o0, 0o0, 0o0, time.Local)
+	hibernation := &hibernationPeriod{
+		HibernationDuration: hibernationStopTime.Sub(hibernationStartTime),
+		DehibernationTime:   hibernationStopTime,
 	}
 	tests := []struct {
 		name string
@@ -88,40 +83,32 @@ func TestHibernatedTooLong(t *testing.T) {
 	}{
 		// TODO: Add test cases.
 		{
-			name: "Cluster that hibernated for 10 days is ok",
+			name: "Cluster with a hibernation that was longer ago does not count as recently resumed",
 			args: args{
-				hibernations: []*hibernationPeriod{shortHibernation},
-				now:          hibernationShortStopTime.Add(1 * time.Hour),
+				hibernations: []*hibernationPeriod{hibernation},
+				now:          hibernationStopTime.Add(24 * time.Hour),
 			},
 			want: false,
 		},
 		{
-			name: "Cluster that hibernated for 30+ days is too long",
+			name: "Cluster that woke up 30 min ago counts as recently resumed",
 			args: args{
-				hibernations: []*hibernationPeriod{longHibernation},
-				now:          hibernationLongStopTime.Add(1 * time.Hour),
+				hibernations: []*hibernationPeriod{hibernation},
+				now:          hibernationStopTime.Add(30 * time.Minute),
 			},
 			want: true,
 		},
 		{
-			name: "Cluster that never hibernated is ok",
+			name: "Cluster that never hibernated does not count as recently resumed",
 			args: args{},
 			want: false,
 		},
-		{
-			name: "Cluster that woke up for 2+ hours ago ok",
-			args: args{
-				hibernations: []*hibernationPeriod{longHibernation},
-				now:          hibernationLongStopTime.Add(3 * time.Hour),
-			},
-			want: false,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := hibernatedTooLong(tt.args.hibernations, tt.args.now)
+			got := hasRecentlyResumed(tt.args.hibernations, tt.args.now)
 			if got != tt.want {
-				t.Errorf("HibernatedTooLong() = %v, want %v", got, tt.want)
+				t.Errorf("hasRecentlyResumed() = %v, want %v", got, tt.want)
 			}
 		})
 	}