You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uniffle.apache.org by ro...@apache.org on 2022/10/08 06:10:48 UTC

[incubator-uniffle] branch master updated: [ISSUE-48][FEATURE][FOLLOW UP] add unit test for validating rss objects (#248)

This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 4408359f [ISSUE-48][FEATURE][FOLLOW UP] add unit test for validating rss objects (#248)
4408359f is described below

commit 4408359fcc9420571d1c8a1fb15c6b551c47f7ca
Author: jasonawang <ja...@tencent.com>
AuthorDate: Sat Oct 8 14:10:43 2022 +0800

    [ISSUE-48][FEATURE][FOLLOW UP] add unit test for validating rss objects (#248)
    
    ### What changes were proposed in this pull request?
    For issue #48
    I add some unit test about validating rss objects in webhook components.
    
    ### Why are the changes needed?
    Support K8S
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    Just test
---
 deploy/kubernetes/docker/Dockerfile                |   3 +
 deploy/kubernetes/docker/build.sh                  |   8 +-
 deploy/kubernetes/docker/rss-env.sh                |  33 ++++
 deploy/kubernetes/operator/Makefile                |  22 ++-
 .../pkg/controller/controller/process_rss_test.go  |   2 +
 .../controller/controller/shuffle_server_test.go   |   2 +-
 .../controller/sync/shuffleserver/shuffleserver.go |   3 +-
 .../operator/pkg/webhook/inspector/rss.go          |  38 ++--
 .../operator/pkg/webhook/inspector/rss_test.go     | 218 +++++++++++++++++++++
 9 files changed, 299 insertions(+), 30 deletions(-)

diff --git a/deploy/kubernetes/docker/Dockerfile b/deploy/kubernetes/docker/Dockerfile
index d1ed46bd..d2045cd2 100644
--- a/deploy/kubernetes/docker/Dockerfile
+++ b/deploy/kubernetes/docker/Dockerfile
@@ -38,8 +38,11 @@ USER rssadmin
 COPY rss-${RSS_VERSION}.tgz /data/rssadmin
 RUN tar -xvf /data/rssadmin/rss-${RSS_VERSION}.tgz -C /data/rssadmin
 RUN mv /data/rssadmin/rss-${RSS_VERSION} /data/rssadmin/rss
+RUN rm /data/rssadmin/rss/bin/rss-env.sh
 RUN rm -rf /data/rssadmin/rss-${RSS_VERSION}.tgz
 
+COPY rss-env.sh /data/rssadmin/rss/bin
+
 COPY start.sh /data/rssadmin/rss/bin
 
 COPY hadoop-${HADOOP_VERSION}.tar.gz /data/rssadmin
diff --git a/deploy/kubernetes/docker/build.sh b/deploy/kubernetes/docker/build.sh
index cdebba0d..86972934 100644
--- a/deploy/kubernetes/docker/build.sh
+++ b/deploy/kubernetes/docker/build.sh
@@ -83,17 +83,17 @@ cd $RSS_DIR || exit
 RSS_VERSION=$(mvn help:evaluate -Dexpression=project.version 2>/dev/null | grep -v "INFO" | grep -v "WARNING" | tail -n 1)
 RSS_FILE=rss-${RSS_VERSION}.tgz
 if [ ! -e "$RSS_FILE" ]; \
-  then sh build_distribution.sh \
+  then sh ./build_distribution.sh; \
   else echo "$RSS_FILE has been built"; \
 fi
 cd "$OLDPWD" || exit
 cp "$RSS_DIR/$RSS_FILE" .
 
-IMAGE=$REGISTRY/rss-server:$RSS_VERSION
-
-echo "building image: $IMAGE"
 GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
 GIT_COMMIT=$(git describe --dirty --always --tags | sed 's/-/./g')
+echo "image version: ${IMAGE_VERSION:=$RSS_VERSION-$GIT_COMMIT}"
+IMAGE=$REGISTRY/rss-server:$IMAGE_VERSION
+echo "building image: $IMAGE"
 docker build -t "$IMAGE" \
              --build-arg RSS_VERSION="$RSS_VERSION" \
              --build-arg HADOOP_VERSION="$HADOOP_VERSION" \
diff --git a/deploy/kubernetes/docker/rss-env.sh b/deploy/kubernetes/docker/rss-env.sh
new file mode 100644
index 00000000..26fd6a73
--- /dev/null
+++ b/deploy/kubernetes/docker/rss-env.sh
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -o pipefail
+set -o nounset   # exit the script if you try to use an uninitialised variable
+set -o errexit   # exit the script if any statement returns a non-true return value
+
+HADOOP_HOME="/data/rssadmin/hadoop"
+RUNNER="${JAVA_HOME}/bin/java"
+JPS="${JAVA_HOME}/bin/jps"
+
+# RSS_HOME, RSS home directory (Default: parent directory of the script)
+# RSS_CONF_DIR, RSS configuration directory (Default: ${RSS_HOME}/conf)
+# HADOOP_CONF_DIR, Hadoop configuration directory (Default: ${HADOOP_HOME}/etc/hadoop)
+# RSS_PID_DIR, Where the pid file is stored (Default: ${RSS_HOME})
+# RSS_LOG_DIR, Where log files are stored (Default: ${RSS_HOME}/logs)
+# RSS_IP, IP address Shuffle Server binds to on this node (Default: first non-loopback ipv4)
diff --git a/deploy/kubernetes/operator/Makefile b/deploy/kubernetes/operator/Makefile
index 4e40d0dd..f02969f0 100644
--- a/deploy/kubernetes/operator/Makefile
+++ b/deploy/kubernetes/operator/Makefile
@@ -20,6 +20,11 @@ REGISTRY ?= UNKNOWN_REGISTRY
 
 MODULES ?= webhook controller
 
+# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
+ENVTEST_K8S_VERSION = 1.22.1
+
+LOCAL_DIR=$(shell pwd)/local
+
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
 GOBIN=$(shell go env GOPATH)/bin
@@ -59,7 +64,8 @@ vet: ## Run go vet against code.
 	go vet ./...
 
 .PHONY: test
-test: manifests generate fmt vet ## Run tests.
+test: manifests generate fmt vet envtest ## Run tests.
+	export KUBEBUILDER_ASSETS=$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCAL_DIR) -p path); \
 	go test ./... -coverprofile cover.out
 
 .PHONY: build
@@ -69,7 +75,7 @@ build: test ## Build manager binary.
     done
 
 # Build the docker image
-docker-build: test version
+docker-build: build version
 	@for module in ${MODULES}; do \
 		image=${REGISTRY}/rss-$$module:$$(cat VERSION); \
 		echo building $$image;\
@@ -77,36 +83,36 @@ docker-build: test version
 	done
 
 # Push the docker image
-docker-push: version docker-build
+docker-push: docker-build
 	@for module in ${MODULES}; do \
 		image=${REGISTRY}/rss-$$module:$$(cat VERSION); \
 		echo pushing $$image;\
 		docker push $$image; \
 	done
 
-CONTROLLER_GEN = $(shell pwd)/local/controller-gen
+CONTROLLER_GEN = $(LOCAL_DIR)/controller-gen
 .PHONY: controller-gen
 controller-gen: ## Download controller-gen locally if necessary.
 	$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0)
 	./hack/update-codegen.sh
 
