|
1 | 1 | # Contributing to ci-tools |
2 | 2 |
|
| 3 | +Thank you for your interest in contributing to CI-Tools! This guide will help you understand how to contribute effectively. |
| 4 | + |
| 5 | +## How to Contribute |
| 6 | + |
| 7 | +### 1. Fork and Clone |
| 8 | + |
| 9 | +```bash |
| 10 | +# Fork the repository on GitHub, then clone your fork |
| 11 | +git clone https://github.com/YOUR_USERNAME/ci-tools.git |
| 12 | +cd ci-tools |
| 13 | + |
| 14 | +# Add upstream remote |
| 15 | +git remote add upstream https://github.com/openshift/ci-tools.git |
| 16 | +``` |
| 17 | + |
| 18 | +### 2. Create a Branch |
| 19 | + |
| 20 | +```bash |
| 21 | +# Create a feature branch from main |
| 22 | +git checkout -b feature/my-feature |
| 23 | + |
| 24 | +# Or a bugfix branch |
| 25 | +git checkout -b fix/bug-description |
| 26 | +``` |
| 27 | + |
| 28 | +### 3. Make Your Changes |
| 29 | + |
| 30 | +- Write clear, readable code |
| 31 | +- Follow Go conventions and style |
| 32 | +- Add tests for new functionality |
| 33 | +- Update documentation as needed |
| 34 | + |
| 35 | +### 4. Test Your Changes |
| 36 | + |
| 37 | +```bash |
| 38 | +# Run unit tests |
| 39 | +make test |
| 40 | + |
| 41 | +# Run linters |
| 42 | +make lint |
| 43 | + |
| 44 | +# Run integration tests (if applicable) |
| 45 | +make integration |
| 46 | + |
| 47 | +# Verify code generation |
| 48 | +make verify-gen |
| 49 | +``` |
| 50 | + |
| 51 | +### 5. Commit Your Changes |
| 52 | + |
| 53 | +```bash |
| 54 | +# Stage changes |
| 55 | +git add . |
| 56 | + |
| 57 | +# Commit with descriptive message |
| 58 | +git commit -m "Add feature: description of changes" |
| 59 | +``` |
| 60 | + |
| 61 | +**Commit Message Guidelines:** |
| 62 | +- Use imperative mood ("Add feature" not "Added feature") |
| 63 | +- Keep first line under 72 characters |
| 64 | +- Add detailed description if needed |
| 65 | +- Reference issues: "Fix #123: description" |
| 66 | + |
| 67 | +### 6. Push and Create Pull Request |
| 68 | + |
| 69 | +```bash |
| 70 | +# Push to your fork |
| 71 | +git push origin feature/my-feature |
| 72 | +``` |
| 73 | + |
| 74 | +Then create a Pull Request on GitHub with: |
| 75 | +- Clear title and description |
| 76 | +- Reference to related issues |
| 77 | +- Screenshots/logs if applicable |
| 78 | +- Checklist of what was tested |
| 79 | + |
| 80 | +## Branching Model |
| 81 | + |
| 82 | +### Branch Naming |
| 83 | + |
| 84 | +- `feature/description` - New features |
| 85 | +- `fix/description` - Bug fixes |
| 86 | +- `docs/description` - Documentation updates |
| 87 | +- `refactor/description` - Code refactoring |
| 88 | +- `test/description` - Test improvements |
| 89 | + |
| 90 | +### Branch Strategy |
| 91 | + |
| 92 | +- **main** - Production-ready code |
| 93 | +- **Feature branches** - Created from main, merged back via PR |
| 94 | +- **Release branches** - For release-specific fixes (if needed) |
| 95 | + |
| 96 | +## Coding Standards |
| 97 | + |
| 98 | +### Go Style |
| 99 | + |
| 100 | +Follow [Effective Go](https://golang.org/doc/effective_go) and [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments). |
| 101 | + |
| 102 | +**Formatting:** |
| 103 | +```bash |
| 104 | +# Use gofmt |
| 105 | +make gofmt |
| 106 | + |
| 107 | +# Or goimports (handles imports) |
| 108 | +go run ./vendor/github.com/openshift-eng/openshift-goimports/ -m github.com/openshift/ci-tools |
| 109 | +``` |
| 110 | + |
| 111 | +**Key Guidelines:** |
| 112 | +- Use `gofmt` for formatting |
| 113 | +- Run `goimports` to organize imports |
| 114 | +- Follow naming conventions (exported = Capital, unexported = lowercase) |
| 115 | +- Keep functions focused and small |
| 116 | +- Add comments for exported functions/types |
| 117 | + |
| 118 | +### Code Organization |
| 119 | + |
| 120 | +- **Packages**: Group related functionality |
| 121 | +- **Files**: Keep files focused (one main type per file when possible) |
| 122 | +- **Tests**: `*_test.go` files alongside source |
| 123 | +- **Test Data**: Use `testdata/` directories |
| 124 | + |
| 125 | +### Error Handling |
| 126 | + |
| 127 | +```go |
| 128 | +// Good: Wrap errors with context |
| 129 | +if err != nil { |
| 130 | + return fmt.Errorf("failed to load config: %w", err) |
| 131 | +} |
| 132 | + |
| 133 | +// Good: Use errors.Is and errors.As for error checking |
| 134 | +if errors.Is(err, os.ErrNotExist) { |
| 135 | + // handle |
| 136 | +} |
| 137 | +``` |
| 138 | + |
| 139 | +### Logging |
| 140 | + |
| 141 | +```go |
| 142 | +// Use logrus for structured logging |
| 143 | +import "github.com/sirupsen/logrus" |
| 144 | + |
| 145 | +logrus.WithFields(logrus.Fields{ |
| 146 | + "config": configPath, |
| 147 | + "error": err, |
| 148 | +}).Error("Failed to load config") |
| 149 | +``` |
| 150 | + |
| 151 | +### Testing |
| 152 | + |
| 153 | +**Unit Tests:** |
| 154 | +```go |
| 155 | +func TestFunction(t *testing.T) { |
| 156 | + // Arrange |
| 157 | + input := "test" |
| 158 | + |
| 159 | + // Act |
| 160 | + result := Function(input) |
| 161 | + |
| 162 | + // Assert |
| 163 | + if result != expected { |
| 164 | + t.Errorf("Expected %v, got %v", expected, result) |
| 165 | + } |
| 166 | +} |
| 167 | +``` |
| 168 | + |
| 169 | +**Table-Driven Tests:** |
| 170 | +```go |
| 171 | +func TestFunction(t *testing.T) { |
| 172 | + tests := []struct { |
| 173 | + name string |
| 174 | + input string |
| 175 | + expected string |
| 176 | + }{ |
| 177 | + { |
| 178 | + name: "normal case", |
| 179 | + input: "test", |
| 180 | + expected: "result", |
| 181 | + }, |
| 182 | + } |
| 183 | + |
| 184 | + for _, tt := range tests { |
| 185 | + t.Run(tt.name, func(t *testing.T) { |
| 186 | + result := Function(tt.input) |
| 187 | + if result != tt.expected { |
| 188 | + t.Errorf("got %v, want %v", result, tt.expected) |
| 189 | + } |
| 190 | + }) |
| 191 | + } |
| 192 | +} |
| 193 | +``` |
| 194 | + |
3 | 195 | ## The Code Review Process |
4 | 196 | ### Submit a PR |
5 | 197 |
|
@@ -89,7 +281,145 @@ For the critical or complex changes, it is acceptable to review the code partial |
89 | 281 | ### Good Things |
90 | 282 | _If you see something nice in the CL, tell the developer, especially when they addressed one of your comments in a great way. Code reviews often just focus on mistakes, but they should offer encouragement and appreciation for good practices, as well. It’s sometimes even more valuable, in terms of mentoring, to tell a developer what they did right than to tell them what they did wrong._ [1] |
91 | 283 |
|
| 284 | +## FAQ |
| 285 | + |
| 286 | +### General Questions |
| 287 | + |
| 288 | +**What is CI-Tools?** |
| 289 | +CI-Tools is a collection of command-line utilities and services that power the OpenShift Continuous Integration system. It provides tools for managing CI configurations, orchestrating builds, running tests, and managing the CI infrastructure. |
| 290 | + |
| 291 | +**Who maintains CI-Tools?** |
| 292 | +The OpenShift Test Platform (TP) team maintains CI-Tools. See the [OWNERS](OWNERS) file for the list of maintainers. |
| 293 | + |
| 294 | +**How do I get started?** |
| 295 | +1. Read the [docs/README.md](docs/README.md) for overview |
| 296 | +2. Set up your environment ([docs/SETUP.md](docs/SETUP.md)) |
| 297 | +3. Explore the codebase ([docs/ARCHITECTURE.md](docs/ARCHITECTURE.md)) |
| 298 | + |
| 299 | +### Development Questions |
| 300 | + |
| 301 | +**How do I build the project?** |
| 302 | +```bash |
| 303 | +# Build all tools |
| 304 | +make build |
| 305 | + |
| 306 | +# Install to $GOPATH/bin |
| 307 | +make install |
| 308 | + |
| 309 | +# Build specific tool |
| 310 | +go build ./cmd/ci-operator |
| 311 | +``` |
| 312 | + |
| 313 | +**How do I run tests?** |
| 314 | +```bash |
| 315 | +# Run all unit tests |
| 316 | +make test |
| 317 | + |
| 318 | +# Run specific package tests |
| 319 | +go test ./pkg/api/... |
| 320 | + |
| 321 | +# Run integration tests |
| 322 | +make integration |
| 323 | +``` |
| 324 | + |
| 325 | +**What's the difference between unit tests, integration tests, and e2e tests?** |
| 326 | +- **Unit Tests**: Test individual functions/units in isolation |
| 327 | +- **Integration Tests**: Test components working together |
| 328 | +- **E2E Tests**: Test complete workflows end-to-end (require cluster access) |
| 329 | + |
| 330 | +### CI Operator Questions |
| 331 | + |
| 332 | +**What is CI Operator?** |
| 333 | +CI Operator is the core orchestration engine that reads YAML configurations and executes multi-stage image builds and tests on OpenShift clusters. |
| 334 | + |
| 335 | +**How do I test a CI Operator config locally?** |
| 336 | +```bash |
| 337 | +ci-operator \ |
| 338 | + --config=path/to/config.yaml \ |
| 339 | + --git-ref=org/repo@branch \ |
| 340 | + --target=test-name \ |
| 341 | + --namespace=my-namespace |
| 342 | +``` |
| 343 | + |
| 344 | +**What's the difference between `--target` and running all steps?** |
| 345 | +`--target` runs only the specified target and its dependencies. Without `--target`, all steps are run. |
| 346 | + |
| 347 | +### Troubleshooting |
| 348 | + |
| 349 | +**My build is failing, how do I debug it?** |
| 350 | +1. Check the config: `ci-operator-checkconfig --config=config.yaml` |
| 351 | +2. Run locally: `ci-operator --config=config.yaml --git-ref=...` |
| 352 | +3. Check logs: `oc logs -n namespace pod-name` |
| 353 | +4. Check artifacts: `oc rsync -n namespace pod:/artifacts ./local` |
| 354 | + |
| 355 | +**I'm getting "permission denied" errors** |
| 356 | +1. Check kubeconfig: `kubectl config current-context` |
| 357 | +2. Verify cluster access: `kubectl get nodes` |
| 358 | +3. Check RBAC permissions |
| 359 | +4. Try logging in: `oc login <cluster-url>` |
| 360 | + |
| 361 | +**Config changes aren't taking effect** |
| 362 | +1. Verify config is in correct location |
| 363 | +2. Check if Prow jobs were regenerated |
| 364 | +3. Verify PR was merged |
| 365 | +4. Check if Prow picked up changes (may take a few minutes) |
| 366 | + |
| 367 | +## Onboarding for New Contributors |
| 368 | + |
| 369 | +### What to Learn Before Contributing |
| 370 | + |
| 371 | +**Essential Knowledge:** |
| 372 | +1. **Go Programming Language** - Go basics, concurrency, testing, modules |
| 373 | +2. **Kubernetes/OpenShift** - Kubernetes API, Pods, Services, Controllers |
| 374 | +3. **YAML** - YAML syntax and working with YAML in Go |
| 375 | +4. **Git and GitHub** - Git workflow, GitHub Pull Request process |
| 376 | + |
| 377 | +**Recommended Knowledge:** |
| 378 | +1. **CI/CD Concepts** - Continuous Integration principles, build pipelines |
| 379 | +2. **Prow** - Prow architecture, job types, plugins |
| 380 | +3. **Container Technology** - Docker basics, container images, ImageStreams |
| 381 | +4. **Distributed Systems** - Event-driven architecture, controller pattern |
| 382 | + |
| 383 | +### Important Concepts |
| 384 | + |
| 385 | +**CI Operator Configuration**: Declarative YAML configurations that define base images, images to build, tests to run, and image promotion rules. See `pkg/api/config.go` and `pkg/config/load.go`. |
| 386 | + |
| 387 | +**Dependency Graph**: CI Operator builds a dependency graph to determine execution order. Steps have inputs (Requires) and outputs (Creates). See `pkg/api/graph.go`. |
| 388 | + |
| 389 | +**Step Execution Framework**: Steps are composable building blocks that implement a common interface. See `pkg/steps/step.go`. |
| 390 | + |
| 391 | +**Controller Pattern**: Many tools use the Kubernetes controller pattern - watch resources, reconcile desired state, handle errors gracefully. See `pkg/controller/util/reconciler.go`. |
| 392 | + |
| 393 | +### Beginner Roadmap |
| 394 | + |
| 395 | +**Phase 1: Understanding the Basics (Week 1-2)** |
| 396 | +- Read [docs/README.md](docs/README.md) and [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) |
| 397 | +- Set up development environment ([docs/SETUP.md](docs/SETUP.md)) |
| 398 | +- Build and run a simple tool locally |
| 399 | +- Read through `cmd/ci-operator/main.go` |
| 400 | + |
| 401 | +**Phase 2: Understanding CI Operator (Week 3-4)** |
| 402 | +- Study `pkg/api/config.go` to understand configuration structure |
| 403 | +- Study `pkg/api/graph.go` to understand dependency graphs |
| 404 | +- Study `pkg/steps/` to understand step execution |
| 405 | +- Create a simple test config and run it locally |
| 406 | + |
| 407 | +**Phase 3: Making Your First Contribution (Week 5+)** |
| 408 | +- Find a good first issue (labeled "good first issue" or "help wanted") |
| 409 | +- Understand the problem and proposed solution |
| 410 | +- Implement the fix or feature |
| 411 | +- Write tests |
| 412 | +- Create a Pull Request |
| 413 | + |
| 414 | +### Getting Help |
| 415 | + |
| 416 | +- **GitHub Issues**: Search existing issues or create new ones |
| 417 | +- **Pull Requests**: Ask questions in PR comments |
| 418 | +- **Slack**: #forum-testplatform (for OpenShift team members) |
| 419 | +- **Documentation**: Read the docs in `docs/` directory |
| 420 | + |
92 | 421 | ## References |
93 | | -1. [Google Engineering Practices Documentation](https://google.github.io/eng-practices/): [How to do a code review](https://google.github.io/eng-practices/review/reviewer/) and [The CL author’s guide to getting through code review](https://google.github.io/eng-practices/review/developer/) |
| 422 | + |
| 423 | +1. [Google Engineering Practices Documentation](https://google.github.io/eng-practices/): [How to do a code review](https://google.github.io/eng-practices/review/reviewer/) and [The CL author's guide to getting through code review](https://google.github.io/eng-practices/review/developer/) |
94 | 424 | 1. [The Code Review Process in Kubernetes community](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#the-code-review-process) |
95 | 425 | 1. [Submitting patches: the essential guide to getting your code into the kernel](https://www.kernel.org/doc/html/latest/process/submitting-patches.html) |
0 commit comments