core-services/prow/02_config/generate-boskos: For spitting out lots of names #11752

wking · 2020-09-10T21:22:16Z

Quotas for some providers depend on both account-scoped resources (e.g. AWS elastic IPs default to five per account), region-scoped resources (e.g. AWS VPCs default to five per region), potentialy other scoping (e.g. AWS NAT gateways default to five per availability zone). This commit moves us away from account-scoped leases and towards region-scoped leases in account-scoped buckets. That allows us to say things like "on Azure, we have more capacity in centralus than we do in our other regions".

The generating script is because typing out us-east-1 50 times is tedious and hard to review, and today there is apparently no support in the Boskos config directly to ask for $N copies of a given name.

I'm also adding myself as an owner, so DPTP doesn't have to bother with lease adjustment. Other CI-platform maintainers should feel free to add themselves as well, if they want to approve lease adjustments to their platforms.

core-services/prow/02_config/generate-boskos.py

wking · 2020-09-15T22:25:55Z

I've pushed cc5e7eb -> e303bcd070, rebasing around #11772 and adding make check-boskos, make boskos-config, and a boskos-config presubmit. Which seems to be failing...

wking · 2020-09-15T22:37:34Z

Fixed the position of the new presubmit job in the YAML file with e303bcd070 -> 0cf4ca96f8. Just have to figure out which image to use for the presubmit so we get make, or whether we want to skip make and just inline the command it wraps.

wking · 2020-09-15T23:04:34Z

I've pushed 0cf4ca96f8 -> 2dd18757cf, dropping make and using the registry.svc.ci.openshift.org/ci/python-validation image, at @stevekuznetsov 's suggestion.

I want this so '#!/usr/bin/env python' gets a version of Python that has PyYAML installed. We currently only install it for Python 3, but as long as the default Python is Python 2, it will not be available to Python-agnostic scripts [1]. Auditing the existing users: $ git --no-pager grep -l python-validation ci-operator ci-operator/jobs/openshift/release/openshift-release-master-presubmits.yaml So all the consumers are jobs in that one file. Getting the commands they run: $ yaml2json <ci-operator/jobs/openshift/release/openshift-release-master-presubmits.yaml | jq -r '.presubmits["openshift/release"][].spec.containers[] | select(.image | endswith("python-validation")) | (.command | join(" ")) + " " + (.args | join(" "))' hack/validate-ci-operator-config-filename.py --config-dir ./ci-operator/config hack/validate-prow-job-semantics.py ./ hack/validate-release-controller-config.sh ./ pylint --rcfile=hack/.pylintrc --ignore=lib --persistent=n hack Don't have to worry about pylint, because we install that in the Dockerfile. Looking for how the shell script is associated with Python: $ grep 'python\|\.py' hack/validate-release-controller-config.sh ${base_dir}/hack/generators/release-controllers/generate-release-controllers.py "${base_dir}" And checking all three Python scripts: $ head -n1 hack/validate-ci-operator-config-filename.py hack/validate-prow-job-semantics.py hack/generators/release-controllers/generate-release-controllers.py ==> hack/validate-ci-operator-config-filename.py <== #!/usr/bin/env python3 ==> hack/validate-prow-job-semantics.py <== #!/usr/bin/env python3 ==> hack/generators/release-controllers/generate-release-controllers.py <== #!/usr/bin/env python3 So we are uniformly python3 today, and changing the default to Python 3 should not cause problems. Confirming the default in the current image: $ podman run --rm registry.svc.ci.openshift.org/ci/python-validation /usr/bin/env python --version Python 2.7.5 $ podman run --rm registry.svc.ci.openshift.org/ci/python-validation rpm -q --whatprovides /usr/bin/python2 python-2.7.5-69.el7_5.x86_64 Apparently the preferred way to change the default is via 'alternatives' [2], but that fails for me with no useful error message: $ podman run --rm registry.svc.ci.openshift.org/ci/python-validation sh -c 'env python --version && command -v python3 && alternatives --set python $(command -v python3) || echo "$?"'Python 2.7.5 /usr/local/bin/python3 2 So instead I've installed a new virtual environment [3] and set the PATH and VIRTUAL_ENV variables to produce the core environment you'd get from sourcing ~/.venv/bin/activate: $ podman run --rm registry.svc.ci.openshift.org/ci/python-validation sh -c 'python3 -m venv ~/.venv && grep "^export\|^PATH=\|^VIRTUAL_ENV=\|^# unset PYTHONHOME" ~/.venv/bin/activate' VIRTUAL_ENV="/root/.venv" export VIRTUAL_ENV PATH="$VIRTUAL_ENV/bin:$PATH" export PATH # unset PYTHONHOME if set [1]: openshift#11752 [2]: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/configuring_basic_system_settings/using-python3_configuring-basic-system-settings#configuring-the-unversioned-python_installing-and-using-python [3]: https://docs.python.org/3/library/venv.html

