Add review changes

weaveworks · Nov 8, 2023 · c5e789e · c5e789e
1 parent 19700a6
commit c5e789e
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 23 deletions.
diff --git a/internal/controller/automatedclusterdiscovery_controller.go b/internal/controller/automatedclusterdiscovery_controller.go
@@ -215,9 +215,6 @@ func (r *AutomatedClusterDiscoveryReconciler) reconcileClusters(ctx context.Cont
 
 		inventoryResources = append(inventoryResources, clusterRef)
 
-		// publish event for ClusterCreated
-		r.event(cd, corev1.EventTypeNormal, "ClusterCreated", fmt.Sprintf("Cluster %s created", cluster.Name))
-
 		secret := newSecret(types.NamespacedName{
 			Name:      secretName,
 			Namespace: cd.Namespace,
@@ -233,6 +230,9 @@ func (r *AutomatedClusterDiscoveryReconciler) reconcileClusters(ctx context.Cont
 			return inventoryResources, fmt.Errorf("failed to set ownership on created Secret: %w", err)
 		}
 
+		// publish event for ClusterCreated
+		r.event(cd, corev1.EventTypeNormal, "ClusterCreated", fmt.Sprintf("Cluster %s created", cluster.Name))
+
 		secret.SetLabels(labelsForResource(*cd))
 		_, err = controllerutil.CreateOrPatch(ctx, r.Client, secret, func() error {
 			value, err := clientcmd.Write(*cluster.KubeConfig)

diff --git a/internal/controller/automatedclusterdiscovery_controller_test.go b/internal/controller/automatedclusterdiscovery_controller_test.go
@@ -487,6 +487,86 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) {
 		_, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(aksCluster)})
 		assert.NoError(t, err)
 	})
+
+	t.Run("Reconcile publishes events on cluster creation and removal", func(t *testing.T) {
+		ctx := context.TODO()
+		aksCluster := &clustersv1alpha1.AutomatedClusterDiscovery{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-aks",
+				Namespace: "default",
+			},
+			Spec: clustersv1alpha1.AutomatedClusterDiscoverySpec{
+				Type: "aks",
+				AKS: &clustersv1alpha1.AKS{
+					SubscriptionID: "subscription-123",
+				},
+				Interval: metav1.Duration{Duration: time.Minute},
+			},
+		}
+
+		err := k8sClient.Create(ctx, aksCluster)
+		assert.NoError(t, err)
+		defer deleteClusterDiscoveryAndInventory(t, k8sClient, aksCluster)
+
+		cluster := &providers.ProviderCluster{
+			Name: "cluster-1",
+			KubeConfig: &kubeconfig.Config{
+				APIVersion: "v1",
+				Clusters: map[string]*kubeconfig.Cluster{
+					"cluster-1": {
+						Server:                   "https://cluster-prod.example.com/",
+						CertificateAuthorityData: []uint8(testCAData),
+					},
+				},
+			},
+		}
+
+		testProvider := stubProvider{
+			response: []*providers.ProviderCluster{
+				cluster,
+			},
+		}
+
+		mockEventRecorder := &mockEventRecorder{}
+
+		reconciler := &AutomatedClusterDiscoveryReconciler{
+			Client: k8sClient,
+			Scheme: scheme,
+			AKSProvider: func(providerID string) providers.Provider {
+				return &testProvider
+			},
+			EventRecorder: mockEventRecorder,
+		}
+		assert.NoError(t, reconciler.SetupWithManager(mgr))
+
+		_, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(aksCluster)})
+		assert.NoError(t, err)
+
+		err = k8sClient.Get(ctx, client.ObjectKeyFromObject(aksCluster), aksCluster)
+		assert.NoError(t, err)
+
+		secret := newSecret(types.NamespacedName{Name: "cluster-1-kubeconfig", Namespace: aksCluster.GetNamespace()})
+		gitopsCluster := newGitopsCluster(secret.GetName(), types.NamespacedName{Name: "cluster-1", Namespace: aksCluster.GetNamespace()})
+		assertInventoryHasItems(t, aksCluster, secret, gitopsCluster)
+
+		assert.Equal(t, "Normal", mockEventRecorder.CapturedType)
+		assert.Equal(t, "ClusterCreated", mockEventRecorder.CapturedReason)
+		assert.Equal(t, "Cluster cluster-1 created", mockEventRecorder.CapturedMessage)
+
+		testProvider.response = []*providers.ProviderCluster{}
+
+		_, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(aksCluster)})
+		assert.NoError(t, err)
+
+		err = k8sClient.Get(ctx, client.ObjectKeyFromObject(aksCluster), aksCluster)
+		assert.NoError(t, err)
+
+		assertInventoryHasNoItems(t, aksCluster)
+
+		assert.Equal(t, "Normal", mockEventRecorder.CapturedType)
+		assert.Equal(t, "ClusterRemoved", mockEventRecorder.CapturedReason)
+		assert.Equal(t, "Cluster cluster-1 removed", mockEventRecorder.CapturedMessage)
+	})
 }
 
 func TestReconcilingWithAnnotationChange(t *testing.T) {
@@ -572,26 +652,6 @@ func TestReconcilingWithAnnotationChange(t *testing.T) {
 	assert.Equal(t, aksCluster.Status.LastHandledReconcileAt, "testing")
 }
 
-func TestEvent(t *testing.T) {
-	mockEventRecorder := &mockEventRecorder{}
-
-	reconciler := &AutomatedClusterDiscoveryReconciler{
-		EventRecorder: mockEventRecorder,
-	}
-
-	obj := &clustersv1alpha1.AutomatedClusterDiscovery{}
-	eventtype := "Normal"
-	reason := "TestReason"
-	message := "TestMessage"
-
-	reconciler.event(obj, eventtype, reason, message)
-
-	assert.Equal(t, mockEventRecorder.CapturedObj, obj)
-	assert.Equal(t, mockEventRecorder.CapturedType, eventtype)
-	assert.Equal(t, mockEventRecorder.CapturedReason, reason)
-	assert.Equal(t, mockEventRecorder.CapturedMessage, message)
-}
-
 type mockEventRecorder struct {
 	CapturedObj     runtime.Object
 	CapturedType    string