Skip to content

Commit

Permalink
Change cleanup func to take pointer to names (#8566)
Browse files Browse the repository at this point in the history
* Change cleanup func to take pointer to names

if the names change, it will still cleanup the proper resource.
Fixes #8560

* also perf tests

* :upgrade:

* one more
  • Loading branch information
vagababov authored Jul 7, 2020
1 parent 77c03f0 commit d74ecbe
Show file tree
Hide file tree
Showing 70 changed files with 120 additions and 124 deletions.
4 changes: 2 additions & 2 deletions test/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func CleanupOnInterrupt(cleanup func()) {
}

// TearDown will delete created names using clients.
func TearDown(clients *Clients, names ResourceNames) {
func TearDown(clients *Clients, names *ResourceNames) {
if clients != nil && clients.ServingClient != nil {
clients.ServingClient.Delete(
[]string{names.Route},
Expand All @@ -79,6 +79,6 @@ func EnsureCleanup(t *testing.T, cleanup func()) {

// EnsureTearDown will delete created names when the test ends, either via
// t.Cleanup, or on interrupt via CleanupOnInterrupt.
func EnsureTearDown(t *testing.T, clients *Clients, names ResourceNames) {
func EnsureTearDown(t *testing.T, clients *Clients, names *ResourceNames) {
EnsureCleanup(t, func() { TearDown(clients, names) })
}
2 changes: 1 addition & 1 deletion test/conformance/api/v1/blue_green_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestBlueGreenRoute(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup Initial Service
t.Log("Creating a new Service in runLatest")
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestUpdateConfigurationMetadata(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Logf("Creating new configuration %s", names.Config)
if _, err := v1test.CreateConfiguration(t, clients, names); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/api/v1/errorcondition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestContainerErrorMsg(t *testing.T) {
Image: test.InvalidHelloWorld,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Specify an invalid image path
// A valid DockerRepo is still needed, otherwise will get UNAUTHORIZED instead of container missing error
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestContainerExitingMsg(t *testing.T) {
Image: test.Failing,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Logf("Creating a new Configuration %s", names.Config)

Expand Down
4 changes: 2 additions & 2 deletions test/conformance/api/v1/generatename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestServiceGenerateName(t *testing.T) {
}

// Cleanup on test failure.
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Create the service using the generate name field. If the service does not become ready this will fail.
t.Log("Creating new service with generateName", generateName)
Expand Down Expand Up @@ -155,7 +155,7 @@ func TestRouteAndConfigGenerateName(t *testing.T) {
Image: test.HelloWorld,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating new configuration with generateName", generateName)
config, err := v1test.CreateConfiguration(t, clients, names, setConfigurationGenerateName(generateName))
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/api/v1/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestTranslation(t *testing.T) {
Image: "helloworld",
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service")
// Create a legacy RunLatest service. This should perform conversion during the webhook
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestV1beta1Rejection(t *testing.T) {
Image: "helloworld",
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service")
// Create a legacy RunLatest service, but give it the TypeMeta of v1.
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestCustomResourcesLimits(t *testing.T) {
Image: test.Autoscale,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

objects, err := v1test.CreateServiceReady(t, clients, &names, withResources)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/revision_timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestRevisionTimeout(t *testing.T) {
Image: test.Timeout,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service ")
resources, err := v1test.CreateServiceReady(t, clients, &names, WithRevisionTimeoutSeconds(tc.timeoutSeconds))
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestRouteCreation(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Route and Configuration")
config, err := v1test.CreateConfiguration(t, clients, names)
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestServiceAccountValidation(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Logf("Creating a new Service %s", names.Service)
service := v1test.Service(names, WithServiceAccountName(invalidServiceAccountName))
Expand Down
8 changes: 4 additions & 4 deletions test/conformance/api/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestService(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup initial Service
objects, err := v1test.CreateServiceReady(t, clients, &names)
Expand Down Expand Up @@ -189,7 +189,7 @@ func TestServiceBYOName(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

revName := names.Service + "-byoname"

Expand Down Expand Up @@ -252,7 +252,7 @@ func TestServiceWithTrafficSplit(t *testing.T) {
Service: test.ObjectNameForTest(t),
Image: test.PizzaPlanet1,
}
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Expected Text for different revisions.
const (
Expand Down Expand Up @@ -490,7 +490,7 @@ func TestAnnotationPropagation(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup initial Service
objects, err := v1test.CreateServiceReady(t, clients, &names)
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1/single_threaded_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestSingleConcurrency(t *testing.T) {
Service: test.ObjectNameForTest(t),
Image: test.SingleThreadedImage,
}
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

objects, err := v1test.CreateServiceReady(t, clients, &names, rtesting.WithContainerConcurrency(1))
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions test/conformance/api/v1/volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestConfigMapVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().ConfigMaps(test.ServingNamespace).Delete(configMap.Name, nil); err != nil {
t.Errorf("ConfigMaps().Delete() = %v", err)
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func TestProjectedConfigMapVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().ConfigMaps(test.ServingNamespace).Delete(configMap.Name, nil); err != nil {
t.Errorf("ConfigMaps().Delete() = %v", err)
}
Expand Down Expand Up @@ -198,7 +198,7 @@ func TestSecretVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down Expand Up @@ -261,7 +261,7 @@ func TestProjectedSecretVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestProjectedComplex(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/blue_green_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestBlueGreenRoute(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup Initial Service
t.Log("Creating a new Service in runLatest")
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestUpdateConfigurationMetadata(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Logf("Creating new configuration %s", names.Config)
if _, err := v1a1test.CreateConfiguration(t, clients, names); err != nil {
Expand Down
5 changes: 2 additions & 3 deletions test/conformance/api/v1alpha1/errorcondition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestContainerErrorMsg(t *testing.T) {
Image: test.InvalidHelloWorld,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Specify an invalid image path
// A valid DockerRepo is still needed, otherwise will get UNAUTHORIZED instead of container missing error
Expand Down Expand Up @@ -162,8 +162,7 @@ func TestContainerExitingMsg(t *testing.T) {
Image: test.Failing,
}

test.EnsureTearDown(t, clients, names)

test.EnsureTearDown(t, clients, &names)
t.Logf("Creating a new Configuration %s", names.Config)

if _, err := v1a1test.CreateConfiguration(t, clients, names, v1a1opts.WithConfigReadinessProbe(tt.ReadinessProbe)); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/api/v1alpha1/generatename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestServiceGenerateName(t *testing.T) {
}

// Cleanup on test failure.
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Create the service using the generate name field. If the service does not become ready this will fail.
t.Log("Creating new service with generateName", generateName)
Expand Down Expand Up @@ -156,7 +156,7 @@ func TestRouteAndConfigGenerateName(t *testing.T) {
Image: test.HelloWorld,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating new configuration with generateName", generateName)
config, err := v1a1test.CreateConfiguration(t, clients, names, setConfigurationGenerateName(generateName))
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestCustomResourcesLimits(t *testing.T) {
Image: test.Autoscale,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

objects, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names,
v1a1opts.WithResourceRequirements(resources))
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/revision_timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestRevisionTimeout(t *testing.T) {
Image: test.Timeout,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service ")
resources, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names, WithRevisionTimeoutSeconds(tc.timeoutSeconds))
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestRouteCreation(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Route and Configuration")
config, err := v1a1test.CreateConfiguration(t, clients, names)
Expand Down
8 changes: 4 additions & 4 deletions test/conformance/api/v1alpha1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestRunLatestService(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup initial Service
objects, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names)
Expand Down Expand Up @@ -189,7 +189,7 @@ func TestRunLatestServiceBYOName(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

revName := names.Service + "-byoname"

Expand Down Expand Up @@ -253,7 +253,7 @@ func TestReleaseService(t *testing.T) {
Service: test.ObjectNameForTest(t),
Image: test.PizzaPlanet1,
}
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Expected Text for different revisions.
const (
Expand Down Expand Up @@ -523,7 +523,7 @@ func TestAnnotationPropagation(t *testing.T) {
}

// Clean up on test failure or interrupt
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup initial Service
objects, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names)
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1alpha1/single_threaded_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestSingleConcurrency(t *testing.T) {
Service: test.ObjectNameForTest(t),
Image: test.SingleThreadedImage,
}
test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

objects, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names,
v1a1opts.WithContainerConcurrency(1))
Expand Down
10 changes: 5 additions & 5 deletions test/conformance/api/v1alpha1/volumes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestConfigMapVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().ConfigMaps(test.ServingNamespace).Delete(configMap.Name, nil); err != nil {
t.Errorf("ConfigMaps().Delete() = %v", err)
}
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestProjectedConfigMapVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().ConfigMaps(test.ServingNamespace).Delete(configMap.Name, nil); err != nil {
t.Errorf("ConfigMaps().Delete() = %v", err)
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func TestSecretVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func TestProjectedSecretVolume(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down Expand Up @@ -349,7 +349,7 @@ func TestProjectedComplex(t *testing.T) {

// Clean up on test failure or interrupt
test.EnsureCleanup(t, func() {
test.TearDown(clients, names)
test.TearDown(clients, &names)
if err := clients.KubeClient.Kube.CoreV1().Secrets(test.ServingNamespace).Delete(secret.Name, nil); err != nil {
t.Errorf("Secrets().Delete() = %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion test/conformance/api/v1beta1/blue_green_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestBlueGreenRoute(t *testing.T) {
Image: test.PizzaPlanet1,
}

test.EnsureTearDown(t, clients, names)
test.EnsureTearDown(t, clients, &names)

// Setup Initial Service
t.Log("Creating a new Service in runLatest")
Expand Down
Loading

0 comments on commit d74ecbe

Please sign in to comment.