petr-muller

LGTM but this needs the other PR to make the underlying image have yaml so holding

/hold

…f names Quotas for some providers depend on both account-scoped resources (e.g. AWS elastic IPs default to five per account [1]), region-scoped resources (e.g. AWS VPCs default to five per region [1]), potentialy other scoping (e.g. AWS NAT gateways default to five per availability zone [1]). This commit moves us away from account-scoped leases and towards region-scoped leases in account-scoped buckets. That allows us to say things like "on Azure, we have more capacity in centralus than we do in our other regions". The generating script is because typing out "us-east-1" 50 times is tedious and hard to review, and today there is apparently no support in the Boskos config directly to ask for $N copies of a given name. I'm also adding myself as an owner, so DPTP doesn't have to bother with lease adjustment. Other CI-platform maintainers should feel free to add themselves as well, if they want to approve lease adjustments to their platforms. Currently a mostly-no-op reshuffling of the exisiting Boskos config. The python-validation image follows the pattern set by the existing prow-config-semantics job. This script works with both Python 2 and Python 3, but I'm using python3 in the shebang because the verification image currently has 'python' pointed at Python 2 but only installs PyYAML for Python 3, and I've been unable to land a pivot to point it at Python 3 [2]. [1]: https://docs.openshift.com/container-platform/4.5/installing/installing_aws/installing-aws-account.html#installation-aws-limits_installing-aws-account [2]: openshift#11869

wking · 2020-10-08T21:02:33Z

With 2dd18757cf -> 5cf5ba3, I've rebased onto master, claimed python3 because #11869 is still unsettled, and limited this change to a no-op reshuffling. I'll come back with follow-up work after this PR lands to actually move AWS/GCP/Azure to per-region leases.

openshift-ci-robot · 2020-10-08T21:04:21Z

@wking: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/ci-testgrid-allow-list	2dd18757cf29a64a766870e36aa4927deed6c20a	link	`/test ci-testgrid-allow-list`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

hack/validate-boskos.sh