-KUSTOMIZE = $(shell pwd)/local/kustomize
+KUSTOMIZE = $(LOCAL_DIR)/kustomize
 .PHONY: kustomize
 kustomize: ## Download kustomize locally if necessary.
 	$(call go-get-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v4@v4.5.2)
 
-ENVTEST = $(shell pwd)/local/setup-envtest
+ENVTEST = $(LOCAL_DIR)/setup-envtest
 .PHONY: envtest
 envtest: ## Download envtest-setup locally if necessary.
 	$(call go-get-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@latest)
 
-GOIMPORTS = $(shell pwd)/local/goimports
+GOIMPORTS = $(LOCAL_DIR)/goimports
 .PHONY: goimports
 goimports: ## Download goimports locally if necessary.
 	$(call go-get-tool,$(GOIMPORTS),golang.org/x/tools/cmd/goimports@latest)
 	$(GOIMPORTS) -local github.com/apache/incubator-uniffle/deploy/kubernetes/operator -w .
 
-REVIVE = $(shell pwd)/local/revive
+REVIVE = $(LOCAL_DIR)/revive
 .PHONY: revive
 revive: ## Download revive locally if necessary.
 	$(call go-get-tool,$(REVIVE),github.com/mgechev/revive@latest)
diff --git a/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go b/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
index 8eba2abe..a867bb62 100644
--- a/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
+++ b/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
@@ -31,6 +31,7 @@ import (
 	"github.com/apache/incubator-uniffle/deploy/kubernetes/operator/pkg/utils"
 )
 
