Merge pull request #4083 from petr-muller/config-cleanup-final

DPTP-360: Config cleanup kick-off

Makefile

@@ -1,3 +1,29 @@
+APPLY := oc apply
+
+.PHONY: check check-core dry-core-admin core-admin dry-core core
+
+check: check-core
+	@echo "Service config check: PASS"
+
+check-core:
+	make -C core-services check
+	@echo "Core service config check: PASS"
+
+dry-core-admin:
+	make core-admin APPLY="$(APPLY) --dry-run=true"
+
+core-admin:
+	make -C core-services admin-resources APPLY="$(APPLY) --as=system:admin"
+
+dry-core:
+	make core APPLY="$(APPLY) --dry-run=true"
+
+core:
+	make -C core-services resources APPLY="$(APPLY)"
+
+# LEGACY TARGETS
+# You should not need to add new targets here.
+
 export RELEASE_URL=https://github.com/openshift/release.git
 export RELEASE_REF=master
 export SKIP_PERMISSIONS_JOB=0
@@ -347,10 +373,10 @@ metal-secrets:
 	-o yaml --dry-run | oc apply -n ocp -f -
 .PHONY: metal-secrets
 
-check:
+check-prow-config:
 	# test that the prow config is parseable
 	mkpj --config-path cluster/ci/config/prow/config.yaml --job-config-path ci-operator/jobs/ --job branch-ci-origin-images --base-ref master --base-sha abcdef
-.PHONY: check
+.PHONY: check-prow-config
 
 libpod:
 	$(MAKE) apply WHAT=projects/libpod/libpod.yaml

README.md

@@ -26,23 +26,46 @@ and are split into the following categories:
    for general information on template tests.
  - [`ci-operator/infra`](./ci-operator/infra/) contains manifests for infrastructure
    components used by the `ci-operator`. Contact a CI Administrator if you feel
-   like one of these should be edited.
+   like one of these should be edited. (**legacy**: do not add new services
+   here. Use [`core-services`](./core-services) or [`projects`](./projects)
+   instead.)
 
