Skip to content

Ensure we are testing scheme registration for termination handler #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 3 commits into from
Jul 29, 2020
Merged

Ensure we are testing scheme registration for termination handler #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4474c9
Ensure scheme registration is tested in handler tests
JoelSpeed Jul 15, 2020
d6e676f
Ensure tests aren't racing by only running handler when needed
JoelSpeed Jul 15, 2020
e463604
Use atomic to adjust counter in termination test
JoelSpeed Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 116 additions & 108 deletions pkg/termination/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,18 @@ var _ = Describe("Handler Suite", func() {
httpHandler = nil
nodeName = "test-node"
httpHandler = newMockHTTPHandler(notFoundFunc)
stop = nil
Copy link

@Danil-Grigorev Danil-Grigorev Jul 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this should be the fix for unit flakes, re: #340 (comment)

errs = nil

h = &handler{
client: k8sClient,
pollInterval: 100 * time.Millisecond,
nodeName: nodeName,
log: klogr.New(),
}
// use NewHandler() instead of manual construction in order to test NewHandler() logic
// like checking that machine api is added to scheme
handlerInterface, err := NewHandler(klogr.New(), cfg, 100*time.Millisecond, "", nodeName)
Expect(err).ToNot(HaveOccurred())

h = handlerInterface.(*handler)

// set pollURL so we can override initial value later
h.pollURL = nil
})

