Skip to content

Commit

Permalink
Cleanup Webhook server setup
Browse files Browse the repository at this point in the history
- Called the webhook server cleanup function

- Only ignore controlplane cleanup when using existing cluster

- Added test to check the directory exists post stop
  • Loading branch information
Ayush Rangwala committed May 5, 2021
1 parent 0c99fc7 commit 090a9cf
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 10 deletions.
19 changes: 17 additions & 2 deletions pkg/envtest/envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -65,8 +67,8 @@ var _ = Describe("Test", func() {
Name: crd.GetName(),
}
var placeholder v1beta1.CustomResourceDefinition
err := c.Get(context.TODO(), crdObjectKey, &placeholder)
if err != nil && apierrors.IsNotFound(err) {
if err = c.Get(context.TODO(), crdObjectKey, &placeholder); err != nil &&
apierrors.IsNotFound(err) {
// CRD doesn't need to be deleted.
continue
}
Expand Down Expand Up @@ -845,4 +847,17 @@ var _ = Describe("Test", func() {
close(done)
}, 30)
})

Describe("Stop", func() {
It("should cleanup webhook /tmp folder with no error when using existing cluster", func(done Done) {
env := &Environment{}
_, err := env.Start()
Expect(err).NotTo(HaveOccurred())
Expect(env.Stop()).To(Succeed())

// check if the /tmp/envtest-serving-certs-* dir doesnt exists any more
Expect(env.WebhookInstallOptions.LocalServingCertDir).ShouldNot(BeADirectory())
close(done)
}, 30)
})
})
10 changes: 6 additions & 4 deletions pkg/envtest/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,15 @@ func (te *Environment) Stop() error {
return err
}
}

if err := te.WebhookInstallOptions.Cleanup(); err != nil {
return err
}

if te.useExistingCluster() {
return nil
}
err := te.WebhookInstallOptions.Cleanup()
if err != nil {
return err
}

return te.ControlPlane.Stop()
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/envtest/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,12 @@ func (o *WebhookInstallOptions) PrepWithoutInstalling() error {
if err != nil {
return err
}

if err := parseWebhook(o); err != nil {
return err
}

err = o.ModifyWebhookDefinitions(hookCA)
if err != nil {
if err = o.ModifyWebhookDefinitions(hookCA); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/envtest/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,6 @@ var _ = Describe("Test", func() {
type rejectingValidator struct {
}

func (v *rejectingValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
func (v *rejectingValidator) Handle(_ context.Context, _ admission.Request) admission.Response {
return admission.Denied(fmt.Sprint("Always denied"))
}
2 changes: 1 addition & 1 deletion pkg/webhook/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var _ = Describe("Webhook Server", func() {

BeforeEach(func() {
ctx, ctxCancel = context.WithCancel(context.Background())
// closed in indivual tests differently
// closed in individual tests differently

servingOpts = envtest.WebhookInstallOptions{}
Expect(servingOpts.PrepWithoutInstalling()).To(Succeed())
Expand Down

0 comments on commit 090a9cf

Please sign in to comment.