-## Cluster Configuration Manifests
+## Cluster and Service Configuration Manifests
+
+### Core Services and Configuration
+
+Manifests for important services (like [OpenShift CI cluster](https://api.ci.openshift.org/)
+and the CI service components that run on it) are present in the
+[`core-services`](./core-services) directory. The services configured in this
+directory are critical for some part of the OpenShift project development
+workflow, must meet basic quality criteria and must be deployed to the cluster
+automatically by a postsubmit job. See its [README](./core-services/README.md)
+for details.
+
+### Cluster Configuration Manifests (legacy)
 
 Manifests for cluster provisioning and installation live under [`cluster/`](./cluster/).
 The [OpenShift CI cluster](https://api.ci.openshift.org/) is configured with the
-manifests under [`cluster/ci/`](./cluster/ci/); clusters that are created by the
-testing infrastructure for validating OpenShift are configured with the profiles
-under [`cluster/test-deploy/`](./cluster/test-deploy/). For directions on how to
-set up clusters for development, see the [README](./cluster/test-deploy/README.md).
+manifests under [`cluster/ci/`](./cluster/ci/). (**legacy**: do not add new
+services here. Use [`core-services`](./core-services) or
+[`projects`](./projects) instead.)
+
+Clusters that are created by the testing infrastructure for validating OpenShift
+are configured with the profiles under [`cluster/test-deploy/`](./cluster/test-deploy/).
+For directions on how to set up clusters for development, see the [README](./cluster/test-deploy/README.md).
+
+### Custom and Legacy Service Configuration
 
-## Component Project Build Manifests
+Manifests for services that are in development, experimental, legacy or not
+critical in some other way are present in the [`projects`](./projects)
+directory. Compared to the [core services configuration](#core-services-and-configuration),
+these projects do not need to follow any common structure or conventions other
+than clear ownership. They must not interfere with the core services in any way.
 
-Manifests for building container images for component repositories live under
-[`projects/`](./projects/). This directory is deprecated; authors of components
-built by manifests in this directory should remove them and ensure that their
-component is appropriately built by the `ci-operator` instead.
+Additionally, manifests for building container images for component repositories
+live under [`projects/`](./projects/). This purpose is deprecated; authors of
+components built by manifests in this directory should remove them and ensure
+that their component is appropriately built by the `ci-operator` instead.
 
 ## Tooling Build Manifests
 

core-services/Makefile

@@ -0,0 +1,17 @@
+ifeq ($(APPLY),)
+	APPLY := oc apply
+endif
+
+SERVICES =
+
+.PHONY: resources admin-resources check
+
+check:
+	_hack/validate-core-service.sh _TEMPLATE # keep the template valid
+	$(foreach service,$(SERVICES),_hack/validate-core-service.sh $(service);)
+
+resources:
+	$(foreach service,$(SERVICES),make -C $(service) APPLY="$(APPLY)" resources;)
+
+admin-resources:
+	$(foreach service,$(SERVICES),make -C $(service) APPLY="$(APPLY)" admin-resources;)

core-services/README.md

@@ -0,0 +1,65 @@
+# Core Services and Configuration
+
+Manifests for important services (like [OpenShift CI cluster](https://api.ci.openshift.org/)
+and the CI service components that run on it) are present in this directory. The
+services configured here are critical for some part of the OpenShift project
+development workflow, must meet basic quality criteria and must be deployed to
+the cluster automatically by a postsubmit job.
+
+## How to add new service
+
+Create a new directory for your service, containing all [necessary files](#quality-criteria-and-conventions).
+You may copy the `_TEMPLATE` directory and start using the files there. Add
+manifests and other configuration as needed, and make sure the `Makefile` in
+your directory applies all config when its `resources` and `admin-resources` are
+built.
+
+Add the name of the directory to the `SERVICES` list in the [Makefile](./Makefile).
+You should not need to modify this or any other Makefile in any way.
+
+## Quality criteria and conventions
+
+1. All directories should contain `OWNERS`, `README.md` and `Makefile` files.
+   This is enforced by `make check` locally and by the `ci/prow/core-valid`
+   check on pull requests.
+2. The `Makefile` should provide `resources` and `admin-resources` targets.
+   Calling the former should create all resources for which admin permissions
+   are not necessary. The `config-updater` service account in the `ci` namespace
+   must have permissions to perform all actions done in the `resources` targets.
+   Calling `admin-resources` should create all resources for which admin
+   permissions is necessary. Presence of these targets is enforced by
+   `make check` locally and by the `ci/prow/core-valid` check on pull requests.
+   Additionally, `make dry-core{-admin}` runs the appropriate target in dry-run
+   mode. Passing `make dry-core` is enforced by the `ci/prow/core-dry` check.
+3. Makefiles and scripts called by them should use `$(APPLY)` variable instead
+   of `oc apply`. This allows the universal dry-run to work.
+4. Destination namespaces should always be specified within a manifest, never
+   by a `-n/--namespace` option or by relying on a currently set OpenShift
+   project.
+5. All ConfigMaps need to be set up for automated updates by the
+   `config-updater` Prow plugin.
+
+## How to apply
+
+There are three types of configuration: admin resources, other resources and
+ConfigMaps.
+
+### Automation
+
+1. Admin resources are not automatically applied to the cluster.
+2. Other resources are automatically applied to the cluster by a Prow
+   [postsubmit](https://prow.svc.ci.openshift.org/?job=branch-ci-openshift-release-master-core-apply)
+   after each PR is merged, and also [periodically](https://prow.svc.ci.openshift.org/?job=openshift-release-master-core-apply).
+3. ConfigMaps are automatically updated by the `config-updater` Prow plugin,
+   configured in its [config.yaml](../cluster/ci/config/prow/config.yaml) file.
+   Additionally, they are [periodically](https://prow.svc.ci.openshift.org/?job=openshift-release-master-config-bootstrapper)
+   synced by a Prow job.
+
+### Manual
+
+1. Admin resources can be created by users with `--as=system:admin` rights by
+   `make core-admin`.
+2. Other resources can be created by `make core`, provided the user has rights
+   to perform all necessary actions
+3. ConfigMaps can be manually created by the [config-bootstrapper](https://github.com/kubernetes/test-infra/tree/master/prow/cmd/config-bootstrapper)
+   tool.

core-services/_TEMPLATE/Makefile

@@ -0,0 +1,10 @@
+ifeq ($(APPLY),)
+	APPLY := oc apply
+endif
+
+export APPLY
+
+.PHONY: resources admin-resources
+
+resources:
+	$(APPLY) -f resource.yaml -n somewhere

core-services/_TEMPLATE/OWNERS

@@ -0,0 +1,2 @@
+approvers:
+  - dptp

core-services/_TEMPLATE/README.md

@@ -0,0 +1,3 @@
+# What is this
+
+Brief information about why this exists, plus any documentation that might be useful.

core-services/_hack/validate-core-service.sh

@@ -0,0 +1,41 @@
+#!/bin/bash
+# This script validates all core services meet quality criteria
+
+set -euo pipefail
+
+function validate_required_files() {
+    local service_path="$1"
+    for required in OWNERS README.md Makefile; do
+        local required_path="$service_path/$required"
+        if [[ ! -s "$required_path" ]]; then
+            echo "[ERROR] $required file not found: $required_path"
+            echo "[ERROR] All core services should have $required file"
+            return 1
+        fi
+    done
+
+    return 0
+}
+
+validate_makefile() {
+    local service_path="$1"
+
+    for target in "resources" "admin-resources"; do
+        if ! make -C "$service_path" "$target" --dry-run; then
+            echo "[ERROR] Dry-run of 'make $target' did not succeed, Makefile likely does not provide this required target"
+            return 1
+        fi
+    done
+
+    return 0
+}
+
+to_validate="$1"
+if [[ ! -d "$to_validate" ]]; then
+    echo "[ERROR] Directory not found: $to_validate"
+    echo "Usage: validate-core-service.sh DIRECTORY"
+    exit 1
+fi
+
+validate_required_files "$to_validate" &&
+    validate_makefile "$to_validate"