JustBeforeEach(func() {
Expand All @@ -68,149 +73,152 @@ var _ = Describe("Handler Suite", func() {
Expect(err).ToNot(HaveOccurred())
h.pollURL = pollURL
}

stop, errs = StartTestHandler(h)
})

AfterEach(func() {
if !isClosed(stop) {
if stop != nil && !isClosed(stop) {
close(stop)
}
terminationServer.Close()

Expect(deleteAllMachines(k8sClient)).To(Succeed())
})

Context("when the handler is stopped", func() {
Context("when running the handler", func() {
JustBeforeEach(func() {
close(stop)
})

It("should not return an error", func() {
Eventually(errs).Should(Receive(BeNil()))
})
})

Context("when no machine exists for the node", func() {
It("should return an error upon starting", func() {
Eventually(errs).Should(Receive(MatchError("error fetching machine for node (\"test-node\"): machine not found for node \"test-node\"")))
})
})

Context("when a machine exists for the node", func() {
var counter int32
var testMachine *machinev1.Machine

BeforeEach(func() {
testMachine = newTestMachine("test-machine", testNamespace, nodeName)
createMachine(testMachine)

// Ensure the polling logic is excercised in tests
httpHandler = newMockHTTPHandler(func(rw http.ResponseWriter, req *http.Request) {
if atomic.LoadInt32(&counter) == 4 {
rw.WriteHeader(200)
} else {
atomic.AddInt32(&counter, 1)
rw.WriteHeader(404)
}
})
stop, errs = StartTestHandler(h)
})

JustBeforeEach(func() {
// Ensure the polling logic is excercised in tests
for atomic.LoadInt32(&counter) < 4 {
continue
}
})

Context("and the handler is stopped", func() {
Context("when the handler is stopped", func() {
JustBeforeEach(func() {
close(stop)
})

It("should not return an error", func() {
Eventually(errs).Should(Receive(BeNil()))
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})

Context("and the instance termination notice is fulfilled", func() {
It("should delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Eventually(func() error {
m := &machinev1.Machine{}
err := k8sClient.Get(ctx, key, m)
if err != nil && errors.IsNotFound(err) {
return nil
} else if err != nil {
return err
}
return fmt.Errorf("machine not yet deleted")
}).Should(Succeed())
Context("when no machine exists for the node", func() {
It("should return an error upon starting", func() {
Eventually(errs).Should(Receive(MatchError("error fetching machine for node (\"test-node\"): machine not found for node \"test-node\"")))
})
})

Context("and the instance termination notice is not fulfilled", func() {
BeforeEach(func() {
httpHandler = newMockHTTPHandler(notFoundFunc)
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})
Context("when a machine exists for the node", func() {
var counter int32
var testMachine *machinev1.Machine

Context("and the instance termination endpoint returns an unknown status", func() {
BeforeEach(func() {
testMachine = newTestMachine("test-machine", testNamespace, nodeName)
createMachine(testMachine)

// Ensure the polling logic is excercised in tests
httpHandler = newMockHTTPHandler(func(rw http.ResponseWriter, req *http.Request) {
if counter == 4 {
rw.WriteHeader(500)
if atomic.LoadInt32(&counter) == 4 {
rw.WriteHeader(200)
} else {
counter++
atomic.AddInt32(&counter, 1)
rw.WriteHeader(404)
}
})
})

It("should return an error", func() {
Eventually(errs).Should(Receive(MatchError("error polling termination endpoint: unexpected status: 500")))
JustBeforeEach(func() {
// Ensure the polling logic is excercised in tests
for atomic.LoadInt32(&counter) < 4 {
continue
}
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
Context("and the handler is stopped", func() {
JustBeforeEach(func() {
close(stop)
})

It("should not return an error", func() {
Eventually(errs).Should(Receive(BeNil()))
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})
})

Context("and the poll URL cannot be reached", func() {
BeforeEach(func() {
h.pollURL = &url.URL{Opaque: "abc#1://localhost"}
Context("and the instance termination notice is fulfilled", func() {
It("should delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Eventually(func() error {
m := &machinev1.Machine{}
err := k8sClient.Get(ctx, key, m)
if err != nil && errors.IsNotFound(err) {
return nil
} else if err != nil {
return err
}
return fmt.Errorf("machine not yet deleted")
}).Should(Succeed())
})
})

It("should return an error", func() {
Eventually(errs).Should(Receive(MatchError("error polling termination endpoint: could not get URL \"abc#1://localhost\": Get abc#1://localhost: unsupported protocol scheme \"\"")))
Context("and the instance termination notice is not fulfilled", func() {
BeforeEach(func() {
httpHandler = newMockHTTPHandler(notFoundFunc)
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})

Context("and the instance termination endpoint returns an unknown status", func() {
BeforeEach(func() {
httpHandler = newMockHTTPHandler(func(rw http.ResponseWriter, req *http.Request) {
if atomic.LoadInt32(&counter) == 4 {
rw.WriteHeader(500)
} else {
atomic.AddInt32(&counter, 1)
rw.WriteHeader(404)
}
})
})

It("should return an error", func() {
Eventually(errs).Should(Receive(MatchError("error polling termination endpoint: unexpected status: 500")))
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
Context("and the poll URL cannot be reached", func() {
BeforeEach(func() {
h.pollURL = &url.URL{Opaque: "abc#1://localhost"}
})

It("should return an error", func() {
Eventually(errs).Should(Receive(MatchError("error polling termination endpoint: could not get URL \"abc#1://localhost\": Get abc#1://localhost: unsupported protocol scheme \"\"")))
})

It("should not delete the machine", func() {
key := client.ObjectKey{Namespace: testMachine.Namespace, Name: testMachine.Name}
Consistently(func() error {
m := &machinev1.Machine{}
return k8sClient.Get(ctx, key, m)
}).Should(Succeed())
})
})
})
})
Expand Down
12 changes: 9 additions & 3 deletions pkg/termination/termination_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/apimachinery/pkg/runtime"
kubernetesscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
Expand Down Expand Up @@ -56,14 +57,19 @@ var _ = BeforeSuite(func() {
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crds")},
}
machinev1.AddToScheme(scheme.Scheme)

// Use our own scheme so we don't interfere with any test cases
// that would initialise the scheme themselves.
scheme := runtime.NewScheme()
kubernetesscheme.AddToScheme(scheme)
machinev1.AddToScheme(scheme)

var err error
cfg, err = testEnv.Start()
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())

k8sClient, err = client.New(cfg, client.Options{})
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme})
Expect(err).ToNot(HaveOccurred())

Expect(k8sClient.Create(ctx, &corev1.Namespace{
Expand Down