Skip to content

AGENTS.md

Latest commit

 

History

History
97 lines (74 loc) · 9.18 KB

AGENTS.md

File metadata and controls

97 lines (74 loc) · 9.18 KB
Raw
applyTo **

Project guiding principles

Follow existing conventions

Before writing any code, analyze the existing codebase to understand and adopt its naming conventions, coding style, and language usage. Consistency is key to maintaining a readable and maintainable codebase.

Avoid code duplication and minimize dependencies

This repository contains a rich set of utility packages. Look for opportunities to use them before writing new code.

  • Look for broadly reusable utilities in the pkg/ directory, especially for concurrency, resiliency, data structures and data manipulation, operating system interaction, and test utilities.
  • For internal, project-specific utilities, use internal/ directory.
  • Add new dependencies only as a last resort, or when explicitly requested by the developer.

Reliability and performance are critical

This codebase implements several custom Kubernetes types and controllers. Implementation is highly parallel, and leverages multiple operating system processes and Docker/Podman containers. Clear, simple, reliable code is essential to ensure that the system works correctly and efficiently. Specifically:

  • All errors and edge cases must be handled.
  • Prefer simple solutions with proven run time and memory consumption characteristics over complex ones.
  • Long-running operations must accept a context.Context and respect its cancellation.
  • Release all resources (e.g., file descriptors, network connections) when they are no longer needed (lifetime context cancelled).
  • Avoid goroutine leaks by ensuring that all goroutines are properly terminated when they are no longer needed.
  • When reading from channels, always check that a channel is not closed and that the value read is valid (and not a zero value of some type).
  • All utility packages must have unit tests.

Code quality must not be compromised

  • Every change must build successfully and pass all tests (refer to Working inside the repository for details on how to run tests).
  • Code must be lint-free. Use make lint to check for linting issues.

Additional guidelines

Go language usage

Eliminate variable shadowing

  • When declaring a new variable, use a different name than the one used in the outer scope.

Avoid variable reuse (especially for errors)

  • If a function invokes multiple error-returning functions, use a different variable name for each error to avoid confusion.

Names of types, members and functions should reflect their PURPOSE

  • Names should reflect what things ARE and what they DO. They should be descriptive but concise. A developer reading code should be able to determine the purpose of a member/function/type from their name, without referring to the implementation.
  • Do not use meaningless, filler words such as "helper" or "fixture". For example, a component that creates sub-processes should be named subprocessLauncher, not subprocessHelper.

No single-line wrapper functions

Do not create "convenience" functions that merely wrap the underlying call to another function, without doing any additional work. Instead, call the target function directly.

Prefer extending existing interfaces over creating new ones

  • It's safe to extend existing interfaces when we need new functionality; we don't need to worry about breaking existing implementations because we control all of them. This also helps to keep the codebase simpler and more consistent.

Adhere to Code Placement Rules

Place new code in the correct location according to the project's structure:

  • API Definitions: Go in api/v1/.
  • Controller Logic: Goes in controllers/.
  • Integration Tests: Go in test/integration/.
  • Broadly Reusable Packages: Go in pkg/. These should be suitable for use in other projects.
  • Internal Packages: All other packages go in internal/. If in doubt, use internal/.

Errors and logging

  • Wrap inner errors with context using fmt.Errorf("context: %w", err) as appropriate.
  • Use lowercase for error messages to enable easy error composition.
  • When logging, start the log message with an uppercase letter.

File handling

  • Use OpenFile(), or (for temporary files) OpenTempFile() functions from github.com/microsoft/dcp/pkg/io package to open files. This function takes care of using appropriate file permissions in a cross-platform way.
  • Always close files after no longer needed, either by calling Close() from the method that opened the file (with defer statement), or when the lifetime context.Context of the file owner expires.

Code generation

  • Run make generate after making changes to API definitions (files under api/v1 folder).
  • Run make generate-grpc after making changes to protobuf definitions (files with .proto extension).

Test authoring

  • Integration tests must not use fake.NewClientBuilder or fake API clients. Use the standard integration test environment to exercise supported behavior through the real API server and running controllers.
  • Keep fault injection at the relevant executor, orchestrator, or API-operation boundary. Do not replace the API server with a fake store, bypass API validation, or build a separate manually driven reconciliation harness.
  • Use the shared delay tool for process lifecycle subprocesses. Do not re-execute the test binary with -test.run to implement subprocess fixtures.
  • Every test function should have a 1-3 sentence documentation comment describing its purpose. Use the following template for the test comments: "Verifies that [name of a type/method/function] does A, B, and C". The name of the test function should reflect its purpose too. Both the test function name, and the documentation comment, should be updated as necessary if the test function implementation is changed substantially.
  • Unless a tight, specific timeout is ESSENTIAL for testing a specific function, avoid using hard-coded timeouts (e.g. obtained via time.After()). Instead, use test context (created via testutil.GetTestContext()) and test context Done() channel for timeouts in tests.

Commenting code

  • Try to keep comments terse and focused. 1-2 sentences for structs and public method descriptions, and 1 sentence for struct members is ideal, but do not use this guideline to omit information that is important for proper use of the type or member. If necessary, make a longer comment.
  • Comments on structs, struct members, and public methods should describe ONLY the purpose and how to use it. Do not describe how a struct or method is used in other parts of the codebase, and do not tell stories about how design or implementation evolved over time.
  • Comments inside code blocks should be reserved for describing the INTENT for the code IF AND ONLY IF the intent is not obvious from the code itself (for example: why certain API is called when it is seemingly not necessary, or why operations are done in certain order if it seems a different order might work even better). Do not add comments merely describing what the code does, or where the name of used members or methods indicates the intent (for example: do not attach "we are closing the socket" comment to a closeSocket() method call).

Working inside the repository

Running commands

  • Do not run make without a target. It does not do anything useful (just displays a list of available targets, in human-readable form).

Running tests

  • Before running tests, make sure test prerequisites are built by running make test-prereqs.
  • Use make test to run unit tests and integration tests.
  • Tests for specific packages can be run with go test -count 1 -parallel 32 <package> (after building prerequisites), where <package> indicates the package that was changed and needs to be tested. This is significantly faster than running all tests, and should be used when working on a specific code change. For final verification, run make test to ensure all tests pass.
  • If working on a change involving a lot of goroutine synchronization, channel operations, and locking, run tests with -race flag to enable race detection. This works on MacOS and Linux ONLY, do not try this when the current OS is Windows.
  • Our test context (obtained via GetTestContext function from pkg/testutil package) can be adjusted via an environment variable to use non-standard test timeout. Set TEST_CONTEXT_TIMEOUT environment variable to number of seconds that will be used as timeout value. For example, TEST_CONTEXT_TIMEOUT=30 will make the test context expire after 30 seconds. 30 seconds is a good value when running individual tests.
  • No single test (not even those classified as "advanced" e.g. tests using real container orchestrator) should take more than 3 minutes (180 seconds) to run. Use AT MOST 3 minutes as a timeout when running individual tests, or subset of tests, with "go test" command ("-timeout" flag).
  • When doing final change verification, leverage a container runtime, if available, to verify the change on Linux too. E.g. on a Windows or a Mac machine with Docker, Podman, or WSLC, do final test run (and any other, change-specific verification, as applicable) using a Linux container. One container-based test run is sufficient unless the change affects container runtime support in DCP; in the latter case, do a test run with all available container runtimes.