From 42090669c88bf3463f344b48e47a8502d85381a5 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 26 Jun 2026 14:54:35 +0200 Subject: [PATCH 1/2] Add docs for Agentic SDLC, architecture and contributing Add AGENTS.md, ARCHITECTURE.md, CLAUDE.md, and CONTRIBUTING.md with verified codebase documentation. Move OTE test instructions from README.md into CONTRIBUTING.md to avoid duplication. --- AGENTS.md | 71 ++++++++++++++++ ARCHITECTURE.md | 150 ++++++++++++++++++++++++++++++++++ CLAUDE.md | 1 + CONTRIBUTING.md | 212 ++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 48 ++--------- 5 files changed, 443 insertions(+), 39 deletions(-) create mode 100644 AGENTS.md create mode 100644 ARCHITECTURE.md create mode 100644 CLAUDE.md create mode 100644 CONTRIBUTING.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..5b3747e11 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,71 @@ +# Cluster Kube Controller Manager Operator + +A static pod operator that manages the lifecycle of `kube-controller-manager` on OpenShift control plane nodes. Built on the [library-go](https://github.com/openshift/library-go) static pod operator framework, it observes cluster configuration, rotates CSR signing certificates, manages the cluster-policy-controller sidecar, and reconciles the target kube-controller-manager config into static pod manifests. Installed by the [Cluster Version Operator](https://github.com/openshift/cluster-version-operator) (CVO). + +See [ARCHITECTURE.md](ARCHITECTURE.md) for the full design and data flow. + +## Build and Test + +```bash +make build # Build all binaries (operator + OTE test runner) +make test-unit # Unit tests (./pkg/... ./cmd/...) +make verify # Formatting, vetting, golang version checks +make test-e2e # E2E operator tests (30m timeout) +make test-e2e-preferred-host # Preferred host e2e tests (1h timeout) +``` + +Go version: see `go.mod`. + +## Project Structure + +| Directory | Purpose | +|-----------|---------| +| `cmd/cluster-kube-controller-manager-operator/` | Operator binary entry point (operator, render, installer, pruner, resource-graph, cert-sync, recovery-controller) | +| `cmd/cluster-kube-controller-manager-operator-tests-ext/` | OTE test runner entry point | +| `pkg/operator/starter.go` | Operator initialization — creates clients, informers, and starts all controllers | +| `pkg/operator/targetconfigcontroller/` | Renders observed config + defaults into kube-controller-manager ConfigMaps/Secrets | +| `pkg/operator/configobservation/` | Configuration observers — each subdirectory watches a cluster resource type | +| `pkg/operator/certrotationcontroller/` | CSR signer certificate rotation and SA token signer controller | +| `pkg/operator/resourcesynccontroller/` | Syncs ConfigMaps/Secrets between namespaces | +| `pkg/operator/operatorclient/` | Namespace constants and operator client interfaces | +| `pkg/operator/gcwatchercontroller/` | Monitors garbage collector metrics via Prometheus | +| `pkg/cmd/operator/` | Operator subcommand — wires `RunOperator()` into the binary's command tree | +| `pkg/cmd/render/` | Bootstrap manifest renderer for cluster installation | +| `pkg/cmd/recoverycontroller/` | Certificate recovery controller (CSR signer + CSR approval) | +| `pkg/cmd/resourcegraph/` | Resource dependency chain visualization | +| `bindata/` | Embedded assets: default config, static pod template, RBAC, bootstrap manifests, vSphere resources | +| `manifests/` | CVO deployment manifests (namespace, deployment, RBAC, ServiceMonitors, alerts) | +| `test/e2e/` | E2E test suite (operator, network policy, SA token signer) | +| `test/e2e-preferred-host/` | Preferred host communication tests | +| `test/library/` | Shared test utilities | + +## Controller Pattern + +Controllers use the library-go `factory.Controller` base. Each controller has a `sync(ctx, syncContext)` method called by the framework on informer events or periodic resyncs. The operator wires them in `pkg/operator/starter.go` via `RunOperator()`. + +Config observers follow a specific pattern: each observer function receives the existing config and returns `(observedConfig, errors)`. Observers are registered in `pkg/operator/configobservation/configobservercontroller/observe_config_controller.go`. + +## Key Conventions + +- **Namespaces:** `openshift-kube-controller-manager-operator` (operator), `openshift-kube-controller-manager` (operand), `openshift-config` (user config), `openshift-config-managed` (platform config). Constants in `pkg/operator/operatorclient/interfaces.go`. +- **Logging:** `k8s.io/klog/v2` with verbosity levels +- **Error handling:** wrap with `fmt.Errorf("context: %w", err)` +- **Feature gates:** controllers that depend on feature gates use `FeatureGateAccessor` from library-go; wait for gates before starting +- **Platform conditionals:** vSphere legacy cloud provider resources are only deployed when `Infrastructure.Status.PlatformStatus.Type == VSpherePlatformType` +- **Upstream changes:** controllers that wrap library-go functionality should have fixes made upstream in [library-go](https://github.com/openshift/library-go), not here + +## Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md) for full guidelines. Key rules: + +- Do not modify files under `vendor/`. Use `go mod tidy && go mod vendor`. +- `bindata/assets.go` uses Go's `embed` directive to embed asset files — update the embedded files, not this file. +- Write unit tests for every change. E2E tests for significant features. +- Backwards compatibility matters — deprecate before removing. +- Before modifying the operator API, ensure there is a corresponding enhancement proposal in [openshift/enhancements](https://github.com/openshift/enhancements). API changes require design review and approval. + +## Testing + +- **Unit tests:** co-located `*_test.go` files, table-driven, `go test ./pkg/... ./cmd/...` +- **E2E tests:** suites under `test/e2e/` and `test/e2e-preferred-host/`, using Ginkgo v2. +- **OTE framework:** `cluster-kube-controller-manager-operator-tests-ext` binary. See [CONTRIBUTING.md](CONTRIBUTING.md#openshift-tests-extension-ote) for usage. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 000000000..a95f253a6 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,150 @@ +# Architecture + +## Overview + +The cluster-kube-controller-manager-operator is a static pod operator that manages the `kube-controller-manager` on OpenShift control plane nodes. It is deployed by the Cluster Version Operator (CVO) and uses the [library-go](https://github.com/openshift/library-go) static pod operator framework. + +The operator's primary responsibilities: +- Observe cluster configuration from multiple sources and synthesize kube-controller-manager config +- Manage kube-controller-manager static pods across control plane nodes (install, revision, prune) +- Rotate CSR signing certificates +- Manage service account token signing keys +- Run the cluster-policy-controller as a sidecar in the same static pod +- Report status via the `ClusterOperator/kube-controller-manager` resource + +## Data Flow + +```text + config.openshift.io resources Secrets/ConfigMaps + (Infrastructure, Network, Node, (service-account-private-key, + FeatureGate, Proxy, ...) service-ca, cloud-config, ...) + | | + v v + +--------------------------------------------------+ + | Config Observer Controllers | + | (observe external state, produce observedConfig) | + +------------------------+--------------------------+ + | observedConfig (sparse JSON) + v + +--------------------------------------------------+ + | Target Config Controller | + | (merge defaults + observedConfig + overrides | + | -> render ConfigMaps/Secrets in target ns) | + +------------------------+--------------------------+ + | ConfigMaps, Secrets + v + +--------------------------------------------------+ + | Static Pod Controllers (library-go) | + | Installer -> Revision Controller -> Pruner | + | (roll out new revisions to each control plane | + | node as static pod manifests) | + +------------------------+--------------------------+ + | + v + kube-controller-manager static pods + (one per control plane node) +``` + +## Operator Startup + +Entry point: `cmd/cluster-kube-controller-manager-operator/main.go` -> `pkg/cmd/operator/cmd.go` -> `pkg/operator/starter.go:RunOperator()`. + +Startup sequence: +1. Create clients (Kubernetes, config, operator) +2. Create informers for watched namespaces (see [Namespaces](#namespaces)) +3. Initialize feature gates via `FeatureGateAccessor` and wait for observation (1-minute timeout) +4. Create and start all controllers concurrently +5. Block until context cancellation + +## Namespaces + +| Namespace | Constant | Purpose | +|-----------|----------|---------| +| `openshift-config` | `GlobalUserSpecifiedConfigNamespace` | User-provided configuration (certs, CAs, cloud config) | +| `openshift-config-managed` | `GlobalMachineSpecifiedConfigNamespace` | Platform-managed configuration (generated CAs, signing certs) | +| `openshift-kube-controller-manager-operator` | `OperatorNamespace` | Operator deployment and its resources (CSR signer secrets) | +| `openshift-kube-controller-manager` | `TargetNamespace` | Operand: kube-controller-manager pods, config, certs | +| `kube-system` | — | Additional watched namespace | +| `openshift-infra` | — | Created by static resource controller | + +The `ResourceSyncController` copies ConfigMaps and Secrets between these namespaces as needed. + +## Static Pod Management + +The operator uses library-go's `staticpod.NewBuilder()` to manage kube-controller-manager static pods. This framework provides: + +- **Installer controller** — creates new static pod revisions on each control plane node. Uses a custom installer command (`cluster-kube-controller-manager-operator installer`). +- **Revision controller** — tracks revisions of ConfigMaps and Secrets. When any revisioned resource changes, a new revision is created. The first ConfigMap in the list (`kube-controller-manager-pod`) contains the static pod manifest template. +- **Pruner** — removes old static pod revisions to free disk space. +- **PDB guard** — ensures availability during upgrades (only on multi-node clusters; disabled for single-node). + +Resources are split into two categories (see the `deploymentConfigMaps`, `deploymentSecrets`, `CertConfigMaps`, and `CertSecrets` variables in `pkg/operator/starter.go` for the authoritative list): +- **Revisioned** — ConfigMaps and Secrets passed to `WithRevisionedResources`. A change to any of these triggers a new static pod revision. +- **Unrevisioned certs** — ConfigMaps and Secrets passed to `WithUnrevisionedCerts`. These are updated in-place without triggering a revision. + +## Configuration Observers + +Configuration observers watch external cluster resources and produce a sparse JSON config (`observedConfig`) that gets merged into the kube-controller-manager configuration. Each observer function receives the existing config and returns `(observedConfig, errors)`. + +Observers are registered in `pkg/operator/configobservation/configobservercontroller/observe_config_controller.go`: + +| Observer | Watches | Config paths set | +|----------|---------|-----------------| +| `CloudProviderObserver` | `Infrastructure` CR | Cloud provider config for the target namespace | +| `FeatureGatesObserver` (KCM) | `FeatureGate` CR | `extendedArguments.feature-gates` (excludes OpenShift-only gates) | +| `FeatureGatesObserver` (cluster-policy-controller) | `FeatureGate` CR | `featureGates` | +| `ObserveClusterCIDRs` | `Network` CR | `extendedArguments.cluster-cidr` | +| `ObserveServiceClusterIPRanges` | `Network` CR | `extendedArguments.service-cluster-ip-range` | +| `LatencyProfileObserver` | `Node` CR | `extendedArguments.node-monitor-grace-period` (Default: 40s, Medium: 2m, Low: 5m) | +| `ProxyObserver` | `Proxy` CR | `targetconfigcontroller.proxy` | +| `ObserveServiceCA` | Service CA ConfigMap | `serviceServingCert.certFile` | +| `ObserveInfraID` | `Infrastructure` CR | `extendedArguments.cluster-name` | +| `ObserveTLSSecurityProfile` | `APIServer` CR | TLS cipher suites and min version | + +Several observers include latency profile suppression logic to prevent config updates during extreme profile transitions. + +## Target Config Controller + +`pkg/operator/targetconfigcontroller/` takes the merged configuration (defaults + observedConfig + unsupportedConfigOverrides) and renders it into concrete resources in the target namespace: + +- `config` ConfigMap — the main kube-controller-manager configuration +- `kube-controller-manager-pod` ConfigMap — the static pod manifest template (kube-controller-manager + cluster-policy-controller containers) +- `cluster-policy-controller-config` ConfigMap — configuration for the cluster-policy-controller sidecar +- `controller-manager-kubeconfig` and `kube-controller-cert-syncer-kubeconfig` ConfigMaps +- `recycler-config` ConfigMap — persistent volume recycler configuration +- `serviceaccount-ca` ConfigMap — CA bundle for service account token verification + +The default kube-controller-manager configuration lives in `bindata/assets/config/defaultconfig.yaml`. It enables leader election, dynamic provisioning, and all controllers except `ttl`, `bootstrapsigner`, and `tokencleaner`. + +## Certificate Rotation + +`pkg/operator/certrotationcontroller/` manages CSR signing certificate rotation: + +- **CSR signer signer** (`csr-signer-signer`) — the CA that signs the CSR signer, stored in the operator namespace. Validity: 2x refresh period, refresh: 30 days (or 2 hours with `ShortCertRotation` feature gate). +- **CSR controller signer CA** (`csr-controller-signer-ca`) — CA bundle ConfigMap in the operator namespace. +- **CSR signer** (`csr-signer`) — the active CSR signing certificate, stored in the operator namespace and synced to the target namespace. Validity: 1x refresh period, refresh: half the refresh period. Signs kubelet CSRs. + +## SA Token Signer + +`pkg/operator/certrotationcontroller/satokensigner_controller.go` manages the `next-service-account-private-key` secret. When the current `service-account-private-key` is about to expire (or is missing), it generates a new key pair and stores it as the "next" key for rotation. + +## Recovery Controller + +`pkg/cmd/recoverycontroller/` provides a certificate recovery mechanism for when CSR signing certificates have expired: + +- Runs the cert rotation controller in `RefreshOnlyWhenExpired` mode to regenerate expired certificates +- Includes a CSR approval controller (`pkg/cmd/recoverycontroller/csrcontroller.go`) that auto-approves kubelet CSRs signed by the recovered signer +- Invoked via the `cert-recovery-controller` subcommand + +## Render Command + +`pkg/cmd/render/` is a bootstrap manifest renderer used during cluster installation. It takes installer-provided inputs (cloud provider config, feature gates, cluster CIDRs, images) and renders the initial set of manifests needed to bootstrap kube-controller-manager before the operator is running. The templates live in `bindata/bootkube/`. + +## Other Controllers + +| Controller | Purpose | +|-----------|---------| +| `StaticResourceController` | Applies static manifests from `bindata/` (namespace, service, RBAC, network policies). Conditionally deploys vSphere legacy cloud provider resources. | +| `ClusterOperatorStatusController` | Reports operator status, versions, and related objects to `ClusterOperator/kube-controller-manager` | +| `GarbageCollectorWatcherController` | Monitors garbage collector sync failures via Prometheus metrics on the kube-controller-manager | +| `LatencyProfileController` | Manages latency profile configuration, coordinates with the installer to reject extreme profiles during transitions | diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..97ed974ea --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,212 @@ +# Contributing to the OpenShift Control Plane Components/Repositories + +This document serves as a guide for contributing to the OpenShift components/repositories +that the OpenShift Control Plane group is responsible for maintaining. + +This document is explicitly for contributions to individual component repositories and not for high-level +feature proposals within OpenShift. + +Feature proposals should follow the OpenShift Enhancement Proposal process outlined in https://github.com/openshift/enhancements/blob/master/dev-guide/feature-zero-to-hero.md#openshift-feature-development-zero-to-hero-guide. +If you are looking for a review on an OpenShift Enhancement Proposal that involves changes to components +maintained by the control plane group, please request a review in the [`#forum-ocp-apiserver`](https://redhat.enterprise.slack.com/archives/CB48XQ4KZ) Slack channel. Requests for review may be redirected to more appropriate and/or more focused channels for discussion. + +This document contains the following sections: + +- [Code conventions](#code-conventions) - A collection of guidelines, style suggestions, and tips for writing code. +- [Testing guidelines](#testing-guidelines) - Guidelines and expectations for testing of contributions. +- [Pull Request process/guidelines](#pull-request-process-and-guidelines) - Guidelines and expectations of pull requests containing contributions. +- [Review expectations](#review-expectations) - Guidelines and expectations for requesting reviews and interacting with reviewers. + +## Code Conventions + +We largely follow the [Kubernetes Code Conventions](https://github.com/kubernetes/community/blob/main/contributors/guide/coding-conventions.md#code-conventions). + +Review both the Kubernetes Code Conventions and the ones specified here. +There will be some overlap. If any conventions are at odds with one another, prefer the conventions explicitly documented here. + +### Bash + +- Follow the [shell styleguide](https://google.github.io/styleguide/shellguide.html). +- Use [`shellcheck`](https://github.com/koalaman/shellcheck) to identify common mistakes or caveats. +- Ensure that all scripts run consistently across Linux and macOS. + +### Golang (Go) + +- Review [Effective Go](https://go.dev/doc/effective_go). +- Review common [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments). +- Review and avoid [Go Landmines](https://gist.github.com/lavalamp/4bd23295a9f32706a48f) +- Comment your code following the [Go comment conventions](https://go.dev/doc/comment). + - Comments should be meaningful and add context and/or explain choices that cannot be expressed through clear code. + - All exported types, functions, and methods must have descriptive comments. + - All unexported types, functions, and methods should have descriptive comments. +- When adding command-line flags, use dashes/hyphens (`-`) and not underscores (`_`). +- Naming + - Please consider package name when selecting an interface name, and avoid redundancy. For example, `storage.Interface` is better than `storage.StorageInterface`. + - Do not use uppercase characters, underscores, or dashes in package names. + - Please consider parent directory name when choosing a package name. For example, `pkg/controllers/autoscaler/foo.go` should say `package autoscaler` not `package autoscalercontroller`. + - Unless there's a good reason, the package foo line should match the name of the directory in which the .go file exists. + - Importers can use a different name if they need to disambiguate. + - Locks should be called `lock` and should never be embedded (always `lock sync.Mutex`). When multiple locks are present, give each lock a distinct name following Go conventions: `stateLock`, `mapLock` etc. +- Error handling + - Wrap errors with meaningful context before returning or logging them. +- When logging, follow the [Kubernetes Logging Conventions](https://github.com/kubernetes/community/blob/main/contributors/devel/sig-instrumentation/logging.md). +- When patching OpenShift-maintained forks of "upstream" repositories, patches should be as small as reasonably possible and should minimize touch points with code that is likely to change and impact the rebasing process. +- Dependencies must be vendored. When making changes to dependencies, ensure you've run `go mod tidy` and `go mod vendor`. + +### General + +Regardless of the programming language, make sure to take the following into consideration: +- Keep readability / maintainability in mind when writing code. + - Clever code and abstractions are often harder to reason about after the fact. Keep clever code and abstractions to the minimum necessary to accomplish the end-goal. +- Do not reinvent the wheel. Where possible, use existing standard library or vendored library functionality. If you are adding a net-new dependency, stop and think if you _really_ need to add the new dependency to achieve your goals. +- When writing tests, focus on testing the functional behaviors your code exercises. Avoid writing tests that are testing that the standard library works as expected or is trivial. Do not write tests just for line coverage. + +### Directory and File Conventions + +- Avoid package sprawl. Find an appropriate subdirectory for new packages. + - Libraries with no appropriate home belong in new package subdirectories of `pkg/util`. +- Avoid general utility packages. Packages called "util" are suspect. Instead, derive a name that describes your desired function. For example, the utility functions dealing with waiting for operations are in the `wait` package and include functionality like `Poll`. The full name is `wait.Poll`. +- All filenames should be lowercase. +- Go source files and directories use underscores, not dashes. + - Package directories should generally avoid using separators as much as possible. When package names are multiple words, they usually should be in nested subdirectories. + +## Testing Guidelines + +These are high-level testing guidelines. Where individual component repositories may have +additional testing guidelines to follow when making contributions. + +- All changes must include unit test additions/changes. + - Exceptions are at reviewer/approver discretion. +- Table-driven unit tests are preferred for testing multiple scenarios/inputs. For an example, see https://github.com/openshift/cluster-authentication-operator/blob/a493799952e9b6838021ccc7d15d3d37d7ad3508/pkg/controllers/externaloidc/externaloidc_controller_test.go#L108 . +- Unit tests must pass on all platforms (at the very least, Linux + macOS). +- Significant features should come with integration and/or end-to-end (e2e) tests where appropriate. + - End-to-end tests _may_ be scoped as a separate work item when the end-to-end tests for the component must be added to the openshift/origin repository instead of the component repository. Adding e2e tests to the component repository is preferred where possible. It is up to reviewer/approver discretion whether a contribution can be merged without end-to-end tests being implemented. +- Do not expect an asynchronous thing to happen immediately. Do not wait for one second and expect a pod to be running. Wait and retry instead. + + +If necessary, manual integration testing can be done by creating a cluster using the [`Cluster Bot` Slack App](https://redhat.enterprise.slack.com/archives/D03KX7M1CRJ). +Once you have a cluster created, you can follow some of the instructions in https://github.com/openshift/enhancements/blob/master/dev-guide/operators.md for guidance on how to +build component images and modify cluster-operators to deploy those images. + +Most component repos have existing tooling to run unit tests. Check for Makefiles and shell scripts that might run the unit tests. If none exist, you should be able to use standard testing tooling like `go test ./...` to run all tests for the project. https://pkg.go.dev/cmd/go/internal/test is a good reference for how the `go test` command works. + +## Pull Request Process and Guidelines + +This section assumes that you have a functional understanding of `git` and how to create a pull request on GitHub. + +If you do not, start with [GitHub's "Getting Started" guide](https://docs.github.com/en/get-started/start-your-journey). + +### Prerequisites + +Before you commit any changes or create any pull requests, you must adhere to OpenShift contribution policies. +Currently, that means enabling commit signature verification. + +See https://docs.google.com/document/d/1184EPSGunUkcSQYUK8T4a6iyawwi6f2zxdbB2jtG9nQ/edit?usp=sharing for more details on +how to adhere to the commit signature verification policy of OpenShift. + +### Creating a Pull Request + +When creating a pull request, include the following: + +- A brief, but descriptive, title. + - All pull requests _should_ link to a Jira ticket associated with the work. There is automation that performs this linking when prefixing the title with the Jira ticket identifier like: `CNTRLPLANE-XXXX: my pull request title`. For pull requests that have no Jira ticket associated with it, you can prefix it with `NO-JIRA:` to signal that there is not a Jira ticket associated with it. +- A useful description of the changes being made and why they are important. Include links to supporting documents and any additional context that reviewers may need. + +### CI / CD + +For CI/CD, OpenShift uses Prow to run various checks. This can include unit tests, e2e tests, linters, etc. + +The jobs configured for each repository are in https://github.com/openshift/release/tree/main/ci-operator/config/openshift . If you find yourself needing to add additional jobs, review the documentation at https://docs.ci.openshift.org/how-tos/contributing-openshift-release/ . + +There are often a mixture of required and optional checks as well as merge criteria that must be met before a pull request can merge. +When any of these checks fail, the GitHub Prow bot will leave a comment on the PR with links to the run of that check that failed. + +As the PR author, it is your responsibility to evaluate the failed checks and determine if there are any changes necessary to pass the checks. +If you suspect that the check failure was a flake, you can trigger retests by commenting `/retest` (or `/retest-required` for retesting only the required checks) on the PR. + +### Verifying your changes / Creating an OpenShift cluster from a PR + +As part of merging a PR, there is a requirement to verify that the changes you've made are working as expected using the `/verified` comment command. + +While there are a lot of scenarios where the existing CI/CD checks may be sufficient to verify your changes are working (and can be denoted by commenting `/verified by ci`), +there may be scenarios where manual verification is required. + +You can use the `Cluster Bot` Slack App to create a cluster from a PR by sending it a message in the format of `launch ${OCP_VERSION},${PR_LINK} ${PLATFORM},${VARIANT}`. +As an example, `launch 4.23,https://github.com/openshift/cluster-authentication-operator/pull/928 aws,techpreview` would launch an OpenShift 4.23 cluster with the changes made in openshift/cluster-authentication-operator#928 running on AWS with the TechPreviewNoUpgrade feature-set enabled. +For more information on what `Cluster Bot` can do, you can send it a message saying `help` and it will respond with additional documentation on how it can be used. + +Once you've verified your changes work as expected, you can mark the PR as verified by commenting `/verified by @{your_github_handle}` on the PR. + +### Additional Resources + +For more information regarding more general OpenShift pull request processes, the following resources are helpful: + +- https://docs.ci.openshift.org/architecture/jira +- https://docs.ci.openshift.org/ +- https://steps.ci.openshift.org/ + +## Review Expectations + +### Requesting a review + +If you are not a member of the OpenShift control plane team and you need a review on a PR, post it in the [#forum-ocp-apiserver](https://redhat.enterprise.slack.com/archives/CB48XQ4KZ) Slack channel or +reach out to folks outlined in the OWNERS file directly. + +If you are a member of the OpenShift control plane team, reviews should come from your feature team. In the event your feature team does not have someone that can approve +a PR, post it in the [#control-plane](https://redhat.enterprise.slack.com/archives/CC3CZCQHM) Slack channel. + +OpenShift uses AI code review tools as part of the code review process. +Before requesting a review, address all feedback from the code review agent(s). +It is up to your discretion as the contributor how you would like to address that feedback. +Responding with an explanation as to why you are not going to take action on a comment made +by the agent is an acceptable way to "address" its feedback. + +### Interacting with reviewers + +When interacting with reviewers/approvers: + +- Be professional. +- Be respectful of differing opinions, viewpoints, and experiences. +- Gracefully give and receive constructive feedback. +- Focus on what is best for the product/organization, not just us as individuals. + +A special note on the usage of AI - to respect the time of those that are reviewing your contribution, please do not use AI to respond to review comments. + +## OpenShift Tests Extension (OTE) + +This repository is compatible with the [OpenShift Tests Extension (OTE)](https://github.com/openshift-eng/openshift-tests-extension) framework. +The test binary and suite definitions live in [`cmd/cluster-kube-controller-manager-operator-tests-ext`](cmd/cluster-kube-controller-manager-operator-tests-ext/main.go). + +### Building the test binary + +```bash +make build +``` + +### Running test suites and tests + +```bash +# Run a suite +./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/parallel + +# Run a specific test +./cluster-kube-controller-manager-operator-tests-ext run-test "test-name" + +# Run a suite with concurrency limited to 1 (serial execution) +./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/disruptive -c 1 + +# Run with JUnit output +./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/parallel --junit-path=/tmp/junit.xml +``` + +### Listing available tests and suites + +```bash +# List all test suites +./cluster-kube-controller-manager-operator-tests-ext list suites + +# List tests in a suite +./cluster-kube-controller-manager-operator-tests-ext list tests --suite=openshift/cluster-kube-controller-manager-operator/operator/parallel +``` + +For more information about the OTE framework, see the [openshift-tests-extension documentation](https://github.com/openshift-eng/openshift-tests-extension). diff --git a/README.md b/README.md index 031c24adc..37b56b471 100644 --- a/README.md +++ b/README.md @@ -93,12 +93,12 @@ spec: Currently the operator log levels correspond to: -| logLevel | log level | -| -------- | --------- | -| Normal | 2 | -| Debug | 4 | -| Trace | 6 | -| TraceAll | 8 | +| operatorLogLevel | log level | +| ---------------- | --------- | +| Normal | 2 | +| Debug | 4 | +| Trace | 6 | +| TraceAll | 8 | ``` @@ -167,38 +167,8 @@ $ OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE=docker.io//origin-release:lates ## Tests -This repository is compatible with the [OpenShift Tests Extension (OTE)](https://github.com/openshift-eng/openshift-tests-extension) framework. - -### Building the test binary - -```bash -make build -``` - -### Running test suites and tests +See the [OpenShift Tests Extension (OTE)](CONTRIBUTING.md#openshift-tests-extension-ote) section in `CONTRIBUTING.md` for instructions on building and running tests. -```bash -# Run a specific test suite -./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/parallel - -# Run with parallel execution (4 workers) -./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/parallel -c 4 - -# Run with JUnit output -./cluster-kube-controller-manager-operator-tests-ext run-suite openshift/cluster-kube-controller-manager-operator/operator/parallel --junit-path "${ARTIFACT_DIR}/junit.xml" - -# Run a specific test -./cluster-kube-controller-manager-operator-tests-ext run-test "test-name" -``` - -### Listing available tests and suites - -```bash -# List all test suites -./cluster-kube-controller-manager-operator-tests-ext list suites - -# List tests in a suite -./cluster-kube-controller-manager-operator-tests-ext list tests --suite=openshift/cluster-kube-controller-manager-operator/operator/parallel -``` +## Contributing -For more information about the OTE framework, see the [openshift-tests-extension documentation](https://github.com/openshift-eng/openshift-tests-extension). +See [CONTRIBUTING.md](CONTRIBUTING.md). From ed53fca2156802d47049347d51d5a786c7e2ca82 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Mon, 20 Jul 2026 11:32:53 +0200 Subject: [PATCH 2/2] Pull in critical rules, design decisions, and README rewrite from PR #944 - AGENTS.md: add Critical Rules and What NOT to Do sections - ARCHITECTURE.md: add Design Decisions section (8 items) - README.md: streamline with Quick Start, concise sections, doc links --- AGENTS.md | 23 ++++++- ARCHITECTURE.md | 18 +++++ README.md | 172 +++++++++++++++--------------------------------- 3 files changed, 93 insertions(+), 120 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5b3747e11..c76b52f28 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,13 +54,34 @@ Config observers follow a specific pattern: each observer function receives the - **Platform conditionals:** vSphere legacy cloud provider resources are only deployed when `Infrastructure.Status.PlatformStatus.Type == VSpherePlatformType` - **Upstream changes:** controllers that wrap library-go functionality should have fixes made upstream in [library-go](https://github.com/openshift/library-go), not here +## Critical Rules + +1. **Never edit `vendor/` directly.** Change `go.mod`, then `go mod tidy && go mod vendor`. Always commit vendor changes separately from code changes for reviewable diffs. + +2. **Static pod template changes affect all control plane nodes.** `bindata/assets/kube-controller-manager/pod.yaml` defines four containers (kube-controller-manager, cluster-policy-controller, cert-syncer, recovery-controller). Changes here trigger rolling restarts across control plane. + +3. **CVO manifest ordering matters.** Files in `manifests/` are prefixed `0000_25_` for run-level 25. This operator must upgrade after kube-apiserver (run-level 20). Don't change the prefix. + +4. **Cert rotation has safety gates.** The SA token signer waits for bootstrap node departure and uses a 5-minute promotion delay. Don't bypass these — they prevent token validation failures cluster-wide. + +5. **This operator does not run in HyperShift.** The operator Deployment is excluded from hosted control plane topologies. HyperShift's control-plane-operator manages KCM directly. + +6. **Feature gate changes cause operator exit.** The operator process calls `os.Exit(0)` when the resolved feature gate set changes (by design, via library-go). This is a restart, not a crash. + +## What NOT to Do + +- **Don't read cluster config directly in TargetConfigController.** Use the ObservedConfig pattern — ConfigObserver writes to the CR status, TargetConfigController reads from there. +- **Don't add HyperShift logic.** This operator is standalone-only. HyperShift has its own KCM management. +- **Don't modify `pkg/operator/configobservation/network/` without networking team review.** It has separate OWNERS. +- **Don't skip `make verify` before submitting.** CI runs gofmt, govet, and Go version checks. + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md) for full guidelines. Key rules: - Do not modify files under `vendor/`. Use `go mod tidy && go mod vendor`. - `bindata/assets.go` uses Go's `embed` directive to embed asset files — update the embedded files, not this file. -- Write unit tests for every change. E2E tests for significant features. +- Write unit tests for behavior and implementation changes. E2E tests for significant features. Documentation-only changes need `make verify` to pass. - Backwards compatibility matters — deprecate before removing. - Before modifying the operator API, ensure there is a corresponding enhancement proposal in [openshift/enhancements](https://github.com/openshift/enhancements). API changes require design review and approval. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a95f253a6..85d435136 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -148,3 +148,21 @@ The default kube-controller-manager configuration lives in `bindata/assets/confi | `ClusterOperatorStatusController` | Reports operator status, versions, and related objects to `ClusterOperator/kube-controller-manager` | | `GarbageCollectorWatcherController` | Monitors garbage collector sync failures via Prometheus metrics on the kube-controller-manager | | `LatencyProfileController` | Manages latency profile configuration, coordinates with the installer to reject extreme profiles during transitions | + +## Design Decisions + +1. **Static pod pattern over Deployment:** The kube-controller-manager runs as a static pod managed by kubelet, not as a Deployment. This avoids a circular dependency — KCM manages controllers that Deployments depend on (e.g., service account token controller). The operator writes pod manifests that kubelet picks up directly. + +2. **Revision-based rollouts:** Configuration changes create new revisions (numbered copies of ConfigMaps/Secrets). The installer controller rolls out one node at a time by writing a new static pod manifest referencing the latest revision. This provides rollback capability and audit trail. + +3. **Cluster-policy-controller as a sidecar:** The cluster-policy-controller ([openshift/cluster-policy-controller](https://github.com/openshift/cluster-policy-controller)) runs as a container in the KCM static pod rather than as a separate Deployment. This was a deliberate decision made for OpenShift 4.3 (PR #297, October 2019). The primary driver is a **bootstrap chicken-and-egg problem**: CPC's controllers (namespace SCC allocation, quota reconciliation, PSA label syncing) must be running before any Deployments can be scheduled — pods cannot be created without UID range and SELinux label allocation. Placing these controllers in a static pod breaks the circular dependency. The KCM pod was chosen because CPC shares the same service account (`system:kube-controller-manager`), RBAC, certificates, kubeconfig, and leader election namespace, avoiding infrastructure duplication. + +4. **5-minute SA key promotion delay:** New service account signing keys are staged in `next-service-account-private-key` for 5 minutes before promotion. This gives the kube-apiserver time to observe the new public key via the `sa-token-signing-certs` bundle, preventing token validation failures during rotation. + +5. **Bootstrap node departure gate:** SA token key rotation is blocked until the bootstrap node has left the cluster. The bootstrap node uses a different signing key; rotating before it departs could cause token validation failures for workloads it started. + +6. **ObservedConfig indirection:** Rather than reading cluster config directly in the target config controller, a separate ConfigObserver writes a merged JSON blob to `.status.observedConfig` on the CR. This decouples config sources from config consumers and makes the effective config inspectable via `oc get kubecontrollermanager cluster -o jsonpath='{.status.observedConfig}'`. + +7. **24-hour bootstrap signer with operator takeover:** The installer deliberately creates a short-lived CSR signer (24h) to minimize trust window during bootstrap. This operator's cert rotation controller takes ownership and issues a longer-lived replacement, ensuring the cluster transitions from minimal-trust bootstrap to managed cert lifecycle. + +8. **UseMoreSecureServiceCA bypasses ObservedConfig (tech debt):** The TargetConfigController reads `.spec.useMoreSecureServiceCA` directly from the operator spec rather than going through the ObservedConfig pattern. This is acknowledged in code as needing migration to a config observer, but requires changes to the observedConfig format. diff --git a/README.md b/README.md index 37b56b471..a64648168 100644 --- a/README.md +++ b/README.md @@ -1,127 +1,70 @@ # Kubernetes Controller Manager operator -The Kubernetes Controller Manager operator manages and updates the [Kubernetes Controller Manager](https://github.com/kubernetes/kubernetes) deployed on top of -[OpenShift](https://openshift.io). The operator is based on OpenShift [library-go](https://github.com/openshift/library-go) framework and it -is installed via [Cluster Version Operator](https://github.com/openshift/cluster-version-operator) (CVO). +The Kube Controller Manager operator manages and updates the [kube-controller-manager](https://github.com/kubernetes/kubernetes) deployed on top of [OpenShift](https://openshift.io). The operator is based on the OpenShift [library-go](https://github.com/openshift/library-go) framework and is installed via the [Cluster Version Operator](https://github.com/openshift/cluster-version-operator) (CVO). It contains the following components: * Operator * Bootstrap manifest renderer -* Installer based on static pods +* Static pod installer * Configuration observer -By default, the operator exposes [Prometheus](https://prometheus.io) metrics via `metrics` service. -The metrics are collected from following components: +## Quick Start -* Kubernetes Controller Manager operator +### Prerequisites +- Go (see version in `go.mod`) +- Access to an OpenShift cluster (for e2e testing) -## Configuration - -The configuration for the Kubernetes Controller Manager is coming from: - -* a [default config](https://github.com/openshift/cluster-kube-controller-manager-operator/blob/master/bindata/assets/config/defaultconfig.yaml) +### Building +```bash +make build +``` -## Debugging +### Running Tests -Operator also expose events that can help debugging issues. To get operator events, run following command: +```bash +# Unit tests +make test-unit -``` -$ oc get events -n openshift-kube-controller-manager-operator +# E2E tests (requires a running OpenShift cluster) +make test-e2e ``` -This operator is configured via [`KubeControllerManager`](https://github.com/openshift/api/blob/master/operator/v1/types_kubecontrollermanager.go) custom resource: +### Verification +```bash +make verify ``` -$ oc describe kubecontrollermanager -``` -```yaml -apiVersion: operator.openshift.io/v1 -kind: KubeControllerManager -metadata: - name: cluster -spec: - managementState: Managed - ... -``` -The log level of individual kube-controller-manager instances can be increased by setting `.spec.logLevel` field: -``` -$ oc explain KubeControllerManager.spec.logLevel -KIND: KubeControllerManager -VERSION: operator.openshift.io/v1 -FIELD: logLevel -DESCRIPTION: - logLevel is an intent based logging for an overall component. It does not - give fine grained control, but it is a simple way to manage coarse grained - logging choices that operators have to interpret for their operands. Valid - values are: "Normal", "Debug", "Trace", "TraceAll". Defaults to "Normal". -``` -For example: -```yaml -apiVersion: operator.openshift.io/v1 -kind: KubeControllerManager -metadata: - name: cluster -spec: - logLevel: Debug - ... -``` - -Currently the log levels correspond to: -| logLevel | log level | -| -------- | --------- | -| Normal | 2 | -| Debug | 4 | -| Trace | 6 | -| TraceAll | 10 | +## Configuration +The Kube Controller Manager is configured via the [`KubeControllerManager`](https://github.com/openshift/api/blob/main/operator/v1/types_kubecontrollermanager.go) custom resource: -Similarly, the log level of cluster-kube-controller-manager-operator can be increased by setting the `.spec.operatorLogLevel` field: -For example: -```yaml -apiVersion: operator.openshift.io/v1 -kind: KubeControllerManager -metadata: - name: cluster -spec: - operatorLogLevel: Debug - ... +```bash +oc describe kubecontrollermanager cluster ``` -Currently the operator log levels correspond to: +The default configuration is in [bindata/assets/config/defaultconfig.yaml](bindata/assets/config/defaultconfig.yaml). -| operatorLogLevel | log level | -| ---------------- | --------- | -| Normal | 2 | -| Debug | 4 | -| Trace | 6 | -| TraceAll | 8 | +Log verbosity can be tuned via `.spec.logLevel` (for the operand) and `.spec.operatorLogLevel` (for the operator). Valid values: `Normal`, `Debug`, `Trace`, `TraceAll`. +## Debugging -``` -$ oc explain kubecontrollermanager -``` -to learn more about the resource itself. - -The current operator status is reported using the `ClusterOperator` resource. To get the current status you can run follow command: +```bash +# Operator events +oc get events -n openshift-kube-controller-manager-operator +# Operator status +oc get clusteroperator/kube-controller-manager ``` -$ oc get clusteroperator/kube-controller-manager -``` - - -## Developing and debugging the operator -In the running cluster [cluster-version-operator](https://github.com/openshift/cluster-version-operator/) is responsible -for maintaining functioning and non-altered elements. In that case to be able to use custom operator image one has to -perform one of these operations: +## Developing -1. Set your operator in umanaged state, see [here](https://github.com/openshift/enhancements/blob/master/dev-guide/cluster-version-operator/dev/clusterversion.md) for details, in short: +To use a custom operator image on a running cluster, override CVO management for the operator deployment: -``` +```bash oc patch clusterversion/version --type='merge' -p "$(cat <<- EOF spec: overrides: @@ -134,41 +77,32 @@ EOF )" ``` -2. Scale down cluster-version-operator: - -``` -oc scale --replicas=0 deploy/cluster-version-operator -n openshift-cluster-version -``` - -IMPORTANT: This apprach disables cluster-version-operator completly, whereas previous only tells it to not manage a kube-controller-manager-operator! - -After doing this you can now change the image of the operator to the desired one: +Then patch the deployment to use your image: +```bash +oc patch deployment/kube-controller-manager-operator -n openshift-kube-controller-manager-operator \ + -p '{"spec":{"template":{"spec":{"containers":[{"name":"kube-controller-manager-operator","image":"","env":[{"name":"OPERATOR_IMAGE","value":""}]}]}}}}' ``` -oc patch deployment/kube-controller-manager-operator -n openshift-kube-controller-manager-operator -p '{"spec":{"template":{"spec":{"containers":[{"name":"kube-controller-manager-operator","image":"/cluster-kube-controller-manager-operator","env":[{"name":"OPERATOR_IMAGE","value":"/cluster-kube-controller-manager-operator"}]}]}}}}' -``` - - -## Developing and debugging the bootkube bootstrap phase -The operator image version used by the [installer](https://github.com/openshift/installer/blob/master/pkg/asset/ignition/bootstrap/) bootstrap phase can be overridden by creating a custom origin-release image pointing to the developer's operator `:latest` image: +## Tests -``` -$ IMAGE_ORG= make images -$ docker push /origin-cluster-kube-controller-manager-operator +This repository uses the [OpenShift Tests Extension (OTE)](https://github.com/openshift-eng/openshift-tests-extension) framework. See the [OTE section in CONTRIBUTING.md](CONTRIBUTING.md#openshift-tests-extension-ote) for build and run instructions. -$ cd ../cluster-kube-apiserver-operator -$ IMAGES=cluster-kube-controller-manager-operator IMAGE_ORG= make origin-release -$ docker push /origin-release:latest +## Metrics -$ cd ../installer -$ OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE=docker.io//origin-release:latest bin/openshift-install cluster ... -``` +The operator exposes [Prometheus](https://prometheus.io) metrics via the `metrics` service by default. -## Tests +## Documentation -See the [OpenShift Tests Extension (OTE)](CONTRIBUTING.md#openshift-tests-extension-ote) section in `CONTRIBUTING.md` for instructions on building and running tests. +- [ARCHITECTURE.md](ARCHITECTURE.md) — Design decisions and component architecture +- [CONTRIBUTING.md](CONTRIBUTING.md) — How to submit changes +- [AGENTS.md](AGENTS.md) — AI agent instructions -## Contributing +## Related Repositories -See [CONTRIBUTING.md](CONTRIBUTING.md). +- [openshift/api](https://github.com/openshift/api) — API types including `KubeControllerManager` +- [openshift/library-go](https://github.com/openshift/library-go) — Shared operator framework +- [openshift/cluster-version-operator](https://github.com/openshift/cluster-version-operator) — Manages this operator's lifecycle +- [openshift/cluster-kube-apiserver-operator](https://github.com/openshift/cluster-kube-apiserver-operator) — Sibling control plane operator +- [openshift/cluster-kube-scheduler-operator](https://github.com/openshift/cluster-kube-scheduler-operator) — Sibling control plane operator +- [openshift/cluster-policy-controller](https://github.com/openshift/cluster-policy-controller) — Runs as a sidecar in the KCM static pod