Skip to content

Commit

Permalink
Refactoring and additional tests
Browse files Browse the repository at this point in the history 
Also requeue if check clusterdeployment returns error
  • Loading branch information
@jewzaam
jewzaam committed Oct 8, 2019
1 parent 078f08e commit a45baea
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 89 deletions.
8 changes: 8 additions & 0 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,12 @@ package config
const (
OperatorName string = "deadmanssnitch-operator"
OperatorNamespace string = "deadmanssnitch-operator"
SyncSetPostfix string = "-dms"
KeySnitchURL string = "SNITCH_URL"

// ClusterDeploymentManagedLabel is the label the clusterdeployment will have that determines
// if the cluster is OSD (managed) or not
ClusterDeploymentManagedLabel string = "api.openshift.com/managed"
// ClusterDeploymentNoalertsLabel is the label the clusterdeployment will have if the cluster should not send alerts
ClusterDeploymentNoalertsLabel string = "api.openshift.com/noalerts"
)
67 changes: 13 additions & 54 deletions pkg/controller/deadmanssnitch/deadmanssnitch_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/go-logr/logr"
"github.com/openshift/deadmanssnitch-operator/config"
"github.com/openshift/deadmanssnitch-operator/pkg/dmsclient"
"github.com/openshift/deadmanssnitch-operator/pkg/utils"
hivev1alpha1 "github.com/openshift/hive/pkg/apis/hive/v1alpha1"
Expand Down Expand Up @@ -35,11 +36,6 @@ const (
DeadMansSnitchAPISecretKey string = "deadmanssnitch-api-key"
// DeadMansSnitchTagKey is the secret where to fetch the DMS API Key
DeadMansSnitchTagKey string = "hive-cluster-tag"
// ClusterDeploymentManagedLabel is the label the clusterdeployment will have that determines
// if the cluster is OSD (managed) or now
ClusterDeploymentManagedLabel string = "api.openshift.com/managed"
// ClusterDeploymentNoalertsLabel is the label the clusterdeployment will have if the cluster should not send alerts
ClusterDeploymentNoalertsLabel string = "api.openshift.com/noalerts"
)

var log = logf.Log.WithName("controller_deadmanssnitch")
Expand Down Expand Up @@ -130,47 +126,14 @@ func (r *ReconcileDeadMansSnitch) Reconcile(request reconcile.Request) (reconcil
reqLogger.Info("Reconciling DeadMansSnitch")

// Fetch the ClusterDeployment instance
instance := &hivev1alpha1.ClusterDeployment{}
err := r.client.Get(context.TODO(), request.NamespacedName, instance)
processCD, instance, err := utils.CheckClusterDeployment(request, r.client, reqLogger)

if err != nil {
if errors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
return reconcile.Result{}, nil
}
// Error reading the object - requeue the request.
// something went wrong, requeue
return reconcile.Result{}, err
}

// Just return if this is not a managed cluster OR has noalerts label set
if val, ok := instance.Labels[ClusterDeploymentManagedLabel]; ok {
if val != "true" {
reqLogger.Info("Not a managed cluster", "Namespace", request.Namespace, "Name", request.Name)
return reconcile.Result{}, nil
}
} else {
// Managed tag is not present which implies it is not a managed cluster
reqLogger.Info("Not a managed cluster", "Namespace", request.Namespace, "Name", request.Name)
return reconcile.Result{}, nil
}

// Cleanup DMS then return if alerts are disabled on the cluster
if _, ok := instance.Labels[ClusterDeploymentNoalertsLabel]; ok {
reqLogger.Info("Managed cluster with Alerts disabled", "Namespace", request.Namespace, "Name", request.Name)
return reconcile.Result{}, deleteDMS(r, request, instance, reqLogger)
}

// cluster isn't installed yet, just return
if !instance.Status.Installed {
// Cluster isn't installed yet, return
reqLogger.Info("Cluster installation is not complete, returning...", "Namespace", request.Namespace, "Name", request.Name)
return reconcile.Result{}, nil
}
reqLogger.Info("Checking to see if CD is deleted", "Namespace", request.Namespace, "Name", request.Name)
// Check to see if the ClusterDeployment is deleted
if instance.DeletionTimestamp != nil {
// cleanup all DMS things
if !processCD {
return reconcile.Result{}, deleteDMS(r, request, instance, reqLogger)
}

Expand All @@ -191,7 +154,7 @@ func (r *ReconcileDeadMansSnitch) Reconcile(request reconcile.Request) (reconcil
return reconcile.Result{}, nil
}

ssName := request.Name + "-dms"
ssName := request.Name + config.SyncSetPostfix
// Check to see if the SyncSet exists
err = r.client.Get(context.TODO(),
types.NamespacedName{Name: ssName, Namespace: request.Namespace},
Expand Down Expand Up @@ -260,17 +223,17 @@ func (r *ReconcileDeadMansSnitch) Reconcile(request reconcile.Request) (reconcil
reqLogger.Info("Done creating a new SyncSet", "Namespace", request.Namespace, "Name", request.Name)
} else {
reqLogger.Info("SyncSet Already Present, nothing to do here...", "Namespace", request.Namespace, "Name", request.Name)

}

return reconcile.Result{}, nil

}

func newSyncSet(namespace string, clusterDeploymentName string, snitchURL string) *hivev1alpha1.SyncSet {

newSS := &hivev1alpha1.SyncSet{
ObjectMeta: metav1.ObjectMeta{
Name: clusterDeploymentName + "-dms",
Name: clusterDeploymentName + config.SyncSetPostfix,
Namespace: namespace,
},
Spec: hivev1alpha1.SyncSetSpec{
Expand All @@ -294,7 +257,7 @@ func newSyncSet(namespace string, clusterDeploymentName string, snitchURL string
Namespace: "openshift-monitoring",
},
Data: map[string][]byte{
"SNITCH_URL": []byte(snitchURL),
config.KeySnitchURL: []byte(snitchURL),
},
},
},
Expand Down Expand Up @@ -330,14 +293,10 @@ func deleteDMS(r *ReconcileDeadMansSnitch, request reconcile.Request, instance *
}

reqLogger.Info("Deleting DMS SyncSet", "Namespace", request.Namespace, "Name", request.Name)
syncset := &hivev1alpha1.SyncSet{}
err = r.client.Get(context.TODO(), types.NamespacedName{Namespace: request.Namespace, Name: request.Name + "-dms"}, syncset)
if err == nil {
err = r.client.Delete(context.TODO(), syncset)
if err != nil {
reqLogger.Error(err, "Error deleting SyncSet", "Namespace", request.Namespace, "Name", request.Name+"-dms")
return err
}
err = utils.DeleteSyncSet(request.Name+config.SyncSetPostfix, request.Namespace, r.client, reqLogger)
if err != nil {
reqLogger.Error(err, "Error deleting SyncSet", "Namespace", request.Namespace, "Name", request.Name+config.SyncSetPostfix)
return err
}

reqLogger.Info("Deleting DMS finalizer from ClusterDeployment", "Namespace", request.Namespace, "Name", request.Name)
Expand Down
152 changes: 117 additions & 35 deletions pkg/controller/deadmanssnitch/deadmanssnitch_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"testing"

"github.com/openshift/deadmanssnitch-operator/config"
"github.com/openshift/deadmanssnitch-operator/pkg/dmsclient"
mockdms "github.com/openshift/deadmanssnitch-operator/pkg/dmsclient/mock"

Expand All @@ -28,12 +29,13 @@ import (
)

const (
testClusterName = "testCluster"
testNamespace = "testNamespace"
testSnitchURL = "https://deadmanssnitch.com/12345"
testSnitchToken = "abcdefg"
testTag = "hive-test"
testAPIKey = "abc123"
testClusterName = "testCluster"
testNamespace = "testNamespace"
testSnitchURL = "https://deadmanssnitch.com/12345"
testSnitchToken = "abcdefg"
testTag = "hive-test"
testAPIKey = "abc123"
testOtherSyncSetPostfix = "-something-else"
)

type SyncSetEntry struct {
Expand Down Expand Up @@ -107,7 +109,7 @@ func testSecret() *corev1.Secret {

// return a simple test ClusterDeployment
func testClusterDeployment() *hivev1alpha1.ClusterDeployment {
labelMap := map[string]string{ClusterDeploymentManagedLabel: "true"}
labelMap := map[string]string{config.ClusterDeploymentManagedLabel: "true"}
finalizers := []string{DeadMansSnitchFinalizer}

cd := hivev1alpha1.ClusterDeployment{
Expand All @@ -126,6 +128,40 @@ func testClusterDeployment() *hivev1alpha1.ClusterDeployment {
return &cd
}

// testSyncSet returns a SyncSet for an existing testClusterDeployment to use in testing.
func testSyncSet() *hivev1alpha1.SyncSet {
return &hivev1alpha1.SyncSet{
ObjectMeta: metav1.ObjectMeta{
Name: testClusterName + config.SyncSetPostfix,
Namespace: testNamespace,
},
Spec: hivev1alpha1.SyncSetSpec{
ClusterDeploymentRefs: []corev1.LocalObjectReference{
{
Name: testClusterName,
},
},
},
}
}

// testOtherSyncSet returns a SyncSet that is not for PD for an existing testClusterDeployment to use in testing.
func testOtherSyncSet() *hivev1alpha1.SyncSet {
return &hivev1alpha1.SyncSet{
ObjectMeta: metav1.ObjectMeta{
Name: testClusterName + testOtherSyncSetPostfix,
Namespace: testNamespace,
},
Spec: hivev1alpha1.SyncSetSpec{
ClusterDeploymentRefs: []corev1.LocalObjectReference{
{
Name: testClusterName,
},
},
},
}
}

// return a deleted ClusterDeployment
func deletedClusterDeployment() *hivev1alpha1.ClusterDeployment {
cd := testClusterDeployment()
Expand All @@ -139,13 +175,14 @@ func deletedClusterDeployment() *hivev1alpha1.ClusterDeployment {
func uninstalledClusterDeployment() *hivev1alpha1.ClusterDeployment {
cd := testClusterDeployment()
cd.Status.Installed = false
cd.ObjectMeta.Finalizers = nil // operator will not have set a finalizer if it was never installed

return cd
}

// return a ClusterDeployment with Label["managed"] == false
func nonManagedClusterDeployment() *hivev1alpha1.ClusterDeployment {
labelMap := map[string]string{ClusterDeploymentManagedLabel: "false"}
labelMap := map[string]string{config.ClusterDeploymentManagedLabel: "false"}
cd := hivev1alpha1.ClusterDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: testClusterName,
Expand All @@ -164,8 +201,8 @@ func nonManagedClusterDeployment() *hivev1alpha1.ClusterDeployment {
// return a ClusterDeployment with Label["noalerts"] == ""
func noalertsManagedClusterDeployment() *hivev1alpha1.ClusterDeployment {
labelMap := map[string]string{
ClusterDeploymentManagedLabel: "true",
ClusterDeploymentNoalertsLabel: "",
config.ClusterDeploymentManagedLabel: "true",
config.ClusterDeploymentNoalertsLabel: "",
}
cd := hivev1alpha1.ClusterDeployment{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -199,7 +236,7 @@ func TestReconcileClusterDeployment(t *testing.T) {
testSecret(),
},
expectedSyncSets: &SyncSetEntry{
name: testClusterName + "-dms",
name: testClusterName + config.SyncSetPostfix,
snitchURL: testSnitchURL,
clusterDeploymentRefName: testClusterName,
},
Expand Down Expand Up @@ -251,7 +288,7 @@ func TestReconcileClusterDeployment(t *testing.T) {
},
},
{
name: "Test Managed ClusterDeployment with Alerts disabled",
name: "Test Create Managed ClusterDeployment with Alerts disabled",
localObjects: []runtime.Object{
noalertsManagedClusterDeployment(),
},
Expand Down Expand Up @@ -302,22 +339,12 @@ func TestRemoveAlertsAfterCreate(t *testing.T) {
mocks := setupDefaultMocks(t, []runtime.Object{
testClusterDeployment(),
testSecret(),
testSyncSet(),
testOtherSyncSet(),
})
//test.setupDMSMock(mocks.mockDMSClient.EXPECT())
setupDMSMock :=
func(r *mockdms.MockClientMockRecorder) {
// create
r.Create(gomock.Any()).Return(dmsclient.Snitch{CheckInURL: testSnitchURL, Tags: []string{testTag}}, nil).Times(1)
r.FindSnitchesByName(gomock.Any()).Return([]dmsclient.Snitch{}, nil).Times(1)
r.FindSnitchesByName(gomock.Any()).Return([]dmsclient.Snitch{
{
CheckInURL: testSnitchURL,
Status: "pending",
},
}, nil).Times(1)
r.CheckIn(gomock.Any()).Return(nil).Times(1)

// delete
r.Delete(gomock.Any()).Return(true, nil).Times(1)
r.FindSnitchesByName(gomock.Any()).Return([]dmsclient.Snitch{
{Token: testSnitchToken},
Expand Down Expand Up @@ -348,20 +375,40 @@ func TestRemoveAlertsAfterCreate(t *testing.T) {
// can't set to empty string, it won't update.. value does not matter
clusterDeployment := &hivev1alpha1.ClusterDeployment{}
err = mocks.fakeKubeClient.Get(context.TODO(), types.NamespacedName{Namespace: testNamespace, Name: testClusterName}, clusterDeployment)
clusterDeployment.Labels[ClusterDeploymentNoalertsLabel] = "X"
clusterDeployment.Labels[config.ClusterDeploymentNoalertsLabel] = "X"
err = mocks.fakeKubeClient.Update(context.TODO(), clusterDeployment)

// ACT (delete)
// Act (delete) [2x because was seeing other SyncSet's getting deleted]
_, err = rdms.Reconcile(reconcile.Request{
NamespacedName: types.NamespacedName{
Name: testClusterName,
Namespace: testNamespace,
},
})
_, err = rdms.Reconcile(reconcile.Request{
NamespacedName: types.NamespacedName{
Name: testClusterName,
Namespace: testNamespace,
},
})
_, err = rdms.Reconcile(reconcile.Request{
NamespacedName: types.NamespacedName{
Name: testClusterName,
Namespace: testNamespace,
},
})
_, err = rdms.Reconcile(reconcile.Request{
NamespacedName: types.NamespacedName{
Name: testClusterName,
Namespace: testNamespace,
},
})

// ASSERT (no syncset)
// ASSERT (no unexpected syncset)
assert.NoError(t, err, "Unexpected Error")
assert.True(t, verifyNoSyncSet(mocks.fakeKubeClient, &SyncSetEntry{}))
// verify the "other" syncset didn't get deleted
assert.True(t, verifyOtherSyncSetExists(mocks.fakeKubeClient, &SyncSetEntry{}))
})
}

Expand All @@ -387,19 +434,54 @@ func verifySyncSetExists(c client.Client, expected *SyncSetEntry) bool {
return false
}

return string(secret.Data["SNITCH_URL"]) == expected.snitchURL
return string(secret.Data[config.KeySnitchURL]) == expected.snitchURL
}

func verifyNoSyncSet(c client.Client, expected *SyncSetEntry) bool {
ss := hivev1alpha1.SyncSet{}
err := c.Get(context.TODO(),
types.NamespacedName{Name: expected.name, Namespace: testNamespace},
&ss)
ssList := &hivev1alpha1.SyncSetList{}
opts := client.ListOptions{Namespace: testNamespace}
err := c.List(context.TODO(), &opts, ssList)

if err != nil {
if errors.IsNotFound(err) {
// no syncsets are defined, this is OK
return true
}
}

if errors.IsNotFound(err) {
return true
for _, ss := range ssList.Items {
if ss.Name != testClusterName+testOtherSyncSetPostfix {
// too bad, found a syncset associated with this operator
return false
}
}
return false

// if we got here, it's good. list was empty or everything passed
return true
}

// verifyOtherSyncSetExists verifies that there is the "other" SyncSet present
func verifyOtherSyncSetExists(c client.Client, expected *SyncSetEntry) bool {
ssList := &hivev1alpha1.SyncSetList{}
opts := client.ListOptions{Namespace: testNamespace}
err := c.List(context.TODO(), &opts, ssList)

if err != nil {
if errors.IsNotFound(err) {
// no syncsets are defined, this is bad
return false
}
}

found := false
for _, ss := range ssList.Items {
if ss.Name == testClusterName+testOtherSyncSetPostfix {
// too bad, found a syncset associated with this operator
found = true
}
}

return found
}

func stringInSlice(a string, list []string) bool {
Expand Down
Loading

0 comments on commit a45baea

Please sign in to comment.