Merge pull request #50719 from crimsonfaith91/immutability

Automatic merge from submit-queue (batch tested with PRs 50719, 51216, 50212, 51408, 51381)

Make selector immutable for v1beta2 deployment, replicaset and daemonset prior update

**What this PR does / why we need it**:
This PR ensures controller selector is immutable for deployment and replicaset prior update by ignoring any change to `Spec`.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #50808

**Special notes for your reviewer**:
This will be a breaking change.

**Release note**:

```release-note
For Deployment, ReplicaSet, and DaemonSet, selectors are now immutable when updating via the new `apps/v1beta2` API. For backward compatibility, selectors can still be changed when updating via `apps/v1beta1` or `extensions/v1beta1`.
```

test/integration/deployment/BUILD

@@ -18,6 +18,7 @@ go_test(
         "//pkg/controller/deployment/util:go_default_library",
         "//test/integration/framework:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
     ],
 )
 

test/integration/deployment/deployment_test.go

@@ -17,9 +17,12 @@ limitations under the License.
 package deployment
 
 import (
+	"reflect"
+	"strings"
 	"testing"
 
 	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 	"k8s.io/kubernetes/test/integration/framework"
 )
@@ -69,3 +72,65 @@ func TestNewDeployment(t *testing.T) {
 		t.Errorf("expected new ReplicaSet last-applied annotation not copied from Deployment %s", deploy.Name)
 	}
 }
+
+// selectors are IMMUTABLE for all API versions except apps/v1beta1 and extensions/v1beta1
+func TestDeploymentSelectorImmutability(t *testing.T) {
+	s, closeFn, c := dcSimpleSetup(t)
+	defer closeFn()
+	name := "test-deployment-selector-immutability"
+	ns := framework.CreateTestingNamespace(name, s, t)
+	defer framework.DeleteTestingNamespace(ns, s, t)
+
+	tester := &deploymentTester{t: t, c: c, deployment: newDeployment(name, ns.Name, int32(20))}
+	deploymentExtensionsV1beta1, err := c.ExtensionsV1beta1().Deployments(ns.Name).Create(tester.deployment)
+	if err != nil {
+		t.Fatalf("failed to create extensions/v1beta1 deployment %s: %v", tester.deployment.Name, err)
+	}
+
+	// test to ensure extensions/v1beta1 selector is mutable
+	newSelectorLabels := map[string]string{"name_extensions_v1beta1": "test_extensions_v1beta1"}
+	deploymentExtensionsV1beta1.Spec.Selector.MatchLabels = newSelectorLabels
+	deploymentExtensionsV1beta1.Spec.Template.Labels = newSelectorLabels
+	updatedDeploymentExtensionsV1beta1, err := c.ExtensionsV1beta1().Deployments(ns.Name).Update(deploymentExtensionsV1beta1)
+	if err != nil {
+		t.Fatalf("failed to update extensions/v1beta1 deployment %s: %v", deploymentExtensionsV1beta1.Name, err)
+	}
+	if !reflect.DeepEqual(updatedDeploymentExtensionsV1beta1.Spec.Selector.MatchLabels, newSelectorLabels) {
+		t.Errorf("selector should be changed for extensions/v1beta1, expected: %v, got: %v", newSelectorLabels, updatedDeploymentExtensionsV1beta1.Spec.Selector.MatchLabels)
+	}
+
+	// test to ensure apps/v1beta1 selector is mutable
+	deploymentAppsV1beta1, err := c.AppsV1beta1().Deployments(ns.Name).Get(updatedDeploymentExtensionsV1beta1.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("failed to get apps/v1beta1 deployment %s: %v", updatedDeploymentExtensionsV1beta1.Name, err)
+	}
+
+	newSelectorLabels = map[string]string{"name_apps_v1beta1": "test_apps_v1beta1"}
+	deploymentAppsV1beta1.Spec.Selector.MatchLabels = newSelectorLabels
+	deploymentAppsV1beta1.Spec.Template.Labels = newSelectorLabels
+	updatedDeploymentAppsV1beta1, err := c.AppsV1beta1().Deployments(ns.Name).Update(deploymentAppsV1beta1)
+	if err != nil {
+		t.Fatalf("failed to update apps/v1beta1 deployment %s: %v", deploymentAppsV1beta1.Name, err)
+	}
+	if !reflect.DeepEqual(updatedDeploymentAppsV1beta1.Spec.Selector.MatchLabels, newSelectorLabels) {
+		t.Errorf("selector should be changed for apps/v1beta1, expected: %v, got: %v", newSelectorLabels, updatedDeploymentAppsV1beta1.Spec.Selector.MatchLabels)
+	}
+
+	// test to ensure apps/v1beta2 selector is immutable
+	deploymentAppsV1beta2, err := c.AppsV1beta2().Deployments(ns.Name).Get(updatedDeploymentAppsV1beta1.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("failed to get apps/v1beta2 deployment %s: %v", updatedDeploymentAppsV1beta1.Name, err)
+	}
+	newSelectorLabels = map[string]string{"name_apps_v1beta2": "test_apps_v1beta2"}
+	deploymentAppsV1beta2.Spec.Selector.MatchLabels = newSelectorLabels
+	deploymentAppsV1beta2.Spec.Template.Labels = newSelectorLabels
+	_, err = c.AppsV1beta2().Deployments(ns.Name).Update(deploymentAppsV1beta2)
+	if err == nil {
+		t.Fatalf("failed to provide validation error when changing immutable selector when updating apps/v1beta2 deployment %s", deploymentAppsV1beta2.Name)
+	}
+	expectedErrType := "Invalid value"
+	expectedErrDetail := "field is immutable"
+	if !strings.Contains(err.Error(), expectedErrType) || !strings.Contains(err.Error(), expectedErrDetail) {
+		t.Errorf("error message does not match, expected type: %s, expected detail: %s, got: %s", expectedErrType, expectedErrDetail, err.Error())
+	}
+}

