chore: reject node-selector-once on instance api (#10144)

(cherry picked from commit cf10be1)

Author: leon-ape

pkg/controller/instance/reconciler_status.go

@@ -65,11 +65,6 @@ func (r *statusReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 	ready, available, updated := false, false, false
 	notReadyName, notAvailableName := "", ""
 
-	// podToNodeMapping, err := ParseNodeSelectorOnceAnnotation(inst)
-	// if err != nil {
-	//	return kubebuilderx.Continue, err
-	// }
-
 	if isCreated(pod) {
 		notReadyName = pod.Name
 	}
@@ -88,17 +83,6 @@ func (r *statusReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 			return kubebuilderx.Continue, err
 		}
 	}
-
-	// TODO: ???
-	// if nodeName, ok := podToNodeMapping[pod.Name]; ok {
-	//	// there's chance that a pod is currently running and wait to be deleted so that it can be rescheduled
-	//	if pod.Spec.NodeName == nodeName {
-	//		if err := deleteNodeSelectorOnceAnnotation(its, pod.Name); err != nil {
-	//			return kubebuilderx.Continue, err
-	//		}
-	//	}
-	// }
-
 	inst.Status.CurrentRevision = getPodRevision(pod)
 	if updated {
 		inst.Status.CurrentRevision = inst.Status.UpdateRevision

pkg/controller/instanceset2/instance_util.go

@@ -21,7 +21,6 @@ package instanceset2
 
 import (
 	"encoding/json"
-	"fmt"
 	"reflect"
 	"sort"
 	"strconv"
@@ -108,18 +107,6 @@ func baseSort(x any, getNameNOrdinalFunc func(i int) (string, int), getRolePrior
 	})
 }
 
-func parseNodeSelectorOnceAnnotation(its *workloads.InstanceSet) (map[string]string, error) {
-	podToNodeMapping := make(map[string]string)
-	data, ok := its.Annotations[constant.NodeSelectorOnceAnnotationKey]
-	if !ok {
-		return podToNodeMapping, nil
-	}
-	if err := json.Unmarshal([]byte(data), &podToNodeMapping); err != nil {
-		return nil, fmt.Errorf("can't unmarshal scheduling information: %w", err)
-	}
-	return podToNodeMapping, nil
-}
-
 func buildInstanceByTemplate(tree *kubebuilderx.ObjectTree,
 	instName string, template *instancetemplate.InstanceTemplateExt, its *workloads.InstanceSet) (*workloads.Instance, error) {
 	labels := getMatchLabels(its.Name)
@@ -143,14 +130,6 @@ func buildInstanceByTemplate(tree *kubebuilderx.ObjectTree,
 	// set these immutable fields only on initial Pod creation, not updates.
 	b.SetHostname(instName).
 		SetSubdomain(getHeadlessSvcName(its.Name))
-	podToNodeMapping, err := parseNodeSelectorOnceAnnotation(its)
-	if err != nil {
-		return nil, err
-	}
-	if nodeName, ok := podToNodeMapping[instName]; ok {
-		// don't specify nodeName directly here, because it may affect WaitForFirstConsumer StorageClass
-		b.SetNodeSelector(map[string]string{corev1.LabelHostname: nodeName})
-	}
 
 	for i := range template.VolumeClaimTemplates {
 		b.AddVolumeClaimTemplate(template.VolumeClaimTemplates[i])

pkg/controller/instanceset2/reconciler_status.go

@@ -77,11 +77,6 @@ func (r *statusReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 		template2TotalReplicas[template.Name] = templateReplicas
 	}
 
-	// podToNodeMapping, err := ParseNodeSelectorOnceAnnotation(its)
-	// if err != nil {
-	//	return kubebuilderx.Continue, err
-	// }
-
 	for _, inst := range instanceList {
 		templateName := inst.Labels[instancetemplate.TemplateNameLabelKey]
 		if template2TemplatesStatus[templateName] == nil {
@@ -114,16 +109,6 @@ func (r *statusReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 				template2TemplatesStatus[templateName].CurrentReplicas++
 			}
 		}
-
-		// TODO: ???
-		// if nodeName, ok := podToNodeMapping[inst.Name]; ok {
-		//	// there's chance that a pod is currently running and wait to be deleted so that it can be rescheduled
-		//	if inst.Spec.NodeName == nodeName {
-		//		if err := deleteNodeSelectorOnceAnnotation(its, inst.Name); err != nil {
-		//			return kubebuilderx.Continue, err
-		//		}
-		//	}
-		// }
 	}
 	its.Status.Replicas = replicas
 	its.Status.ReadyReplicas = readyReplicas

pkg/controller/instanceset2/reconciler_validation.go

@@ -20,7 +20,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package instanceset2
 
 import (
+	"fmt"
+
 	workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
+	"github.com/apecloud/kubeblocks/pkg/constant"
 	"github.com/apecloud/kubeblocks/pkg/controller/instancetemplate"
 	"github.com/apecloud/kubeblocks/pkg/controller/kubebuilderx"
 )
@@ -41,8 +44,22 @@ func (r *validationReconciler) PreCondition(tree *kubebuilderx.ObjectTree) *kube
 }
 
 func (r *validationReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilderx.Result, error) {
-	if err := instancetemplate.ValidateInstanceTemplates(tree.GetRoot().(*workloads.InstanceSet), tree); err != nil {
+	its := tree.GetRoot().(*workloads.InstanceSet)
+	if err := validateUnsupportedFeatures(its); err != nil {
+		return kubebuilderx.Commit, err
+	}
+	if err := instancetemplate.ValidateInstanceTemplates(its, tree); err != nil {
 		return kubebuilderx.Commit, err
 	}
 	return kubebuilderx.Continue, nil
 }
+
+func validateUnsupportedFeatures(its *workloads.InstanceSet) error {
+	if its == nil || its.Annotations == nil {
+		return nil
+	}
+	if _, ok := its.Annotations[constant.NodeSelectorOnceAnnotationKey]; ok {
+		return fmt.Errorf("annotation %q is not supported when enableInstanceAPI=true", constant.NodeSelectorOnceAnnotationKey)
+	}
+	return nil
+}

pkg/controller/instanceset2/reconciler_validation_test.go

@@ -0,0 +1,57 @@
+/*
+Copyright (C) 2022-2025 ApeCloud Co., Ltd
+
+This file is part of KubeBlocks project
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package instanceset2
+
+import (
+	"strings"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+
+	workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
+	"github.com/apecloud/kubeblocks/pkg/constant"
+	"github.com/apecloud/kubeblocks/pkg/controller/kubebuilderx"
+)
+
+func TestValidationRejectsNodeSelectorOnce(t *testing.T) {
+	tree := kubebuilderx.NewObjectTree()
+	tree.SetRoot(&workloads.InstanceSet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-its",
+			Namespace: "default",
+			Annotations: map[string]string{
+				constant.NodeSelectorOnceAnnotationKey: `{"test-its-0":"node-a"}`,
+			},
+		},
+		Spec: workloads.InstanceSetSpec{
+			Replicas:          ptr.To[int32](1),
+			EnableInstanceAPI: ptr.To(true),
+		},
+	})
+
+	_, err := NewValidationReconciler().Reconcile(tree)
+	if err == nil {
+		t.Fatalf("expected validation error")
+	}
+	if !strings.Contains(err.Error(), constant.NodeSelectorOnceAnnotationKey) {
+		t.Fatalf("expected error to mention %s, got: %v", constant.NodeSelectorOnceAnnotationKey, err)
+	}
+}