Skip to content

Commit

Permalink
Merge pull request intel#744 from pohly/operator-testing
Browse files Browse the repository at this point in the history 
operator testing
  • Loading branch information
@pohly
pohly authored Sep 25, 2020
2 parents cf0e8d2 + 48a0250 commit 882b7f2
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 63 deletions.
3 changes: 2 additions & 1 deletion deploy/kustomize/operator/operator.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ spec:
fieldRef:
fieldPath: metadata.name
- name: OPERATOR_NAME
# This should match to the operator container name
# This must match to the operator container name.
# The operator uses it to find its image.
value: "pmem-csi-operator"
volumeMounts:
- name: tmp
Expand Down
71 changes: 36 additions & 35 deletions pkg/pmem-csi-operator/controller/deployment/deployment_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,12 @@ func TestDeploymentController(t *testing.T) {

testIt := func(t *testing.T, testK8sVersion version.Version) {
type testContext struct {
c client.Client
cs kubernetes.Interface
rc reconcile.Reconciler
evWatcher watch.Interface
events []*corev1.Event
c client.Client
cs kubernetes.Interface
rc reconcile.Reconciler
evWatcher watch.Interface
events []*corev1.Event
resourceVersions map[string]string
}
const (
testNamespace = "test-namespace"
Expand All @@ -209,9 +210,11 @@ func TestDeploymentController(t *testing.T) {
}

setup := func(t *testing.T) *testContext {
tc := &testContext{}
tc.c = fake.NewFakeClient()
tc.cs = cgfake.NewSimpleClientset()
tc := &testContext{
c: fake.NewFakeClient(),
cs: cgfake.NewSimpleClientset(),
resourceVersions: map[string]string{},
}
tc.rc = newReconcileDeployment(tc.c, tc.cs)
tc.evWatcher = tc.rc.(*deployment.ReconcileDeployment).EventBroadcaster().StartEventWatcher(func(ev *corev1.Event) {
// Discard consecutive duplicate events, mimicking the EventAggregator behavior
Expand Down Expand Up @@ -245,14 +248,20 @@ func TestDeploymentController(t *testing.T) {
require.ElementsMatch(t, events, expectedEvents, "events must match")
}

validateDriver := func(t *testing.T, tc *testContext, dep *api.Deployment, expectedEvents []string) {
validateDriver := func(t *testing.T, tc *testContext, dep *api.Deployment, expectedEvents []string, wasUpdated bool) {
// We may have to fill in some defaults, so make a copy first.
dep = dep.DeepCopyObject().(*api.Deployment)
if dep.Spec.Image == "" {
dep.Spec.Image = testDriverImage
}

require.NoError(t, validate.DriverDeployment(tc.c, testK8sVersion, testNamespace, *dep), "validate deployment")
// If the CR was not updated, then objects should still be the same as they were initially.
rv := tc.resourceVersions
if wasUpdated {
rv = nil
}
_, err := validate.DriverDeployment(tc.c, testK8sVersion, testNamespace, *dep, rv)
require.NoError(t, err, "validate deployment")
validateEvents(t, tc, dep, expectedEvents)
}

Expand All @@ -271,7 +280,7 @@ func TestDeploymentController(t *testing.T) {
require.NoError(t, err, "failed to create deployment")

testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("deployment with explicit values", func(t *testing.T) {
Expand All @@ -297,7 +306,7 @@ func TestDeploymentController(t *testing.T) {

// Reconcile now should change Phase to running
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("multiple deployments", func(t *testing.T) {
Expand All @@ -320,9 +329,9 @@ func TestDeploymentController(t *testing.T) {
require.NoError(t, err, "failed to create deployment2")

testReconcilePhase(t, tc.rc, tc.c, d1.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep1, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep1, []string{api.EventReasonNew, api.EventReasonRunning}, false)
testReconcilePhase(t, tc.rc, tc.c, d2.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep2, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep2, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("invalid device mode", func(t *testing.T) {
Expand Down Expand Up @@ -354,7 +363,7 @@ func TestDeploymentController(t *testing.T) {
err := tc.c.Create(context.TODO(), dep)
require.NoError(t, err, "failed to create deployment")
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("direct mode", func(t *testing.T) {
Expand All @@ -369,7 +378,7 @@ func TestDeploymentController(t *testing.T) {
err := tc.c.Create(context.TODO(), dep)
require.NoError(t, err, "failed to create deployment")
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("provided private keys", func(t *testing.T) {
Expand All @@ -391,7 +400,7 @@ func TestDeploymentController(t *testing.T) {

// First deployment expected to be successful
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("provided private keys and certificates", func(t *testing.T) {
Expand Down Expand Up @@ -424,7 +433,7 @@ func TestDeploymentController(t *testing.T) {

// First deployment expected to be successful
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
})

t.Run("invalid private keys and certificates", func(t *testing.T) {
Expand Down Expand Up @@ -517,13 +526,13 @@ func TestDeploymentController(t *testing.T) {
tc.rc.(*deployment.ReconcileDeployment).AddHook(&hook)

testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)

tc.rc.(*deployment.ReconcileDeployment).RemoveHook(&hook)

// Next reconcile phase should catch the deployment changes
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, updatedDep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, updatedDep, []string{api.EventReasonNew, api.EventReasonRunning}, true)
})

t.Run("updating", func(t *testing.T) {
Expand All @@ -543,10 +552,11 @@ func TestDeploymentController(t *testing.T) {
require.NoError(t, err, "create deployment")

testReconcilePhase(t, tc.rc, tc.c, dep.Name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)

// Reconcile now should keep phase as running.
testReconcilePhase(t, tc.rc, tc.c, dep.Name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)
validateEvents(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})

// Retrieve existing object before updating it.
Expand All @@ -567,7 +577,7 @@ func TestDeploymentController(t *testing.T) {
testReconcilePhase(t, tc.rc, tc.c, dep.Name, false, false, api.DeploymentPhaseRunning)

// Recheck the container resources are updated
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, true)
}

t.Run("while running", func(t *testing.T) {
Expand All @@ -593,7 +603,7 @@ func TestDeploymentController(t *testing.T) {
err := tc.c.Create(context.TODO(), dep)
require.NoError(t, err, "failed to create deployment")
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, false)

err = tc.c.Get(context.TODO(), client.ObjectKey{Name: d.name}, dep)
require.NoError(t, err, "get deployment")
Expand Down Expand Up @@ -629,25 +639,16 @@ func TestDeploymentController(t *testing.T) {
err = tc.c.Create(context.TODO(), cm2)
require.NoError(t, err, "create configmap: %s", cm2.Name)

defer func() {
err := tc.c.Delete(context.TODO(), cm1)
if err != nil && !errors.IsNotFound(err) {
require.NoErrorf(t, err, "delete configmap: %s", cm1.Name)
}
err = tc.c.Delete(context.TODO(), cm2)
if err != nil && !errors.IsNotFound(err) {
require.NoErrorf(t, err, "delete configmap: %s", cm2.Name)
}
}()

// Use a fresh reconciler to mimic operator restart
tc.rc = newReconcileDeployment(tc.c, tc.cs)

// A fresh reconcile should delete the newly created above ConfigMap
testReconcilePhase(t, tc.rc, tc.c, d.name, false, false, api.DeploymentPhaseRunning)
err = tc.c.Get(context.TODO(), client.ObjectKey{Name: d.name}, dep)
require.NoError(t, err, "get deployment")
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning})
// It is debatable whether the operator should update all objects after
// a restart. Currently it does.
validateDriver(t, tc, dep, []string{api.EventReasonNew, api.EventReasonRunning}, true)

cm := &corev1.ConfigMap{}
err = tc.c.Get(context.TODO(), client.ObjectKey{Name: cm1.Name, Namespace: testNamespace}, cm)
Expand Down
39 changes: 27 additions & 12 deletions test/e2e/operator/deployment_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,15 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
if what == nil {
what = []interface{}{"validate driver"}
}
framework.ExpectNoErrorWithOffset(1, validate.DriverDeploymentEventually(ctx, client, k8sver, d.Namespace, deployment), what...)

// We cannot check for unexpected object modifications
// by the operator during E2E testing because the app
// controllers themselves will also modify the same
// objects with status changes. We can only test
// that during unit testing.
initialCreation := false

framework.ExpectNoErrorWithOffset(1, validate.DriverDeploymentEventually(ctx, client, k8sver, d.Namespace, deployment, initialCreation), what...)
framework.Logf("got expected driver deployment %s", deployment.Name)
}

Expand Down Expand Up @@ -197,7 +205,7 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {

deployment2 = deploy.CreateDeploymentCR(f, deployment2)
defer deploy.DeleteDeploymentCR(f, deployment2.Name)
validateDriver(deployment1 /* TODO 2 */, "validate driver #2")
validateDriver(deployment1, true /* TODO 2 */, "validate driver #2")
})

It("shall support dots in the name", func() {
Expand All @@ -214,7 +222,7 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {

deployment = deploy.CreateDeploymentCR(f, deployment)
defer deploy.DeleteDeploymentCR(f, deployment.Name)
validateDriver(deployment)
validateDriver(deployment, true)
})

It("driver deployment shall be running even after operator exit", func() {
Expand All @@ -223,16 +231,21 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
deployment = deploy.CreateDeploymentCR(f, deployment)

defer deploy.DeleteDeploymentCR(f, deployment.Name)
validateDriver(deployment)
validateDriver(deployment, true)

// Stop the operator
stopOperator(c, d)
// restore the deployment so that next test should not effect
defer startOperator(c, d)

// Ensure that the driver is running consistently
resourceVersions := map[string]string{}
Consistently(func() error {
return validate.DriverDeployment(client, k8sver, d.Namespace, deployment)
final, err := validate.DriverDeployment(client, k8sver, d.Namespace, deployment, resourceVersions)
if final {
framework.Failf("final error during driver validation after restarting: %v", err)
}
return err
}, "1m", "20s").ShouldNot(HaveOccurred(), "driver validation failure after restarting")
})

Expand Down Expand Up @@ -283,7 +296,7 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {

// Deleting the existing secret should make the deployment succeed.
deleteSecret(sec.Name)
validateDriver(deployment)
validateDriver(deployment, true)
})
})

Expand Down Expand Up @@ -378,7 +391,7 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
deployment = deploy.CreateDeploymentCR(f, deployment)
defer deploy.DeleteDeploymentCR(f, deployment.Name)
deploy.WaitForPMEMDriver(c, deployment.Name, corev1.NamespaceDefault, false /* testing */)
validateDriver(deployment)
validateDriver(deployment, true)

// NOTE(avalluri): As the current operator does not support deploying
// the driver in 'testing' mode, we cannot directely access CSI
Expand Down Expand Up @@ -434,8 +447,10 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
validateDriver(deployment, "validate driver before update")

// We have to get a fresh copy before updating it because the
// operator should have modified the status.
deployment = deploy.GetDeploymentCR(f, deployment.Name)
// operator should have modified the status, and only the status.
modifiedDeployment := deploy.GetDeploymentCR(f, deployment.Name)
Expect(modifiedDeployment.Spec).To(Equal(deployment.Spec), "spec unmodified")
Expect(modifiedDeployment.Status.Phase).To(Equal(api.DeploymentPhaseRunning), "deployment phase")

restored := false
if restart {
Expand All @@ -447,15 +462,15 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
}()
}

testcase.Mutate(&deployment)
deployment = deploy.UpdateDeploymentCR(f, deployment)
testcase.Mutate(&modifiedDeployment)
deployment = deploy.UpdateDeploymentCR(f, modifiedDeployment)

if restart {
startOperator(c, d)
restored = true
}

validateDriver(deployment, "validate driver after update and restart")
validateDriver(modifiedDeployment, "validate driver after update and restart")
}

It("while running", func() {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/operator/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ var _ = deploy.DescribeForSome("driver", func(d *deploy.Deployment) bool {
deployment := d.GetDriverDeployment()
deployment = deploy.GetDeploymentCR(f, deployment.Name)

framework.ExpectNoError(validate.DriverDeploymentEventually(ctx, client, *k8sver, d.Namespace, deployment), "validate driver")
framework.ExpectNoError(validate.DriverDeploymentEventually(ctx, client, *k8sver, d.Namespace, deployment, true), "validate driver")
})

// Just very minimal testing at the moment.
Expand Down
Loading

0 comments on commit 882b7f2

Please sign in to comment.