+// buildEmptyPhaseRssObj builds a rss object with empty phase for testing.
 func buildEmptyPhaseRssObj() *unifflev1alpha1.RemoteShuffleService {
 	return &unifflev1alpha1.RemoteShuffleService{
 		ObjectMeta: metav1.ObjectMeta{
@@ -47,6 +48,7 @@ func buildEmptyPhaseRssObj() *unifflev1alpha1.RemoteShuffleService {
 	}
 }
 
+// TestProcessEmptyPhaseRss tests rss objects' process of rss-controller
 func TestProcessEmptyPhaseRss(t *testing.T) {
 	rss := buildEmptyPhaseRssObj()
 
diff --git a/deploy/kubernetes/operator/pkg/controller/controller/shuffle_server_test.go b/deploy/kubernetes/operator/pkg/controller/controller/shuffle_server_test.go
index 85cb5c98..669bf318 100644
--- a/deploy/kubernetes/operator/pkg/controller/controller/shuffle_server_test.go
+++ b/deploy/kubernetes/operator/pkg/controller/controller/shuffle_server_test.go
@@ -66,7 +66,7 @@ func buildTestShuffleServerPod() *corev1.Pod {
 			},
 		},
 		Status: corev1.PodStatus{
-			PodIP: "10.0.0.1",
+			PodIP: "xxx.xxx.xxx.xxx",
 		},
 	}
 }
diff --git a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
index 9a65842d..cfab93d9 100644
--- a/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
+++ b/deploy/kubernetes/operator/pkg/controller/sync/shuffleserver/shuffleserver.go
@@ -136,7 +136,8 @@ func GenerateNodePortSVC(rss *unifflev1alpha1.RemoteShuffleService) *corev1.Serv
 
 // getReplicas returns replicas of shuffle servers.
 func getReplicas(rss *unifflev1alpha1.RemoteShuffleService) *int32 {
-	// TODO: we will support hpa for rss object, and when we enable hpa, wo should not return replicas in .spec.shuffleServer field.
+	// TODO: we will support hpa for rss object,
+	// and when we enable hpa, we should not return replicas in .spec.shuffleServer field.
 	return rss.Spec.ShuffleServer.Replicas
 }
 
diff --git a/deploy/kubernetes/operator/pkg/webhook/inspector/rss.go b/deploy/kubernetes/operator/pkg/webhook/inspector/rss.go
index 31bdff1d..e10890d4 100644
--- a/deploy/kubernetes/operator/pkg/webhook/inspector/rss.go
+++ b/deploy/kubernetes/operator/pkg/webhook/inspector/rss.go
@@ -53,22 +53,10 @@ func (i *inspector) validateRSS(ar *admissionv1.AdmissionReview) *admissionv1.Ad
 			string(ar.Request.Object.Raw), err)
 		return util.AdmissionReviewFailed(ar, err)
 	}
