Skip to content

Commit e1f993f

Browse files
wkingwking
committed
populate-owners: Also slurp OWNERS_ALIASES
Some slurped repositories use OWNERS_ALIASES [1], so we need to pull these in to resolve entries in their OWNERS. OWNERS_ALIASES can only exist in the repo root [2]. That means we may need to namespace the upstream aliases to avoid collisions when assembing the composite OWNERS_ALIASES here. Defining aliases in upstream repos will be useful for situations where a particular repo team (e.g. the installer team) should have control over other directories within this repositories. For example, $ cat cluster/test-deploy/aws/OWNERS approvers: - installer-approvers reviewers: - installer-reviewers is simpler and more maintainable with an auto-maintained OWNERS_ALIASES than the copy/paste we used in 49f60b7 (cluster/test-deploy/aws/OWNERS: Delegate to openshift/installer, 2018-08-27, #1290). With script control over OWNERS_ALIASES, *this* repo can't define its own aliases. But that doesn't seem like a large limitation, because this repo is unlikely to need aliases that are not defined in *any* of the upstream repositories. And if we need to grow a location for local alias configuration, we can always add that in the future. I'm using 'git clone...' over SSH to access the files, because that provides access to private repositories. Using: https://github.com/raw/{organization}/{repository}/HEAD/{path} only works if you have an auth token or the repository is public. Using clone for two files is a bit heavy. Ideally we could pull just the two files we're interested. Git's archive command supports that use-case, but GitHub doesn't seem to have enabled the server side of that transaction: $ git archive --format=tar --remote ssh://[email protected]/openshift/installer.git HEAD OWNERS OWNERS_ALIASES Invalid command: 'git-upload-archive '/openshift/installer.git'' You appear to be using ssh to clone a git:// URL. Make sure your core.gitProxy config option and the GIT_PROXY_COMMAND environment variable are NOT set. fatal: The remote end hung up unexpectedly $ git --version git version 1.8.3.1 Until they enable that, I'm fine consuming more of their bandwidth than is strictly necessary ;). Using reflect in assertEqual gives us a nice, compact implementation. But the output can be a bit hard to read. With an introduced error: $ go test . ... --- FAIL: TestOrgRepos (0.00s) main_test.go:15: unexpected result: [0xc42006e1e0 0xc42006e240] != [0xc42006e2a0] --- FAIL: TestGetOwners (2.30s) main_test.go:15: unexpected result: &{Directories:[] Organization:openshift Repository:installer Owners:0xc42006e420 Aliases:<nil> Commit:aaa47f6a54f0fa0449d36de01ccb9f56729b608a} != &{Directories:[] Organiza ... I'd mocked up an assertEqual based on comparing json.MarshalIndent strings which produced: --- FAIL: TestOrgRepos (0.00s) main_test.go:30: unexpected result: [ { "directories": [ "/tmp/populate-owners-389376240/a/b" ], "organization": "a", "repository": "b" }, { "directories": [ "/tmp/populate-owners-389376240/c/d" ], "organization": "c", "repository": "d" } ] != [ { "directories": [ "/tmp/populate-owners-389376240/a/b" ], "organization": "a", "repository": "b" } ] --- FAIL: TestGetOwners (2.38s) main_test.go:30: unexpected result: { "organization": "openshift", "repository": "installer", "owners": { "approvers": [ "aaronlevy", "abhinavdahiya", "crawford", "smarterclayton", "wking", "yifan-gu" ], "reviewers": [ "vikramsk" ] }, "commit": "aaa47f6a54f0fa0449d36de01ccb9f56729b608a" } != { "organization": "openshift", "repository": "installer", "owners": { "approvers": [ "aaronlevy", "abhinavdahiya", "crawford", "smarterclayton", "wking", "yifan-gu" ], "reviewers": [ "vikramsk" ] }, "commit": "2587b3ed493c18747f2b37e1ab6daebdd277631a" } and of course, there are a number of third-party libraries that do an even better job (e.g. [3]). But I don't think this functionality is worth an external dependency, and Steve doesn't think it's worth maintaining much local code for comparison [4], so we're going with the more compact reflect approach despite the less-useful output. And the tests should all be passing anyway, right? ;) The GIT_*_DATE tick business is based on [5]. I haven't bothered running a new commit after each test.setup change, because we don't need a different commit hash. In production, extractOwners will be run on a fresh clone, so we will always have a different commit hash if the content changes. The stub populate-owners.sh wraps the 'go run' at Steve's request [6]. We no longer need anything Bash-specific, so I'm using a POSIX shebang in the wrapper. The 'exec' allows us to replace the shell command with Go without executing a new process [7] (although Go itself will proceed to launch a few processes). The O_TRUNC is an attempt to avoid occasional corruption like: diff --git a/ci-operator/jobs/openshift/azure-misc/OWNERS b/ci-operator/jobs/openshift/azure-misc/OWNERS index 132c684..bf0dec5 100644 --- a/ci-operator/jobs/openshift/azure-misc/OWNERS +++ b/ci-operator/jobs/openshift/azure-misc/OWNERS @@ -3,10 +3,11 @@ # See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md approvers: +- pweil- - jim-minter - kargakis - kwoodson -- mjudeikis +is - pweil- reviewers: - jim-minter although the exact source of that corruption is not clear to me. [1]: https://github.com/openshift/cluster-api-provider-aws/blob/44e974de04f496f9552ecd37b73cad01b6d69f4d/OWNERS_ALIASES [2]: https://github.com/kubernetes/community/blame/0741bdfbb56dcd4829754560f79a2f3da32cb34f/contributors/guide/owners.md#L54 [3]: https://godoc.org/github.com/stretchr/testify/assert#Equal [4]: #1285 (comment) [5]: https://github.com/git/git/blob/v2.18.0/t/test-lib-functions.sh#L128-L138 [6]: #1285 (comment) [7]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#exec
1 parent a68ff97 commit e1f993f

File tree

5 files changed

+823
-40
lines changed

5 files changed

+823
-40
lines changed

‎ci-operator/populate-owners.sh

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,6 @@
1-
#!/bin/bash
1+
#!/bin/sh
22

3-
# This script populates ConfigMaps using the configuration
4-
# files in these directories. To be used to bootstrap the
5-
# build cluster after a redeploy.
3+
# This script runs /tools/populate-owners
64

7-
set -o errexit
8-
set -o nounset
9-
set -o pipefail
10-
11-
12-
temp_workdir=$( mktemp -d )
13-
trap "rm -rf ${temp_workdir}" EXIT
14-
15-
function populate_owners() {
16-
local org="$1"
17-
local repo="$2"
18-
local target_dir="${temp_workdir}/${org}/${repo}"
19-
mkdir -p "${target_dir}"
20-
git clone --depth 1 --single-branch "[email protected]:${org}/${repo}.git" "${target_dir}"
21-
if [[ -f "${target_dir}/OWNERS" ]]; then
22-
cp "${target_dir}/OWNERS" "${jobs}/${org}/${repo}"
23-
if [[ -d "${config}/${org}/${repo}" ]]; then
24-
cp "${target_dir}/OWNERS" "${config}/${org}/${repo}"
25-
fi
26-
fi
27-
}
28-
29-
jobs="$( dirname "${BASH_SOURCE[0]}" )/jobs"
30-
config="$( dirname "${BASH_SOURCE[0]}" )/config"
31-
32-
for org_dir in $( find "${jobs}" -mindepth 1 -maxdepth 1 -type d ); do
33-
org="$( basename "${org_dir}" )"
34-
for repo_dir in $( find "${jobs}/${org}" -mindepth 1 -maxdepth 1 -type d ); do
35-
repo="$( basename "${repo_dir}" )"
36-
populate_owners "${org}" "${repo}" &
37-
done
38-
done
39-
40-
for job in $( jobs -p ); do
41-
wait "${job}"
42-
done
5+
REPO_ROOT="$(git rev-parse --show-toplevel)" &&
6+
exec go run "${REPO_ROOT}/tools/populate-owners/main.go"

‎tools/populate-owners/OWNERS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
approvers:
2+
- stevekuznetsov
3+
- wking

‎tools/populate-owners/README.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Populating `OWNERS` and `OWNERS_ALIASES`
2+
3+
This utility pulls `OWNERS` and `OWNERS_ALIASES` from upstream OpenShift repositories.
4+
Usage:
5+
6+
```console
7+
$ go run main.go
8+
```
9+
10+
Or, equivalently, execute [`populate-owners.sh`](../../ci-operator/populate-owners.sh) from anywhere in this repository.
11+
12+
Upstream repositories are calculated from `ci-operator/jobs/{organization}/{repository}`.
13+
For example, the presence of [`ci-operator/jobs/openshift/origin`](../../ci-operator/jobs/openshift/origin) inserts [openshift/origin][] as an upstream repository.
14+
15+
The `HEAD` branch for each upstream repository is pulled to extract its `OWNERS` and `OWNERS_ALIASES`.
16+
If `OWNERS` is missing, the utility will ignore `OWNERS_ALIASES`, even if it is present upstream.
17+
18+
Once all the upstream content has been fetched, the utility namespaces any colliding upstream aliases.
19+
Collisions only occur if multiple upstreams define the same alias with different member sets.
20+
When that happens, the utility replaces the upstream alias with a `{organization}-{repository}-{upstream-alias}`.
21+
For example, if [openshift/origin][] and [openshift/installer][] both defined an alias for `security` with different member sets, the utility would rename them to `openshift-origin-security` and `openshift-installer-security` respectively.
22+
23+
After namespacing aliases, the utility writes `OWNERS_ALIASES` to the root of this repository.
24+
If no upstreams define aliases, then the utility removes `OWNER_ALIASES` from the root of this repository.
25+
26+
The utility also iterates through the `ci-operator/jobs/{organization}/{repository}` directories, writing `OWNERS` to reflect the upstream configuration.
27+
If the upstream did not have an `OWNERS` file, the utility removes the associated `ci-operator/jobs/{organization}/{repository}/OWNERS`.
28+
29+
[openshift/origin]: https://github.com/openshift/origin
30+
[openshift/installer]: https://github.com/openshift/installer

0 commit comments

Comments
 (0)