Skip to content

Commit

Permalink
Update Workspace controller reconciliation logic to avoid pulling def…
Browse files Browse the repository at this point in the history
…ault project ID on every reconciliation when workspace gets updated
  • Loading branch information
arybolovlev committed Aug 30, 2024
1 parent 6127ef0 commit 299e48b
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 29 deletions.
4 changes: 4 additions & 0 deletions api/v1alpha2/workspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,10 @@ type WorkspaceStatus struct {
//
//+optional
Variables []VariableStatus `json:"variables,omitempty"`
// Default organization project ID.
//
//+optional
DefaultProjectID string `json:"defaultProjectID,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,9 @@ spec:
status:
description: WorkspaceStatus defines the observed state of Workspace.
properties:
defaultProjectID:
description: Default organization project ID.
type: string
observedGeneration:
description: Real world state generation.
format: int64
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/app.terraform.io_workspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@ spec:
status:
description: WorkspaceStatus defines the observed state of Workspace.
properties:
defaultProjectID:
description: Default organization project ID.
type: string
observedGeneration:
description: Real world state generation.
format: int64
Expand Down
44 changes: 26 additions & 18 deletions controllers/workspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,15 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI
options.GlobalRemoteState = tfc.Bool(spec.RemoteStateSharing.AllWorkspaces)
}

org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization)
if err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization")
r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization")
return nil, err
}

if spec.Project != nil {
prjID, err := r.getProjectID(ctx, w)
prjID, err := w.getProjectID(ctx)
if err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to get project ID")
r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get project ID")
Expand All @@ -300,6 +307,9 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI
}

patch := client.MergeFrom(w.instance.DeepCopy())
if org != nil && org.DefaultProject != nil {
w.instance.Status.DefaultProjectID = org.DefaultProject.ID
}
w.instance.Status.WorkspaceID = workspace.ID
if err = r.Status().Patch(ctx, &w.instance, patch); err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to update status with workspace ID")
Expand Down Expand Up @@ -397,7 +407,7 @@ func (r *WorkspaceReconciler) updateWorkspace(ctx context.Context, w *workspaceI
}

if spec.Project != nil {
prjID, err := r.getProjectID(ctx, w)
prjID, err := w.getProjectID(ctx)
if err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to get project ID")
r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get project ID")
Expand All @@ -406,22 +416,20 @@ func (r *WorkspaceReconciler) updateWorkspace(ctx context.Context, w *workspaceI
w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("project ID %s will be used", prjID))
updateOptions.Project = &tfc.Project{ID: prjID}
} else {
// Setting up `Project` to nil(tfc.WorkspaceUpdateOptions{Project: nil}) won't move the workspace to the default project after the update.
// TODO:
// - The default project can be renamed, but cannot be deleted.
// We should do this API call once on a workspace creation and keep the default project ID in the status for future reference.
// We could validate whether or not the default project ID in the status is empty or not during the update stage.
// If the default project ID in the status is empty, then we make an API call to fill in this gap.
// Ideally, it will never happen but can during the TFE upgrade.
org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization)
if err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization")
r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization")
return nil, err
}
if org.DefaultProject != nil {
w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", org.DefaultProject.ID))
updateOptions.Project = &tfc.Project{ID: org.DefaultProject.ID}
if w.instance.Status.DefaultProjectID != "" {
updateOptions.Project = &tfc.Project{ID: w.instance.Status.DefaultProjectID}
w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", w.instance.Status.DefaultProjectID))
} else {
org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization)
if err != nil {
w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization")
r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization")
return nil, err
}
if org != nil && org.DefaultProject != nil {
w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", org.DefaultProject.ID))
updateOptions.Project = &tfc.Project{ID: org.DefaultProject.ID}
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions controllers/workspace_controller_projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
tfc "github.com/hashicorp/go-tfe"
)

func (r *WorkspaceReconciler) getProjectIDByName(ctx context.Context, w *workspaceInstance) (string, error) {
func (w *workspaceInstance) getProjectIDByName(ctx context.Context) (string, error) {
projectName := w.instance.Spec.Project.Name

listOpts := &tfc.ProjectListOptions{
Expand Down Expand Up @@ -38,7 +38,7 @@ func (r *WorkspaceReconciler) getProjectIDByName(ctx context.Context, w *workspa
return "", fmt.Errorf("project ID not found for project name %q", projectName)
}

func (r *WorkspaceReconciler) getProjectID(ctx context.Context, w *workspaceInstance) (string, error) {
func (w *workspaceInstance) getProjectID(ctx context.Context) (string, error) {
specProject := w.instance.Spec.Project

if specProject == nil {
Expand All @@ -47,7 +47,7 @@ func (r *WorkspaceReconciler) getProjectID(ctx context.Context, w *workspaceInst

if specProject.Name != "" {
w.log.Info("Reconcile Project", "msg", "getting project ID by name")
return r.getProjectIDByName(ctx, w)
return w.getProjectIDByName(ctx)
}

w.log.Info("Reconcile Project", "msg", "getting project ID from the spec.Project.ID")
Expand Down
17 changes: 9 additions & 8 deletions controllers/workspace_controller_projects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ var _ = Describe("Workspace controller", Ordered, func() {
})

Context("Project", func() {
It("can handle project by name", func() {
It("can be handled by name", func() {
instance.Spec.Project = &appv1alpha2.WorkspaceProject{Name: projectName}
// Create a new Kubernetes workspace object and wait until the controller finishes the reconciliation
createWorkspace(instance)
Expand All @@ -103,7 +103,7 @@ var _ = Describe("Workspace controller", Ordered, func() {
isReconciledDefaultProject(instance)
})

It("can handle project by id", func() {
It("can be handled by ID", func() {
instance.Spec.Project = &appv1alpha2.WorkspaceProject{ID: projectID}
// Create a new Kubernetes workspace object and wait until the controller finishes the reconciliation
createWorkspace(instance)
Expand Down Expand Up @@ -145,8 +145,8 @@ func isReconciledProjectByID(instance *appv1alpha2.Workspace) {
Eventually(func() bool {
Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed())
ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID)
Expect(ws).ShouldNot(BeNil())
Expect(err).Should(Succeed())
Expect(ws).ShouldNot(BeNil())
return ws.Project.ID == instance.Spec.Project.ID
}).Should(BeTrue())
}
Expand All @@ -157,11 +157,11 @@ func isReconciledProjectByName(instance *appv1alpha2.Workspace) {
Eventually(func() bool {
Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed())
ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID)
Expect(ws).ShouldNot(BeNil())
Expect(err).Should(Succeed())
Expect(ws).ShouldNot(BeNil())
prj, err := tfClient.Projects.Read(ctx, ws.Project.ID)
Expect(prj).ShouldNot(BeNil())
Expect(err).Should(Succeed())
Expect(prj).ShouldNot(BeNil())
return prj.Name == instance.Spec.Project.Name
}).Should(BeTrue())
}
Expand All @@ -172,11 +172,12 @@ func isReconciledDefaultProject(instance *appv1alpha2.Workspace) {
Eventually(func() bool {
Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed())
ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID)
Expect(ws).ShouldNot(BeNil())
Expect(err).Should(Succeed())
Expect(ws).ShouldNot(BeNil())
org, err := tfClient.Organizations.Read(ctx, instance.Spec.Organization)
Expect(org).ShouldNot(BeNil())
Expect(err).Should(Succeed())
return ws.Project.ID == org.DefaultProject.ID
Expect(org).ShouldNot(BeNil())
Expect(org.DefaultProject).ShouldNot(BeNil())
return org.DefaultProject.ID == ws.Project.ID && org.DefaultProject.ID == instance.Status.DefaultProjectID
}).Should(BeTrue())
}

0 comments on commit 299e48b

Please sign in to comment.