test/integration/deployment/util.go

@@ -115,6 +115,20 @@ func dcSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *replicaset.R
 	return s, closeFn, rm, dc, informers, clientSet
 }
 
+// dcSimpleSetup sets up necessities for Deployment integration test, including master, apiserver,
+// and clientset, but not controllers and informers
+func dcSimpleSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, clientset.Interface) {
+	masterConfig := framework.NewIntegrationTestMasterConfig()
+	_, s, closeFn := framework.RunAMaster(masterConfig)
+
+	config := restclient.Config{Host: s.URL}
+	clientSet, err := clientset.NewForConfig(&config)
+	if err != nil {
+		t.Fatalf("error in create clientset: %v", err)
+	}
+	return s, closeFn, clientSet
+}
+
 // addPodConditionReady sets given pod status to ready at given time
 func addPodConditionReady(pod *v1.Pod, time metav1.Time) {
 	pod.Status = v1.PodStatus{

test/integration/replicaset/replicaset_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/http/httptest"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -149,6 +150,18 @@ func rmSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *replicaset.R
 	return s, closeFn, rm, informers, clientSet
 }
 
+func rmSimpleSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, clientset.Interface) {
+	masterConfig := framework.NewIntegrationTestMasterConfig()
+	_, s, closeFn := framework.RunAMaster(masterConfig)
+
+	config := restclient.Config{Host: s.URL}
+	clientSet, err := clientset.NewForConfig(&config)
+	if err != nil {
+		t.Fatalf("Error in create clientset: %v", err)
+	}
+	return s, closeFn, clientSet
+}
+
 // wait for the podInformer to observe the pods. Call this function before
 // running the RS controller to prevent the rc manager from creating new pods
 // rather than adopting the existing ones.
@@ -462,3 +475,43 @@ func TestUpdateLabelToBeAdopted(t *testing.T) {
 	}
 	close(stopCh)
 }
+
+// selectors are IMMUTABLE for all API versions except extensions/v1beta1
+func TestRSSelectorImmutability(t *testing.T) {
+	s, closeFn, clientSet := rmSimpleSetup(t)
+	defer closeFn()
+	ns := framework.CreateTestingNamespace("rs-selector-immutability", s, t)
+	defer framework.DeleteTestingNamespace(ns, s, t)
+	rs := newRS("rs", ns.Name, 0)
+	createRSsPods(t, clientSet, []*v1beta1.ReplicaSet{rs}, []*v1.Pod{}, ns.Name)
+
+	// test to ensure extensions/v1beta1 selector is mutable
+	newSelectorLabels := map[string]string{"changed_name_extensions_v1beta1": "changed_test_extensions_v1beta1"}
+	rs.Spec.Selector.MatchLabels = newSelectorLabels
+	rs.Spec.Template.Labels = newSelectorLabels
+	replicaset, err := clientSet.ExtensionsV1beta1().ReplicaSets(ns.Name).Update(rs)
+	if err != nil {
+		t.Fatalf("failed to update extensions/v1beta1 replicaset %s: %v", replicaset.Name, err)
+	}
+	if !reflect.DeepEqual(replicaset.Spec.Selector.MatchLabels, newSelectorLabels) {
+		t.Errorf("selector should be changed for extensions/v1beta1, expected: %v, got: %v", newSelectorLabels, replicaset.Spec.Selector.MatchLabels)
+	}
+
+	// test to ensure apps/v1beta2 selector is immutable
+	rsV1beta2, err := clientSet.AppsV1beta2().ReplicaSets(ns.Name).Get(replicaset.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("failed to get apps/v1beta2 replicaset %s: %v", replicaset.Name, err)
+	}
+	newSelectorLabels = map[string]string{"changed_name_apps_v1beta2": "changed_test_apps_v1beta2"}
+	rsV1beta2.Spec.Selector.MatchLabels = newSelectorLabels
+	rsV1beta2.Spec.Template.Labels = newSelectorLabels
+	_, err = clientSet.AppsV1beta2().ReplicaSets(ns.Name).Update(rsV1beta2)
+	if err == nil {
+		t.Fatalf("failed to provide validation error when changing immutable selector when updating apps/v1beta2 replicaset %s", rsV1beta2.Name)
+	}
+	expectedErrType := "Invalid value"
+	expectedErrDetail := "field is immutable"
+	if !strings.Contains(err.Error(), expectedErrType) || !strings.Contains(err.Error(), expectedErrDetail) {
+		t.Errorf("error message does not match, expected type: %s, expected detail: %s, got: %s", expectedErrType, expectedErrDetail, err.Error())
+	}
+}