-	// validate configurations for coordinators.
-	coordinator := newRSS.Spec.Coordinator
-	if len(coordinator.RPCNodePort) != int(*coordinator.Count) ||
-		len(coordinator.HTTPNodePort) != int(*coordinator.Count) {
-		return util.AdmissionReviewFailed(ar,
-			fmt.Errorf("invalid number of http or rpc node ports (%v/%v) <> (%v)",
-				len(coordinator.RPCNodePort), len(coordinator.HTTPNodePort), coordinator.Count))
-	}
-	if len(coordinator.ExcludeNodesFilePath) == 0 {
-		return util.AdmissionReviewFailed(ar,
-			fmt.Errorf("empty exclude nodes file path for coordinators"))
-	}
-	// validate configurations of logHostPath for coordinators.
-	coordinatorLogPath := newRSS.Spec.Coordinator.LogHostPath
-	if len(coordinatorLogPath) > 0 && len(newRSS.Spec.Coordinator.HostPathMounts[coordinatorLogPath]) == 0 {
-		return util.AdmissionReviewFailed(ar, fmt.Errorf("empty log volume mount path for coordinators"))
+	if err := validateCoordinator(newRSS.Spec.Coordinator); err != nil {
+		klog.Errorf("validate coordinator config of rss (%v) failed: %v",
+			utils.UniqueName(newRSS), err)
+		return util.AdmissionReviewFailed(ar, err)
 	}
 	// validate configurations of logHostPath for shuffle servers.
 	shuffleServerLogPath := newRSS.Spec.ShuffleServer.LogHostPath
@@ -155,3 +143,21 @@ func generateRSSPatches(ar *admissionv1.AdmissionReview,
 	klog.V(4).Infof("patch body (%v) for rss (%v)", string(patchBody), utils.UniqueName(rss))
 	return patchBody, nil
 }
+
+// validateCoordinator validates configurations for coordinators.
+func validateCoordinator(coordinator *unifflev1alpha1.CoordinatorConfig) error {
+	if len(coordinator.RPCNodePort) != int(*coordinator.Count) ||
+		len(coordinator.HTTPNodePort) != int(*coordinator.Count) {
+		return fmt.Errorf("invalid number of http or rpc node ports (%v/%v) <> (%v)",
+			len(coordinator.RPCNodePort), len(coordinator.HTTPNodePort), coordinator.Count)
+	}
+	if len(coordinator.ExcludeNodesFilePath) == 0 {
+		return fmt.Errorf("empty exclude nodes file path for coordinators")
+	}
+	// validate configurations of logHostPath for coordinators.
+	coordinatorLogPath := coordinator.LogHostPath
+	if len(coordinatorLogPath) > 0 && len(coordinator.HostPathMounts[coordinatorLogPath]) == 0 {
+		return fmt.Errorf("empty log volume mount path for coordinators")
+	}
+	return nil
+}
diff --git a/deploy/kubernetes/operator/pkg/webhook/inspector/rss_test.go b/deploy/kubernetes/operator/pkg/webhook/inspector/rss_test.go
new file mode 100644
index 00000000..8c7b08bb
--- /dev/null
+++ b/deploy/kubernetes/operator/pkg/webhook/inspector/rss_test.go
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package inspector
+
+import (
+	"encoding/json"
+	"testing"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/utils/pointer"
+
+	unifflev1alpha1 "github.com/apache/incubator-uniffle/deploy/kubernetes/operator/api/uniffle/v1alpha1"
+	"github.com/apache/incubator-uniffle/deploy/kubernetes/operator/pkg/webhook/config"
+)
+
+func wrapTestRssObj(rss *unifflev1alpha1.RemoteShuffleService) *unifflev1alpha1.RemoteShuffleService {
+	rss.Name = "test"
+	rss.Namespace = corev1.NamespaceDefault
+	rss.UID = "uid-test"
+	return rss
+}
+
+// convertRssToRawExtension converts a rss object to runtime.RawExtension for testing.
+func convertRssToRawExtension(rss *unifflev1alpha1.RemoteShuffleService) (runtime.RawExtension, error) {
+	if rss == nil {
+		return convertRssToRawExtension(&unifflev1alpha1.RemoteShuffleService{})
+	}
+	body, err := json.Marshal(rss)
+	if err != nil {
+		return runtime.RawExtension{}, err
+	}
+	return runtime.RawExtension{
+		Raw:    body,
+		Object: rss,
+	}, nil
+}
+
+// buildTestAdmissionReview builds an AdmissionReview object for testing.
+func buildTestAdmissionReview(op admissionv1.Operation,
+	oldRss, newRss *unifflev1alpha1.RemoteShuffleService) *admissionv1.AdmissionReview {
+	oldObject, err := convertRssToRawExtension(oldRss)
+	if err != nil {
+		panic(err)
+	}
+	var object runtime.RawExtension
+	object, err = convertRssToRawExtension(newRss)
+	if err != nil {
+		panic(err)
+	}
+	return &admissionv1.AdmissionReview{
+		Request: &admissionv1.AdmissionRequest{
+			Operation: op,
+			Object:    object,
+			OldObject: oldObject,
+		},
+	}
+}
+
+// TestValidateRSS tests rss objects' validation in rss-webhook.
+func TestValidateRSS(t *testing.T) {
+	testInspector := newInspector(&config.Config{}, nil)
+
+	rssWithCooNodePort := &unifflev1alpha1.RemoteShuffleService{
+		Spec: unifflev1alpha1.RemoteShuffleServiceSpec{
+			Coordinator: &unifflev1alpha1.CoordinatorConfig{
+				Count:        pointer.Int32(2),
+				RPCNodePort:  []int32{30001, 30002},
+				HTTPNodePort: []int32{30011, 30012},
+			},
+		},
+	}
+
+	rssWithoutLogInCooMounts := rssWithCooNodePort.DeepCopy()
+	rssWithoutLogInCooMounts.Spec.Coordinator.ExcludeNodesFilePath = "/exclude_nodes"
+	rssWithoutLogInCooMounts.Spec.Coordinator.CommonConfig = &unifflev1alpha1.CommonConfig{
+		RSSPodSpec: &unifflev1alpha1.RSSPodSpec{
+			LogHostPath:    "/data/logs",
+			HostPathMounts: map[string]string{},
+		},
+	}
+
+	rssWithoutLogInServerMounts := rssWithoutLogInCooMounts.DeepCopy()
+	rssWithoutLogInServerMounts.Spec.Coordinator.CommonConfig.RSSPodSpec.HostPathMounts["/data/logs"] = "/data/logs"
+	rssWithoutLogInServerMounts.Spec.ShuffleServer = &unifflev1alpha1.ShuffleServerConfig{
+		CommonConfig: &unifflev1alpha1.CommonConfig{
+			RSSPodSpec: &unifflev1alpha1.RSSPodSpec{
+				LogHostPath:    "/data/logs",
+				HostPathMounts: map[string]string{},
+			},
+		},
+	}
+
+	rssWithoutPartition := rssWithoutLogInServerMounts.DeepCopy()
+	rssWithoutPartition.Spec.ShuffleServer.CommonConfig.RSSPodSpec.HostPathMounts["/data/logs"] = "/data/logs"
+	rssWithoutPartition.Spec.ShuffleServer.UpgradeStrategy = &unifflev1alpha1.ShuffleServerUpgradeStrategy{
+		Type: unifflev1alpha1.PartitionUpgrade,
+	}
+
+	rssWithInvalidPartition := rssWithoutLogInServerMounts.DeepCopy()
+	rssWithInvalidPartition.Spec.ShuffleServer.CommonConfig.RSSPodSpec.HostPathMounts["/data/logs"] = "/data/logs"
+	rssWithInvalidPartition.Spec.ShuffleServer.UpgradeStrategy = &unifflev1alpha1.ShuffleServerUpgradeStrategy{
+		Type:      unifflev1alpha1.PartitionUpgrade,
+		Partition: pointer.Int32(-1),
+	}
+
+	rssWithoutSpecificNames := rssWithoutLogInServerMounts.DeepCopy()
+	rssWithoutSpecificNames.Spec.ShuffleServer.CommonConfig.RSSPodSpec.HostPathMounts["/data/logs"] = "/data/logs"
+	rssWithoutSpecificNames.Spec.ShuffleServer.UpgradeStrategy = &unifflev1alpha1.ShuffleServerUpgradeStrategy{
+		Type: unifflev1alpha1.SpecificUpgrade,
+	}
+
+	rssWithoutUpgradeStrategyType := rssWithoutLogInServerMounts.DeepCopy()
+	rssWithoutUpgradeStrategyType.Spec.ShuffleServer.CommonConfig.RSSPodSpec.HostPathMounts["/data/logs"] = "/data/logs"
+	rssWithoutUpgradeStrategyType.Spec.ShuffleServer.UpgradeStrategy = &unifflev1alpha1.ShuffleServerUpgradeStrategy{}
+
+	for _, tt := range []struct {
+		name    string
+		ar      *admissionv1.AdmissionReview
+		allowed bool
+	}{
+		{
+			name: "try to modify a upgrading rss object",
+			ar: buildTestAdmissionReview(admissionv1.Update, wrapTestRssObj(&unifflev1alpha1.RemoteShuffleService{
+				Status: unifflev1alpha1.RemoteShuffleServiceStatus{
+					Phase: unifflev1alpha1.RSSUpgrading,
+				},
+			}), nil),
+			allowed: false,
+		},
+		{
+			name: "invalid rpc node port number in a rss object",
+			ar: buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(&unifflev1alpha1.RemoteShuffleService{
+				Spec: unifflev1alpha1.RemoteShuffleServiceSpec{
+					Coordinator: &unifflev1alpha1.CoordinatorConfig{
+						Count:       pointer.Int32(2),
+						RPCNodePort: []int32{30001},
+					},
+				},
+			})),
+			allowed: false,
+		},
+		{
+			name: "invalid http node port number in a rss object",
+			ar: buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(&unifflev1alpha1.RemoteShuffleService{
+				Spec: unifflev1alpha1.RemoteShuffleServiceSpec{
+					Coordinator: &unifflev1alpha1.CoordinatorConfig{
+						Count:        pointer.Int32(2),
+						RPCNodePort:  []int32{30001, 30002},
+						HTTPNodePort: []int32{30011},
+					},
+				},
+			})),
+			allowed: false,
+		},
+		{
+			name:    "empty exclude nodes file path field in a rss object",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithCooNodePort)),
+			allowed: false,
+		},
+		{
+			name:    "can not find log host path in coordinators' host path mounts field in a rss object",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithoutLogInCooMounts)),
+			allowed: false,
+		},
+		{
+			name:    "can not find log host path in shuffle server' host path mounts field in a rss object",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithoutLogInServerMounts)),
+			allowed: false,
+		},
+		{
+			name:    "empty partition field when shuffler server of a rss object need partition upgrade",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithoutPartition)),
+			allowed: false,
+		},
+		{
+			name:    "invalid partition field when shuffler server of a rss object need partition upgrade",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithInvalidPartition)),
+			allowed: false,
+		},
+		{
+			name:    "empty specific names field when shuffler server of a rss object need specific upgrade",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithoutSpecificNames)),
+			allowed: false,
+		},
+		{
+			name:    "empty upgrade strategy type in shuffler server of a rss object",
+			ar:      buildTestAdmissionReview(admissionv1.Update, nil, wrapTestRssObj(rssWithoutUpgradeStrategyType)),
+			allowed: false,
+		},
+	} {
+		t.Run(tt.name, func(tc *testing.T) {
+			updatedAR := testInspector.validateRSS(tt.ar)
+			if !updatedAR.Response.Allowed {
+				tc.Logf("==> message in result: %+v", updatedAR.Response.Result.Message)
+			}
+			if updatedAR.Response.Allowed != tt.allowed {
+				tc.Errorf("invalid 'allowed' field in response: %v <> %v", updatedAR.Response.Allowed, tt.allowed)
+			}
+		})
+	}
+}