Tests to make sure we pass through fields of corev1.Container (#242)

* Add validation for ContainerSpec of Configurations. Even though ContainerSpec accepts all fields of core/v1/Container, the controller will silently overwrite some fields, like Name, Resources, Ports, and VolumeMounts. This will cause confusions if the users expect their settings to be effective. This change disallows specifying those fields to avoid the confusions.
knative · Feb 28, 2018 · 820c2a3 · 820c2a3
1 parent 1b81975
commit 820c2a3
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 8 deletions.
diff --git a/pkg/controller/configuration/BUILD.bazel b/pkg/controller/configuration/BUILD.bazel
@@ -44,6 +44,7 @@ go_test(
         "//pkg/client/informers/externalversions:go_default_library",
         "//pkg/controller/testing:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",

diff --git a/pkg/controller/configuration/controller_test.go b/pkg/controller/configuration/controller_test.go
@@ -32,6 +32,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -66,6 +67,26 @@ func getTestConfiguration() *v1alpha1.Configuration {
 			Template: v1alpha1.Revision{
 				Spec: v1alpha1.RevisionSpec{
 					Service: "test-service",
+					// corev1.Container has a lot of setting.  We try to pass many
+					// of them here to verify that we pass through the settings to
+					// the derived Revisions.
+					ContainerSpec: &corev1.Container{
+						Image:      "gcr.io/repo/image",
+						Command:    []string{"echo"},
+						Args:       []string{"hello", "world"},
+						WorkingDir: "/tmp",
+						Env: []corev1.EnvVar{{
+							Name:  "EDITOR",
+							Value: "emacs",
+						}},
+						LivenessProbe: &corev1.Probe{
+							TimeoutSeconds: 42,
+						},
+						ReadinessProbe: &corev1.Probe{
+							TimeoutSeconds: 43,
+						},
+						TerminationMessagePath: "/dev/null",
+					},
 				},
 			},
 		},
@@ -147,6 +168,10 @@ func TestCreateConfigurationsCreatesRevision(t *testing.T) {
 			t.Errorf("rev service was not %s", config.Spec.Template.Spec.Service)
 		}
 
+		if diff := cmp.Diff(config.Spec.Template.Spec, rev.Spec); diff != "" {
+			t.Errorf("rev spec != config template spec (-want +got): %v", diff)
+		}
+
 		if rev.Labels[ConfigurationLabelKey] != config.Name {
 			t.Errorf("rev does not have label <%s:%s>", ConfigurationLabelKey, config.Name)
 		}

diff --git a/pkg/controller/revision/BUILD.bazel b/pkg/controller/revision/BUILD.bazel
@@ -56,6 +56,8 @@ go_test(
         "//pkg/client/informers/externalversions:go_default_library",
         "//pkg/controller/testing:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/extensions/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

diff --git a/pkg/controller/revision/controller_test.go b/pkg/controller/revision/controller_test.go
@@ -17,13 +17,6 @@ limitations under the License.
 package revision
 
 /* TODO tests:
-- When a Revision is created:
-	- a namespace is created
-	- a deployment is created
-	- an autoscaler is created
-	- an nginx configmap is created
-	- Revision status is updated
-
 - When a Revision is updated TODO
 - When a Revision is deleted TODO
 */
@@ -33,6 +26,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+
 	"github.com/golang/glog"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -61,8 +57,25 @@ func getTestRevision() *v1alpha1.Revision {
 		},
 		Spec: v1alpha1.RevisionSpec{
 			Service: "test-service",
+			// corev1.Container has a lot of setting.  We try to pass many
+			// of them here to verify that we pass through the settings to
+			// derived objects.
 			ContainerSpec: &corev1.Container{
-				Image: "test-image",
+				Image:      "gcr.io/repo/image",
+				Command:    []string{"echo"},
+				Args:       []string{"hello", "world"},
+				WorkingDir: "/tmp",
+				Env: []corev1.EnvVar{{
+					Name:  "EDITOR",
+					Value: "emacs",
+				}},
+				LivenessProbe: &corev1.Probe{
+					TimeoutSeconds: 42,
+				},
+				ReadinessProbe: &corev1.Probe{
+					TimeoutSeconds: 43,
+				},
+				TerminationMessagePath: "/dev/null",
 			},
 		},
 	}
@@ -158,6 +171,12 @@ func TestCreateRevCreatesStuff(t *testing.T) {
 		func(t *testing.T) {
 			for _, c := range p.Spec.Containers {
 				if c.Image == rev.Spec.ContainerSpec.Image {
+					// Ignoring fields set by Elafros controller.
+					ignored := cmpopts.IgnoreFields(corev1.Container{}, "Name", "Ports", "Resources", "VolumeMounts")
+					// All other fields must match.
+					if diff := cmp.Diff(rev.Spec.ContainerSpec, &c, ignored); diff != "" {
+						t.Errorf("Pod container spec != revision container spec (-want +got): %v", diff)
+					}
 					return
 				}
 			}