diff --git a/pkg/reconciler/v1alpha1/revision/table_test.go b/pkg/reconciler/v1alpha1/revision/table_test.go index a64936e87bd4..87ad82c49e15 100644 --- a/pkg/reconciler/v1alpha1/revision/table_test.go +++ b/pkg/reconciler/v1alpha1/revision/table_test.go @@ -19,12 +19,9 @@ package revision import ( "context" "testing" - "time" caching "github.com/knative/caching/pkg/apis/caching/v1alpha1" - "github.com/knative/pkg/apis" "github.com/knative/pkg/apis/duck" - duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" "github.com/knative/pkg/configmap" "github.com/knative/pkg/controller" "github.com/knative/pkg/logging" @@ -46,55 +43,6 @@ import ( . "github.com/knative/serving/pkg/reconciler/v1alpha1/testing" ) -var ( - conditionsOnFailure = duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }} - - allUnknownConditions = duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }} -) - // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method. func TestReconcile(t *testing.T) { table := TableTest{{ @@ -121,14 +69,9 @@ func TestReconcile(t *testing.T) { image("foo", "first-reconcile", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "first-reconcile", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "first-reconcile", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + Object: rev("foo", "first-reconcile", "busybox", + // The first reconciliation Populates the following status properties. + WithK8sServiceName, WithLogURL, AllUnknownConditions), }}, Key: "foo/first-reconcile", }, { @@ -144,20 +87,15 @@ func TestReconcile(t *testing.T) { kpa("foo", "update-status-failure", "busybox"), }, WantCreates: []metav1.Object{ - // The first reconciliation of a Revision creates the following resources. + // We still see the following creates before the failure is induced. deploy("foo", "update-status-failure", "busybox"), svc("foo", "update-status-failure", "busybox"), image("foo", "update-status-failure", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "update-status-failure", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "update-status-failure", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + Object: rev("foo", "update-status-failure", "busybox", + // Despite failure, the following status properties are set. + WithK8sServiceName, WithLogURL, AllUnknownConditions), }}, Key: "foo/update-status-failure", }, { @@ -172,22 +110,17 @@ func TestReconcile(t *testing.T) { rev("foo", "create-kpa-failure", "busybox"), }, WantCreates: []metav1.Object{ - // The first reconciliation of a Revision creates the following resources. + // We still see the following creates before the failure is induced. kpa("foo", "create-kpa-failure", "busybox"), deploy("foo", "create-kpa-failure", "busybox"), svc("foo", "create-kpa-failure", "busybox"), image("foo", "create-kpa-failure", "busybox"), - // The user service and autoscaler resources are not created. }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "create-kpa-failure", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - ServiceName: svc("foo", "create-kpa-failure", "busybox").Name, - Conditions: conditionsOnFailure, - }), + Object: rev("foo", "create-kpa-failure", "busybox", + // Despite failure, the following status properties are set. + WithK8sServiceName, WithLogURL, WithInitRevConditions, + WithNoBuild, MarkDeploying("Deploying")), }}, Key: "foo/create-kpa-failure", }, { @@ -203,18 +136,14 @@ func TestReconcile(t *testing.T) { kpa("foo", "create-user-deploy-failure", "busybox"), }, WantCreates: []metav1.Object{ - // The first reconciliation of a Revision creates the following resources. + // We still see the following creates before the failure is induced. deploy("foo", "create-user-deploy-failure", "busybox"), - // The user service and autoscaler resources are not created. }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "create-user-deploy-failure", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: conditionsOnFailure, - }), + Object: rev("foo", "create-user-deploy-failure", "busybox", + // Despite failure, the following status properties are set. + WithLogURL, WithInitRevConditions, + WithNoBuild, MarkDeploying("Deploying")), }}, Key: "foo/create-user-deploy-failure", }, { @@ -230,21 +159,16 @@ func TestReconcile(t *testing.T) { kpa("foo", "create-user-service-failure", "busybox"), }, WantCreates: []metav1.Object{ - // The first reconciliation of a Revision creates the following resources. + // We still see the following creates before the failure is induced. deploy("foo", "create-user-service-failure", "busybox"), svc("foo", "create-user-service-failure", "busybox"), image("foo", "create-user-service-failure", "busybox"), - // No autoscaler resources created. }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "create-user-service-failure", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "create-user-service-failure", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: conditionsOnFailure, - }), + Object: rev("foo", "create-user-service-failure", "busybox", + // Despite failure, the following status properties are set. + WithK8sServiceName, WithLogURL, WithInitRevConditions, + WithNoBuild, MarkDeploying("Deploying")), }}, Key: "foo/create-user-service-failure", }, { @@ -254,13 +178,8 @@ func TestReconcile(t *testing.T) { // state (immediately post-creation), and verify that no changes // are necessary. Objects: []runtime.Object{ - makeStatus( - rev("foo", "stable-reconcile", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "stable-reconcile", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + rev("foo", "stable-reconcile", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "stable-reconcile", "busybox"), deploy("foo", "stable-reconcile", "busybox"), svc("foo", "stable-reconcile", "busybox"), @@ -270,51 +189,21 @@ func TestReconcile(t *testing.T) { Key: "foo/stable-reconcile", }, { Name: "deactivated revision is stable", - // Test a simple stable reconciliation of a Reserve Revision. + // Test a simple stable reconciliation of an inactive Revision. // We feed in a Revision and the resources it controls in a steady // state (port-Reserve), and verify that no changes are necessary. Objects: []runtime.Object{ - makeStatus( - rev("foo", "stable-deactivation", "busybox"), - // The Revision status matches that of a properly deactivated Revision. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "stable-deactivation", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), - kpa("foo", "stable-deactivation", "busybox"), - // The Deployments match what we'd expect of an Reserve revision. + rev("foo", "stable-deactivation", "busybox", + WithK8sServiceName, WithLogURL, MarkRevisionReady, + MarkInactive("NoTraffic", "This thing is inactive.")), + kpa("foo", "stable-deactivation", "busybox", + WithNoTraffic("NoTraffic", "This thing is inactive.")), deploy("foo", "stable-deactivation", "busybox"), + endpoints("foo", "stable-deactivation", "busybox", WithSubsets), svc("foo", "stable-deactivation", "busybox"), image("foo", "stable-deactivation", "busybox"), }, Key: "foo/stable-deactivation", - }, { - Name: "create resources in reserve", - // Test a reconcile of a Revision in the Reserve state. - // This tests the initial set of resources that we create for a Revision - // when it is in a Reserve state and none of its resources exist. The main - // place we should expect this transition to happen is Retired -> Reserve. - Objects: []runtime.Object{ - rev("foo", "create-in-reserve", "busybox"), - }, - WantCreates: []metav1.Object{ - kpa("foo", "create-in-reserve", "busybox"), - // Only Deployments are created and they have no replicas. - deploy("foo", "create-in-reserve", "busybox"), - svc("foo", "create-in-reserve", "busybox"), - image("foo", "create-in-reserve", "busybox"), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "create-in-reserve", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "create-in-reserve", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), - }}, - Key: "foo/create-in-reserve", }, { Name: "endpoint is created (not ready)", // Test the transition when a Revision's Endpoints are created (but not yet ready) @@ -327,39 +216,8 @@ func TestReconcile(t *testing.T) { // from thinking we've been waiting for this Endpoint since the beginning of time // and declaring a timeout (this is the main difference from that test below). Objects: []runtime.Object{ - makeStatus( - rev("foo", "endpoint-created-not-ready", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "endpoint-created-not-ready", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - // We set the LTT so that we don't give up on the Endpoints yet. - LastTransitionTime: apis.VolatileTime{metav1.NewTime(time.Now())}, - Severity: "Error", - }}, - }), + rev("foo", "endpoint-created-not-ready", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "endpoint-created-not-ready", "busybox"), deploy("foo", "endpoint-created-not-ready", "busybox"), svc("foo", "endpoint-created-not-ready", "busybox"), @@ -375,91 +233,22 @@ func TestReconcile(t *testing.T) { // we've been waiting since the dawn of time because we omit LastTransitionTime from // our Conditions. We should see an update to put us into a ServiceTimeout state. Objects: []runtime.Object{ - makeStatus( - rev("foo", "endpoint-created-timeout", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "endpoint-created-timeout", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "True", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - // The LTT defaults and is long enough ago that we expire waiting - // on the Endpoints to become ready. - Severity: "Error", - }}, - }), - addKPAStatus( - kpa("foo", "endpoint-created-timeout", "busybox"), - kpav1alpha1.PodAutoscalerStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "True", - Severity: "Info", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }}, - }), + rev("foo", "endpoint-created-timeout", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions, + MarkActive, WithEmptyLTTs), + kpa("foo", "endpoint-created-timeout", "busybox", WithTraffic), deploy("foo", "endpoint-created-timeout", "busybox"), svc("foo", "endpoint-created-timeout", "busybox"), endpoints("foo", "endpoint-created-timeout", "busybox"), image("foo", "endpoint-created-timeout", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "endpoint-created-timeout", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "endpoint-created-timeout", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "True", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "False", - Reason: "ServiceTimeout", - Message: "Timed out waiting for a service endpoint to become ready", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "False", - Reason: "ServiceTimeout", - Message: "Timed out waiting for a service endpoint to become ready", - Severity: "Error", - }}, - }), + Object: rev("foo", "endpoint-created-timeout", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions, MarkActive, + // When the LTT is cleared, a reconcile will result in the + // following mutation. + MarkServiceTimeout), }}, - // We update the Revision to timeout waiting on Endpoints. Key: "foo/endpoint-created-timeout", }, { Name: "endpoint and kpa are ready", @@ -468,258 +257,61 @@ func TestReconcile(t *testing.T) { // Revision. It then creates an Endpoints resource with active subsets. // This signal should make our Reconcile mark the Revision as Ready. Objects: []runtime.Object{ - makeStatus( - rev("foo", "endpoint-ready", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "endpoint-ready", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }}, - }), - addKPAStatus( - kpa("foo", "endpoint-ready", "busybox"), - kpav1alpha1.PodAutoscalerStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "True", - Severity: "Info", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }}, - }), + rev("foo", "endpoint-ready", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), + kpa("foo", "endpoint-ready", "busybox", WithTraffic), deploy("foo", "endpoint-ready", "busybox"), svc("foo", "endpoint-ready", "busybox"), - addEndpoint(endpoints("foo", "endpoint-ready", "busybox")), + endpoints("foo", "endpoint-ready", "busybox", WithSubsets), image("foo", "endpoint-ready", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "endpoint-ready", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "endpoint-ready", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "True", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "True", - Severity: "Error", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "True", - Severity: "Error", - }}, - }), + Object: rev("foo", "endpoint-ready", "busybox", WithK8sServiceName, WithLogURL, + // When the endpoint and KPA are ready, then we will see the + // Revision become ready. + MarkRevisionReady), }}, - // We update the Revision to timeout waiting on Endpoints. Key: "foo/endpoint-ready", }, { Name: "kpa not ready", // Test propagating the KPA status to the Revision. Objects: []runtime.Object{ - makeStatus( - rev("foo", "kpa-not-ready", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "kpa-not-ready", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "True", - Severity: "Error", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }}, - }), - addKPAStatus( - kpa("foo", "kpa-not-ready", "busybox"), - kpav1alpha1.PodAutoscalerStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Something", - Message: "This is something longer", - Severity: "Info", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Something", - Message: "This is something longer", - Severity: "Error", - }}, - }), + rev("foo", "kpa-not-ready", "busybox", + WithK8sServiceName, WithLogURL, MarkRevisionReady), + kpa("foo", "kpa-not-ready", "busybox", + WithBufferedTraffic("Something", "This is something longer")), deploy("foo", "kpa-not-ready", "busybox"), svc("foo", "kpa-not-ready", "busybox"), - addEndpoint(endpoints("foo", "kpa-not-ready", "busybox")), + endpoints("foo", "kpa-not-ready", "busybox", WithSubsets), image("foo", "kpa-not-ready", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "kpa-not-ready", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "kpa-not-ready", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Something", - Message: "This is something longer", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "True", - Severity: "Error", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "True", - Severity: "Error", - }}, - }), + Object: rev("foo", "kpa-not-ready", "busybox", + WithK8sServiceName, WithLogURL, MarkRevisionReady, + // When we reconcile a ready state and our KPA is in an activating + // state, we should see the following mutation. + MarkActivating("Something", "This is something longer")), }}, Key: "foo/kpa-not-ready", }, { Name: "kpa inactive", // Test propagating the inactivity signal from the KPA to the Revision. Objects: []runtime.Object{ - makeStatus( - rev("foo", "kpa-inactive", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "kpa-inactive", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "True", - Severity: "Error", - }, { - Type: "Ready", - Status: "True", - Severity: "Error", - }}, - }), - addKPAStatus( - kpa("foo", "kpa-inactive", "busybox"), - kpav1alpha1.PodAutoscalerStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "False", - Reason: "NoTraffic", - Message: "This thing is inactive.", - Severity: "Info", - }, { - Type: "Ready", - Status: "False", - Reason: "NoTraffic", - Message: "This thing is inactive.", - Severity: "Error", - }}, - }), + rev("foo", "kpa-inactive", "busybox", + WithK8sServiceName, WithLogURL, MarkRevisionReady), + kpa("foo", "kpa-inactive", "busybox", + WithNoTraffic("NoTraffic", "This thing is inactive.")), deploy("foo", "kpa-inactive", "busybox"), svc("foo", "kpa-inactive", "busybox"), + endpoints("foo", "kpa-inactive", "busybox", WithSubsets), image("foo", "kpa-inactive", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "kpa-inactive", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "kpa-inactive", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "False", - Reason: "NoTraffic", - Message: "This thing is inactive.", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }}, - }), + Object: rev("foo", "kpa-inactive", "busybox", + WithK8sServiceName, WithLogURL, MarkRevisionReady, + // When we reconcile an "all ready" revision when the KPA + // is inactive, we should see the following change. + MarkInactive("NoTraffic", "This thing is inactive.")), }}, Key: "foo/kpa-inactive", }, { @@ -730,73 +322,20 @@ func TestReconcile(t *testing.T) { // verify that Reconcile posts the appropriate updates to correct the // services back to our desired specification. Objects: []runtime.Object{ - makeStatus( - rev("foo", "fix-mutated-service", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "fix-mutated-service", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - // We set the LTT so that we don't give up on the Endpoints yet. - LastTransitionTime: apis.VolatileTime{metav1.NewTime(time.Now())}, - Severity: "Error", - }}, - }), + rev("foo", "fix-mutated-service", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "fix-mutated-service", "busybox"), deploy("foo", "fix-mutated-service", "busybox"), - changeService(svc("foo", "fix-mutated-service", "busybox")), + svc("foo", "fix-mutated-service", "busybox", MutateK8sService), endpoints("foo", "fix-mutated-service", "busybox"), image("foo", "fix-mutated-service", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - // Reason changes from Deploying to Updating. - Object: makeStatus( - rev("foo", "fix-mutated-service", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "fix-mutated-service", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Updating", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Updating", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Updating", - Severity: "Error", - }}, - }), + Object: rev("foo", "fix-mutated-service", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions, + // When our reconciliation has to change the service + // we should see the following mutations to status. + MarkDeploying("Updating"), MarkActivating("Deploying", "")), }, { Object: svc("foo", "fix-mutated-service", "busybox"), }}, @@ -809,41 +348,11 @@ func TestReconcile(t *testing.T) { InduceFailure("update", "services"), }, Objects: []runtime.Object{ - makeStatus( - rev("foo", "update-user-svc-failure", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "update-user-svc-failure", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - // We set the LTT so that we don't give up on the Endpoints yet. - LastTransitionTime: apis.VolatileTime{metav1.NewTime(time.Now())}, - Severity: "Error", - }}, - }), + rev("foo", "update-user-svc-failure", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "update-user-svc-failure", "busybox"), deploy("foo", "update-user-svc-failure", "busybox"), - changeService(svc("foo", "update-user-svc-failure", "busybox")), + svc("foo", "update-user-svc-failure", "busybox", MutateK8sService), endpoints("foo", "update-user-svc-failure", "busybox"), image("foo", "update-user-svc-failure", "busybox"), }, @@ -859,70 +368,20 @@ func TestReconcile(t *testing.T) { // condition. It then verifies that Reconcile propagates this into the // status of the Revision. Objects: []runtime.Object{ - makeStatus( - rev("foo", "deploy-timeout", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "deploy-timeout", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - // We set the LTT so that we don't give up on the Endpoints yet. - LastTransitionTime: apis.VolatileTime{metav1.NewTime(time.Now())}, - Severity: "Error", - }}, - }), - kpa("foo", "deploy-timeout", "busybox"), + rev("foo", "deploy-timeout", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions, MarkActive), + kpa("foo", "deploy-timeout", "busybox", WithTraffic), timeoutDeploy(deploy("foo", "deploy-timeout", "busybox")), svc("foo", "deploy-timeout", "busybox"), endpoints("foo", "deploy-timeout", "busybox"), image("foo", "deploy-timeout", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "deploy-timeout", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "deploy-timeout", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "False", - Reason: "ProgressDeadlineExceeded", - Message: "Unable to create pods for more than 120 seconds.", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "False", - Reason: "ProgressDeadlineExceeded", - Message: "Unable to create pods for more than 120 seconds.", - Severity: "Error", - }}, - }), + Object: rev("foo", "deploy-timeout", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions, MarkActive, + // When the revision is reconciled after a Deployment has + // timed out, we should see it marked with the PDE state. + MarkProgressDeadlineExceeded), }}, Key: "foo/deploy-timeout", }, { @@ -933,32 +392,14 @@ func TestReconcile(t *testing.T) { // the conditions of this Revision. It is notable that unlike the tests // above, this will include a BuildSucceeded condition. Objects: []runtime.Object{ - addBuild(rev("foo", "missing-build", "busybox"), "the-build"), + rev("foo", "missing-build", "busybox", WithBuildRef("the-build")), }, WantErr: true, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - addBuild(rev("foo", "missing-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }}, - }), + Object: rev("foo", "missing-build", "busybox", WithBuildRef("the-build"), + // When we first reconcile a revision with a Build (that's missing) + // we should see the following status changes. + WithLogURL, WithInitRevConditions), }}, Key: "foo/missing-build", }, { @@ -969,39 +410,14 @@ func TestReconcile(t *testing.T) { // the conditions of this Revision. It is notable that unlike the tests // above, this will include a BuildSucceeded condition. Objects: []runtime.Object{ - addBuild(rev("foo", "running-build", "busybox"), "the-build"), - build("foo", "the-build", - duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Severity: "Error", - }), + rev("foo", "running-build", "busybox", WithBuildRef("the-build")), + build("foo", "the-build", WithSucceededUnknown("", "")), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - addBuild(rev("foo", "running-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "Unknown", - Reason: "Building", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Building", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }}, - }), + Object: rev("foo", "running-build", "busybox", WithBuildRef("the-build"), + // When we first reconcile a revision with a Build (not done) + // we should see the following status changes. + WithLogURL, WithInitRevConditions, WithOngoingBuild), }}, Key: "foo/running-build", }, { @@ -1012,33 +428,8 @@ func TestReconcile(t *testing.T) { // Reconcile toggles the BuildSucceeded status and then acts similarly to // the first reconcile of a BYO-Container Revision. Objects: []runtime.Object{ - makeStatus( - addBuild(rev("foo", "done-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }}, - }), - build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionTrue, - Severity: "Error", - }), + rev("foo", "done-build", "busybox", WithBuildRef("the-build"), WithInitRevConditions), + build("foo", "the-build", WithSucceededTrue), }, WantCreates: []metav1.Object{ // The first reconciliation of a Revision creates the following resources. @@ -1048,37 +439,11 @@ func TestReconcile(t *testing.T) { image("foo", "done-build", "busybox"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - addBuild(rev("foo", "done-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "done-build", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }}, - }), + Object: rev("foo", "done-build", "busybox", WithBuildRef("the-build"), WithInitRevConditions, + // When we reconcile a Revision after the Build completes, we should + // see the following updates to its status. + WithK8sServiceName, WithLogURL, WithSuccessfulBuild, + MarkDeploying("Deploying"), MarkActivating("Deploying", "")), }}, Key: "foo/done-build", }, { @@ -1088,42 +453,12 @@ func TestReconcile(t *testing.T) { // state (immediately post-build completion), and verify that no changes // are necessary. Objects: []runtime.Object{ - makeStatus( - addBuild(rev("foo", "stable-reconcile-with-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "stable-reconcile-with-build", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Info", - }, { - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }}, - }), + rev("foo", "stable-reconcile-with-build", "busybox", + WithBuildRef("the-build"), WithK8sServiceName, WithLogURL, + WithInitRevConditions, WithSuccessfulBuild, + MarkDeploying("Deploying"), MarkActivating("Deploying", "")), kpa("foo", "stable-reconcile-with-build", "busybox"), - build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionTrue, - }), + build("foo", "the-build", WithSucceededTrue), deploy("foo", "stable-reconcile-with-build", "busybox"), svc("foo", "stable-reconcile-with-build", "busybox"), image("foo", "stable-reconcile-with-build", "busybox"), @@ -1137,63 +472,16 @@ func TestReconcile(t *testing.T) { // and a Build that has just failed. We then verify that a Reconcile toggles // the BuildSucceeded status and stops. Objects: []runtime.Object{ - makeStatus( - addBuild(rev("foo", "failed-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }}, - }), - build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }), + rev("foo", "failed-build", "busybox", WithBuildRef("the-build"), WithLogURL, WithInitRevConditions), + build("foo", "the-build", + WithSucceededFalse("SomeReason", "This is why the build failed.")), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - addBuild(rev("foo", "failed-build", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "False", - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "Ready", - Status: "False", - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }}, - }), + Object: rev("foo", "failed-build", "busybox", + WithBuildRef("the-build"), WithLogURL, WithInitRevConditions, + // When we reconcile a Revision whose build has failed, we sill see that + // failure reflected in the Revision status as follows: + WithFailedBuild("SomeReason", "This is why the build failed.")), }}, Key: "foo/failed-build", }, { @@ -1203,43 +491,10 @@ func TestReconcile(t *testing.T) { // has failed, which has been previously reconcile. We then verify that a // Reconcile has nothing to change. Objects: []runtime.Object{ - makeStatus( - addBuild(rev("foo", "failed-build-stable", "busybox"), "the-build"), - v1alpha1.RevisionStatus{ - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Info", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Severity: "Error", - }, { - Type: "BuildSucceeded", - Status: "False", - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }, { - Type: "Ready", - Status: "False", - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }}, - }), - build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: "SomeReason", - Message: "This is why the build failed.", - Severity: "Error", - }), + rev("foo", "failed-build-stable", "busybox", WithBuildRef("the-build"), WithInitRevConditions, + WithLogURL, WithFailedBuild("SomeReason", "This is why the build failed.")), + build("foo", "the-build", + WithSucceededFalse("SomeReason", "This is why the build failed.")), }, Key: "foo/failed-build-stable", }} @@ -1284,14 +539,9 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { image("foo", "first-reconcile-var-log", "busybox", EnableVarLog), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "first-reconcile-var-log", "busybox"), + Object: rev("foo", "first-reconcile-var-log", "busybox", // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "first-reconcile-var-log", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + WithK8sServiceName, WithLogURL, AllUnknownConditions), }}, Key: "foo/first-reconcile-var-log", }, { @@ -1305,54 +555,26 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { rev("foo", "create-configmap-failure", "busybox"), }, WantCreates: []metav1.Object{ - // The first reconciliation of a Revision creates the following resources. deploy("foo", "create-configmap-failure", "busybox", EnableVarLog), svc("foo", "create-configmap-failure", "busybox"), fluentdConfigMap("foo", "create-configmap-failure", "busybox", EnableVarLog), image("foo", "create-configmap-failure", "busybox", EnableVarLog), - // We don't create the autoscaler resources if we fail to create the fluentd configmap }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: makeStatus( - rev("foo", "create-configmap-failure", "busybox"), - // After the first reconciliation of a Revision the status looks like this. - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "create-configmap-failure", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: duckv1alpha1.Conditions{{ - Type: "BuildSucceeded", - Status: "True", - Severity: "Error", - }, { - Type: "ContainerHealthy", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }, { - Type: "ResourcesAvailable", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", - }}, - }), + Object: rev("foo", "create-configmap-failure", "busybox", + // When our first reconciliation is interrupted by a failure creating + // the fluentd configmap, we should still see the following reflected + // in our status. + WithK8sServiceName, WithLogURL, WithInitRevConditions, + WithNoBuild, MarkDeploying("Deploying")), }}, Key: "foo/create-configmap-failure", }, { Name: "steady state after initial creation", // Verify that after creating the things from an initial reconcile that we're stable. Objects: []runtime.Object{ - makeStatus( - rev("foo", "steady-state", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "steady-state", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + rev("foo", "steady-state", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "steady-state", "busybox"), deploy("foo", "steady-state", "busybox", EnableVarLog), svc("foo", "steady-state", "busybox"), @@ -1364,19 +586,15 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { Name: "update a bad fluentd configmap", // Verify that after creating the things from an initial reconcile that we're stable. Objects: []runtime.Object{ - makeStatus( - rev("foo", "update-fluentd-config", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "update-fluentd-config", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + rev("foo", "update-fluentd-config", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), kpa("foo", "update-fluentd-config", "busybox"), deploy("foo", "update-fluentd-config", "busybox", EnableVarLog), svc("foo", "update-fluentd-config", "busybox"), &corev1.ConfigMap{ // Use the ObjectMeta, but discard the rest. - ObjectMeta: fluentdConfigMap("foo", "update-fluentd-config", "busybox", EnableVarLog).ObjectMeta, + ObjectMeta: fluentdConfigMap("foo", "update-fluentd-config", "busybox", + EnableVarLog).ObjectMeta, Data: map[string]string{ "bad key": "bad value", }, @@ -1396,18 +614,14 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { InduceFailure("update", "configmaps"), }, Objects: []runtime.Object{ - makeStatus( - rev("foo", "update-configmap-failure", "busybox"), - v1alpha1.RevisionStatus{ - ServiceName: svc("foo", "update-configmap-failure", "busybox").Name, - LogURL: "http://logger.io/test-uid", - Conditions: allUnknownConditions, - }), + rev("foo", "update-configmap-failure", "busybox", + WithK8sServiceName, WithLogURL, AllUnknownConditions), deploy("foo", "update-configmap-failure", "busybox", EnableVarLog), svc("foo", "update-configmap-failure", "busybox"), &corev1.ConfigMap{ // Use the ObjectMeta, but discard the rest. - ObjectMeta: fluentdConfigMap("foo", "update-configmap-failure", "busybox", EnableVarLog).ObjectMeta, + ObjectMeta: fluentdConfigMap("foo", "update-configmap-failure", "busybox", + EnableVarLog).ObjectMeta, Data: map[string]string{ "bad key": "bad value", }, @@ -1441,38 +655,6 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { })) } -var ( - boolTrue = true -) - -func makeStatus(rev *v1alpha1.Revision, status v1alpha1.RevisionStatus) *v1alpha1.Revision { - rev.Status = status - return rev -} - -func addBuild(rev *v1alpha1.Revision, name string) *v1alpha1.Revision { - rev.Spec.BuildName = name - rev.Spec.BuildRef = &corev1.ObjectReference{ - APIVersion: "testing.build.knative.dev/v1alpha1", - Kind: "Build", - Name: name, - } - return rev -} - -func addEndpoint(ep *corev1.Endpoints) *corev1.Endpoints { - ep.Subsets = []corev1.EndpointSubset{{ - Addresses: []corev1.EndpointAddress{{IP: "127.0.0.1"}}, - }} - return ep -} - -func changeService(svc *corev1.Service) *corev1.Service { - // An effective hammer ;-P - svc.Spec = corev1.ServiceSpec{} - return svc -} - func timeoutDeploy(deploy *appsv1.Deployment) *appsv1.Deployment { deploy.Status.Conditions = []appsv1.DeploymentCondition{{ Type: appsv1.DeploymentProgressing, @@ -1482,14 +664,9 @@ func timeoutDeploy(deploy *appsv1.Deployment) *appsv1.Deployment { return deploy } -func addKPAStatus(kpa *kpav1alpha1.PodAutoscaler, status kpav1alpha1.PodAutoscalerStatus) *kpav1alpha1.PodAutoscaler { - kpa.Status = status - return kpa -} - // Build is a special case of resource creation because it isn't owned by // the Revision, just tracked. -func build(namespace, name string, conds ...duckv1alpha1.Condition) *unstructured.Unstructured { +func build(namespace, name string, bo ...BuildOption) *unstructured.Unstructured { b := &unstructured.Unstructured{} b.SetGroupVersionKind(schema.GroupVersionKind{ Group: "testing.build.knative.dev", @@ -1498,14 +675,17 @@ func build(namespace, name string, conds ...duckv1alpha1.Condition) *unstructure }) b.SetName(name) b.SetNamespace(namespace) - b.Object["status"] = map[string]interface{}{"conditions": conds} u := &unstructured.Unstructured{} duck.FromUnstructured(b, u) // prevent panic in b.DeepCopy() + + for _, opt := range bo { + opt(u) + } return u } -func rev(namespace, name string, image string) *v1alpha1.Revision { - return &v1alpha1.Revision{ +func rev(namespace, name string, image string, ro ...RevisionOption) *v1alpha1.Revision { + r := &v1alpha1.Revision{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -1515,8 +695,27 @@ func rev(namespace, name string, image string) *v1alpha1.Revision { Container: corev1.Container{Image: image}, }, } + + for _, opt := range ro { + opt(r) + } + return r +} + +func WithK8sServiceName(r *v1alpha1.Revision) { + r.Status.ServiceName = svc(r.Namespace, r.Name, r.Spec.Container.Image).Name +} + +// TODO(mattmoor): Come up with a better name for this. +func AllUnknownConditions(r *v1alpha1.Revision) { + WithInitRevConditions(r) + WithNoBuild(r) + MarkDeploying("Deploying")(r) + MarkActivating("Deploying", "")(r) } +type configOption func(*config.Config) + func deploy(namespace, name string, image string, co ...configOption) *appsv1.Deployment { config := ReconcilerTestConfig() for _, opt := range co { @@ -1554,24 +753,37 @@ func fluentdConfigMap(namespace, name string, image string, co ...configOption) return resources.MakeFluentdConfigMap(rev, config.Observability) } -func kpa(namespace, name string, image string) *kpav1alpha1.PodAutoscaler { +func kpa(namespace, name string, image string, ko ...KPAOption) *kpav1alpha1.PodAutoscaler { rev := rev(namespace, name, image) - return resources.MakeKPA(rev) + k := resources.MakeKPA(rev) + + for _, opt := range ko { + opt(k) + } + return k } -func svc(namespace, name string, image string) *corev1.Service { +func svc(namespace, name string, image string, so ...K8sServiceOption) *corev1.Service { rev := rev(namespace, name, image) - return resources.MakeK8sService(rev) + s := resources.MakeK8sService(rev) + for _, opt := range so { + opt(s) + } + return s } -func endpoints(namespace, name string, image string) *corev1.Endpoints { +func endpoints(namespace, name string, image string, eo ...EndpointsOption) *corev1.Endpoints { service := svc(namespace, name, image) - return &corev1.Endpoints{ + ep := &corev1.Endpoints{ ObjectMeta: metav1.ObjectMeta{ Namespace: service.Namespace, Name: service.Name, }, } + for _, opt := range eo { + opt(ep) + } + return ep } type testConfigStore struct { @@ -1590,8 +802,6 @@ func (t *testConfigStore) WatchConfigs(w configmap.Watcher) {} var _ configStore = (*testConfigStore)(nil) -type configOption func(*config.Config) - func ReconcilerTestConfig() *config.Config { return &config.Config{ Controller: getTestControllerConfig(), diff --git a/pkg/reconciler/v1alpha1/service/service_test.go b/pkg/reconciler/v1alpha1/service/service_test.go index c333f0191640..ff660012d41f 100644 --- a/pkg/reconciler/v1alpha1/service/service_test.go +++ b/pkg/reconciler/v1alpha1/service/service_test.go @@ -17,9 +17,9 @@ limitations under the License. package service import ( + "fmt" "testing" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" fakekubeclientset "k8s.io/client-go/kubernetes/fake" @@ -37,53 +37,6 @@ import ( . "github.com/knative/serving/pkg/reconciler/v1alpha1/testing" ) -var ( - boolTrue = true - configSpec = v1alpha1.ConfigurationSpec{ - RevisionTemplate: v1alpha1.RevisionTemplateSpec{ - Spec: v1alpha1.RevisionSpec{ - Container: corev1.Container{ - Image: "busybox", - }, - }, - }, - } - - manualConditions = duckv1alpha1.Conditions{{ - Type: v1alpha1.ServiceConditionConfigurationsReady, - Status: corev1.ConditionUnknown, - Reason: "Manual", - Message: "Service is set to Manual, and is not managing underlying resources.", - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionReady, - Status: corev1.ConditionUnknown, - Reason: "Manual", - Message: "Service is set to Manual, and is not managing underlying resources.", - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionRoutesReady, - Status: corev1.ConditionUnknown, - Reason: "Manual", - Message: "Service is set to Manual, and is not managing underlying resources.", - Severity: "Error", - }} - - initialConditions = duckv1alpha1.Conditions{{ - Type: v1alpha1.ServiceConditionConfigurationsReady, - Status: corev1.ConditionUnknown, - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionReady, - Status: corev1.ConditionUnknown, - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionRoutesReady, - Status: corev1.ConditionUnknown, - Severity: "Error", - }} -) - // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method. func TestReconcile(t *testing.T) { table := TableTest{{ @@ -95,90 +48,102 @@ func TestReconcile(t *testing.T) { }, { Name: "incomplete service", Objects: []runtime.Object{ - // There is no spec.{runLatest,pinned} in this Service to trigger the error condition. - svc("incomplete", "foo", v1alpha1.ServiceSpec{}, initialConditions...), + // There is no spec.{runLatest,pinned} in this Service to + // trigger the error condition. + svc("incomplete", "foo", WithInitSvcConditions), }, Key: "foo/incomplete", WantErr: true, }, { Name: "runLatest - create route and service", Objects: []runtime.Object{ - svcRL("run-latest", "foo"), + svc("run-latest", "foo", WithRunLatestRollout), }, Key: "foo/run-latest", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcRL("run-latest", "foo")), - mustMakeRoute(t, svcRL("run-latest", "foo")), + config("run-latest", "foo", WithRunLatestRollout), + route("run-latest", "foo", WithRunLatestRollout), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("run-latest", "foo", initialConditions...), + Object: svc("run-latest", "foo", WithRunLatestRollout, + // The first reconciliation will initialize the status conditions. + WithInitSvcConditions), }}, }, { Name: "pinned - create route and service", Objects: []runtime.Object{ - svcPin("pinned", "foo"), + svc("pinned", "foo", WithPinnedRollout("pinned-0001")), }, Key: "foo/pinned", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcPin("pinned", "foo")), - mustMakeRoute(t, svcPin("pinned", "foo")), + config("pinned", "foo", WithPinnedRollout("pinned-0001")), + route("pinned", "foo", WithPinnedRollout("pinned-0001")), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcPin("pinned", "foo", initialConditions...), + Object: svc("pinned", "foo", WithPinnedRollout("pinned-0001"), + // The first reconciliation will initialize the status conditions. + WithInitSvcConditions), }}, }, { Name: "release - create route and service", Objects: []runtime.Object{ - svcRelease("release", "foo"), + svc("release", "foo", WithReleaseRollout("release-00001", "release-00002")), }, Key: "foo/release", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcRelease("release", "foo")), - mustMakeRoute(t, svcRelease("release", "foo")), + config("release", "foo", WithReleaseRollout("release-00001", "release-00002")), + route("release", "foo", WithReleaseRollout("release-00001", "release-00002")), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRelease("release", "foo", initialConditions...), + Object: svc("release", "foo", WithReleaseRollout("release-00001", "release-00002"), + // The first reconciliation will initialize the status conditions. + WithInitSvcConditions), }}, }, { Name: "manual- no creates", Objects: []runtime.Object{ - svcManual("manual", "foo"), + svc("manual", "foo", WithManualRollout), }, Key: "foo/manual", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcManual("manual", "foo", manualConditions...), + Object: svc("manual", "foo", WithManualRollout, + // The first reconciliation will initialize the status conditions. + WithManualStatus), }}, }, { Name: "runLatest - no updates", Objects: []runtime.Object{ - svcRL("no-updates", "foo", initialConditions...), - mustMakeRoute(t, svcRL("no-updates", "foo", initialConditions...)), - mustMakeConfig(t, svcRL("no-updates", "foo", initialConditions...)), + svc("no-updates", "foo", WithRunLatestRollout, WithInitSvcConditions), + route("no-updates", "foo", WithRunLatestRollout), + config("no-updates", "foo", WithRunLatestRollout), }, Key: "foo/no-updates", }, { Name: "runLatest - update route and service", Objects: []runtime.Object{ - svcRL("update-route-and-config", "foo", initialConditions...), - // Update the skeletal Config/Route to have the appropriate {Config,Route}Specs - mutateConfig(mustMakeConfig(t, svcRL("update-route-and-config", "foo", initialConditions...))), - mutateRoute(mustMakeRoute(t, svcRL("update-route-and-config", "foo", initialConditions...))), + svc("update-route-and-config", "foo", WithRunLatestRollout, WithInitSvcConditions), + // Mutate the Config/Route to have a different body than we want. + config("update-route-and-config", "foo", WithRunLatestRollout, + // Change the concurrency model to ensure it is corrected. + WithConfigConcurrencyModel("Single")), + route("update-route-and-config", "foo", WithRunLatestRollout, MutateRoute), }, Key: "foo/update-route-and-config", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: mustMakeConfig(t, svcRL("update-route-and-config", "foo", initialConditions...)), + Object: config("update-route-and-config", "foo", WithRunLatestRollout), }, { - Object: mustMakeRoute(t, svcRL("update-route-and-config", "foo", initialConditions...)), + Object: route("update-route-and-config", "foo", WithRunLatestRollout), }}, }, { Name: "runLatest - bad config update", Objects: []runtime.Object{ // There is no spec.{runLatest,pinned} in this Service, which triggers the error // path updating Configuration. - svc("bad-config-update", "foo", v1alpha1.ServiceSpec{}, initialConditions...), - // Update the skeletal Config/Route to have the appropriate {Config,Route}Specs - mutateConfig(mustMakeConfig(t, svcRL("bad-config-update", "foo", initialConditions...))), - mutateRoute(mustMakeRoute(t, svcRL("bad-config-update", "foo", initialConditions...))), + svc("bad-config-update", "foo", WithInitSvcConditions), + config("bad-config-update", "foo", WithRunLatestRollout, + // Change the concurrency model to ensure it is corrected. + WithConfigConcurrencyModel("Single")), + route("bad-config-update", "foo", WithRunLatestRollout, MutateRoute), }, Key: "foo/bad-config-update", WantErr: true, @@ -190,15 +155,17 @@ func TestReconcile(t *testing.T) { InduceFailure("create", "routes"), }, Objects: []runtime.Object{ - svcRL("create-route-failure", "foo"), + svc("create-route-failure", "foo", WithRunLatestRollout), }, Key: "foo/create-route-failure", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcRL("create-route-failure", "foo")), - mustMakeRoute(t, svcRL("create-route-failure", "foo")), + config("create-route-failure", "foo", WithRunLatestRollout), + route("create-route-failure", "foo", WithRunLatestRollout), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("create-route-failure", "foo", initialConditions...), + Object: svc("create-route-failure", "foo", WithRunLatestRollout, + // First reconcile initializes conditions. + WithInitSvcConditions), }}, }, { Name: "runLatest - configuration creation failure", @@ -208,15 +175,17 @@ func TestReconcile(t *testing.T) { InduceFailure("create", "configurations"), }, Objects: []runtime.Object{ - svcRL("create-config-failure", "foo"), + svc("create-config-failure", "foo", WithRunLatestRollout), }, Key: "foo/create-config-failure", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcRL("create-config-failure", "foo")), + config("create-config-failure", "foo", WithRunLatestRollout), // We don't get to creating the Route. }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("create-config-failure", "foo", initialConditions...), + Object: svc("create-config-failure", "foo", WithRunLatestRollout, + // First reconcile initializes conditions. + WithInitSvcConditions), }}, }, { Name: "runLatest - update route failure", @@ -226,16 +195,14 @@ func TestReconcile(t *testing.T) { InduceFailure("update", "routes"), }, Objects: []runtime.Object{ - svcRL("update-route-failure", "foo", initialConditions...), - // Update the skeletal Config/Route to have the appropriate {Config,Route}Specs - mutateRoute(mustMakeRoute(t, svcRL("update-route-failure", "foo", initialConditions...))), - mutateConfig(mustMakeConfig(t, svcRL("update-route-failure", "foo", initialConditions...))), + svc("update-route-failure", "foo", WithRunLatestRollout, WithInitSvcConditions), + // Mutate the Route to have an unexpected body to trigger an update. + route("update-route-failure", "foo", WithRunLatestRollout, MutateRoute), + config("update-route-failure", "foo", WithRunLatestRollout), }, Key: "foo/update-route-failure", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: mustMakeConfig(t, svcRL("update-route-failure", "foo", initialConditions...)), - }, { - Object: mustMakeRoute(t, svcRL("update-route-failure", "foo", initialConditions...)), + Object: route("update-route-failure", "foo", WithRunLatestRollout), }}, }, { Name: "runLatest - update config failure", @@ -245,15 +212,16 @@ func TestReconcile(t *testing.T) { InduceFailure("update", "configurations"), }, Objects: []runtime.Object{ - svcRL("update-config-failure", "foo", initialConditions...), - // Update the skeletal Config/Route to have the appropriate {Config,Route}Specs - mutateRoute(mustMakeRoute(t, svcRL("update-config-failure", "foo", initialConditions...))), - mutateConfig(mustMakeConfig(t, svcRL("update-config-failure", "foo", initialConditions...))), + svc("update-config-failure", "foo", WithRunLatestRollout, WithInitSvcConditions), + route("update-config-failure", "foo", WithRunLatestRollout), + // Mutate the Config to have an unexpected body to trigger an update. + config("update-config-failure", "foo", WithRunLatestRollout, + // Change the concurrency model to ensure it is corrected. + WithConfigConcurrencyModel("Single")), }, Key: "foo/update-config-failure", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: mustMakeConfig(t, svcRL("update-config-failure", "foo", initialConditions...)), - // We don't get to updating the Route. + Object: config("update-config-failure", "foo", WithRunLatestRollout), }}, }, { Name: "runLatest - failure updating service status", @@ -263,135 +231,72 @@ func TestReconcile(t *testing.T) { InduceFailure("update", "services"), }, Objects: []runtime.Object{ - svcRL("run-latest", "foo"), + svc("run-latest", "foo", WithRunLatestRollout), }, Key: "foo/run-latest", WantCreates: []metav1.Object{ - mustMakeConfig(t, svcRL("run-latest", "foo")), - mustMakeRoute(t, svcRL("run-latest", "foo")), + config("run-latest", "foo", WithRunLatestRollout), + route("run-latest", "foo", WithRunLatestRollout), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("run-latest", "foo", initialConditions...), + Object: svc("run-latest", "foo", WithRunLatestRollout, + // We attempt to update the Service to initialize its + // conditions, which is where we induce the failure. + WithInitSvcConditions), }}, }, { Name: "runLatest - route and config ready, propagate ready", // When both route and config are ready, the service should become ready. Objects: []runtime.Object{ - svcRL("all-ready", "foo", initialConditions...), - routeWithStatus(mustMakeRoute(t, svcRL("all-ready", "foo", initialConditions...)), - v1alpha1.RouteStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.RouteConditionReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}, - }), - cfgWithStatus(mustMakeConfig(t, svcRL("all-ready", "foo", initialConditions...)), - v1alpha1.ConfigurationStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.ConfigurationConditionReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}, - }), + svc("all-ready", "foo", WithRunLatestRollout, WithInitSvcConditions), + route("all-ready", "foo", WithRunLatestRollout, RouteReady), + config("all-ready", "foo", WithRunLatestRollout, WithGeneration(1), + // These turn a Configuration to Ready=true + WithLatestCreated, WithLatestReady), }, Key: "foo/all-ready", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("all-ready", "foo", duckv1alpha1.Conditions{{ - Type: v1alpha1.ServiceConditionConfigurationsReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionRoutesReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}...), + Object: svc("all-ready", "foo", WithRunLatestRollout, + // When the Route and Configuration come back with ready + // our initial conditions transition to Ready=True. + // TODO(mattmoor): Add Latest{Created,Ready} + WithReadyRoute, WithReadyConfig("all-ready-00001")), }}, }, { Name: "runLatest - config fails, propagate failure", // When config fails, the service should fail. Objects: []runtime.Object{ - svcRL("config-fails", "foo", initialConditions...), - routeWithStatus(mustMakeRoute(t, svcRL("config-fails", "foo", initialConditions...)), - v1alpha1.RouteStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.RouteConditionReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}, - }), - cfgWithStatus(mustMakeConfig(t, svcRL("config-fails", "foo", initialConditions...)), - v1alpha1.ConfigurationStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.ConfigurationConditionReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }}, - }), + svc("config-fails", "foo", WithRunLatestRollout, WithInitSvcConditions), + route("config-fails", "foo", WithRunLatestRollout, RouteReady), + config("config-fails", "foo", WithRunLatestRollout, WithGeneration(1), + WithLatestCreated, MarkLatestCreatedFailed("blah")), }, Key: "foo/config-fails", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("config-fails", "foo", duckv1alpha1.Conditions{{ - Type: v1alpha1.ServiceConditionConfigurationsReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionRoutesReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}...), + Object: svc("config-fails", "foo", WithRunLatestRollout, WithInitSvcConditions, + // When the Route is Ready, and the Configuration has failed, + // we expect the following changes to our status conditions. + WithReadyRoute, WithFailedConfig( + "config-fails-00001", "RevisionFailed", "blah")), }}, }, { Name: "runLatest - route fails, propagate failure", // When route fails, the service should fail. Objects: []runtime.Object{ - svcRL("route-fails", "foo", initialConditions...), - routeWithStatus(mustMakeRoute(t, svcRL("route-fails", "foo", initialConditions...)), - v1alpha1.RouteStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.RouteConditionReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }}, - }), - cfgWithStatus(mustMakeConfig(t, svcRL("route-fails", "foo", initialConditions...)), - v1alpha1.ConfigurationStatus{ - Conditions: duckv1alpha1.Conditions{{ - Type: v1alpha1.ConfigurationConditionReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }}, - }), + svc("route-fails", "foo", WithRunLatestRollout, WithInitSvcConditions), + route("route-fails", "foo", WithRunLatestRollout, + RouteFailed("Propagate me, please", "")), + config("route-fails", "foo", WithRunLatestRollout, WithGeneration(1), + // These turn a Configuration to Ready=true + WithLatestCreated, WithLatestReady), }, Key: "foo/route-fails", WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svcRL("route-fails", "foo", duckv1alpha1.Conditions{{ - Type: v1alpha1.ServiceConditionConfigurationsReady, - Status: corev1.ConditionTrue, - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }, { - Type: v1alpha1.ServiceConditionRoutesReady, - Status: corev1.ConditionFalse, - Reason: "Propagate me, please", - Severity: "Error", - }}...), + Object: svc("route-fails", "foo", WithRunLatestRollout, WithInitSvcConditions, + // When the Configuration is Ready, and the Route has failed, + // we expect the following changed to our status conditions. + WithReadyConfig("route-fails-00001"), + WithFailedRoute("Propagate me, please", "")), }}, }} @@ -427,79 +332,66 @@ func TestNew(t *testing.T) { } } -func svc(name, namespace string, spec v1alpha1.ServiceSpec, conditions ...duckv1alpha1.Condition) *v1alpha1.Service { - return &v1alpha1.Service{ +func svc(name, namespace string, so ...ServiceOption) *v1alpha1.Service { + s := &v1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, - Spec: spec, - Status: v1alpha1.ServiceStatus{ - Conditions: conditions, - }, } + for _, opt := range so { + opt(s) + } + return s } -func svcRL(name, namespace string, conditions ...duckv1alpha1.Condition) *v1alpha1.Service { - return svc(name, namespace, v1alpha1.ServiceSpec{ - RunLatest: &v1alpha1.RunLatestType{Configuration: configSpec}, - }, conditions...) -} - -func svcPin(name, namespace string, conditions ...duckv1alpha1.Condition) *v1alpha1.Service { - return svc(name, namespace, v1alpha1.ServiceSpec{ - Pinned: &v1alpha1.PinnedType{RevisionName: "pinned-0001", Configuration: configSpec}, - }, conditions...) -} - -func svcRelease(name, namespace string, conditions ...duckv1alpha1.Condition) *v1alpha1.Service { - return svc(name, namespace, v1alpha1.ServiceSpec{ - Release: &v1alpha1.ReleaseType{Revisions: []string{"release-00001", "release-00002"}, RolloutPercent: 49, Configuration: configSpec}, - }, conditions...) -} - -func svcManual(name, namespace string, conditions ...duckv1alpha1.Condition) *v1alpha1.Service { - return svc(name, namespace, v1alpha1.ServiceSpec{ - Manual: &v1alpha1.ManualType{}, - }, conditions...) -} - -func mustMakeConfig(t *testing.T, svc *v1alpha1.Service) *v1alpha1.Configuration { - cfg, err := resources.MakeConfiguration(svc) +func config(name, namespace string, so ServiceOption, co ...ConfigOption) *v1alpha1.Configuration { + s := svc(name, namespace, so) + cfg, err := resources.MakeConfiguration(s) if err != nil { - t.Fatalf("MakeConfiguration() = %v", err) + panic(fmt.Sprintf("MakeConfiguration() = %v", err)) + } + for _, opt := range co { + opt(cfg) } return cfg } -func mustMakeRoute(t *testing.T, svc *v1alpha1.Service) *v1alpha1.Route { - route, err := resources.MakeRoute(svc) +func route(name, namespace string, so ServiceOption, ro ...RouteOption) *v1alpha1.Route { + s := svc(name, namespace, so) + route, err := resources.MakeRoute(s) if err != nil { - t.Fatalf("MakeRoute() = %v", err) + panic(fmt.Sprintf("MakeRoute() = %v", err)) + } + for _, opt := range ro { + opt(route) } return route } -// mutateConfig mutates the specification of the Configuration to simulate someone editing it around our controller. -func mutateConfig(cfg *v1alpha1.Configuration) *v1alpha1.Configuration { - cfg.Spec = v1alpha1.ConfigurationSpec{} - return cfg -} - -// mutateRoute mutates the specification of the Route to simulate someone editing it around our controller. -func mutateRoute(rt *v1alpha1.Route) *v1alpha1.Route { +// TODO(mattmoor): Replace these when we refactor Route's table_test.go +func MutateRoute(rt *v1alpha1.Route) { rt.Spec = v1alpha1.RouteSpec{} - return rt } -// TODO(#1762): Replace with builders. -func cfgWithStatus(cfg *v1alpha1.Configuration, s v1alpha1.ConfigurationStatus) *v1alpha1.Configuration { - cfg.Status = s - return cfg +func RouteReady(cfg *v1alpha1.Route) { + cfg.Status = v1alpha1.RouteStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "True", + }}, + } } -// TODO(#1762): Replace with builders. -func routeWithStatus(rt *v1alpha1.Route, s v1alpha1.RouteStatus) *v1alpha1.Route { - rt.Status = s - return rt +func RouteFailed(reason, message string) RouteOption { + return func(cfg *v1alpha1.Route) { + cfg.Status = v1alpha1.RouteStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "False", + Reason: reason, + Message: message, + }}, + } + } } diff --git a/pkg/reconciler/v1alpha1/testing/functional.go b/pkg/reconciler/v1alpha1/testing/functional.go index 7657a8d1f3ca..5b55ab719c06 100644 --- a/pkg/reconciler/v1alpha1/testing/functional.go +++ b/pkg/reconciler/v1alpha1/testing/functional.go @@ -17,8 +17,11 @@ limitations under the License. package testing import ( + "fmt" "time" + "github.com/knative/pkg/apis" + "github.com/knative/pkg/apis/duck" duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1" kpav1alpha1 "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1" "github.com/knative/serving/pkg/apis/serving/v1alpha1" @@ -31,6 +34,47 @@ import ( // BuildOption enables further configuration of a Build. type BuildOption func(*unstructured.Unstructured) +// WithSucceededTrue updates the status of the provided unstructured Build object with the +// expected success condition. +func WithSucceededTrue(orig *unstructured.Unstructured) { + cp := orig.DeepCopy() + cp.Object["status"] = map[string]interface{}{"conditions": []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}} + duck.FromUnstructured(cp, orig) // prevent panic in b.DeepCopy() +} + +// WithSucceededUnknown updates the status of the provided unstructured Build object with the +// expected in-flight condition. +func WithSucceededUnknown(reason, message string) BuildOption { + return func(orig *unstructured.Unstructured) { + cp := orig.DeepCopy() + cp.Object["status"] = map[string]interface{}{"conditions": []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: reason, + Message: message, + }}} + duck.FromUnstructured(cp, orig) // prevent panic in b.DeepCopy() + } +} + +// WithSucceededFalse updates the status of the provided unstructured Build object with the +// expected failure condition. +func WithSucceededFalse(reason, message string) BuildOption { + return func(orig *unstructured.Unstructured) { + cp := orig.DeepCopy() + cp.Object["status"] = map[string]interface{}{"conditions": []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: reason, + Message: message, + }}} + duck.FromUnstructured(cp, orig) // prevent panic in b.DeepCopy() + } +} + // ServiceOption enables further configuration of a Service. type ServiceOption func(*v1alpha1.Service) @@ -47,6 +91,116 @@ var ( } ) +// WithRunLatestRollout configures the Service to use a "runLatest" rollout. +func WithRunLatestRollout(s *v1alpha1.Service) { + s.Spec = v1alpha1.ServiceSpec{ + RunLatest: &v1alpha1.RunLatestType{ + Configuration: configSpec, + }, + } +} + +// WithPinnedRollout configures the Service to use a "pinned" rollout, +// which is pinned to the named revision. +func WithPinnedRollout(name string) ServiceOption { + return func(s *v1alpha1.Service) { + s.Spec = v1alpha1.ServiceSpec{ + Pinned: &v1alpha1.PinnedType{ + RevisionName: name, + Configuration: configSpec, + }, + } + } +} + +// WithReleaseRollout configures the Service to use a "release" rollout, +// which spans the provided revisions. +func WithReleaseRollout(names ...string) ServiceOption { + return func(s *v1alpha1.Service) { + s.Spec = v1alpha1.ServiceSpec{ + Release: &v1alpha1.ReleaseType{ + Revisions: names, + Configuration: configSpec, + }, + } + } +} + +// WithManualRollout configures the Service to use a "manual" rollout. +func WithManualRollout(s *v1alpha1.Service) { + s.Spec = v1alpha1.ServiceSpec{ + Manual: &v1alpha1.ManualType{}, + } +} + +// WithInitSvcConditions initializes the Service's conditions. +func WithInitSvcConditions(s *v1alpha1.Service) { + s.Status.InitializeConditions() +} + +// WithManualStatus configures the Service to have the appropriate +// status for a "manual" rollout type. +func WithManualStatus(s *v1alpha1.Service) { + s.Status.SetManualStatus() +} + +// WithReadyRoute reflects the Route's readiness in the Service resource. +func WithReadyRoute(s *v1alpha1.Service) { + s.Status.PropagateRouteStatus(v1alpha1.RouteStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "True", + }}, + }) +} + +// WithFailedRoute reflects a Route's failure in the Service resource. +func WithFailedRoute(reason, message string) ServiceOption { + return func(s *v1alpha1.Service) { + s.Status.PropagateRouteStatus(v1alpha1.RouteStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "False", + Reason: reason, + Message: message, + }}, + }) + } +} + +// WithReadyConfig reflects the Configuration's readiness in the Service +// resource. This must coincide with the setting of Latest{Created,Ready} +// to the provided revision name. +func WithReadyConfig(name string) ServiceOption { + return func(s *v1alpha1.Service) { + s.Status.PropagateConfigurationStatus(v1alpha1.ConfigurationStatus{ + LatestCreatedRevisionName: name, + LatestReadyRevisionName: name, + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "True", + }}, + }) + } +} + +// WithFailedConfig reflects the Configuration's failure in the Service +// resource. The failing revision's name is reflected in LatestCreated. +func WithFailedConfig(name, reason, message string) ServiceOption { + return func(s *v1alpha1.Service) { + s.Status.PropagateConfigurationStatus(v1alpha1.ConfigurationStatus{ + LatestCreatedRevisionName: name, + Conditions: []duckv1alpha1.Condition{{ + Type: "Ready", + Status: "False", + Reason: reason, + Message: fmt.Sprintf("Revision %q failed with message: %q.", + name, message), + }}, + }) + } +} + // RouteOption enables further configuration of a Route. type RouteOption func(*v1alpha1.Route) @@ -147,6 +301,11 @@ func WithRevConcurrencyModel(ss v1alpha1.RevisionRequestConcurrencyModelType) Re } } +// WithLogURL sets the .Status.LogURL to the expected value. +func WithLogURL(r *v1alpha1.Revision) { + r.Status.LogURL = "http://logger.io/test-uid" +} + // WithCreationTimestamp sets the Revision's timestamp to the provided time. // TODO(mattmoor): Ideally this could be a more generic Option and use meta.Accessor, // but unfortunately Go's type system cannot support that. @@ -168,6 +327,53 @@ func WithNoBuild(r *v1alpha1.Revision) { }) } +// WithOngoingBuild propagates the status of an in-progress Build to the Revision's status. +func WithOngoingBuild(r *v1alpha1.Revision) { + r.Status.PropagateBuildStatus(duckv1alpha1.KResourceStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }}, + }) +} + +// WithSuccessfulBuild propagates the status of a successful Build to the Revision's status. +func WithSuccessfulBuild(r *v1alpha1.Revision) { + r.Status.PropagateBuildStatus(duckv1alpha1.KResourceStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}, + }) +} + +// WithFailedBuild propagates the status of a failed Build to the Revision's status. +func WithFailedBuild(reason, message string) RevisionOption { + return func(r *v1alpha1.Revision) { + r.Status.PropagateBuildStatus(duckv1alpha1.KResourceStatus{ + Conditions: []duckv1alpha1.Condition{{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: reason, + Message: message, + }}, + }) + } +} + +// WithEmptyLTTs clears the LastTransitionTime fields on all of the conditions of the +// provided Revision. +func WithEmptyLTTs(r *v1alpha1.Revision) { + conds := r.Status.Conditions + for i, c := range conds { + // The LTT defaults and is long enough ago that we expire waiting + // on the Endpoints to become ready. + c.LastTransitionTime = apis.VolatileTime{} + conds[i] = c + } + r.Status.SetConditions(conds) +} + // WithLastPinned updates the "last pinned" annotation to the provided timestamp. func WithLastPinned(t time.Time) RevisionOption { return func(rev *v1alpha1.Revision) { @@ -188,6 +394,38 @@ func MarkActive(r *v1alpha1.Revision) { r.Status.MarkActive() } +// MarkInactive calls .Status.MarkInactive on the Revision. +func MarkInactive(reason, message string) RevisionOption { + return func(r *v1alpha1.Revision) { + r.Status.MarkInactive(reason, message) + } +} + +// MarkActivating calls .Status.MarkActivating on the Revision. +func MarkActivating(reason, message string) RevisionOption { + return func(r *v1alpha1.Revision) { + r.Status.MarkActivating(reason, message) + } +} + +// MarkDeploying calls .Status.MarkDeploying on the Revision. +func MarkDeploying(reason string) RevisionOption { + return func(r *v1alpha1.Revision) { + r.Status.MarkDeploying(reason) + } +} + +// MarkProgressDeadlineExceeded calls the method of the same name on the Revision +// with the message we expect the Revision Reconciler to pass. +func MarkProgressDeadlineExceeded(r *v1alpha1.Revision) { + r.Status.MarkProgressDeadlineExceeded("Unable to create pods for more than 120 seconds.") +} + +// MarkServiceTimeout calls .Status.MarkServiceTimeout on the Revision. +func MarkServiceTimeout(r *v1alpha1.Revision) { + r.Status.MarkServiceTimeout() +} + // MarkContainerMissing calls .Status.MarkContainerMissing on the Revision. func MarkContainerMissing(rev *v1alpha1.Revision) { rev.Status.MarkContainerMissing("It's the end of the world as we know it") @@ -205,8 +443,42 @@ func MarkRevisionReady(r *v1alpha1.Revision) { // KPAOption enables further configuration of the PodAutoscaler. type KPAOption func(*kpav1alpha1.PodAutoscaler) +// WithTraffic updates the KPA to reflect it receiving traffic. +func WithTraffic(kpa *kpav1alpha1.PodAutoscaler) { + kpa.Status.MarkActive() +} + +// WithBufferedTraffic updates the KPA to reflect that it has received +// and buffered traffic while it is being activated. +func WithBufferedTraffic(reason, message string) KPAOption { + return func(kpa *kpav1alpha1.PodAutoscaler) { + kpa.Status.MarkActivating(reason, message) + } +} + +// WithNoTraffic updates the KPA to reflect the fact that it is not +// receiving traffic. +func WithNoTraffic(reason, message string) KPAOption { + return func(kpa *kpav1alpha1.PodAutoscaler) { + kpa.Status.MarkInactive(reason, message) + } +} + // K8sServiceOption enables further configuration of the Kubernetes Service. type K8sServiceOption func(*corev1.Service) +// MutateK8sService changes the service in a way that must be reconciled. +func MutateK8sService(svc *corev1.Service) { + // An effective hammer ;-P + svc.Spec = corev1.ServiceSpec{} +} + // EndpointsOption enables further configuration of the Kubernetes Endpoints. type EndpointsOption func(*corev1.Endpoints) + +// WithSubsets adds subsets to the body of a Revision, enabling us to refer readiness. +func WithSubsets(ep *corev1.Endpoints) { + ep.Subsets = []corev1.EndpointSubset{{ + Addresses: []corev1.EndpointAddress{{IP: "127.0.0.1"}}, + }} +}