Merge pull request #843 from reedcort/SREP-2822-remove-override

[SREP-2822] Add remove override flag to request-serving-nodes

Author: openshift-merge-bot[bot]

cmd/cluster/resize/requestserving_node.go

@@ -24,10 +24,11 @@ const (
 )
 
 type requestServingNodesOpts struct {
-	clusterID string
-	cluster   *cmv1.Cluster
-	size      string
-	reason    string
+	clusterID      string
+	cluster        *cmv1.Cluster
+	size           string
+	reason         string
+	removeOverride bool
 
 	// mgmtClient is a K8s client to management cluster
 	mgmtClient client.Client
@@ -58,6 +59,9 @@ func newCmdResizeRequestServingNodes() *cobra.Command {
 
   # Resize a ROSA HCP cluster's request-serving nodes to a specific size
   osdctl cluster resize request-serving-nodes --cluster-id "${CLUSTER_ID}" --size m54xl --reason "${OHSS}"
+
+  # Remove the cluster-size-override annotation to revert to default sizing behavior
+  osdctl cluster resize request-serving-nodes --cluster-id "${CLUSTER_ID}" --remove-override --reason "${OHSS}"
 `,
 		Args:              cobra.NoArgs,
 		DisableAutoGenTag: true,
@@ -69,8 +73,10 @@ func newCmdResizeRequestServingNodes() *cobra.Command {
 	cmd.Flags().StringVarP(&opts.clusterID, "cluster-id", "C", "", "The internal ID of the cluster to perform actions on")
 	cmd.Flags().StringVar(&opts.size, "size", "", "The target request-serving node size (e.g. m54xl). If not specified, will auto-select the next size up")
 	cmd.Flags().StringVar(&opts.reason, "reason", "", "The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket)")
+	cmd.Flags().BoolVar(&opts.removeOverride, "remove-override", false, "Remove the cluster-size-override annotation to revert to default sizing behavior")
 	_ = cmd.MarkFlagRequired("cluster-id")
 	_ = cmd.MarkFlagRequired("reason")
+	cmd.MarkFlagsMutuallyExclusive("size", "remove-override")
 
 	return cmd
 }
@@ -154,6 +160,11 @@ func (r *requestServingNodesOpts) run(ctx context.Context) error {
 	printer.PrintlnGreen(fmt.Sprintf("HostedCluster namespace: %s", hcNamespace))
 	printer.PrintlnGreen(fmt.Sprintf("HostedCluster name: %s", hcName))
 
+	// If remove-override flag is set, remove the annotation and exit
+	if r.removeOverride {
+		return r.handleRemoveOverride(ctx, hostedCluster, cluster.Name(), hcNamespace)
+	}
+
 	// Get current hosted-cluster-size from label
 	currentSize := hostedCluster.Labels["hypershift.openshift.io/hosted-cluster-size"]
 	if currentSize == "" {
@@ -402,3 +413,73 @@ func (r *requestServingNodesOpts) sendCustomerServiceLog() error {
 
 	return postCmd.Run()
 }
+
+func (r *requestServingNodesOpts) handleRemoveOverride(ctx context.Context, hostedCluster *hypershiftv1beta1.HostedCluster, clusterName, hcNamespace string) error {
+	overrideAnnotation := "hypershift.openshift.io/cluster-size-override"
+	currentOverride, hasOverride := hostedCluster.Annotations[overrideAnnotation]
+
+	if !hasOverride || currentOverride == "" {
+		printer.PrintlnGreen("\nNo cluster-size-override annotation found. Cluster is already using default sizing behavior.")
+		return nil
+	}
+
+	printer.PrintlnGreen(fmt.Sprintf("Current cluster-size-override: %s", currentOverride))
+
+	currentSize := hostedCluster.Labels["hypershift.openshift.io/hosted-cluster-size"]
+	if currentSize != "" {
+		printer.PrintlnGreen(fmt.Sprintf("Current hosted-cluster-size: %s", currentSize))
+	}
+
+	fmt.Printf("\nThis will remove the cluster-size-override annotation from cluster %s\n", clusterName)
+	if !utils.ConfirmPrompt() {
+		return errors.New("operation cancelled by user")
+	}
+
+	printer.PrintlnGreen("\nRemoving cluster-size-override annotation...")
+	if err := r.removeClusterSizeOverride(ctx, hostedCluster); err != nil {
+		return fmt.Errorf("failed to remove cluster-size-override annotation: %v", err)
+	}
+
+	printer.PrintlnGreen("Annotation removed successfully!")
+
+	time.Sleep(10 * time.Second)
+
+	updatedHC := &hypershiftv1beta1.HostedCluster{}
+	if err := r.mgmtClient.Get(ctx, client.ObjectKey{Namespace: hcNamespace, Name: hostedCluster.Name}, updatedHC); err != nil {
+		fmt.Printf("Warning: failed to verify annotation removal: %v\n", err)
+	} else {
+		if _, stillHasOverride := updatedHC.Annotations[overrideAnnotation]; stillHasOverride {
+			fmt.Printf("Warning: cluster-size-override annotation still present\n")
+		} else {
+			printer.PrintlnGreen("Verified cluster-size-override annotation has been removed")
+		}
+
+		newSize := updatedHC.Labels["hypershift.openshift.io/hosted-cluster-size"]
+		if newSize != "" {
+			printer.PrintlnGreen(fmt.Sprintf("Current hosted-cluster-size: %s", newSize))
+		}
+	}
+
+	fmt.Println("\nOverride removed successfully!")
+	fmt.Println("\nUse the following commands to monitor the cluster:")
+	fmt.Printf("\nVerify cluster size (annotation and label):\n")
+	fmt.Printf("  oc get hostedcluster -n %s -oyaml | grep -E '(cluster-size-override|hosted-cluster-size)'\n", hcNamespace)
+	fmt.Printf("\nMonitor nodes:\n")
+	fmt.Printf("  oc get nodes -l hypershift.openshift.io/cluster-namespace=%s\n", hcNamespace)
+
+	return nil
+}
+
+func (r *requestServingNodesOpts) removeClusterSizeOverride(ctx context.Context, hostedCluster *hypershiftv1beta1.HostedCluster) error {
+	patch := client.MergeFrom(hostedCluster.DeepCopy())
+
+	if hostedCluster.Annotations != nil {
+		delete(hostedCluster.Annotations, "hypershift.openshift.io/cluster-size-override")
+	}
+
+	if err := r.mgmtClientAdmin.Patch(ctx, hostedCluster, patch); err != nil {
+		return fmt.Errorf("failed to patch hostedcluster annotation: %v", err)
+	}
+
+	return nil
+}

cmd/cluster/resize/requestserving_node_test.go

@@ -443,3 +443,81 @@ func TestApplyClusterSizeOverride(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoveClusterSizeOverride(t *testing.T) {
+	tests := []struct {
+		name          string
+		hostedCluster *hypershiftv1beta1.HostedCluster
+		patchError    error
+		expectErr     bool
+		errorContains string
+	}{
+		{
+			name: "successful removal - existing override annotation",
+			hostedCluster: &hypershiftv1beta1.HostedCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-hc",
+					Namespace: "test-namespace",
+					Annotations: map[string]string{
+						"hypershift.openshift.io/cluster-size-override": "m54xl",
+						"other": "annotation",
+					},
+				},
+			},
+			expectErr: false,
+		},
+		{
+			name: "successful removal - no annotations",
+			hostedCluster: &hypershiftv1beta1.HostedCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-hc",
+					Namespace: "test-namespace",
+				},
+			},
+			expectErr: false,
+		},
+		{
+			name: "patch error",
+			hostedCluster: &hypershiftv1beta1.HostedCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-hc",
+					Namespace: "test-namespace",
+					Annotations: map[string]string{
+						"hypershift.openshift.io/cluster-size-override": "m54xl",
+					},
+				},
+			},
+			patchError:    fmt.Errorf("api server error"),
+			expectErr:     true,
+			errorContains: "failed to patch hostedcluster annotation",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockClient := &MockClient{}
+			mockClient.On("Patch", mock.Anything, mock.AnythingOfType("*v1beta1.HostedCluster"), mock.Anything, mock.Anything).
+				Return(tt.patchError)
+
+			r := &requestServingNodesOpts{
+				mgmtClientAdmin: mockClient,
+			}
+
+			err := r.removeClusterSizeOverride(context.Background(), tt.hostedCluster)
+
+			if tt.expectErr {
+				assert.Error(t, err)
+				if tt.errorContains != "" {
+					assert.Contains(t, err.Error(), tt.errorContains)
+				}
+			} else {
+				assert.NoError(t, err)
+				// Verify the annotation was removed
+				_, exists := tt.hostedCluster.Annotations["hypershift.openshift.io/cluster-size-override"]
+				assert.False(t, exists, "cluster-size-override annotation should be removed")
+			}
+
+			mockClient.AssertExpectations(t)
+		})
+	}
+}

docs/README.md

@@ -1887,6 +1887,7 @@ osdctl cluster resize request-serving-nodes [flags]
       --kubeconfig string                Path to the kubeconfig file to use for CLI requests.
   -o, --output string                    Valid formats are ['', 'json', 'yaml', 'env']
       --reason string                    The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket)
+      --remove-override                  Remove the cluster-size-override annotation to revert to default sizing behavior
       --request-timeout string           The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests. (default "0")
   -s, --server string                    The address and port of the Kubernetes API server
       --size string                      The target request-serving node size (e.g. m54xl). If not specified, will auto-select the next size up

docs/osdctl_cluster_resize_request-serving-nodes.md

@@ -20,6 +20,9 @@ osdctl cluster resize request-serving-nodes [flags]
   # Resize a ROSA HCP cluster's request-serving nodes to a specific size
   osdctl cluster resize request-serving-nodes --cluster-id "${CLUSTER_ID}" --size m54xl --reason "${OHSS}"
 
+  # Remove the cluster-size-override annotation to revert to default sizing behavior
+  osdctl cluster resize request-serving-nodes --cluster-id "${CLUSTER_ID}" --remove-override --reason "${OHSS}"
+
 ```
 
 ### Options
@@ -28,6 +31,7 @@ osdctl cluster resize request-serving-nodes [flags]
   -C, --cluster-id string   The internal ID of the cluster to perform actions on
   -h, --help                help for request-serving-nodes
       --reason string       The reason for this command, which requires elevation, to be run (usually an OHSS or PD ticket)
+      --remove-override     Remove the cluster-size-override annotation to revert to default sizing behavior
       --size string         The target request-serving node size (e.g. m54xl). If not specified, will auto-select the next size up
 ```