Seems like our python-validation image does not include it [1]: hack/validate-boskos.sh: line 14: git: command not found 'diff -u A B' is in POSIX [2]. The '|| true' catches the exit status, which is when differences were found [2]. The chance of a >1 exit status for other errors seems small enough that I haven't bothered to handle it, although ideally we'd exit out in that situation. [1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/11752/rehearse-11752-pull-ci-openshift-release-master-boskos-config/1314310508639162368 [2]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html

wking · 2020-10-08T23:32:27Z

Me giving up on getting python to point to a Python with PyYAML means we can drop the hold.

/hold cancel

openshift-ci-robot · 2020-10-08T23:34:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, stevekuznetsov, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [petr-muller,stevekuznetsov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-10-08T23:43:04Z

@wking: Updated the following 4 configmaps:

job-config-master configmap in namespace ci at cluster app.ci using the following files:
- key openshift-release-master-presubmits.yaml using file ci-operator/jobs/openshift/release/openshift-release-master-presubmits.yaml
resources configmap in namespace ci at cluster api.ci using the following files:
- key boskos.yaml using file core-services/prow/02_config/_boskos.yaml
resources configmap in namespace ci at cluster app.ci using the following files:
- key boskos.yaml using file core-services/prow/02_config/_boskos.yaml
job-config-master configmap in namespace ci at cluster api.ci using the following files:
- key openshift-release-master-presubmits.yaml using file ci-operator/jobs/openshift/release/openshift-release-master-presubmits.yaml

Details

In response to this:

Quotas for some providers depend on both account-scoped resources (e.g. AWS elastic IPs default to five per account), region-scoped resources (e.g. AWS VPCs default to five per region), potentialy other scoping (e.g. AWS NAT gateways default to five per availability zone). This commit moves us away from account-scoped leases and towards region-scoped leases in account-scoped buckets. That allows us to say things like "on Azure, we have more capacity in centralus than we do in our other regions".

The generating script is because typing out us-east-1 50 times is tedious and hard to review, and today there is apparently no support in the Boskos config directly to ask for $N copies of a given name.

I'm also adding myself as an owner, so DPTP doesn't have to bother with lease adjustment. Other CI-platform maintainers should feel free to add themselves as well, if they want to approve lease adjustments to their platforms.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

coverprice · 2020-10-08T23:45:24Z

core-services/prow/02_config/generate-boskos.py

+        'default': 120,
+    },
+    'libvirt-s390x-quota-slice': {},
+    'libvirt-ppc64le-quota-slice': {},


code golf:

'libvirt-ppc64le-quota-slice': { f'libvirt-ppc64le-{i}-{j}': 1 for j in range(4) for i in range(2) },

Fine with me. File a PR?

wking · 2020-10-08T23:51:16Z

Followed up for per-region Azure leases in #12584.

Catching up with openshift/release#5cf5ba3f87 (core-services/prow/02_config/generate-boskos: For spitting out lots of names, 2020-09-10, openshift/release#11752).

Catching up with openshift/release@5cf5ba3f87 (core-services/prow/02_config/generate-boskos: For spitting out lots of names, 2020-09-10, openshift/release#11752).

I'd added myself as an approver here in 5cf5ba3 (core-services/prow/02_config/generate-boskos: For spitting out lots of names, 2020-10-08, openshift#11752). This comment isn't structured metadata, but it makes it slightly less likely that folks ask me to review things like "I'm adding a new repository".

openshift-ci-robot requested review from hongkailiu and petr-muller September 10, 2020 21:22

stevekuznetsov reviewed Sep 10, 2020

View reviewed changes

core-services/prow/02_config/generate-boskos.py Outdated Show resolved Hide resolved

core-services/prow/02_config/generate-boskos.py Outdated Show resolved Hide resolved

wking force-pushed the per-region-leases branch 3 times, most recently from cc5e7eb to 8ab9d41 Compare September 15, 2020 22:21

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2020

wking force-pushed the per-region-leases branch from 8ab9d41 to e303bcd Compare September 15, 2020 22:23

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2020

wking force-pushed the per-region-leases branch from e303bcd to 0cf4ca9 Compare September 15, 2020 22:36

wking force-pushed the per-region-leases branch from 0cf4ca9 to 2dd1875 Compare September 15, 2020 23:03

wking mentioned this pull request Sep 16, 2020

services/validation-images/python/Dockerfile: Python 3 by default #11869

Closed

petr-muller approved these changes Sep 16, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 16, 2020

openshift-ci-robot assigned petr-muller Sep 16, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 16, 2020

wking force-pushed the per-region-leases branch from 2dd1875 to 032371d Compare October 8, 2020 20:59

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Oct 8, 2020

wking force-pushed the per-region-leases branch from 032371d to 5cf5ba3 Compare October 8, 2020 21:02

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2020

wking commented Oct 8, 2020

View reviewed changes

hack/validate-boskos.sh Outdated Show resolved Hide resolved

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2020

stevekuznetsov approved these changes Oct 8, 2020

View reviewed changes

openshift-ci-robot assigned stevekuznetsov Oct 8, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2020

openshift-merge-robot merged commit d0cc94d into openshift:master Oct 8, 2020

coverprice reviewed Oct 8, 2020

View reviewed changes

wking mentioned this pull request Oct 8, 2020

ci-operator/step-registry/ipi/conf/azure: Get region from Boskos lease #12584

Merged

wking deleted the per-region-leases branch October 8, 2020 23:54

wking mentioned this pull request Oct 15, 2020

content/docs/architecture/quota-and-leases: Doc generate-boskos.py openshift/ci-docs#11

Merged

wking mentioned this pull request Feb 21, 2022

core-services/prow/02_config/OWNERS: Explain wking's approval scope #26436

Merged

core-services/prow/02_config/generate-boskos: For spitting out lots of names #11752

core-services/prow/02_config/generate-boskos: For spitting out lots of names #11752

Uh oh!

Conversation

wking commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wking commented Sep 15, 2020

Uh oh!

wking commented Sep 15, 2020

Uh oh!

wking commented Sep 15, 2020

Uh oh!

petr-muller left a comment

Choose a reason for hiding this comment

Uh oh!

wking commented Oct 8, 2020

Uh oh!

openshift-ci-robot commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

wking commented Oct 8, 2020

Uh oh!

openshift-ci-robot commented Oct 8, 2020

Uh oh!

openshift-ci-robot commented Oct 8, 2020

Uh oh!

coverprice Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

wking commented Oct 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wking commented Sep 10, 2020 •

edited

Loading

openshift-ci-robot commented Oct 8, 2020 •

edited

Loading

coverprice Oct 8, 2020 •

edited

Loading