Skip to content

refactor: reorganize codebase into packages - no functional changes#138

Open
ravisastryk wants to merge 5 commits intoahrtr:mainfrom
ravisastryk:refactor/reorganize-flat-codebase-to-package-structure
Open

refactor: reorganize codebase into packages - no functional changes#138
ravisastryk wants to merge 5 commits intoahrtr:mainfrom
ravisastryk:refactor/reorganize-flat-codebase-to-package-structure

Conversation

@ravisastryk
Copy link
Copy Markdown
Contributor

@ravisastryk ravisastryk commented Jan 25, 2026

Summary

Restructure the etcd-defrag codebase from a flat file layout in the root directory into an organized Go package structure.

This is a pure refactoring with zero functional changes - all CLI flags, behavior, and output remain exactly the same.

Issue: #137

Why This Change Is Needed

Problems with Current Flat Structure

The current codebase has all Go files in the root directory:

etcd-defrag/
├── main.go           # Entry + CLI + some logic mixed
├── agent.go          # ~400 lines with mixed responsibilities
├── config.go         # Configuration handling
├── endpoints.go      # Endpoint management
├── evaluator.go      # Rule evaluation
├── disalarm.go       # Auto-disalarm feature
├── utils.go          # Grab-bag utilities
└── version.go        # Version info

Issues:

Problem Impact
Mixed concerns in agent.go Health checks, defragmentation, compaction, status fetching, and orchestration all in one ~400 line file
Everything in main package No encapsulation, all types are globally accessible, no clear API boundaries
Hard to test Tightly coupled code makes unit testing difficult; can't mock individual components
Difficult onboarding New contributors struggle to understand code organization
Poor discoverability Finding specific functionality requires searching through files
Challenging code reviews PRs often touch unrelated code in the same file
No room for growth Adding features like parallel health checks or metrics has no obvious location

Benefits of Package Structure

Benefit Description
Single Responsibility Each package has one clear purpose
Testability Packages can be tested in isolation with mocked dependencies
Encapsulation internal/ packages are implementation details; pkg/ is public API
Maintainability Changes are localized to relevant packages
Extensibility New features have obvious homes
Go Idioms Follows standard Go project layout conventions

Architecture Changes

Before: Flat Structure

etcd-defrag/
├── main.go
├── agent.go          # ALL core logic (~400 lines)
├── agent_test.go
├── config.go
├── cmd_test.go
├── disalarm.go
├── endpoints.go
├── endpoints_test.go
├── evaluator.go
├── evaluator_test.go
├── main_test.go
├── utils.go
└── version.go

After: Package Structure

etcd-defrag/
├── main.go                              # Minimal entry point
│
├── cmd/
│   └── root.go                          # CLI setup, flag registration, execution
│
├── internal/                            # Private implementation packages
│   ├── agent/
│   │   ├── agent.go                     # Orchestration workflow
│   │   ├── agent_test.go
│   │   ├── compaction.go                # Compaction operations
│   │   ├── defrag.go                    # Defragmentation logic
│   │   ├── disalarm.go                  # Auto-disalarm in agent context
│   │   ├── health.go                    # Health check operations
│   │   └── status.go                    # Member status operations
│   │
│   ├── alarm/
│   │   └── disalarm.go                  # Alarm management utilities
│   │
│   ├── client/
│   │   └── client.go                    # etcd client wrapper & TLS config
│   │
│   ├── config/
│   │   ├── config.go                    # Configuration struct + defaults
│   │   └── validate.go                  # Configuration validation
│   │
│   ├── endpoint/
│   │   ├── endpoints.go                 # Endpoint resolution & filtering
│   │   └── endpoints_test.go
│   │
│   └── eval/
│       ├── evaluator.go                 # Rule expression evaluation
│       ├── evaluator_test.go
│       └── variables.go                 # Evaluation variable definitions
│
└── pkg/                                 # Public API packages
    └── version/
        └── version.go                   # Version information

Testing

# Run all tests
go test ./...

# Run specific package tests
go test ./internal/agent/...
go test ./internal/config/...
go test ./internal/eval/...
go test ./internal/endpoint/...

# Build verification
go build -o etcd-defrag .
./etcd-defrag --version

@ahrtr
Copy link
Copy Markdown
Owner

ahrtr commented Jan 25, 2026

Overall, I think this is a little over-engineering for a repo with only 2.2k line of source code. That said, I don't mind refactoring it. If you have bandwidth, please do it step by step (break down this PR into multiple smaller PRs, one PR for each step) so that it's easier to review.

Thanks for the effort anyway.

@ravisastryk
Copy link
Copy Markdown
Contributor Author

Overall, I think this is a little over-engineering for a repo with only 2.2k line of source code. That said, I don't mind refactoring it. If you have bandwidth, please do it step by step (break down this PR into multiple smaller PRs, one PR for each step) so that it's easier to review.

Thanks for the effort anyway.

Thanks for the feedback @ahrtr! I will split into these smaller PRs(probably will stack them):

  1. PR 1 - Extract pkg/version + internal/config (foundation packages)
  2. PR 2 - Extract internal/eval + internal/endpoint (self-contained modules)
  3. PR 3 - Extract internal/client + internal/alarm (client and alarm handling)
  4. PR 4 - Restructure agent + extract CLI to cmd/ (this PR, after rebase)

I will keep this PR open and rebase it after PRs 1-3 merge, so it becomes the final piece. Will open PRs shortly.

- Move version.go → pkg/version/version.go (public API)
- Move config.go → internal/config/config.go

Part of codebase reorganization (Issue ahrtr#137).
No functional changes.

PR 1 of 4 in the stacked refactoring series.
@ravisastryk ravisastryk force-pushed the refactor/reorganize-flat-codebase-to-package-structure branch from f4d0225 to a5b766a Compare January 25, 2026 21:33
- Move rule evaluation logic to internal/eval
- Move endpoint resolution and filtering to internal/endpoint
- Relocate associated tests to new package locations

Part of codebase reorganization (Issue ahrtr#137).
No functional changes.

PR 2 of 4 in the stacked refactoring series.
- Move disalarm.go → internal/alarm/disalarm.go
- Create internal/client/client.go for etcd client wrapper

Part of codebase reorganization (Issue ahrtr#137).
No functional changes.

PR 3 of 4 in the stacked refactoring series.
@ravisastryk ravisastryk force-pushed the refactor/reorganize-flat-codebase-to-package-structure branch from a5b766a to ad0c820 Compare January 25, 2026 22:07
@ravisastryk
Copy link
Copy Markdown
Contributor Author

@ahrtr Please review these in order, starting with PR1. After confirmation, I will open the remaining PRs sequentially.

PR1: #139
PR2: #140
PR3: #141

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.

2 participants