Allow changes to container image for updates

hack/testdata/nginx-petset.yaml

@@ -0,0 +1,44 @@
+# A headless service to create DNS records
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
+  name: nginx
+  labels:
+    app: nginx-petset
+spec:
+  ports:
+  - port: 80
+    name: web
+  # *.nginx.default.svc.cluster.local
+  clusterIP: None
+  selector:
+    app: nginx-petset
+---
+apiVersion: apps/v1alpha1
+kind: PetSet
+metadata:
+  name: nginx
+spec:
+  serviceName: "nginx"
+  replicas: 0
+  template:
+    metadata:
+      labels:
+        app: nginx-petset
+      annotations:
+        pod.alpha.kubernetes.io/initialized: "true"
+    spec:
+      terminationGracePeriodSeconds: 0
+      containers:
+      - name: nginx
+        image: gcr.io/google_containers/nginx-slim:0.7
+        ports:
+        - containerPort: 80
+          name: web
+        command:
+        - sh
+        - -c
+        - 'while true; do sleep 1; done'
+

pkg/api/validation/validation.go

@@ -1978,7 +1978,7 @@ func ValidatePodSecurityContext(securityContext *api.PodSecurityContext, spec *a
 	return allErrs
 }
 
-func validateContainerUpdates(newContainers, oldContainers []api.Container, fldPath *field.Path) (allErrs field.ErrorList, stop bool) {
+func ValidateContainerUpdates(newContainers, oldContainers []api.Container, fldPath *field.Path) (allErrs field.ErrorList, stop bool) {
 	allErrs = field.ErrorList{}
 	if len(newContainers) != len(oldContainers) {
 		//TODO: Pinpoint the specific container that causes the invalid error after we have strategic merge diff
@@ -2008,12 +2008,12 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
 	// 2.  initContainers[*].image
 	// 3.  spec.activeDeadlineSeconds
 
-	containerErrs, stop := validateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers"))
+	containerErrs, stop := ValidateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers"))
 	allErrs = append(allErrs, containerErrs...)
 	if stop {
 		return allErrs
 	}
-	containerErrs, stop = validateContainerUpdates(newPod.Spec.InitContainers, oldPod.Spec.InitContainers, specPath.Child("initContainers"))
+	containerErrs, stop = ValidateContainerUpdates(newPod.Spec.InitContainers, oldPod.Spec.InitContainers, specPath.Child("initContainers"))
 	allErrs = append(allErrs, containerErrs...)
 	if stop {
 		return allErrs

pkg/apis/apps/validation/validation.go

@@ -101,19 +101,24 @@ func ValidatePetSet(petSet *apps.PetSet) field.ErrorList {
 func ValidatePetSetUpdate(petSet, oldPetSet *apps.PetSet) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMetaUpdate(&petSet.ObjectMeta, &oldPetSet.ObjectMeta, field.NewPath("metadata"))
 
-	// TODO: For now we're taking the safe route and disallowing all updates to spec except for Spec.Replicas.
-	// Enable on a case by case basis.
+	// TODO: For now we're taking the safe route and disallowing all updates to
+	// spec except for Replicas, for scaling, and Template.Spec.containers.image
+	// for rolling-update. Enable others on a case by case basis.
 	restoreReplicas := petSet.Spec.Replicas
 	petSet.Spec.Replicas = oldPetSet.Spec.Replicas
 
+	restoreContainers := petSet.Spec.Template.Spec.Containers
+	petSet.Spec.Template.Spec.Containers = oldPetSet.Spec.Template.Spec.Containers
+
 	if !reflect.DeepEqual(petSet.Spec, oldPetSet.Spec) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to petset spec for fields other than 'replicas' are forbidden."))
 	}
-	if !reflect.DeepEqual(petSet.Status, oldPetSet.Status) {
-		allErrs = append(allErrs, field.Forbidden(field.NewPath("status"), "updates to petset status forbidden."))
-	}
 	petSet.Spec.Replicas = restoreReplicas
+	petSet.Spec.Template.Spec.Containers = restoreContainers
+
 	allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(petSet.Spec.Replicas), field.NewPath("spec", "replicas"))...)
+	containerErrs, _ := apivalidation.ValidateContainerUpdates(petSet.Spec.Template.Spec.Containers, oldPetSet.Spec.Template.Spec.Containers, field.NewPath("spec").Child("template").Child("containers"))
+	allErrs = append(allErrs, containerErrs...)
 	return allErrs
 }
 

pkg/kubectl/stop.go

@@ -366,7 +366,7 @@ func (reaper *PetSetReaper) Stop(namespace, name string, timeout time.Duration,
 		return utilerrors.NewAggregate(errList)
 	}
 
-	// TODO: Cleanup volumes? We don't want to accidentaly delete volumes from
+	// TODO: Cleanup volumes? We don't want to accidentally delete volumes from
 	// stop, so just leave this up to the the petset.
 	return petsets.Delete(name, nil)
 }

pkg/registry/petset/strategy_test.go

@@ -66,7 +66,7 @@ func TestPetSetStrategy(t *testing.T) {
 
 	// Just Spec.Replicas is allowed to change
 	validPs := &apps.PetSet{
-		ObjectMeta: api.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace},
+		ObjectMeta: api.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1},
 		Spec: apps.PetSetSpec{
 			Selector: ps.Spec.Selector,
 			Template: validPodTemplate.Template,
@@ -76,7 +76,7 @@ func TestPetSetStrategy(t *testing.T) {
 	Strategy.PrepareForUpdate(ctx, validPs, ps)
 	errs = Strategy.ValidateUpdate(ctx, validPs, ps)
 	if len(errs) != 0 {
-		t.Errorf("Updating spec.Replicas is allowed on a petset.")
+		t.Errorf("Updating spec.Replicas is allowed on a petset: %v", errs)
 	}
 
 	validPs.Spec.Selector = &unversioned.LabelSelector{MatchLabels: map[string]string{"a": "bar"}}