-
Notifications
You must be signed in to change notification settings - Fork 21
Description
Prerequisites
- I searched existing issues
Feature Summary
Refactor Makefiles globally to enable:
- Single Source of Truth: Each module owns its build logic
- Convention over Configuration: Predictable patterns reduce doc needs
- Flat over Nested: Minimize indirection
- Explicit over Implicit: Clear commands preferred over "magic"
- Optimize for Common Cases:
make build,make test,make runshould be obvious
Why:
For Developers
- Predictable Commands: Always
make build,make test,make docker - Less Cognitive Load: Maximum 2 layers (module → included .mk files)
- Easy Module Creation: Copy template Makefile, change MODULE_NAME
- Auto-Generated Help: Tools can parse .PHONY declarations
- Local = CI: Same commands work everywhere
For CI
- Consistent Invocation: Always
make -C <module> ci-test - Parallel Friendly: Each module is independent
- Easy to Cache: Module-level caching straightforward
- Clear Dependencies: No hidden make recursion
For Maintenance
- Single Source of Truth: Module list auto-discovered
- Less Duplication: Common patterns in
make/*.mk - Type Safety: Go modules include
go.mk - Easy Updates: Change
make/go.mk→ all Go modules updated - Remove 30-40% of Code: Eliminate delegation targets
Problem/Use Case
Cognitive Load
Multiple layers of Makefile indirection make it difficult to understand what's happening. The root one has ~700 lines, and then still delegates to 7+ sub-Makefiles. The common.mk one has ~300 lines and then each module has ~150 line one too. As a developer I must trace through 3-4 layers to understand a single test or build command. For example:
make docker-fault-quarantine-module
→ docker/Makefile: build-fault-quarantine-module
→ fault-quarantine-module/Makefile: docker-build
→ common.mk: docker-build target
→ Actual docker buildx commandTarget Names
Multiple naming schemes for similar operations which makes hard to predict what each command will do. For example:
lint-test(common pattern)lint-test-all(aggregation pattern)docker-build(new standard)image(legacy, calls docker-build)publish(legacy, calls docker-publish)build-gpu-health-monitor(docker/Makefile)docker-gpu-health-monitor(root Makefile)
Duplication
Multiple makefiles means more opportunity for duplication. Changing or adding requires edits in 3-5 files. Easy to break things. For example the Go module lists are defined 3+ times in multiple places:
# Root Makefile
GO_MODULES := health-monitors/syslog-health-monitor ...
# docker/Makefile
GO_MODULES := health-monitors/syslog-health-monitor ...
# health-monitors/Makefile
GO_HEALTH_MONITORS := syslog-health-monitor ...Verbose Targets
The root Makefile has 30+ individual module targets that just delegate. This makes maintenance harder. In most cases these don't offer real value over direct invocation.
.PHONY: lint-test-platform-connectors
lint-test-platform-connectors:
$(MAKE) -C platform-connectors lint-test
.PHONY: lint-test-health-events-analyzer
lint-test-health-events-analyzer:
$(MAKE) -C health-events-analyzer lint-test
# ... repeated 30 more timesHelp Targets
Each Makefile has extensive help text (30-80 lines) that duplicates target definitions. Help text gets out of sync with actual targets; maintenance overhead.
Docker Build Delegation Circle
I know we are moving towards ko for pure Go builds but for a while at least we will have to support both. docker/Makefile seems to centralize Docker builds but it just delegates back. Hard to follow:
# docker/Makefile
build-fault-quarantine-module:
$(MAKE) -C ../fault-quarantine-module docker-buildCI and Local Dev Inconsistent
CI workflows use inconsistent patterns (see below), and the local dev does not provide idiomatic way to run targets; difficult to debug CI issues locally.
- Some use:
make -C module lint-test - Some use:
make lint-test-module - Some use:
make -C docker build-module - Container builds use:
make -C module docker-build
Proposed Solution
Proposed Structure
nvsentinel/
├── Makefile # Simple orchestration only
├── make/ # Shared makefile utilities
│ ├── common.mk # Shared variables only
│ ├── go.mk # Go-specific targets
│ ├── python.mk # Python-specific targets
│ └── docker.mk # Docker-specific targets
└── <module>/
└── Makefile # Includes appropriate make/*.mk files
Core Target Standardization
Every module supports these targets (via included .mk files):
# Development workflow
make build # Build locally
make test # Run tests
make lint # Run linters
make clean # Clean artifacts
make help # Auto-generated help
# CI workflow
make ci-test # Run full CI test suite (lint + test + coverage)
make ci-build # Build for CI (may differ from local build)
# Docker workflow (if applicable)
make docker # Build Docker image locally
make docker-publish # Build and push Docker imageRoot Makefile - Simplified Orchestration
# Makefile - Root orchestration (simple and clear)
# Auto-discover all modules with Makefiles
MODULES := $(dir $(shell find . -maxdepth 3 -name Makefile -not -path "./Makefile"))
# --- Quick Development Commands ---
.PHONY: build test lint clean
build test lint clean: ## Build, test, and ling...
@for module in $(MODULES); do \
echo "Running $@ in $$module..."; \
$(MAKE) -C $$module $@ || exit 1; \
done
.PHONY: ci-test
ci-test:
@for module in $(MODULES); do \
$(MAKE) -C $$module ci-test || exit 1; \
done
# --- Docker Commands ---
.PHONY: docker docker-publish
docker docker-publish: ## Docker build and publish...
@for module in $(MODULES); do \
if $(MAKE) -C $$module -n $@ >/dev/null 2>&1; then \
$(MAKE) -C $$module $@ || exit 1; \
fi; \
done
# --- Utility Commands ---
.PHONY: qualify
qualify: lint test ## Run full pre-commit checks
.PHONY: modules
modules: # List all modules
@echo $(MODULES) | tr ' ' '\n'
.PHONY: help
help: ## Dynamically displays available commands based on their comments
@echo "Available make targets:"; \
grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk \
'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'Module Makefile - Simplified Pattern
Go Module Example (fault-quarantine-module/Makefile):
# fault-quarantine-module/Makefile
MODULE_NAME := fault-quarantine-module
HAS_DOCKER := true
# Include shared functionality
include ../make/common.mk
include ../make/go.mk
include ../make/docker.mk
# Module-specific overrides (if needed)
TEST_FLAGS := -race -timeout=30s
DOCKER_PLATFORMS := linux/amd64,linux/arm64
# Module-specific Docker variants
.PHONY: docker-dcgm3 docker-dcgm4
docker-dcgm3:
docker buildx build --build-arg DCGM_VERSION=3.x ...
docker-dcgm4:
docker buildx build --build-arg DCGM_VERSION=4.x ...Shared Make Files
make/common.mk - Only variables:
# make/common.mk - Shared variables only
REPO_ROOT := $(shell git rev-parse --show-toplevel)
MODULE_PATH := $(subst $(REPO_ROOT)/,,$(CURDIR))
# Docker configuration
NVCR_CONTAINER_REPO ?= nvcr.io
NGC_ORG ?= nv-ngc-devops
SAFE_REF_NAME ?= $(shell git rev-parse --abbrev-ref HEAD | sed 's/\//-/g')
PLATFORMS ?= linux/arm64,linux/amd64
# Common tools
GOLANGCI_LINT := golangci-lint
GOTESTSUM := gotestsummake/go.mk - Go targets:
# make/go.mk - Go-specific build targets
.PHONY: build
build:
go build ./...
.PHONY: test
test:
gotestsum --junitfile report.xml -- -race -coverprofile=coverage.txt ./...
.PHONY: lint
lint:
golangci-lint run --config $(REPO_ROOT)/.golangci.yml
.PHONY: ci-test
ci-test: lint test
go tool cover -func coverage.txt
gocover-cobertura < coverage.txt > coverage.xml
.PHONY: clean
clean:
go clean ./...
rm -f coverage.txt coverage.xml report.xmlmake/docker.mk - Docker targets:
# make/docker.mk - Docker build targets
ifeq ($(HAS_DOCKER),true)
.PHONY: docker
docker:
docker buildx build \
--platform $(PLATFORMS) \
--load \
-t $(MODULE_NAME):local \
-f Dockerfile \
$(REPO_ROOT)
.PHONY: docker-publish
docker-publish:
docker buildx build \
--platform $(PLATFORMS) \
--push \
-t $(NVCR_CONTAINER_REPO)/$(NGC_ORG)/nvsentinel-$(MODULE_NAME):$(SAFE_REF_NAME) \
-f Dockerfile \
$(REPO_ROOT)
endifComponent
Multiple Components