Skip to content

refactor: split controller.go into focused files by concern#69

Open
bdehamer wants to merge 2 commits intomainfrom
bdehamer/refactor-controller-split
Open

refactor: split controller.go into focused files by concern#69
bdehamer wants to merge 2 commits intomainfrom
bdehamer/refactor-controller-split

Conversation

@bdehamer
Copy link
Contributor

Summary

Splits the 682-line internal/controller/controller.go into four focused files organized by responsibility. No behavioral changes — pure structural refactoring.

New file layout

File Lines Responsibility
controller.go ~160 Types, interfaces, Controller struct, New(), newAPIClient(), Run()
handlers.go ~170 registerEventHandlers(), runWorker(), processNextItem(), startWorkers()
reconcile.go ~270 processEvent(), recordContainer(), deploymentExists(), getCacheKey()
pod.go ~130 createInformerFactory(), getARDeploymentName(), getContainerDigest(), getDeploymentName(), podToPartialMetadata()
config.go 42 Unchanged

Key changes

  • registerEventHandlers() extracted from New() as a Controller method — the event handler closures now use c.workqueue instead of closing over a local queue variable.
  • newAPIClient() extracted as a package-level helper to isolate API client construction from the controller constructor.
  • startWorkers() extracted from Run() for clarity.

Testing

All existing unit and integration tests pass without modification. No test changes needed since all files remain in the same package.

Split the 682-line controller.go into four files organized by responsibility:

- controller.go (~160 lines): types, interfaces, Controller struct, New(), Run()
- handlers.go (~170 lines): event handler registration, worker loop
- reconcile.go (~270 lines): event processing, deployment recording, caching
- pod.go (~130 lines): pod utility functions, informer factory

Extract registerEventHandlers() from New() as a Controller method and
newAPIClient() as a package-level helper to simplify the constructor.

No behavioral changes — pure structural refactoring.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the Kubernetes deployment-tracker controller by splitting the previous monolithic internal/controller/controller.go into smaller files organized by responsibility, aiming to keep behavior unchanged while improving readability and maintainability.

Changes:

  • Extracted event handler registration and worker lifecycle logic into handlers.go.
  • Extracted reconciliation / posting logic into reconcile.go.
  • Extracted pod/informer helper utilities into pod.go and simplified controller.go to core wiring (New, Run, API client construction).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
internal/controller/controller.go Slimmed down to core types + constructor + API client creation + Run(), delegating handlers/workers elsewhere.
internal/controller/handlers.go New file containing informer event handlers and queue worker processing logic.
internal/controller/reconcile.go New file containing processEvent/recordContainer and caching logic for posts.
internal/controller/pod.go New file containing informer factory creation + pod/container helper functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI commented Mar 23, 2026

@bdehamer I've opened a new pull request, #70, to work on those changes. Once the pull request is ready, I'll request review from you.

…tone support (#70)

* Initial plan

* fix: use DeletionHandlingMetaNamespaceKeyFunc in DeleteFunc for tombstone support

Co-authored-by: bdehamer <398027+bdehamer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/deployment-tracker/sessions/620a8570-c815-4b5f-95cd-25d107fcd82a

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bdehamer <398027+bdehamer@users.noreply.github.com>
return
}

key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a nice bug fix picked-up by Copilot during this refactor. Even if we decide not to merge this PR, we should apply this change (using DeletionHandlingMetaNamespaceKeyFunc in place of MetaNamespaceKeyFunc)

@bdehamer bdehamer marked this pull request as ready for review March 24, 2026 01:57
@bdehamer bdehamer requested a review from a team as a code owner March 24, 2026 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants