Merge pull request #27 from NautiluX/refactor-ref-controller

Refactor ref controller

pkg/controller/projectreference/projectreference_adapter.go

@@ -103,6 +103,38 @@ func newReferenceAdapter(projectReference *gcpv1alpha1.ProjectReference, logger
 	}, nil
 }
 
+func (r *ReferenceAdapter) EnsureProjectClaimUpdated() (gcpv1alpha1.ClaimStatus, error) {
+	if r.projectReference.Status.State != gcpv1alpha1.ProjectReferenceStatusReady {
+		return r.projectClaim.Status.State, nil
+	}
+
+	if r.projectReference.Status.State == gcpv1alpha1.ProjectReferenceStatusReady && r.projectClaim.Status.State == gcpv1alpha1.ClaimStatusReady {
+		r.logger.Info("ProjectReference and ProjectClaim CR are in READY state nothing to process.")
+		return r.projectClaim.Status.State, nil
+	}
+
+	if r.projectClaim.Spec.GCPProjectID == "" {
+		r.projectClaim.Spec.GCPProjectID = r.projectReference.Spec.GCPProjectID
+		err := r.kubeClient.Update(context.TODO(), r.projectClaim)
+		if err != nil {
+			r.logger.Error(err, "Error updating ProjectClaim GCPProjectID")
+			return r.projectClaim.Status.State, err
+		}
+	}
+
+	//Project Ready update matchingClaim to ready
+	r.projectClaim.Status.State = gcpv1alpha1.ClaimStatusReady
+	// Since conditions as of now are not inititated we need to set an empty one here
+	// This will need to removed and checked when we actually start to use conditions
+	r.projectClaim.Status.Conditions = []gcpv1alpha1.ProjectClaimCondition{}
+	err := r.kubeClient.Status().Update(context.TODO(), r.projectClaim)
+	if err != nil {
+		r.logger.Error(err, "Error updating ProjectClaim Status")
+		return r.projectClaim.Status.State, err
+	}
+	return r.projectClaim.Status.State, nil
+}
+
 func getMatchingClaimLink(projectReference *gcpv1alpha1.ProjectReference, client client.Client) (*gcpv1alpha1.ProjectClaim, error) {
 	projectClaim := &gcpv1alpha1.ProjectClaim{}
 	err := client.Get(context.TODO(), types.NamespacedName{Name: projectReference.Spec.ProjectClaimCRLink.Name, Namespace: projectReference.Spec.ProjectClaimCRLink.Namespace}, projectClaim)

pkg/controller/projectreference/projectreference_adapter_test.go

@@ -1,13 +1,42 @@
-package projectreference_test
+package projectreference
 
 import (
+	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/types"
+	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
-	. "github.com/openshift/gcp-project-operator/pkg/controller/projectreference"
+	api "github.com/openshift/gcp-project-operator/pkg/apis/gcp/v1alpha1"
+	mocks "github.com/openshift/gcp-project-operator/pkg/util/mocks"
+	mockGCP "github.com/openshift/gcp-project-operator/pkg/util/mocks/gcpclient"
+	testStructs "github.com/openshift/gcp-project-operator/pkg/util/mocks/structs"
 )
 
 var _ = Describe("ProjectreferenceAdapter", func() {
+	var (
+		adapter          *ReferenceAdapter
+		projectReference *api.ProjectReference
+		mockKubeClient   *mocks.MockClient
+		mockGCPClient    *mockGCP.MockClient
+		mockStatusWriter *mocks.MockStatusWriter
+		projectClaim     *api.ProjectClaim
+		err              error
+	)
+	BeforeEach(func() {
+		projectReference = testStructs.NewProjectReferenceBuilder().GetProjectReference()
+		projectClaim = testStructs.NewProjectClaimBuilder().GetProjectClaim()
+		ctrl := gomock.NewController(GinkgoT())
+		mockStatusWriter = mocks.NewMockStatusWriter(ctrl)
+		mockKubeClient = mocks.NewMockClient(ctrl)
+		mockGCPClient = mockGCP.NewMockClient(ctrl)
+	})
+	JustBeforeEach(func() {
+		claimLink := types.NamespacedName{Name: projectReference.Spec.ProjectClaimCRLink.Name, Namespace: projectReference.Spec.ProjectClaimCRLink.Namespace}
+		mockKubeClient.EXPECT().Get(gomock.Any(), claimLink, gomock.Any()).SetArg(2, *projectClaim)
+		adapter, err = newReferenceAdapter(projectReference, logf.Log.WithName("Test Logger"), mockKubeClient, mockGCPClient)
+		Expect(err).NotTo(HaveOccurred())
+	})
 	Context("generated project names", func() {
 		It("are shorter than 30 characters", func() {
 			projectID, err := GenerateProjectID()
@@ -35,4 +64,58 @@ var _ = Describe("ProjectreferenceAdapter", func() {
 			}
 		})
 	})
+
+	Context("EnsureProjectClaimUpdated", func() {
+		Context("When ProjectReference is in creating state", func() {
+			BeforeEach(func() {
+				projectReference.Status.State = api.ProjectReferenceStatusCreating
+			})
+
+			It("returns without altering ProjectClaim", func() {
+				oldClaim := projectClaim.DeepCopy()
+				state, err := adapter.EnsureProjectClaimUpdated()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(state).To(Equal(projectClaim.Status.State))
+				Expect(adapter.projectClaim).To(Equal(oldClaim))
+			})
+		})
+		Context("When ProjectReference is in Ready state", func() {
+			BeforeEach(func() {
+				projectReference.Status.State = api.ProjectReferenceStatusReady
+			})
+
+			Context("When ProjectClaim is in Ready state", func() {
+				BeforeEach(func() {
+					projectClaim.Status.State = api.ClaimStatusReady
+				})
+
+				It("returns without altering ProjectClaim", func() {
+					oldClaim := projectClaim.DeepCopy()
+					state, err := adapter.EnsureProjectClaimUpdated()
+					Expect(err).NotTo(HaveOccurred())
+					Expect(state).To(Equal(projectClaim.Status.State))
+					Expect(adapter.projectClaim).To(Equal(oldClaim))
+				})
+			})
+
+			Context("When ProjectClaim is not in Ready state", func() {
+				BeforeEach(func() {
+					projectClaim.Status.State = api.ClaimStatusPending
+					projectClaim.Spec.GCPProjectID = ""
+					projectReference.Spec.GCPProjectID = "fake-gcp-project"
+
+					mockKubeClient.EXPECT().Update(gomock.Any(), gomock.Any())
+					mockKubeClient.EXPECT().Status().Return(mockStatusWriter)
+					mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any())
+				})
+
+				It("updates the ProjectClaim, sets GCPProjectID and the state to Ready", func() {
+					state, err := adapter.EnsureProjectClaimUpdated()
+					Expect(err).NotTo(HaveOccurred())
+					Expect(state).To(Equal(api.ClaimStatusReady))
+					Expect(adapter.projectClaim.Spec.GCPProjectID).To(Equal(adapter.projectReference.Spec.GCPProjectID))
+				})
+			})
+		})
+	})
 })

pkg/controller/projectreference/projectreference_controller.go

@@ -127,36 +127,9 @@ func (r *ReconcileProjectReference) Reconcile(request reconcile.Request) (reconc
 	}
 
 	// Make projectReference  be processed based on state of ProjectClaim and Project Reference
-	switch {
-	// If we are in a creating state break from the loop and conitnue to process CR
-	case projectReference.Status.State == gcpv1alpha1.ProjectReferenceStatusCreating:
-		break
-	// If projectReference and projectClaim are both ready there is nothing to do
-	case projectReference.Status.State == gcpv1alpha1.ProjectReferenceStatusReady && adapter.projectClaim.Status.State == gcpv1alpha1.ClaimStatusReady:
-		reqLogger.Info("ProjectReference and ProjectClaim CR are in READY state nothing to process.")
-		return r.doNotRequeue()
-	case projectReference.Status.State == gcpv1alpha1.ProjectReferenceStatusReady:
-
-		if adapter.projectClaim.Spec.GCPProjectID == "" {
-			adapter.projectClaim.Spec.GCPProjectID = projectReference.Spec.GCPProjectID
-			err = r.client.Update(context.TODO(), adapter.projectClaim)
-			if err != nil {
-				reqLogger.Error(err, "Error updating ProjectClaim GCPProjectID")
-				return r.requeueOnErr(err)
-			}
-		}
-
-		//Project Ready update matchingClaim to ready
-		adapter.projectClaim.Status.State = gcpv1alpha1.ClaimStatusReady
-		// Since conditions as of now are not inititated we need to set an empty one here
-		// This will need to removed and checked when we actually start to use conditions
-		adapter.projectClaim.Status.Conditions = []gcpv1alpha1.ProjectClaimCondition{}
-		err := r.client.Status().Update(context.TODO(), adapter.projectClaim)
-		if err != nil {
-			reqLogger.Error(err, "Error updating ProjectClaim Status")
-			return r.requeueOnErr(err)
-		}
-		return r.doNotRequeue()
+	claimStatus, err := adapter.EnsureProjectClaimUpdated()
+	if claimStatus == gcpv1alpha1.ClaimStatusReady || err != nil {
+		return r.requeueOnErr(err)
 	}
 
 	// only make changes to ProjectReference if ProjelctClaim is pending