diff options
84 files changed, 2894 insertions, 824 deletions
diff --git a/.zuul.yaml b/.zuul.yaml index 420ac11ad..f357c6ce6 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -195,25 +195,37 @@ # Image building jobs - secret: - name: zuul-dockerhub + name: zuul-registry-credentials data: - username: zuulzuul - password: !encrypted/pkcs1-oaep - - DFlbrDM5eUMptMGIVMXV1g455xOJLi92UYF08Z2/JlIGu3t6v052o9FKlVyj1ZmpXs5+2 - JTa5jHkLTvTsYs9fCaNcQc2nmViCyWNlbOMzjB17uiZOaYFNs1sMqZcUZbGEz7Y8ds6Qq - NBXI10jWFPTah4QxUuBvUbT3vmjnUToCzexl5ZGhKgijcnROWfUsnlCdugpgoNIcPsUki - zty5FotDihnrC8n8vIomVK6EClY38ty97pLrADzFDd+Cos/OUlvi2xooUhzx8Bn020rJA - lqEU5v8LGXp5QkHx0MSDx6JY6KppJ/4p/yM+4By6l+A20zdcimxmgiNc9rMWPwDj7xsao - m7NAZWmWqOO0Xkhgt6WOfugwgt9X46sgs2+yDEfbnI5ok8uRbAB/4FWj/KdpyXwhcf+O2 - wEfhxLwDbAoGONQPjb4YcZmCXtmR7Qe5t+n2jyczWXvrbaBDUQP5a+YtVNN/xhmQ7D740 - POlxv7bLxJAixzqaQ3d8Rz9ZEv6zzRuhWph32UQtZ1JxSNww+EvmXm2eEi2Q2z6pT1Cx/ - j2OrFyA2GL/UJOVb15VHKF6bgHPHWJtpjPFhqdcvBhVute4BWB+KPcWH+y+apHN1enK3H - tNJO9iqm34nKwSuj5ExmFw50LtwR5/9FyRuRPq/vBL+8y82v8FDmeYsBeobn5M= + quay.io: + username: zuul-ci+opendevzuul + password: !encrypted/pkcs1-oaep + - B5RM116kdo4uTDHChDVWLbRUvXZHXkndzi9sZVmZ/8EjQRKhtsNfVWWPinr7cbXiN6NjA + ja85RrAMwYic8Y2f8cTRSowitPDmvAs8Av/zZ6PnFap6pGb1vQFuPYYOqEkkeqrQoY9vO + h2PV3Z2A+O48mzDt0CVhI6E8AQdqrMO7R0pO1plb5q9PFTHUlwgUdIbUMkVpndVMER8Ez + IdWs2bcVUC5hChUKFcSX2Jr8peOwQvLnzX5nGRAYATrp5tV/xsC8R/WkOVvKP0ORLJlf8 + T/yZRXm7yw9LdizCsf+3jkzw726YZT+GqavPnygJvKeu+WVMtBs69TPhTdpq4B5WCnQko + Xq+g2WZkqdfeQRBp2BqMsk1wZ+fMnTqKb8iJRO0tjYmFJBq0xRRQHHjwLDd4clM1KBMqT + 17ss4QvTY61ZbbSM33M3FgibBPe96G7vCuTLiUnrmcR8i5M1H+XHPYMSSbCn3DgY5IBl2 + Lr9ism4velFlzXQ9r71VM5v+JCSbcjrMAn5GKFZ5f9MeqWnATqYUO+xwEa/wb+sgPzd/v + VNlB74K3tg7wP1abGM7LtQkVKkWuy44sXayt2PcikE4pDO6EiODOxSv5a5Uxc6ef/cmfl + ADZ08hlob4CFNjL1m5lZSZNYDcsgwYuYXoP2a5kgykerEV+v3q0epjExCCUrEI= + api_token: !encrypted/pkcs1-oaep + - HTgQ6onrqJCCwjwT+TGjgiXT6qgwJMbWYUXlt5mz570RKNN4Ptsa6oqu4zpk+paAIwain + Olw3n4InshNAWIvTSro6zwYurmiiKCbxcocaxLzIKM+zbgIR7haKm+Crqei4tanWNpXHq + ULdu4muJojxu6OdYNhftPY3NLdFFuDA5IF2zv2f4gZSthvmQ9NzvEqPKFDV2yzNa6G4V+ + jicYR3X7f0TjW9QF9p9CuSMvZrOCxp98zRelT8EJBKsb+38JVapvJgbAqAgJhAaGgJYkP + 1W7iqL+eNLMcRCnkoBEnOwZe66WO5gZagqWH25oRhjFnFM/qqMeYEG/AhAUae7vTHd9VK + yHSndEdrdCNpdzBsJQFGS7lxJMUl8ELL61qoojZSW91bGRIziedYEbDuPEzZUkoosaf6r + 50jvdRYOsmYnmr3Q5/T0QxW00qLL6FkPCRHg8wI2EXDPA/X5+vTlEYMhJxEJZ1+unHk9t + PPebXHCZ2B0VA+x3Khnt5BOJt/ewxfbfVu0CVgjSgrAUaSHWY3DLdvbuA9lwjafybkfxC + vIEhQz8AxAgWsdWpFOr9uPCB+5C+ma4jF15k/RUWoODHhvYZEoSNDOz4BRGrK+kHczr3F + P9x7xzzPhVCdbspsf+sV90xpUI0U4vCeVpi+3Ha1zZZR0JAC1SIWXUnJxB2EkM= - job: name: zuul-build-image - parent: opendev-build-docker-image - description: Build Docker images. + parent: opendev-build-container-image + description: Build container images. allowed-projects: zuul/zuul timeout: 2700 # 45 minutes requires: @@ -221,58 +233,85 @@ - python-base-3.11-bullseye-container-image provides: zuul-container-image vars: &zuul_image_vars - docker_images: + promote_container_image_method: intermediate-registry + promote_container_image_job: zuul-upload-image + container_command: docker + container_images: - context: . - repository: zuul/zuul + registry: quay.io + repository: quay.io/zuul-ci/zuul + namespace: zuul-ci + repo_shortname: zuul + repo_description: Base Zuul image. target: zuul tags: # If zuul.tag is defined: [ '3', '3.19', '3.19.0' ]. Only works for 3-component tags. # Otherwise: ['latest'] &imagetag "{{ zuul.tag is defined | ternary([zuul.get('tag', '').split('.')[0], '.'.join(zuul.get('tag', '').split('.')[:2]), zuul.get('tag', '')], ['latest']) }}" - context: . - repository: zuul/zuul-executor + registry: quay.io + repository: quay.io/zuul-ci/zuul-executor + namespace: zuul-ci + repo_shortname: zuul-executor + repo_description: Zuul executor image target: zuul-executor tags: *imagetag - context: . - repository: zuul/zuul-fingergw + registry: quay.io + repository: quay.io/zuul-ci/zuul-fingergw + namespace: zuul-ci + repo_shortname: zuul-fingergw + repo_description: Zuul fingergw image target: zuul-fingergw tags: *imagetag - context: . - repository: zuul/zuul-merger + registry: quay.io + repository: quay.io/zuul-ci/zuul-merger + namespace: zuul-ci + repo_shortname: zuul-merger + repo_description: Zuul merger image target: zuul-merger tags: *imagetag - context: . - repository: zuul/zuul-scheduler + registry: quay.io + repository: quay.io/zuul-ci/zuul-scheduler + namespace: zuul-ci + repo_shortname: zuul-scheduler + repo_description: Zuul scheduler image target: zuul-scheduler tags: *imagetag - context: . - repository: zuul/zuul-web + registry: quay.io + repository: quay.io/zuul-ci/zuul-web + namespace: zuul-ci + repo_shortname: zuul-web + repo_description: Zuul web image target: zuul-web tags: *imagetag - job: name: zuul-upload-image - parent: opendev-upload-docker-image - description: Build Docker images and upload to Docker Hub. + parent: opendev-upload-container-image + description: Build container images and upload. allowed-projects: zuul/zuul requires: - python-builder-3.11-bullseye-container-image - python-base-3.11-bullseye-container-image provides: zuul-container-image secrets: - name: docker_credentials - secret: zuul-dockerhub + name: container_registry_credentials + secret: zuul-registry-credentials pass-to-parent: true vars: *zuul_image_vars - job: name: zuul-promote-image - parent: opendev-promote-docker-image - description: Promote previously uploaded Docker images. + parent: opendev-promote-container-image + description: Promote previously uploaded container images. allowed-projects: zuul/zuul secrets: - name: docker_credentials - secret: zuul-dockerhub + name: container_registry_credentials + secret: zuul-registry-credentials pass-to-parent: true nodeset: nodes: [] @@ -370,11 +409,11 @@ jobs: - zuul-release-python - zuul-publish-nox-docs - - upload-docker-image: + - upload-container-image: secrets: - name: docker_credentials - secret: zuul-dockerhub + name: container_registry_credentials + secret: zuul-registry-credentials pass-to-parent: true vars: <<: *zuul_image_vars - upload_docker_image_promote: false + upload_container_image_promote: false diff --git a/doc/source/drivers/gerrit.rst b/doc/source/drivers/gerrit.rst index 4e7fc7cea..3c16f202a 100644 --- a/doc/source/drivers/gerrit.rst +++ b/doc/source/drivers/gerrit.rst @@ -239,6 +239,10 @@ be able to invoke the ``gerrit stream-events`` command over SSH. .. attr:: require-approval + .. warning:: This is deprecated and will be removed in a future + version. Use :attr:`pipeline.trigger.<gerrit + source>.require` instead. + This may be used for any event. It requires that a certain kind of approval be present for the current patchset of the change (the approval could be added by the event in question). It @@ -246,11 +250,39 @@ be able to invoke the ``gerrit stream-events`` command over SSH. source>.approval`. For each specified criteria there must exist a matching approval. + This is ignored if the :attr:`pipeline.trigger.<gerrit + source>.require` attribute is present. + .. attr:: reject-approval + .. warning:: This is deprecated and will be removed in a future + version. Use :attr:`pipeline.trigger.<gerrit + source>.reject` instead. + This takes a list of approvals in the same format as :attr:`pipeline.trigger.<gerrit source>.require-approval` but - will fail to enter the pipeline if there is a matching approval. + the item will fail to enter the pipeline if there is a matching + approval. + + This is ignored if the :attr:`pipeline.trigger.<gerrit + source>.reject` attribute is present. + + .. attr:: require + + This may be used for any event. It describes conditions that + must be met by the change in order for the trigger event to + match. Those conditions may be satisfied by the event in + question. It follows the same syntax as + :ref:`gerrit_requirements`. + + .. attr:: reject + + This may be used for any event and is the mirror of + :attr:`pipeline.trigger.<gerrit source>.require`. It describes + conditions that when met by the change cause the trigger event + not to match. Those conditions may be satisfied by the event in + question. It follows the same syntax as + :ref:`gerrit_requirements`. Reporter Configuration ---------------------- @@ -283,6 +315,8 @@ with an HTTP password, in which case the HTTP API is used. A :ref:`connection<connections>` that uses the gerrit driver must be supplied to the trigger. +.. _gerrit_requirements: + Requirements Configuration -------------------------- @@ -365,7 +399,7 @@ order to be enqueued into the pipeline. .. attr:: status A string value that corresponds with the status of the change - reported by the trigger. + reported by Gerrit. .. attr:: pipeline.reject.<gerrit source> @@ -375,10 +409,12 @@ order to be enqueued into the pipeline. .. attr:: approval - This takes an approval or a list of approvals. If an approval - matches the provided criteria the change can not be entered - into the pipeline. It follows the same syntax as - :attr:`pipeline.require.<gerrit source>.approval`. + This requires that a certain kind of approval not be present for the + current patchset of the change (the approval could be added by + the event in question). Approval is a dictionary or a list of + dictionaries with attributes listed below, all of which are + optional and are combined together so that there must be no approvals + matching all specified requirements. Example to reject a change with any negative vote: @@ -389,6 +425,55 @@ order to be enqueued into the pipeline. approval: - Code-Review: [-1, -2] + .. attr:: username + + If present, an approval from this username is required. It is + treated as a regular expression. + + .. attr:: email + + If present, an approval with this email address is required. It is + treated as a regular expression. + + .. attr:: older-than + + If present, the approval must be older than this amount of time + to match. Provide a time interval as a number with a suffix of + "w" (weeks), "d" (days), "h" (hours), "m" (minutes), "s" + (seconds). Example ``48h`` or ``2d``. + + .. attr:: newer-than + + If present, the approval must be newer than this amount + of time to match. Same format as "older-than". + + Any other field is interpreted as a review category and value + pair. For example ``Verified: 1`` would require that the + approval be for a +1 vote in the "Verified" column. The value + may either be a single value or a list: ``Verified: [1, 2]`` + would match either a +1 or +2 vote. + + .. attr:: open + + A boolean value (``true`` or ``false``) that indicates whether + the change must be open or closed in order to be rejected. + + .. attr:: current-patchset + + A boolean value (``true`` or ``false``) that indicates whether the + change must be the current patchset in order to be rejected. + + .. attr:: wip + + A boolean value (``true`` or ``false``) that indicates whether the + change must be wip or not wip in order to be rejected. + + .. attr:: status + + A string value that corresponds with the status of the change + reported by Gerrit. + + Reference Pipelines Configuration --------------------------------- diff --git a/doc/source/drivers/github.rst b/doc/source/drivers/github.rst index 148c6f976..7cacf45ac 100644 --- a/doc/source/drivers/github.rst +++ b/doc/source/drivers/github.rst @@ -339,7 +339,7 @@ the following options. format of ``user:context:status``. For example, ``zuul_github_ci_bot:check_pipeline:success``. - .. attr: check + .. attr:: check This is only used for ``check_run`` events. It works similar to the ``status`` attribute and accepts a list of strings each of @@ -363,6 +363,38 @@ the following options. always sends full ref name, eg. ``refs/tags/bar`` and this string is matched against the regular expression. + .. attr:: require-status + + .. warning:: This is deprecated and will be removed in a future + version. Use :attr:`pipeline.trigger.<github + source>.require` instead. + + This may be used for any event. It requires that a certain kind + of status be present for the PR (the status could be added by + the event in question). It follows the same syntax as + :attr:`pipeline.require.<github source>.status`. For each + specified criteria there must exist a matching status. + + This is ignored if the :attr:`pipeline.trigger.<github + source>.require` attribute is present. + + .. attr:: require + + This may be used for any event. It describes conditions that + must be met by the PR in order for the trigger event to match. + Those conditions may be satisfied by the event in question. It + follows the same syntax as :ref:`github_requirements`. + + .. attr:: reject + + This may be used for any event and is the mirror of + :attr:`pipeline.trigger.<github source>.require`. It describes + conditions that when met by the PR cause the trigger event not + to match. Those conditions may be satisfied by the event in + question. It follows the same syntax as + :ref:`github_requirements`. + + Reporter Configuration ---------------------- Zuul reports back to GitHub via GitHub API. Available reports include a PR @@ -462,6 +494,8 @@ itself. Status name, description, and context is taken from the pipeline. .. _Github App: https://developer.github.com/apps/ +.. _github_requirements: + Requirements Configuration -------------------------- diff --git a/doc/source/examples/docker-compose.yaml b/doc/source/examples/docker-compose.yaml index 43c0bec61..0eb05835b 100644 --- a/doc/source/examples/docker-compose.yaml +++ b/doc/source/examples/docker-compose.yaml @@ -11,7 +11,7 @@ services: networks: - zuul gerritconfig: - image: docker.io/zuul/zuul-executor + image: quay.io/zuul-ci/zuul-executor environment: - http_proxy - https_proxy @@ -66,7 +66,7 @@ services: zuul-scheduler -f' # FIXME: The scheduler has no ansible anymore so use the executor image. # This needs to be changes such that ansible is not required for startup. - image: docker.io/zuul/zuul-scheduler + image: quay.io/zuul-ci/zuul-scheduler volumes: - "${ZUUL_TUTORIAL_CONFIG:-./etc_zuul/}:/etc/zuul/:z" - "./playbooks/:/var/playbooks/:z" @@ -83,7 +83,7 @@ services: - mysql ports: - "9000:9000" - image: docker.io/zuul/zuul-web + image: quay.io/zuul-ci/zuul-web environment: ZUUL_MYSQL_PASSWORD: secret volumes: @@ -101,7 +101,7 @@ services: - ZUUL_MYSQL_PASSWORD=secret depends_on: - scheduler - image: docker.io/zuul/zuul-executor + image: quay.io/zuul-ci/zuul-executor volumes: - "${ZUUL_TUTORIAL_CONFIG:-./etc_zuul/}:/etc/zuul/:z" - "./playbooks/:/var/playbooks/:z" @@ -126,7 +126,7 @@ services: launcher: depends_on: - zk - image: docker.io/zuul/nodepool-launcher + image: quay.io/zuul-ci/nodepool-launcher volumes: - "./playbooks/:/var/playbooks/:z" - "./etc_nodepool/:/etc/nodepool/:z" diff --git a/doc/source/examples/etc_nodepool/nodepool.yaml b/doc/source/examples/etc_nodepool/nodepool.yaml index 1c1830635..105b0ef54 100644 --- a/doc/source/examples/etc_nodepool/nodepool.yaml +++ b/doc/source/examples/etc_nodepool/nodepool.yaml @@ -7,7 +7,7 @@ zookeeper-tls: ca: /var/certs/certs/cacert.pem labels: - - name: ubuntu-focal + - name: ubuntu-jammy providers: - name: static-vms @@ -16,7 +16,7 @@ providers: - name: main nodes: - name: node - labels: ubuntu-focal + labels: ubuntu-jammy host-key: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOgHJYejINIKzUiuSJ2MN8uPc+dfFrZ9JH1hLWS8gI+g" python-path: /usr/bin/python3 username: root diff --git a/doc/source/examples/node-Dockerfile b/doc/source/examples/node-Dockerfile index ff74aa592..b588bcf2c 100644 --- a/doc/source/examples/node-Dockerfile +++ b/doc/source/examples/node-Dockerfile @@ -1,4 +1,4 @@ -FROM docker.io/ubuntu:20.04 +FROM docker.io/ubuntu:22.04 RUN apt-get update \ && DEBIAN_FRONTEND="noninteractive" apt-get -y install \ diff --git a/doc/source/examples/zuul-config/zuul.d/jobs.yaml b/doc/source/examples/zuul-config/zuul.d/jobs.yaml index 8ad979e46..bb9822f48 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs.yaml @@ -3,5 +3,5 @@ parent: null nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml index a6ed1a633..c7b4a6878 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml @@ -18,5 +18,5 @@ timeout: 1800 nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/job-content.rst b/doc/source/job-content.rst index d6bb07683..643632d5b 100644 --- a/doc/source/job-content.rst +++ b/doc/source/job-content.rst @@ -669,6 +669,68 @@ of item. - shell: echo example when: zuul_success | bool +.. var:: nodepool + + Information about each host from Nodepool is supplied in the + `nodepool` host variable. Availability of values varies based on + the node and the driver that supplied it. Values may be ``null`` + if they are not applicable. + + .. var:: label + + The nodepool label of this node. + + .. var:: az + + The availability zone in which this node was placed. + + .. var:: cloud + + The name of the cloud in which this node was created. + + .. var:: provider + + The name of the nodepool provider of this node. + + .. var:: region + + The name of the nodepool provider's region. + + .. var:: host_id + + The cloud's host identification for this node's hypervisor. + + .. var:: external_id + + The cloud's identifier for this node. + + .. var:: slot + + If the node supports running multiple jobs on the node, a unique + numeric ID for the subdivision of the node assigned to this job. + This may be used to avoid build directory collisions. + + .. var:: interface_ip + + The best IP address to use to contact the node as determined by + the cloud provider and nodepool. + + .. var:: public_ipv4 + + A public IPv4 address of the node. + + .. var:: private_ipv4 + + A private IPv4 address of the node. + + .. var:: public_ipv6 + + A public IPv6 address of the node. + + .. var:: private_ipv6 + + A private IPv6 address of the node. + Change Items ~~~~~~~~~~~~ diff --git a/playbooks/zuul-stream/fixtures/test-stream.yaml b/playbooks/zuul-stream/fixtures/test-stream.yaml index 488f8cb2f..49ceb092b 100644 --- a/playbooks/zuul-stream/fixtures/test-stream.yaml +++ b/playbooks/zuul-stream/fixtures/test-stream.yaml @@ -1,3 +1,16 @@ +# NOTE: We run this before starting the log streaming to validate that +# if we set zuul_console_disabled, we don't try to connect at all. If +# there is a log streamer running when we run this test, then we have +# no indication that we avoid the connection step. +- name: Run command to show skipping works without zuul_console running + vars: + zuul_console_disabled: true + hosts: node + tasks: + - name: Run quiet command + command: echo 'This command should not stream' + when: new_console | default(false) + - name: Start zuul stream daemon hosts: node tasks: @@ -11,7 +24,7 @@ port: 19887 when: new_console | default(false) -- name: Run command to show skipping works +- name: Run command to show skipping works with zuul_console running vars: zuul_console_disabled: true hosts: node diff --git a/playbooks/zuul-stream/validate.yaml b/playbooks/zuul-stream/validate.yaml index 81c613406..c7069f335 100644 --- a/playbooks/zuul-stream/validate.yaml +++ b/playbooks/zuul-stream/validate.yaml @@ -27,3 +27,8 @@ - name: Validate output - binary data shell: | egrep "^.*\| {{ item.node }} \| \\\\x80abc" {{ item.filename }} + +- name: Validate output - no waiting on logger + shell: | + egrep -v "Waiting on logger" {{ item.filename }} + egrep -v "Log Stream did not terminate" {{ item.filename }} diff --git a/releasenotes/notes/dark-mode-e9b1cca960d4b906.yaml b/releasenotes/notes/dark-mode-e9b1cca960d4b906.yaml new file mode 100644 index 000000000..997626af2 --- /dev/null +++ b/releasenotes/notes/dark-mode-e9b1cca960d4b906.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Added a new dark theme for the Zuul web interface. + - | + Added theme selection for the Zuul web interface. The default theme is set + to Auto which means your system/browsers preference determines if the Light + or Dark theme should be used. Either can be explicitly set in the settings + for the web interface by clicking the cogs in the top right. diff --git a/releasenotes/notes/fix-prune-database-a4873bd4dead7b5f.yaml b/releasenotes/notes/fix-prune-database-a4873bd4dead7b5f.yaml new file mode 100644 index 000000000..6e036a754 --- /dev/null +++ b/releasenotes/notes/fix-prune-database-a4873bd4dead7b5f.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + The `zuul-admin prune-database` command did not completely delete + expected data from the database. It may not have deleted all of + the buildsets older than the specified cutoff time, and it may + have left orphaned data in ancillary tables. This has been + corrected and it should now work as expected. Additionally, a + `--batch-size` argument has been added so that it may delete data + in multiple transactions which can facilitate smoother operation + when run while Zuul is operational. + + Users who have previously run the command may need to manually + delete rows from the `zuul_build`, `zuul_build_event`, + `zuul_artifact`, and `zuul_provides` tables which do not have + corresponding entries in the `zuul_buildset` table. diff --git a/releasenotes/notes/gerrit-trigger-status-88cb4c52bd3ba86a.yaml b/releasenotes/notes/gerrit-trigger-status-88cb4c52bd3ba86a.yaml new file mode 100644 index 000000000..479d6c153 --- /dev/null +++ b/releasenotes/notes/gerrit-trigger-status-88cb4c52bd3ba86a.yaml @@ -0,0 +1,22 @@ +--- +features: + - | + Gerrit pipeline triggers now support embedded require and reject + filters in order to match. Any conditions set for the pipeline in + require or reject filters may also be set for event trigger + filters. + + This can be used to construct pipelines which trigger based on + certain events but only if certain other conditions are met. It + is distinct from pipeline requirements in that it only affects + items that are directly enqueued whereas pipeline requirements + affect dependencies as well. + - | + All Gerrit "requires" filters are now available as "reject" + filters as well. +deprecations: + - | + The `require-approval` and `reject-approval` Gerrit trigger + attributes are deprecated. Use :attr:`pipeline.trigger.<gerrit + source>.require` and :attr:`pipeline.trigger.<gerrit + source>.reject` instead. diff --git a/releasenotes/notes/github-trigger-status-948e81b9f45418f1.yaml b/releasenotes/notes/github-trigger-status-948e81b9f45418f1.yaml new file mode 100644 index 000000000..19cac8642 --- /dev/null +++ b/releasenotes/notes/github-trigger-status-948e81b9f45418f1.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + GitHub pipeline triggers now support embedded require and reject + filters in order to match. Any conditions set for the pipeline in + require or reject filters may also be set for event trigger + filters. + + This can be used to construct pipelines which trigger based on + certain events but only if certain other conditions are met. It + is distinct from pipeline requirements in that it only affects + items that are directly enqueued whereas pipeline requirements + affect dependencies as well. +deprecations: + - | + The `require-status` GitHub trigger attribute is deprecated. + Use :attr:`pipeline.trigger.<github source>.require` instead. diff --git a/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml new file mode 100644 index 000000000..dd5c502d2 --- /dev/null +++ b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + The `cherry-pick` merge mode will now silently skip commits that have + already been applied to the tree when cherry-picking, instead of failing + with an error. + + The exception to this is if the source of the cherry-pick is an empty + commit, in which case it is always kept. + + Skipping commits that have already been applied is important in a pipeline + triggered by the Gerrit `change-merged` event (like the `deploy` pipeline), + since the scheduler would previously try to cherry-pick the change on top + of the commit that just merged and fail. diff --git a/releasenotes/notes/nodepool-slot-2061128253e50580.yaml b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml new file mode 100644 index 000000000..c7ba3e1dc --- /dev/null +++ b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The :var:`nodepool.slot` variable has been added to host vars. + This is supplied by the nodepool static and metastatic drivers + starting with version 8.0.0. It may be used to avoid build + directory collisions on nodes that run more than one job. diff --git a/releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml b/releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml new file mode 100644 index 000000000..14f037156 --- /dev/null +++ b/releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + Zuul will now attempt to honor Gerrit "submit requirements" when + determining whether to enqueue a change into a dependent (i.e., + "gate") pipeline. Zuul previously honored only Gerrit's older + "submit records" feature. The new checks will avoid enqueing + changes in "gate" pipelines in the cases where Zuul can + unambiguously determine that there is no possibility of merging, + but some non-mergable changes may still be enqueued if Zuul can + not be certain whether a rule should apply or be disregarded (in + these cases, Gerrit will fail to merge the change and Zuul will + report the buildset as a MERGE_FAILURE). + + This requires Gerrit version 3.5.0 or later, and Zuul to be + configured with HTTP access for Gerrit. diff --git a/tests/base.py b/tests/base.py index fd927a92c..af10ebe96 100644 --- a/tests/base.py +++ b/tests/base.py @@ -384,7 +384,7 @@ class FakeGerritChange(object): def __init__(self, gerrit, number, project, branch, subject, status='NEW', upstream_root=None, files={}, parent=None, merge_parents=None, merge_files=None, - topic=None): + topic=None, empty=False): self.gerrit = gerrit self.source = gerrit self.reported = 0 @@ -403,6 +403,7 @@ class FakeGerritChange(object): self.comments = [] self.checks = {} self.checks_history = [] + self.submit_requirements = [] self.data = { 'branch': branch, 'comments': self.comments, @@ -429,7 +430,7 @@ class FakeGerritChange(object): self.addMergePatchset(parents=merge_parents, merge_files=merge_files) else: - self.addPatchset(files=files, parent=parent) + self.addPatchset(files=files, parent=parent, empty=empty) if merge_parents: self.data['parents'] = merge_parents elif parent: @@ -503,9 +504,11 @@ class FakeGerritChange(object): repo.heads['master'].checkout() return r - def addPatchset(self, files=None, large=False, parent=None): + def addPatchset(self, files=None, large=False, parent=None, empty=False): self.latest_patchset += 1 - if not files: + if empty: + files = {} + elif not files: fn = '%s-%s' % (self.branch.replace('/', '_'), self.number) data = ("test %s %s %s\n" % (self.branch, self.number, self.latest_patchset)) @@ -786,6 +789,12 @@ class FakeGerritChange(object): return [{'status': 'NOT_READY', 'labels': labels}] + def getSubmitRequirements(self): + return self.submit_requirements + + def setSubmitRequirements(self, reqs): + self.submit_requirements = reqs + def setDependsOn(self, other, patchset): self.depends_on_change = other self.depends_on_patchset = patchset @@ -892,6 +901,7 @@ class FakeGerritChange(object): data['parents'] = self.data['parents'] if 'topic' in self.data: data['topic'] = self.data['topic'] + data['submit_requirements'] = self.getSubmitRequirements() return json.loads(json.dumps(data)) def queryRevisionHTTP(self, revision): @@ -940,6 +950,7 @@ class FakeGerritChange(object): if self.fail_merge: return self.data['status'] = 'MERGED' + self.data['open'] = False self.open = False path = os.path.join(self.upstream_root, self.project) @@ -1330,7 +1341,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): def addFakeChange(self, project, branch, subject, status='NEW', files=None, parent=None, merge_parents=None, - merge_files=None, topic=None): + merge_files=None, topic=None, empty=False): """Add a change to the fake Gerrit.""" self.change_number += 1 c = FakeGerritChange(self, self.change_number, project, branch, @@ -1338,7 +1349,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): status=status, files=files, parent=parent, merge_parents=merge_parents, merge_files=merge_files, - topic=topic) + topic=topic, empty=empty) self.changes[self.change_number] = c return c @@ -1494,8 +1505,9 @@ class FakeGerritConnection(gerritconnection.GerritConnection): msg = msg[1:-1] l = [queryMethod(change) for change in self.changes.values() if msg in change.data['commitMessage']] - elif query.startswith("status:"): + else: cut_off_time = 0 + l = list(self.changes.values()) parts = query.split(" ") for part in parts: if part.startswith("-age"): @@ -1503,17 +1515,18 @@ class FakeGerritConnection(gerritconnection.GerritConnection): cut_off_time = ( datetime.datetime.now().timestamp() - float(age[:-1]) ) - l = [ - queryMethod(change) for change in self.changes.values() - if change.data["lastUpdated"] >= cut_off_time - ] - elif query.startswith('topic:'): - topic = query[len('topic:'):].strip() - l = [queryMethod(change) for change in self.changes.values() - if topic in change.data.get('topic', '')] - else: - # Query all open changes - l = [queryMethod(change) for change in self.changes.values()] + l = [ + change for change in l + if change.data["lastUpdated"] >= cut_off_time + ] + if part.startswith('topic:'): + topic = part[len('topic:'):].strip() + l = [ + change for change in l + if 'topic' in change.data + and topic in change.data['topic'] + ] + l = [queryMethod(change) for change in l] return l def simpleQuerySSH(self, query, event=None): diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 725c083e2..25dcb15da 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -730,7 +730,7 @@ class FakeGithubSession(object): 'message': 'Merge not allowed because of fake reason', } return FakeResponse(data, 405, 'Method not allowed') - pr.setMerged(json["commit_message"]) + pr.setMerged(json.get("commit_message", "")) return FakeResponse({"merged": True}, 200) return FakeResponse(None, 404) diff --git a/tests/fixtures/layouts/gerrit-trigger-requirements.yaml b/tests/fixtures/layouts/gerrit-trigger-requirements.yaml new file mode 100644 index 000000000..72ad6b41e --- /dev/null +++ b/tests/fixtures/layouts/gerrit-trigger-requirements.yaml @@ -0,0 +1,162 @@ +- pipeline: + name: require-open + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test require-open + require: + open: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: reject-open + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test reject-open + reject: + open: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: require-wip + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test require-wip + require: + wip: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: reject-wip + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test reject-wip + reject: + wip: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: require-current-patchset + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test require-current-patchset + require: + current-patchset: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: reject-current-patchset + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test reject-current-patchset + reject: + current-patchset: true + success: + gerrit: + Verified: 1 + +- pipeline: + name: require-status + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test require-status + require: + status: MERGED + success: + gerrit: + Verified: 1 + +- pipeline: + name: reject-status + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test reject-status + reject: + status: MERGED + success: + gerrit: + Verified: 1 + +- pipeline: + name: require-approval + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test require-approval + require: + approval: + username: zuul + Verified: 1 + success: + gerrit: + Verified: 1 + +- pipeline: + name: reject-approval + manager: independent + trigger: + gerrit: + - event: comment-added + comment: test reject-approval + reject: + approval: + username: zuul + Verified: 1 + success: + gerrit: + Verified: 1 + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: {name: require-open} +- job: {name: reject-open} +- job: {name: require-wip} +- job: {name: reject-wip} +- job: {name: require-current-patchset} +- job: {name: reject-current-patchset} +- job: {name: require-status} +- job: {name: reject-status} +- job: {name: require-approval} +- job: {name: reject-approval} + +- project: + name: org/project + require-open: {jobs: [require-open]} + reject-open: {jobs: [reject-open]} + require-wip: {jobs: [require-wip]} + reject-wip: {jobs: [reject-wip]} + require-current-patchset: {jobs: [require-current-patchset]} + reject-current-patchset: {jobs: [reject-current-patchset]} + require-status: {jobs: [require-status]} + reject-status: {jobs: [reject-status]} + require-approval: {jobs: [require-approval]} + reject-approval: {jobs: [reject-approval]} diff --git a/tests/fixtures/layouts/github-trigger-requirements.yaml b/tests/fixtures/layouts/github-trigger-requirements.yaml new file mode 100644 index 000000000..5014df3bb --- /dev/null +++ b/tests/fixtures/layouts/github-trigger-requirements.yaml @@ -0,0 +1,112 @@ +- pipeline: + name: require-status + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test require-status + require: + status: + - zuul:tenant-one/check:success + success: + github: + comment: true + +- pipeline: + name: reject-status + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test reject-status + reject: + status: + - zuul:tenant-one/check:failure + success: + github: + comment: true + +- pipeline: + name: require-review + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test require-review + require: + review: + - type: approved + permission: write + success: + github: + comment: true + +- pipeline: + name: reject-review + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test reject-review + reject: + review: + - type: changes_requested + permission: write + success: + github: + comment: true + +- pipeline: + name: require-label + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test require-label + require: + label: + - approved + success: + github: + comment: true + +- pipeline: + name: reject-label + manager: independent + trigger: + github: + - event: pull_request + action: comment + comment: test reject-label + reject: + label: + - rejected + success: + github: + comment: true + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: {name: require-status} +- job: {name: reject-status} +- job: {name: require-review} +- job: {name: reject-review} +- job: {name: require-label} +- job: {name: reject-label} + +- project: + name: org/project + require-status: {jobs: [require-status]} + reject-status: {jobs: [reject-status]} + require-review: {jobs: [require-review]} + reject-review: {jobs: [reject-review]} + require-label: {jobs: [require-label]} + reject-label: {jobs: [reject-label]} diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index a3f9dda33..28ca528b5 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -2246,6 +2246,70 @@ class TestGerritCircularDependencies(ZuulTestCase): self.assertEqual(B.data["status"], "MERGED") @simple_layout('layouts/deps-by-topic.yaml') + def test_deps_by_topic_git_needs(self): + A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", + topic='test-topic') + B = self.fake_gerrit.addFakeChange('org/project2', "master", "B", + topic='test-topic') + C = self.fake_gerrit.addFakeChange('org/project2', "master", "C", + topic='other-topic') + D = self.fake_gerrit.addFakeChange('org/project1', "master", "D", + topic='other-topic') + + # Git level dependency between B and C + B.setDependsOn(C, 1) + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(D.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(A.patchsets[-1]["approvals"]), 1) + self.assertEqual(A.patchsets[-1]["approvals"][0]["type"], "Verified") + self.assertEqual(A.patchsets[-1]["approvals"][0]["value"], "1") + + self.assertEqual(len(B.patchsets[-1]["approvals"]), 1) + self.assertEqual(B.patchsets[-1]["approvals"][0]["type"], "Verified") + self.assertEqual(B.patchsets[-1]["approvals"][0]["value"], "1") + + self.assertEqual(len(C.patchsets[-1]["approvals"]), 1) + self.assertEqual(C.patchsets[-1]["approvals"][0]["type"], "Verified") + self.assertEqual(C.patchsets[-1]["approvals"][0]["value"], "1") + + self.assertEqual(len(D.patchsets[-1]["approvals"]), 1) + self.assertEqual(D.patchsets[-1]["approvals"][0]["type"], "Verified") + self.assertEqual(D.patchsets[-1]["approvals"][0]["value"], "1") + + # We're about to add approvals to changes without adding the + # triggering events to Zuul, so that we can be sure that it is + # enqueing the changes based on dependencies, not because of + # triggering events. Since it will have the changes cached + # already (without approvals), we need to clear the cache + # first. + for connection in self.scheds.first.connections.connections.values(): + connection.maintainCache([], max_age=0) + + A.addApproval("Code-Review", 2) + B.addApproval("Code-Review", 2) + C.addApproval("Code-Review", 2) + D.addApproval("Code-Review", 2) + A.addApproval("Approved", 1) + C.addApproval("Approved", 1) + D.addApproval("Approved", 1) + self.fake_gerrit.addEvent(B.addApproval("Approved", 1)) + self.waitUntilSettled() + + self.assertEqual(A.reported, 3) + self.assertEqual(B.reported, 3) + self.assertEqual(C.reported, 3) + self.assertEqual(D.reported, 3) + self.assertEqual(A.data["status"], "MERGED") + self.assertEqual(B.data["status"], "MERGED") + self.assertEqual(C.data["status"], "MERGED") + self.assertEqual(D.data["status"], "MERGED") + + @simple_layout('layouts/deps-by-topic.yaml') def test_deps_by_topic_new_patchset(self): # Make sure that we correctly update the change cache on new # patchsets. diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index f241147eb..2e90d3fb4 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -21,10 +21,12 @@ import time import configparser import datetime import dateutil.tz +import uuid import fixtures import jwt import testtools +import sqlalchemy from zuul.zk import ZooKeeperClient from zuul.zk.locks import SessionAwareLock @@ -499,27 +501,107 @@ class TestDBPruneParse(BaseTestCase): class DBPruneTestCase(ZuulTestCase): tenant_config_file = 'config/single-tenant/main.yaml' + # This should be larger than the limit size in sqlconnection + num_buildsets = 55 + + def _createBuildset(self, update_time): + connection = self.scheds.first.sched.sql.connection + buildset_uuid = uuid.uuid4().hex + event_id = uuid.uuid4().hex + with connection.getSession() as db: + start_time = update_time - datetime.timedelta(seconds=1) + end_time = update_time + db_buildset = db.createBuildSet( + uuid=buildset_uuid, + tenant='tenant-one', + pipeline='check', + project='org/project', + change='1', + patchset='1', + ref='refs/changes/1', + oldrev='', + newrev='', + branch='master', + zuul_ref='Zref', + ref_url='http://gerrit.example.com/1', + event_id=event_id, + event_timestamp=update_time, + updated=update_time, + first_build_start_time=start_time, + last_build_end_time=end_time, + result='SUCCESS', + ) + for build_num in range(2): + build_uuid = uuid.uuid4().hex + db_build = db_buildset.createBuild( + uuid=build_uuid, + job_name=f'job{build_num}', + start_time=start_time, + end_time=end_time, + result='SUCCESS', + voting=True, + ) + for art_num in range(2): + db_build.createArtifact( + name=f'artifact{art_num}', + url='http://example.com', + ) + for provides_num in range(2): + db_build.createProvides( + name=f'item{provides_num}', + ) + for event_num in range(2): + db_build.createBuildEvent( + event_type=f'event{event_num}', + event_time=start_time, + ) + + def _query(self, db, model): + table = model.__table__ + q = db.session().query(model).order_by(table.c.id.desc()) + try: + return q.all() + except sqlalchemy.orm.exc.NoResultFound: + return [] + + def _getBuildsets(self, db): + return self._query(db, db.connection.buildSetModel) + + def _getBuilds(self, db): + return self._query(db, db.connection.buildModel) + + def _getProvides(self, db): + return self._query(db, db.connection.providesModel) + + def _getArtifacts(self, db): + return self._query(db, db.connection.artifactModel) + + def _getBuildEvents(self, db): + return self._query(db, db.connection.buildEventModel) def _setup(self): config_file = os.path.join(self.test_root, 'zuul.conf') with open(config_file, 'w') as f: self.config.write(f) - A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') - self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) - self.waitUntilSettled() - - time.sleep(1) - - B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') - self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) - self.waitUntilSettled() + update_time = (datetime.datetime.utcnow() - + datetime.timedelta(minutes=self.num_buildsets)) + for x in range(self.num_buildsets): + update_time = update_time + datetime.timedelta(minutes=1) + self._createBuildset(update_time) connection = self.scheds.first.sched.sql.connection - buildsets = connection.getBuildsets() - builds = connection.getBuilds() - self.assertEqual(len(buildsets), 2) - self.assertEqual(len(builds), 6) + with connection.getSession() as db: + buildsets = self._getBuildsets(db) + builds = self._getBuilds(db) + artifacts = self._getArtifacts(db) + provides = self._getProvides(db) + events = self._getBuildEvents(db) + self.assertEqual(len(buildsets), self.num_buildsets) + self.assertEqual(len(builds), 2 * self.num_buildsets) + self.assertEqual(len(artifacts), 4 * self.num_buildsets) + self.assertEqual(len(provides), 4 * self.num_buildsets) + self.assertEqual(len(events), 4 * self.num_buildsets) for build in builds: self.log.debug("Build %s %s %s", build, build.start_time, build.end_time) @@ -535,6 +617,7 @@ class DBPruneTestCase(ZuulTestCase): start_time = buildsets[0].first_build_start_time self.log.debug("Cutoff %s", start_time) + # Use the default batch size (omit --batch-size arg) p = subprocess.Popen( [os.path.join(sys.prefix, 'bin/zuul-admin'), '-c', config_file, @@ -545,13 +628,20 @@ class DBPruneTestCase(ZuulTestCase): out, _ = p.communicate() self.log.debug(out.decode('utf8')) - buildsets = connection.getBuildsets() - builds = connection.getBuilds() - self.assertEqual(len(buildsets), 1) - self.assertEqual(len(builds), 3) + with connection.getSession() as db: + buildsets = self._getBuildsets(db) + builds = self._getBuilds(db) + artifacts = self._getArtifacts(db) + provides = self._getProvides(db) + events = self._getBuildEvents(db) for build in builds: self.log.debug("Build %s %s %s", build, build.start_time, build.end_time) + self.assertEqual(len(buildsets), 1) + self.assertEqual(len(builds), 2) + self.assertEqual(len(artifacts), 4) + self.assertEqual(len(provides), 4) + self.assertEqual(len(events), 4) def test_db_prune_older_than(self): # Test pruning buildsets older than a relative time @@ -567,15 +657,23 @@ class DBPruneTestCase(ZuulTestCase): '-c', config_file, 'prune-database', '--older-than', '0d', + '--batch-size', '5', ], stdout=subprocess.PIPE) out, _ = p.communicate() self.log.debug(out.decode('utf8')) - buildsets = connection.getBuildsets() - builds = connection.getBuilds() + with connection.getSession() as db: + buildsets = self._getBuildsets(db) + builds = self._getBuilds(db) + artifacts = self._getArtifacts(db) + provides = self._getProvides(db) + events = self._getBuildEvents(db) self.assertEqual(len(buildsets), 0) self.assertEqual(len(builds), 0) + self.assertEqual(len(artifacts), 0) + self.assertEqual(len(provides), 0) + self.assertEqual(len(events), 0) class TestDBPruneMysql(DBPruneTestCase): diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index 2a63d5ef8..4085e8b1b 100644 --- a/tests/unit/test_gerrit.py +++ b/tests/unit/test_gerrit.py @@ -827,6 +827,14 @@ class TestGerritFake(ZuulTestCase): config_file = "zuul-gerrit-github.conf" tenant_config_file = "config/circular-dependencies/main.yaml" + def _make_tuple(self, data): + ret = [] + for c in data: + dep_change = c['number'] + dep_ps = c['currentPatchSet']['number'] + ret.append((int(dep_change), int(dep_ps))) + return sorted(ret) + def _get_tuple(self, change_number): ret = [] data = self.fake_gerrit.get( @@ -903,6 +911,11 @@ class TestGerritFake(ZuulTestCase): ret = self.fake_gerrit._getSubmittedTogether(C1, None) self.assertEqual(ret, [(4, 1)]) + # Test also the query used by the GerritConnection: + ret = self.fake_gerrit._simpleQuery('status:open topic:test-topic') + ret = self._make_tuple(ret) + self.assertEqual(ret, [(3, 1), (4, 1)]) + class TestGerritConnection(ZuulTestCase): config_file = 'zuul-gerrit-web.conf' @@ -958,6 +971,92 @@ class TestGerritConnection(ZuulTestCase): self.assertEqual(A.data['status'], 'MERGED') self.assertEqual(B.data['status'], 'MERGED') + def test_submit_requirements(self): + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + A.addApproval('Code-Review', 2) + # Set an unsatisfied submit requirement + A.setSubmitRequirements([ + { + "name": "Code-Review", + "description": "Disallow self-review", + "status": "UNSATISFIED", + "is_legacy": False, + "submittability_expression_result": { + "expression": "label:Code-Review=MAX,user=non_uploader " + "AND -label:Code-Review=MIN", + "fulfilled": False, + "passing_atoms": [], + "failing_atoms": [ + "label:Code-Review=MAX,user=non_uploader", + "label:Code-Review=MIN" + ] + } + }, + { + "name": "Verified", + "status": "UNSATISFIED", + "is_legacy": True, + "submittability_expression_result": { + "expression": "label:Verified=MAX -label:Verified=MIN", + "fulfilled": False, + "passing_atoms": [], + "failing_atoms": [ + "label:Verified=MAX", + "-label:Verified=MIN" + ] + } + }, + ]) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertHistory([]) + self.assertEqual(A.queried, 1) + self.assertEqual(A.data['status'], 'NEW') + + # Mark the requirement satisfied + A.setSubmitRequirements([ + { + "name": "Code-Review", + "description": "Disallow self-review", + "status": "SATISFIED", + "is_legacy": False, + "submittability_expression_result": { + "expression": "label:Code-Review=MAX,user=non_uploader " + "AND -label:Code-Review=MIN", + "fulfilled": False, + "passing_atoms": [ + "label:Code-Review=MAX,user=non_uploader", + ], + "failing_atoms": [ + "label:Code-Review=MIN" + ] + } + }, + { + "name": "Verified", + "status": "UNSATISFIED", + "is_legacy": True, + "submittability_expression_result": { + "expression": "label:Verified=MAX -label:Verified=MIN", + "fulfilled": False, + "passing_atoms": [], + "failing_atoms": [ + "label:Verified=MAX", + "-label:Verified=MIN" + ] + } + }, + ]) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name="project-merge", result="SUCCESS", changes="1,1"), + dict(name="project-test1", result="SUCCESS", changes="1,1"), + dict(name="project-test2", result="SUCCESS", changes="1,1"), + ], ordered=False) + self.assertEqual(A.queried, 3) + self.assertEqual(A.data['status'], 'MERGED') + class TestGerritUnicodeRefs(ZuulTestCase): config_file = 'zuul-gerrit-web.conf' diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 47e84ca7f..3060f5673 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1430,7 +1430,9 @@ class TestGithubDriver(ZuulTestCase): repo._set_branch_protection( 'master', contexts=['tenant-one/check', 'tenant-one/gate']) - A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') + pr_description = "PR description" + A = self.fake_github.openFakePullRequest('org/project', 'master', 'A', + body_text=pr_description) self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) self.waitUntilSettled() @@ -1448,6 +1450,9 @@ class TestGithubDriver(ZuulTestCase): merges = [report for report in self.fake_github.github_data.reports if report[2] == 'merge'] assert (len(merges) == 1 and merges[0][3] == 'squash') + # Assert that we won't duplicate the PR title in the merge + # message description. + self.assertEqual(A.merge_message, pr_description) @simple_layout('layouts/basic-github.yaml', driver='github') def test_invalid_event(self): diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py index ef1f75944..f3021d41d 100644 --- a/tests/unit/test_github_requirements.py +++ b/tests/unit/test_github_requirements.py @@ -678,3 +678,181 @@ class TestGithubAppRequirements(ZuulGithubAppTestCase): self.fake_github.emitEvent(comment) self.waitUntilSettled() self.assertEqual(len(self.history), 1) + + +class TestGithubTriggerRequirements(ZuulTestCase): + """Test pipeline and trigger requirements""" + config_file = 'zuul-github-driver.conf' + scheduler_count = 1 + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_require_status(self): + # Test trigger require-status + jobname = 'require-status' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No status from zuul so should not be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # An error status should not cause it to be enqueued + self.fake_github.setCommitStatus(project, A.head_sha, 'error', + context='tenant-one/check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A success status goes in + self.fake_github.setCommitStatus(project, A.head_sha, 'success', + context='tenant-one/check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_reject_status(self): + # Test trigger reject-status + jobname = 'reject-status' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No status from zuul so should be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # A failure status should not cause it to be enqueued + self.fake_github.setCommitStatus(project, A.head_sha, 'failure', + context='tenant-one/check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + + # A success status goes in + self.fake_github.setCommitStatus(project, A.head_sha, 'success', + context='tenant-one/check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 2) + self.assertEqual(self.history[1].name, jobname) + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_require_review(self): + # Test trigger require-review + jobname = 'require-review' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + A.writers.extend(('maintainer',)) + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No review so should not be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # An changes requested review should not cause it to be enqueued + A.addReview('maintainer', 'CHANGES_REQUESTED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A positive review goes in + A.addReview('maintainer', 'APPROVED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_reject_review(self): + # Test trigger reject-review + jobname = 'reject-review' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + A.writers.extend(('maintainer',)) + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No review so should be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # An changes requested review should not cause it to be enqueued + A.addReview('maintainer', 'CHANGES_REQUESTED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + + # A positive review goes in + A.addReview('maintainer', 'APPROVED') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 2) + self.assertEqual(self.history[1].name, jobname) + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_require_label(self): + # Test trigger require-label + jobname = 'require-label' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No label so should not be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A random should not cause it to be enqueued + A.addLabel('foobar') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # An approved label goes in + A.addLabel('approved') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/github-trigger-requirements.yaml', driver='github') + def test_reject_label(self): + # Test trigger reject-label + jobname = 'reject-label' + project = 'org/project' + A = self.fake_github.openFakePullRequest(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent(f'test {jobname}') + + # No label so should be enqueued + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # A rejected label should not cause it to be enqueued + A.addLabel('rejected') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + + # Any other label, it goes in + A.removeLabel('rejected') + A.addLabel('okay') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 2) + self.assertEqual(self.history[1].name, jobname) diff --git a/tests/unit/test_requirements.py b/tests/unit/test_requirements.py index 9f3b87187..c5dca56cd 100644 --- a/tests/unit/test_requirements.py +++ b/tests/unit/test_requirements.py @@ -14,7 +14,7 @@ import time -from tests.base import ZuulTestCase +from tests.base import ZuulTestCase, simple_layout class TestRequirementsApprovalNewerThan(ZuulTestCase): @@ -490,3 +490,222 @@ class TestRequirementsTrustedCheck(ZuulTestCase): self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) self.waitUntilSettled() self.assertHistory([]) + + +class TestGerritTriggerRequirements(ZuulTestCase): + scheduler_count = 1 + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_require_open(self): + # Test trigger require-open + jobname = 'require-open' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's open, so it should be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # Not open, so should be ignored + A.setMerged() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_reject_open(self): + # Test trigger reject-open + jobname = 'reject-open' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's open, so it should not be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Not open, so should be enqueued + A.setMerged() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_require_wip(self): + # Test trigger require-wip + jobname = 'require-wip' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's not WIP, so it should be ignored + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # WIP, so should be enqueued + A.setWorkInProgress(True) + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_reject_wip(self): + # Test trigger reject-wip + jobname = 'reject-wip' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's not WIP, so it should be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # WIP, so should be ignored + A.setWorkInProgress(True) + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_require_current_patchset(self): + # Test trigger require-current_patchset + jobname = 'require-current-patchset' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's current, so it should be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # Not current, so should be ignored + A.addPatchset() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_reject_current_patchset(self): + # Test trigger reject-current_patchset + jobname = 'reject-current-patchset' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's current, so it should be ignored + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Not current, so should be enqueued + A.addPatchset() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_require_status(self): + # Test trigger require-status + jobname = 'require-status' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's not merged, so it should be ignored + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Merged, so should be enqueued + A.setMerged() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_reject_status(self): + # Test trigger reject-status + jobname = 'reject-status' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # It's not merged, so it should be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # Merged, so should be ignored + A.setMerged() + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_require_approval(self): + # Test trigger require-approval + jobname = 'require-approval' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # Missing approval, so it should be ignored + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Has approval, so it should be enqueued + A.addApproval('Verified', 1, username='zuul') + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + @simple_layout('layouts/gerrit-trigger-requirements.yaml') + def test_reject_approval(self): + # Test trigger reject-approval + jobname = 'reject-approval' + project = 'org/project' + A = self.fake_gerrit.addFakeChange(project, 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getChangeCommentEvent(1, f'test {jobname}') + + # Missing approval, so it should be enqueued + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) + + # Has approval, so it should be ignored + A.addApproval('Verified', 1, username='zuul') + self.fake_gerrit.addEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, jobname) diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 172ed34dc..131034f17 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -7440,6 +7440,85 @@ class TestSchedulerMerges(ZuulTestCase): result = self._test_project_merge_mode('cherry-pick') self.assertEqual(result, expected_messages) + def test_project_merge_mode_cherrypick_redundant(self): + # A redundant commit (that is, one that has already been applied to the + # working tree) should be skipped + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + files = { + "foo.txt": "ABC", + } + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', files=files) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = True + B = self.fake_gerrit.addFakeChange(project, 'master', 'B', files=files) + B.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + dict(name='project-test1', result='SUCCESS', changes='2,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + + def test_project_merge_mode_cherrypick_empty(self): + # An empty commit (that is, one that doesn't modify any files) should + # be preserved + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', empty=True) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + changed_files = list(repo.commit("HEAD").diff(repo.commit("HEAD~1"))) + self.assertEqual(changed_files, []) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + def test_project_merge_mode_cherrypick_branch_merge(self): "Test that branches can be merged together in cherry-pick mode" self.create_branch('org/project-merge-branches', 'mp') diff --git a/web/src/Misc.jsx b/web/src/Misc.jsx index dae5a84bf..1186f42d3 100644 --- a/web/src/Misc.jsx +++ b/web/src/Misc.jsx @@ -120,4 +120,30 @@ IconProperty.propTypes = { const ConditionalWrapper = ({ condition, wrapper, children }) => condition ? wrapper(children) : children -export { IconProperty, removeHash, ExternalLink, buildExternalLink, buildExternalTableLink, ConditionalWrapper } +function resolveDarkMode(theme) { + let darkMode = false + + if (theme === 'Auto') { + let matchMedia = window.matchMedia || function () { + return { + matches: false, + } + } + + darkMode = matchMedia('(prefers-color-scheme: dark)').matches + } else if (theme === 'Dark') { + darkMode = true + } + + return darkMode +} + +function setDarkMode(darkMode) { + if (darkMode) { + document.documentElement.classList.add('pf-theme-dark') + } else { + document.documentElement.classList.remove('pf-theme-dark') + } +} + +export { IconProperty, removeHash, ExternalLink, buildExternalLink, buildExternalTableLink, ConditionalWrapper, resolveDarkMode, setDarkMode } diff --git a/web/src/containers/FilterToolbar.jsx b/web/src/containers/FilterToolbar.jsx index 328d60d85..bcf74c062 100644 --- a/web/src/containers/FilterToolbar.jsx +++ b/web/src/containers/FilterToolbar.jsx @@ -13,6 +13,7 @@ // under the License. import React, { useState } from 'react' +import { useDispatch } from 'react-redux' import PropTypes from 'prop-types' import { Button, @@ -32,12 +33,14 @@ import { } from '@patternfly/react-core' import { FilterIcon, SearchIcon } from '@patternfly/react-icons' +import { addNotification } from '../actions/notifications' import { FilterSelect } from './filters/Select' import { FilterTernarySelect } from './filters/TernarySelect' import { FilterCheckbox } from './filters/Checkbox' function FilterToolbar(props) { + const dispatch = useDispatch() const [isCategoryDropdownOpen, setIsCategoryDropdownOpen] = useState(false) const [currentCategory, setCurrentCategory] = useState( props.filterCategories[0].title @@ -58,15 +61,22 @@ function FilterToolbar(props) { } function handleInputSend(event, category) { - const { onFilterChange, filters } = props + const { onFilterChange, filters, filterInputValidation } = props // In case the event comes from a key press, only accept "Enter" if (event.key && event.key !== 'Enter') { return } - // Ignore empty values - if (!inputValue) { + const validationResult = filterInputValidation(category.key, inputValue) + if (!validationResult.success) { + dispatch(addNotification( + { + text: validationResult.message, + type: 'error', + status: '', + url: '', + })) return } @@ -250,6 +260,7 @@ FilterToolbar.propTypes = { onFilterChange: PropTypes.func.isRequired, filters: PropTypes.object.isRequired, filterCategories: PropTypes.array.isRequired, + filterInputValidation: PropTypes.func.isRequired, } function getChipsFromFilters(filters, category) { diff --git a/web/src/containers/autohold/HeldBuildList.jsx b/web/src/containers/autohold/HeldBuildList.jsx index d2f45dd20..0aef3a804 100644 --- a/web/src/containers/autohold/HeldBuildList.jsx +++ b/web/src/containers/autohold/HeldBuildList.jsx @@ -62,7 +62,6 @@ class HeldBuildList extends React.Component { to={`${tenant.linkPrefix}/build/${node.build}`} style={{ textDecoration: 'none', - color: 'var(--pf-global--disabled-color--100)', }} > <DataListItemRow> diff --git a/web/src/containers/build/Artifact.jsx b/web/src/containers/build/Artifact.jsx index 3793222d9..0ada93a0c 100644 --- a/web/src/containers/build/Artifact.jsx +++ b/web/src/containers/build/Artifact.jsx @@ -18,15 +18,16 @@ import { TreeView, } from 'patternfly-react' import ReactJson from 'react-json-view' - +import { connect } from 'react-redux' class Artifact extends React.Component { static propTypes = { - artifact: PropTypes.object.isRequired + artifact: PropTypes.object.isRequired, + preferences: PropTypes.object, } render() { - const { artifact } = this.props + const { artifact, preferences } = this.props return ( <table className="table table-striped table-bordered" style={{width:'50%'}}> <tbody> @@ -41,7 +42,8 @@ class Artifact extends React.Component { collapsed={true} sortKeys={true} enableClipboard={false} - displayDataTypes={false}/> + displayDataTypes={false} + theme={preferences.darkMode ? 'tomorrow' : 'rjv-default'}/> :artifact.metadata[key].toString()} </td> </tr> @@ -54,17 +56,18 @@ class Artifact extends React.Component { class ArtifactList extends React.Component { static propTypes = { - artifacts: PropTypes.array.isRequired + artifacts: PropTypes.array.isRequired, + preferences: PropTypes.object, } render() { - const { artifacts } = this.props + const { artifacts, preferences } = this.props const nodes = artifacts.map((artifact, index) => { const node = {text: <a href={artifact.url}>{artifact.name}</a>, icon: null} if (artifact.metadata) { - node['nodes']= [{text: <Artifact key={index} artifact={artifact}/>, + node['nodes']= [{text: <Artifact key={index} artifact={artifact} preferences={preferences}/>, icon: ''}] } return node @@ -83,4 +86,10 @@ class ArtifactList extends React.Component { } } -export default ArtifactList +function mapStateToProps(state) { + return { + preferences: state.preferences, + } +} + +export default connect(mapStateToProps)(ArtifactList) diff --git a/web/src/containers/build/BuildOutput.jsx b/web/src/containers/build/BuildOutput.jsx index 1098ed2c7..58f0e13b5 100644 --- a/web/src/containers/build/BuildOutput.jsx +++ b/web/src/containers/build/BuildOutput.jsx @@ -13,6 +13,7 @@ // under the License. import * as React from 'react' +import { connect } from 'react-redux' import { Fragment } from 'react' import ReAnsi from '@softwarefactory-project/re-ansi' import PropTypes from 'prop-types' @@ -73,6 +74,7 @@ class BuildOutputLabel extends React.Component { class BuildOutput extends React.Component { static propTypes = { output: PropTypes.object, + preferences: PropTypes.object, } renderHosts (hosts) { @@ -109,8 +111,12 @@ class BuildOutput extends React.Component { renderFailedTask (host, task) { const max_lines = 42 + let zuulOutputClass = 'zuul-build-output' + if (this.props.preferences.darkMode) { + zuulOutputClass = 'zuul-build-output-dark' + } return ( - <Card key={host + task.zuul_log_id} className="zuul-task-summary-failed"> + <Card key={host + task.zuul_log_id} className="zuul-task-summary-failed" style={this.props.preferences.darkMode ? {background: 'var(--pf-global--BackgroundColor--300)'} : {}}> <CardHeader> <TimesIcon style={{ color: 'var(--pf-global--danger-color--100)' }}/> Task <strong>{task.name}</strong> @@ -119,25 +125,25 @@ class BuildOutput extends React.Component { <CardBody> {task.invocation && task.invocation.module_args && task.invocation.module_args._raw_params && ( - <pre key="cmd" title="cmd" className={`${'cmd'}`}> + <pre key="cmd" title="cmd" className={'cmd ' + zuulOutputClass}> {task.invocation.module_args._raw_params} </pre> )} {task.msg && ( - <pre key="msg" title="msg">{task.msg}</pre> + <pre key="msg" title="msg" className={zuulOutputClass}>{task.msg}</pre> )} {task.exception && ( - <pre key="exc" style={{ color: 'red' }} title="exc">{task.exception}</pre> + <pre key="exc" style={{ color: 'red' }} title="exc" className={zuulOutputClass}>{task.exception}</pre> )} {task.stdout_lines && task.stdout_lines.length > 0 && ( <Fragment> {task.stdout_lines.length > max_lines && ( <details className={`${'foldable'} ${'stdout'}`}><summary></summary> - <pre key="stdout" title="stdout"> + <pre key="stdout" title="stdout" className={zuulOutputClass}> <ReAnsi log={task.stdout_lines.slice(0, -max_lines).join('\n')} /> </pre> </details>)} - <pre key="stdout" title="stdout"> + <pre key="stdout" title="stdout" className={zuulOutputClass}> <ReAnsi log={task.stdout_lines.slice(-max_lines).join('\n')} /> </pre> </Fragment> @@ -146,12 +152,12 @@ class BuildOutput extends React.Component { <Fragment> {task.stderr_lines.length > max_lines && ( <details className={`${'foldable'} ${'stderr'}`}><summary></summary> - <pre key="stderr" title="stderr"> + <pre key="stderr" title="stderr" className={zuulOutputClass}> <ReAnsi log={task.stderr_lines.slice(0, -max_lines).join('\n')} /> </pre> </details> )} - <pre key="stderr" title="stderr"> + <pre key="stderr" title="stderr" className={zuulOutputClass}> <ReAnsi log={task.stderr_lines.slice(-max_lines).join('\n')} /> </pre> </Fragment> @@ -177,4 +183,10 @@ class BuildOutput extends React.Component { } -export default BuildOutput +function mapStateToProps(state) { + return { + preferences: state.preferences, + } +} + +export default connect(mapStateToProps)(BuildOutput) diff --git a/web/src/containers/build/BuildOutput.test.jsx b/web/src/containers/build/BuildOutput.test.jsx index c76236a2e..defa342c5 100644 --- a/web/src/containers/build/BuildOutput.test.jsx +++ b/web/src/containers/build/BuildOutput.test.jsx @@ -14,6 +14,8 @@ import React from 'react' import ReactDOM from 'react-dom' +import { Provider } from 'react-redux' +import configureStore from '../../store' import BuildOutput from './BuildOutput' const fakeOutput = (width, height) => ({ @@ -31,7 +33,11 @@ it('BuildOutput renders big task', () => { const div = document.createElement('div') const output = fakeOutput(512, 1024) const begin = performance.now() - ReactDOM.render(<BuildOutput output={output} />, div, () => { + const store = configureStore() + ReactDOM.render( + <Provider store={store}> + <BuildOutput output={output} /> + </Provider>, div, () => { const end = performance.now() console.log('Render took ' + (end - begin) + ' milliseconds.') }) diff --git a/web/src/containers/build/Buildset.jsx b/web/src/containers/build/Buildset.jsx index 2ca70549d..5492b7a13 100644 --- a/web/src/containers/build/Buildset.jsx +++ b/web/src/containers/build/Buildset.jsx @@ -47,7 +47,7 @@ import { addNotification, addApiError } from '../../actions/notifications' import { ChartModal } from '../charts/ChartModal' import BuildsetGanttChart from '../charts/GanttChart' -function Buildset({ buildset, timezone, tenant, user }) { +function Buildset({ buildset, timezone, tenant, user, preferences }) { const buildset_link = buildExternalLink(buildset) const [isGanttChartModalOpen, setIsGanttChartModalOpen] = useState(false) @@ -319,7 +319,9 @@ function Buildset({ buildset, timezone, tenant, user }) { value={ <> <strong>Message:</strong> - <pre>{buildset.message}</pre> + <div className={preferences.darkMode ? 'zuul-console-dark' : ''}> + <pre>{buildset.message}</pre> + </div> </> } /> @@ -349,10 +351,12 @@ Buildset.propTypes = { tenant: PropTypes.object, timezone: PropTypes.string, user: PropTypes.object, + preferences: PropTypes.object, } export default connect((state) => ({ tenant: state.tenant, timezone: state.timezone, user: state.user, + preferences: state.preferences, }))(Buildset) diff --git a/web/src/containers/build/Console.jsx b/web/src/containers/build/Console.jsx index 9cd10df92..826ebe3cd 100644 --- a/web/src/containers/build/Console.jsx +++ b/web/src/containers/build/Console.jsx @@ -18,6 +18,7 @@ import * as React from 'react' import ReAnsi from '@softwarefactory-project/re-ansi' import PropTypes from 'prop-types' import ReactJson from 'react-json-view' +import { connect } from 'react-redux' import { Button, @@ -60,6 +61,7 @@ class TaskOutput extends React.Component { static propTypes = { data: PropTypes.object, include: PropTypes.array, + preferences: PropTypes.object, } renderResults(value) { @@ -130,7 +132,8 @@ class TaskOutput extends React.Component { name={null} sortKeys={true} enableClipboard={false} - displayDataTypes={false}/> + displayDataTypes={false} + theme={this.props.preferences.darkMode ? 'tomorrow' : 'rjv-default'}/> </pre> ) } else { @@ -142,7 +145,7 @@ class TaskOutput extends React.Component { } return ( - <div key={key}> + <div className={this.props.preferences.darkMode ? 'zuul-console-dark' : 'zuul-console-light'} key={key}> {ret && <h5>{key}</h5>} {ret && ret} </div> @@ -170,6 +173,7 @@ class HostTask extends React.Component { errorIds: PropTypes.object, taskPath: PropTypes.array, displayPath: PropTypes.array, + preferences: PropTypes.object, } state = { @@ -290,7 +294,7 @@ class HostTask extends React.Component { </DataListCell> ) - const content = <TaskOutput data={this.props.host} include={INTERESTING_KEYS}/> + const content = <TaskOutput data={this.props.host} include={INTERESTING_KEYS} preferences={this.props.preferences}/> let item = null if (interestingKeys) { @@ -354,7 +358,7 @@ class HostTask extends React.Component { isOpen={this.state.showModal} onClose={this.close} description={modalDescription}> - <TaskOutput data={host}/> + <TaskOutput data={host} preferences={this.props.preferences}/> </Modal> </> ) @@ -367,6 +371,7 @@ class PlayBook extends React.Component { errorIds: PropTypes.object, taskPath: PropTypes.array, displayPath: PropTypes.array, + preferences: PropTypes.object, } constructor(props) { @@ -404,8 +409,8 @@ class PlayBook extends React.Component { dataListCells.push( <DataListCell key='name' width={1}> <strong> - {playbook.phase[0].toUpperCase() + playbook.phase.slice(1)} playbook< - /strong> + {playbook.phase[0].toUpperCase() + playbook.phase.slice(1)} playbook + </strong> </DataListCell>) dataListCells.push( <DataListCell key='path' width={5}> @@ -463,7 +468,8 @@ class PlayBook extends React.Component { taskPath={taskPath.concat([ idx.toString(), idx2.toString(), hostname])} displayPath={displayPath} task={task} host={host} - errorIds={errorIds}/> + errorIds={errorIds} + preferences={this.props.preferences}/> ))))} </DataList> @@ -484,6 +490,7 @@ class Console extends React.Component { errorIds: PropTypes.object, output: PropTypes.array, displayPath: PropTypes.array, + preferences: PropTypes.object, } render () { @@ -492,7 +499,7 @@ class Console extends React.Component { return ( <React.Fragment> <br /> - <span className="zuul-console"> + <span className={`zuul-console ${this.props.preferences.darkMode ? 'zuul-console-dark' : 'zuul-console-light'}`}> <DataList isCompact={true} style={{ fontSize: 'var(--pf-global--FontSize--md)' }}> { @@ -500,6 +507,7 @@ class Console extends React.Component { <PlayBook key={idx} playbook={playbook} taskPath={[idx.toString()]} displayPath={displayPath} errorIds={errorIds} + preferences={this.props.preferences} />)) } </DataList> @@ -509,5 +517,11 @@ class Console extends React.Component { } } +function mapStateToProps(state) { + return { + preferences: state.preferences, + } +} + -export default Console +export default connect(mapStateToProps)(Console) diff --git a/web/src/containers/charts/GanttChart.jsx b/web/src/containers/charts/GanttChart.jsx index 5ac065fce..f677d83b8 100644 --- a/web/src/containers/charts/GanttChart.jsx +++ b/web/src/containers/charts/GanttChart.jsx @@ -26,7 +26,7 @@ import { buildResultLegendData, buildsBarStyle } from './Misc' function BuildsetGanttChart(props) { - const { builds, timezone } = props + const { builds, timezone, preferences } = props const sortedByStartTime = builds.sort((a, b) => { if (a.start_time > b.start_time) { return -1 @@ -64,6 +64,10 @@ function BuildsetGanttChart(props) { const chartLegend = buildResultLegendData.filter((legend) => { return uniqueResults.indexOf(legend.name) > -1 }) + let horizontalLegendTextColor = '#000' + if (preferences.darkMode) { + horizontalLegendTextColor = '#ccc' + } return ( <div style={{ height: Math.max(400, 20 * builds.length) + 'px', width: '900px' }}> @@ -81,10 +85,9 @@ function BuildsetGanttChart(props) { legendOrientation='horizontal' legendPosition='top' legendData={legendData} - legendComponent={<ChartLegend data={chartLegend} itemsPerRow={4} />} - + legendComponent={<ChartLegend data={chartLegend} itemsPerRow={4} style={{labels: {fill: horizontalLegendTextColor}}} />} > - <ChartAxis /> + <ChartAxis style={{tickLabels: {fill:horizontalLegendTextColor}}} /> <ChartAxis dependentAxis showGrid @@ -103,15 +106,16 @@ function BuildsetGanttChart(props) { return moment.duration(t, 'seconds').format(format) }} fixLabelOverlap={true} - style={{ tickLabels: { angle: -25, padding: 1, verticalAnchor: 'middle', textAnchor: 'end' } }} /> + style={{ tickLabels: { angle: -25, padding: 1, verticalAnchor: 'middle', textAnchor: 'end', fill: horizontalLegendTextColor } }} + /> <ChartBar data={data} - style={buildsBarStyle} + style={ buildsBarStyle } labelComponent={ - <ChartTooltip constrainToVisibleArea />} + <ChartTooltip constrainToVisibleArea/>} labels={({ datum }) => `${datum.result}\nStarted ${datum.started}\nEnded ${datum.ended}`} /> - </ Chart> + </Chart> </div> ) @@ -120,8 +124,10 @@ function BuildsetGanttChart(props) { BuildsetGanttChart.propTypes = { builds: PropTypes.array.isRequired, timezone: PropTypes.string, + preferences: PropTypes.object, } export default connect((state) => ({ timezone: state.timezone, -}))(BuildsetGanttChart)
\ No newline at end of file + preferences: state.preferences, +}))(BuildsetGanttChart) diff --git a/web/src/containers/config/Config.jsx b/web/src/containers/config/Config.jsx index 3d402a116..652d6702f 100644 --- a/web/src/containers/config/Config.jsx +++ b/web/src/containers/config/Config.jsx @@ -18,10 +18,14 @@ import { ButtonVariant, Modal, ModalVariant, - Switch + Switch, + Select, + SelectOption, + SelectVariant } from '@patternfly/react-core' import { CogIcon } from '@patternfly/react-icons' import { setPreference } from '../../actions/preferences' +import { resolveDarkMode, setDarkMode } from '../../Misc' class ConfigModal extends React.Component { @@ -39,6 +43,8 @@ class ConfigModal extends React.Component { this.state = { isModalOpen: false, autoReload: false, + theme: 'Auto', + isThemeOpen: false, } this.handleModalToggle = () => { this.setState(({ isModalOpen }) => ({ @@ -47,9 +53,39 @@ class ConfigModal extends React.Component { this.resetState() } + this.handleEscape = () => { + if (this.state.isThemeOpen) { + this.setState(({ isThemeOpen }) => ({ + isThemeOpen: !isThemeOpen, + })) + } else { + this.handleModalToggle() + } + } + + this.handleThemeToggle = () => { + this.setState(({ isThemeOpen }) => ({ + isThemeOpen: !isThemeOpen, + })) + } + + this.handleThemeSelect = (event, selection) => { + this.setState({ + theme: selection, + isThemeOpen: false + }) + } + + this.handleTheme = () => { + let darkMode = resolveDarkMode(this.state.theme) + setDarkMode(darkMode) + } + this.handleSave = () => { this.handleModalToggle() this.props.dispatch(setPreference('autoReload', this.state.autoReload)) + this.props.dispatch(setPreference('theme', this.state.theme)) + this.handleTheme() } this.handleAutoReload = () => { @@ -62,11 +98,12 @@ class ConfigModal extends React.Component { resetState() { this.setState({ autoReload: this.props.preferences.autoReload, + theme: this.props.preferences.theme, }) } render() { - const { isModalOpen, autoReload } = this.state + const { isModalOpen, autoReload, theme, isThemeOpen } = this.state return ( <React.Fragment> <Button @@ -80,6 +117,7 @@ class ConfigModal extends React.Component { title="Preferences" isOpen={isModalOpen} onClose={this.handleModalToggle} + onEscapePress={this.handleEscape} actions={[ <Button key="confirm" variant="primary" onClick={this.handleSave}> Confirm @@ -91,6 +129,8 @@ class ConfigModal extends React.Component { > <div> <p key="info">Application settings are saved in browser local storage only. They are applied whether authenticated or not.</p> + </div> + <div> <Switch key="autoreload" id="autoreload" @@ -99,6 +139,24 @@ class ConfigModal extends React.Component { onChange={this.handleAutoReload} /> </div> + <div style={{'paddingTop': '25px'}}> + <p key="theme-info">Select your preferred theme, auto will base it on your system preference.</p> + </div> + <div> + <Select + variant={SelectVariant.single} + label="Select Input" + onToggle={this.handleThemeToggle} + onSelect={this.handleThemeSelect} + selections={theme} + isOpen={isThemeOpen} + menuAppendTo="parent" + > + <SelectOption key="auto" value="Auto"/> + <SelectOption key="light" value="Light"/> + <SelectOption key="dark" value="Dark"/> + </Select> + </div> </Modal> </React.Fragment> ) diff --git a/web/src/containers/job/JobVariant.jsx b/web/src/containers/job/JobVariant.jsx index 9621cf333..eeb6ee52e 100644 --- a/web/src/containers/job/JobVariant.jsx +++ b/web/src/containers/job/JobVariant.jsx @@ -58,7 +58,8 @@ class JobVariant extends React.Component { static propTypes = { parent: PropTypes.object, tenant: PropTypes.object, - variant: PropTypes.object.isRequired + variant: PropTypes.object.isRequired, + preferences: PropTypes.object, } renderStatus (variant) { @@ -161,7 +162,8 @@ class JobVariant extends React.Component { collapsed={true} sortKeys={true} enableClipboard={false} - displayDataTypes={false}/> + displayDataTypes={false} + theme={this.props.preferences.darkMode ? 'tomorrow' : 'rjv-default'}/> </span> ) } @@ -200,7 +202,8 @@ class JobVariant extends React.Component { collapsed={true} sortKeys={true} enableClipboard={false} - displayDataTypes={false}/> + displayDataTypes={false} + theme={this.props.preferences.darkMode ? 'tomorrow' : 'rjv-default'}/> </span> ) nice_label = (<span><CodeIcon /> Job variables</span>) @@ -287,4 +290,7 @@ class JobVariant extends React.Component { } } -export default connect(state => ({tenant: state.tenant}))(JobVariant) +export default connect(state => ({ + tenant: state.tenant, + preferences: state.preferences, +}))(JobVariant) diff --git a/web/src/containers/jobgraph/JobGraphDisplay.jsx b/web/src/containers/jobgraph/JobGraphDisplay.jsx index e5cff9cbc..c8fb938ac 100644 --- a/web/src/containers/jobgraph/JobGraphDisplay.jsx +++ b/web/src/containers/jobgraph/JobGraphDisplay.jsx @@ -21,10 +21,15 @@ import { useHistory } from 'react-router-dom' import { makeJobGraphKey, fetchJobGraphIfNeeded } from '../../actions/jobgraph' import { graphviz } from 'd3-graphviz' -function makeDot(tenant, pipeline, project, branch, jobGraph) { +function makeDot(tenant, pipeline, project, branch, jobGraph, dark) { let ret = 'digraph job_graph {\n' + ret += ' bgcolor="transparent"\n' ret += ' rankdir=LR;\n' - ret += ' node [shape=box];\n' + if (dark) { + ret += ' node [shape=box color="white" fontcolor="white"];\n' + } else { + ret += ' node [shape=box];\n' + } jobGraph.forEach((job) => { const searchParams = new URLSearchParams('') searchParams.append('pipeline', pipeline) @@ -43,8 +48,15 @@ function makeDot(tenant, pipeline, project, branch, jobGraph) { if (job.dependencies.length) { job.dependencies.forEach((dep) => { let soft = ' [dir=back]' + if (dark) { + soft = ' [dir=back color="white" fontcolor="white"]' + } if (dep.soft) { - soft = ' [style=dashed dir=back]' + if (dark) { + soft = ' [style=dashed dir=back color="white" fontcolor="white"]' + } else { + soft = ' [style=dashed dir=back]' + } } ret += ' "' + dep.name + '" -> "' + job.name + '"' + soft + ';\n' }) @@ -99,7 +111,7 @@ GraphViz.propTypes = { function JobGraphDisplay(props) { const [dot, setDot] = useState() - const {fetchJobGraphIfNeeded, tenant, project, pipeline, branch} = props + const {fetchJobGraphIfNeeded, tenant, project, pipeline, branch, preferences } = props useEffect(() => { fetchJobGraphIfNeeded(tenant, project.name, pipeline, branch) @@ -112,9 +124,9 @@ function JobGraphDisplay(props) { const jobGraph = tenantJobGraph ? tenantJobGraph[jobGraphKey] : undefined useEffect(() => { if (jobGraph) { - setDot(makeDot(tenant, pipeline, project, branch, jobGraph)) + setDot(makeDot(tenant, pipeline, project, branch, jobGraph, preferences.darkMode)) } - }, [tenant, pipeline, project, branch, jobGraph]) + }, [tenant, pipeline, project, branch, jobGraph, preferences]) return ( <> {dot && <GraphViz dot={dot}/>} @@ -131,11 +143,13 @@ JobGraphDisplay.propTypes = { jobgraph: PropTypes.object, dispatch: PropTypes.func, state: PropTypes.object, + preferences: PropTypes.object, } function mapStateToProps(state) { return { tenant: state.tenant, jobgraph: state.jobgraph, + preferences: state.preferences, state: state, } } diff --git a/web/src/containers/jobs/Jobs.jsx b/web/src/containers/jobs/Jobs.jsx index 71395f1d1..9af8210a2 100644 --- a/web/src/containers/jobs/Jobs.jsx +++ b/web/src/containers/jobs/Jobs.jsx @@ -163,6 +163,7 @@ class JobsList extends React.Component { <FormGroup controlId='jobs'> <FormControl type='text' + className="pf-c-form-control" placeholder='job name' defaultValue={filter} inputRef={i => this.filter = i} diff --git a/web/src/containers/project/ProjectVariant.jsx b/web/src/containers/project/ProjectVariant.jsx index 36d2c09ce..7ced1e1bb 100644 --- a/web/src/containers/project/ProjectVariant.jsx +++ b/web/src/containers/project/ProjectVariant.jsx @@ -59,7 +59,7 @@ function ProjectVariant(props) { return ( <div> - <table className='table table-striped table-bordered'> + <table className={`table ${props.preferences.darkMode ? 'zuul-table-dark' : 'table-striped table-bordered'}`}> <tbody> {rows.map(item => ( <tr key={item.label}> @@ -75,12 +75,14 @@ function ProjectVariant(props) { ProjectVariant.propTypes = { tenant: PropTypes.object, - variant: PropTypes.object.isRequired + variant: PropTypes.object.isRequired, + preferences: PropTypes.object, } function mapStateToProps(state) { return { tenant: state.tenant, + preferences: state.preferences, } } diff --git a/web/src/containers/status/Change.jsx b/web/src/containers/status/Change.jsx index ac0a4e6e8..b2ab50c5b 100644 --- a/web/src/containers/status/Change.jsx +++ b/web/src/containers/status/Change.jsx @@ -48,7 +48,8 @@ class Change extends React.Component { pipeline: PropTypes.object, tenant: PropTypes.object, user: PropTypes.object, - dispatch: PropTypes.func + dispatch: PropTypes.func, + preferences: PropTypes.object } state = { @@ -268,7 +269,11 @@ class Change extends React.Component { for (i = 0; i < queue._tree_columns; i++) { let className = '' if (i < change._tree.length && change._tree[i] !== null) { - className = ' zuul-change-row-line' + if (this.props.preferences.darkMode) { + className = ' zuul-change-row-line-dark' + } else { + className = ' zuul-change-row-line' + } } row.push( <td key={i} className={'zuul-change-row' + className}> @@ -313,4 +318,5 @@ class Change extends React.Component { export default connect(state => ({ tenant: state.tenant, user: state.user, + preferences: state.preferences, }))(Change) diff --git a/web/src/containers/status/ChangePanel.jsx b/web/src/containers/status/ChangePanel.jsx index dd4fc27e5..4c8c20469 100644 --- a/web/src/containers/status/ChangePanel.jsx +++ b/web/src/containers/status/ChangePanel.jsx @@ -25,7 +25,8 @@ class ChangePanel extends React.Component { static propTypes = { globalExpanded: PropTypes.bool.isRequired, change: PropTypes.object.isRequired, - tenant: PropTypes.object + tenant: PropTypes.object, + preferences: PropTypes.object } constructor () { @@ -126,7 +127,7 @@ class ChangePanel extends React.Component { const interesting_jobs = change.jobs.filter(j => this.jobStrResult(j) !== 'skipped') let jobPercent = (100 / interesting_jobs.length).toFixed(2) return ( - <div className='progress zuul-change-total-result'> + <div className={`progress zuul-change-total-result${this.props.preferences.darkMode ? ' progress-dark' : ''}`}> {change.jobs.map((job, idx) => { let result = this.jobStrResult(job) if (['queued', 'waiting', 'skipped'].includes(result)) { @@ -204,7 +205,7 @@ class ChangePanel extends React.Component { } return ( - <div className='progress zuul-job-result' + <div className={`progress zuul-job-result${this.props.preferences.darkMode ? ' progress-dark' : ''}`} title={title}> <div className={'progress-bar ' + className} role='progressbar' @@ -321,9 +322,9 @@ class ChangePanel extends React.Component { return ( <> - <ul className='list-group zuul-patchset-body'> + <ul className={`list-group ${this.props.preferences.darkMode ? 'zuul-patchset-body-dark' : 'zuul-patchset-body'}`}> {interestingJobs.map((job, idx) => ( - <li key={idx} className='list-group-item zuul-change-job'> + <li key={idx} className={`list-group-item ${this.props.preferences.darkMode ? 'zuul-change-job-dark' : 'zuul-change-job'}`}> {this.renderJob(job, times.jobs[job.name])} </li> ))} @@ -389,8 +390,8 @@ class ChangePanel extends React.Component { } const times = this.calculateTimes(change) const header = ( - <div className='panel panel-default zuul-change'> - <div className='panel-heading zuul-patchset-header' + <div className={`panel panel-default ${this.props.preferences.darkMode ? 'zuul-change-dark' : 'zuul-change'}`}> + <div className={`panel-heading ${this.props.preferences.darkMode ? 'zuul-patchset-header-dark' : 'zuul-patchset-header'}`} onClick={this.onClick}> <div className='row'> <div className='col-xs-8'> @@ -422,4 +423,7 @@ class ChangePanel extends React.Component { } } -export default connect(state => ({tenant: state.tenant}))(ChangePanel) +export default connect(state => ({ + tenant: state.tenant, + preferences: state.preferences, +}))(ChangePanel) diff --git a/web/src/containers/timezone/SelectTz.jsx b/web/src/containers/timezone/SelectTz.jsx index 576645f6c..0740aaed6 100644 --- a/web/src/containers/timezone/SelectTz.jsx +++ b/web/src/containers/timezone/SelectTz.jsx @@ -111,6 +111,7 @@ class SelectTz extends React.Component { <OutlinedClockIcon/> <Select className="zuul-select-tz" + classNamePrefix="zuul-select-tz" styles={customStyles} components={{ DropdownIndicator }} value={this.state.currentValue} diff --git a/web/src/images/line.png b/web/src/images/line.png Binary files differdeleted file mode 100644 index ace6bab3d..000000000 --- a/web/src/images/line.png +++ /dev/null diff --git a/web/src/index.css b/web/src/index.css index 587804cfa..4cedc144f 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -40,8 +40,18 @@ a.refresh { } .zuul-select-tz { - /* That's the color PF4 uses for the dropdown items in the navbar */ - color: var(--pf-global--Color--dark-100); + /* Always use black because when using dark mode the theme will default + to another dark color which is hard to see on a white background */ + color: #000; +} + +.pf-theme-dark .zuul-select-tz .zuul-select-tz__option { + background: #222; + color: #fff; +} + +.pf-theme-dark .zuul-select-tz .zuul-select-tz__option:hover { + background: #000; } /* Config error modal */ @@ -53,6 +63,15 @@ a.refresh { margin-left: var(--pf-global--spacer--md); } +.pf-theme-dark .zuul-config-errors-title, .pf-theme-dark .zuul-config-errors-count { + color: #fff !important; +} + +.pf-theme-dark .pf-c-notification-drawer pre { + background: #000; + color: #fff; +} + /* * Build Lists and Tables */ @@ -66,6 +85,10 @@ a.refresh { font-weight: bold; } +.zuul-menu-dropdown-toggle { + background: transparent !important; +} + .zuul-menu-dropdown-toggle:before { content: none !important; } @@ -167,6 +190,11 @@ a.refresh { margin-bottom: 10px; } +.zuul-change-dark { + margin-bottom: 10px; + border-color: #222; +} + .zuul-change-id { float: right; } @@ -210,6 +238,13 @@ a.refresh { padding: 2px 8px; } +.zuul-change-job-dark { + padding: 2px 8px; + background: #000; + color: #ccc; + border: 1px solid #222; +} + /* Force_break_very_long_non_hyphenated_repo_names */ .change_project { word-break: break-all; @@ -233,6 +268,21 @@ a.refresh { padding: 8px 12px; } +.zuul-patchset-header-dark { + font-size: small; + padding: 8px 12px; + background: #000 !important; + color: #ccc !important; + border-color: #222 !important; +} + +.zuul-patchset-body { +} + +.zuul-patchset-body-dark { + border-top: 1px solid #000; +} + .zuul-log-output { color: black; } @@ -283,7 +333,7 @@ a.refresh { } .zuul-build-status { - background: white; + background: transparent; font-size: 16px; } @@ -292,14 +342,23 @@ a.refresh { } .zuul-change-row-line { - background-image: url('images/line.png'); - background-repeat: 'repeat-y'; + background: linear-gradient(#000, #000) no-repeat center/2px 100%; + background-position-y: 15px; +} + +.zuul-change-row-line-dark { + background: linear-gradient(#fff, #fff) no-repeat center/2px 100%; + background-position-y: 15px; } .progress-bar-animated { animation: progress-bar-stripes 1s linear infinite; } +.progress-dark { + background: #333 !important; +} + /* Job Tree View group gap */ div.tree-view-container ul.list-group { margin: 0px 0px; @@ -325,6 +384,10 @@ pre.version { background-color: var(--pf-global--palette--red-50) !important; } +.pf-theme-dark .zuul-console-task-failed { + background-color: var(--pf-global--palette--red-300) !important; +} + .zuul-console .pf-c-data-list__expandable-content { border: none; } @@ -344,11 +407,21 @@ pre.version { border-radius: 5px; } -.zuul-console .pf-c-data-list__item:hover +.zuul-console-light .pf-c-data-list__item:hover { background: var(--pf-global--palette--blue-50); } +.zuul-console-dark .pf-c-data-list__item:hover +{ + background: var(--pf-global--BackgroundColor--200); +} + +.zuul-console-dark pre { + background: #000; + color: #fff; +} + .zuul-console .pf-c-data-list__item:hover::before { background: var(--pf-global--active-color--400); @@ -451,3 +524,42 @@ details.foldable[open] summary::before { .zuul-task-summary-failed.pf-c-card { background: var(--pf-global--palette--red-50); } + +.pf-theme-dark .pf-c-nav__link { + color: #fff !important; +} + +.pf-theme-dark .pf-c-modal-box__title-text, .pf-theme-dark .pf-c-modal-box__body { + color: #fff !important; +} + +.pf-theme-dark .swagger-ui { + filter: invert(88%) hue-rotate(180deg); +} + +.pf-theme-dark .swagger-ui .highlight-code { + filter: invert(100%) hue-rotate(180deg); +} + +.zuul-table-dark .list-group-item { + background-color: #333 !important; +} + +.zuul-build-output { +} + +.zuul-build-output-dark { + background-color: #000 !important; + color: #fff; +} + +.pf-theme-dark .zuul-log-sev-0 { + color: #ccc !important; +} +.pf-theme-dark .zuul-log-sev-1 { + color: #ccc !important; +} + +.pf-theme-dark .pf-c-empty-state { + color: #fff !important; +} diff --git a/web/src/pages/Autohold.jsx b/web/src/pages/Autohold.jsx index 0d0198b45..cc91cbcd0 100644 --- a/web/src/pages/Autohold.jsx +++ b/web/src/pages/Autohold.jsx @@ -59,6 +59,7 @@ class AutoholdPage extends React.Component { autohold: PropTypes.object, isFetching: PropTypes.bool.isRequired, fetchAutohold: PropTypes.func.isRequired, + preferences: PropTypes.object, } updateData = () => { @@ -147,7 +148,7 @@ class AutoholdPage extends React.Component { return ( <> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Title headingLevel="h2">Autohold Request {autohold.id}</Title> <Flex className="zuul-autohold-attributes"> @@ -211,7 +212,9 @@ class AutoholdPage extends React.Component { value={ <> <strong>Reason:</strong> - <pre>{autohold.reason}</pre> + <div className={this.props.preferences.darkMode ? 'zuul-console-dark' : ''}> + <pre>{autohold.reason}</pre> + </div> </> } /> @@ -221,7 +224,7 @@ class AutoholdPage extends React.Component { </Flex> </Flex> </PageSection> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Title headingLevel="h3"> <BuildIcon style={{ @@ -243,6 +246,7 @@ function mapStateToProps(state) { autohold: state.autoholds.autohold, tenant: state.tenant, isFetching: state.autoholds.isFetching, + preferences: state.preferences, } } diff --git a/web/src/pages/Build.jsx b/web/src/pages/Build.jsx index 0386f8eef..c66af1060 100644 --- a/web/src/pages/Build.jsx +++ b/web/src/pages/Build.jsx @@ -65,6 +65,7 @@ class BuildPage extends React.Component { activeTab: PropTypes.string.isRequired, location: PropTypes.object.isRequired, history: PropTypes.object.isRequired, + preferences: PropTypes.object, } state = { @@ -250,10 +251,10 @@ class BuildPage extends React.Component { return ( <> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Build build={build} active={activeTab} hash={hash} /> </PageSection> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Tabs isFilled activeKey={activeTab} @@ -314,7 +315,7 @@ class BuildPage extends React.Component { </Tabs> </PageSection> {!this.state.topOfPageVisible && ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Button onClick={scrollToTop} variant="primary" style={{position: 'fixed', bottom: 20, right: 20, zIndex: 1}}> Go to top of page <ArrowUpIcon/> </Button> @@ -362,6 +363,7 @@ function mapStateToProps(state, ownProps) { isFetchingManifest: state.build.isFetchingManifest, isFetchingOutput: state.build.isFetchingOutput, isFetchingLogfile: state.logfile.isFetching, + preferences: state.preferences, } } diff --git a/web/src/pages/Builds.jsx b/web/src/pages/Builds.jsx index b89bcd678..f1c449ec0 100644 --- a/web/src/pages/Builds.jsx +++ b/web/src/pages/Builds.jsx @@ -195,6 +195,28 @@ class BuildsPage extends React.Component { this.updateData(filters) } } + + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } handleFilterChange = (newFilters) => { const { location, history } = this.props @@ -261,6 +283,7 @@ class BuildsPage extends React.Component { filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( diff --git a/web/src/pages/Buildset.jsx b/web/src/pages/Buildset.jsx index b19aefb81..6e1cded57 100644 --- a/web/src/pages/Buildset.jsx +++ b/web/src/pages/Buildset.jsx @@ -38,6 +38,7 @@ class BuildsetPage extends React.Component { buildset: PropTypes.object, isFetching: PropTypes.bool.isRequired, fetchBuildset: PropTypes.func.isRequired, + preferences: PropTypes.object, } updateData = () => { @@ -105,10 +106,10 @@ class BuildsetPage extends React.Component { return ( <> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Buildset buildset={buildset} /> </PageSection> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Title headingLevel="h3"> <BuildIcon style={{ @@ -134,6 +135,7 @@ function mapStateToProps(state, ownProps) { buildset, tenant: state.tenant, isFetching: state.build.isFetching, + preferences: state.preferences, } } diff --git a/web/src/pages/Buildsets.jsx b/web/src/pages/Buildsets.jsx index 98d86d640..3ae3b772b 100644 --- a/web/src/pages/Buildsets.jsx +++ b/web/src/pages/Buildsets.jsx @@ -32,6 +32,7 @@ class BuildsetsPage extends React.Component { tenant: PropTypes.object, location: PropTypes.object, history: PropTypes.object, + preferences: PropTypes.object, } constructor(props) { @@ -148,6 +149,28 @@ class BuildsetsPage extends React.Component { } } + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } + handleFilterChange = (newFilters) => { const { location, history } = this.props const { filters, itemCount } = this.state @@ -208,11 +231,12 @@ class BuildsetsPage extends React.Component { const { buildsets, fetching, filters, resultsPerPage, currentPage, itemCount } = this.state return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <FilterToolbar filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( @@ -245,4 +269,7 @@ class BuildsetsPage extends React.Component { } } -export default connect((state) => ({ tenant: state.tenant }))(BuildsetsPage) +export default connect((state) => ({ + tenant: state.tenant, + preferences: state.preferences, +}))(BuildsetsPage) diff --git a/web/src/pages/ConfigErrors.jsx b/web/src/pages/ConfigErrors.jsx index b7a85d074..b43ebf562 100644 --- a/web/src/pages/ConfigErrors.jsx +++ b/web/src/pages/ConfigErrors.jsx @@ -18,7 +18,12 @@ import { connect } from 'react-redux' import { Icon } from 'patternfly-react' -import { PageSection, PageSectionVariants } from '@patternfly/react-core' +import { + PageSection, + PageSectionVariants, + List, + ListItem, +} from '@patternfly/react-core' import { fetchConfigErrorsAction } from '../actions/configErrors' @@ -26,7 +31,8 @@ class ConfigErrorsPage extends React.Component { static propTypes = { configErrors: PropTypes.object, tenant: PropTypes.object, - dispatch: PropTypes.func + dispatch: PropTypes.func, + preferences: PropTypes.object, } updateData = () => { @@ -36,7 +42,7 @@ class ConfigErrorsPage extends React.Component { render () { const { configErrors } = this.props return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <div className="pull-right"> {/* Lint warning jsx-a11y/anchor-is-valid */} {/* eslint-disable-next-line */} @@ -45,22 +51,22 @@ class ConfigErrorsPage extends React.Component { </a> </div> <div className="pull-left"> - <ul className="list-group"> + <List isPlain isBordered> {configErrors.map((item, idx) => { let ctxPath = item.source_context.path if (item.source_context.branch !== 'master') { ctxPath += ' (' + item.source_context.branch + ')' } return ( - <li className="list-group-item" key={idx}> + <ListItem key={idx}> <h3>{item.source_context.project} - {ctxPath}</h3> <p style={{whiteSpace: 'pre-wrap'}}> {item.error} </p> - </li> + </ListItem> ) })} - </ul> + </List> </div> </PageSection> ) @@ -69,5 +75,6 @@ class ConfigErrorsPage extends React.Component { export default connect(state => ({ tenant: state.tenant, - configErrors: state.configErrors.errors + configErrors: state.configErrors.errors, + preferences: state.preferences, }))(ConfigErrorsPage) diff --git a/web/src/pages/FreezeJob.jsx b/web/src/pages/FreezeJob.jsx index a78cc7b56..491a9a1ee 100644 --- a/web/src/pages/FreezeJob.jsx +++ b/web/src/pages/FreezeJob.jsx @@ -97,7 +97,8 @@ function FreezeJobPage(props) { collapsed={false} sortKeys={true} enableClipboard={false} - displayDataTypes={false}/> + displayDataTypes={false} + theme={props.preferences.darkMode ? 'tomorrow' : 'rjv-default'}/> </span> ) } @@ -133,12 +134,14 @@ FreezeJobPage.propTypes = { fetchFreezeJobIfNeeded: PropTypes.func, tenant: PropTypes.object, freezejob: PropTypes.object, + preferences: PropTypes.object, } function mapStateToProps(state) { return { tenant: state.tenant, freezejob: state.freezejob, + preferences: state.preferences, } } diff --git a/web/src/pages/Job.jsx b/web/src/pages/Job.jsx index efb4cfddc..f29ea383d 100644 --- a/web/src/pages/Job.jsx +++ b/web/src/pages/Job.jsx @@ -26,7 +26,8 @@ class JobPage extends React.Component { match: PropTypes.object.isRequired, tenant: PropTypes.object, remoteData: PropTypes.object, - dispatch: PropTypes.func + dispatch: PropTypes.func, + preferences: PropTypes.object, } updateData = (force) => { @@ -53,7 +54,7 @@ class JobPage extends React.Component { const tenantJobs = remoteData.jobs[this.props.tenant.name] const jobName = this.props.match.params.jobName return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode? PageSectionVariants.dark : PageSectionVariants.light}> {tenantJobs && tenantJobs[jobName] && <Job job={tenantJobs[jobName]} />} </PageSection> ) @@ -63,4 +64,5 @@ class JobPage extends React.Component { export default connect(state => ({ tenant: state.tenant, remoteData: state.job, + preferences: state.preferences, }))(JobPage) diff --git a/web/src/pages/Jobs.jsx b/web/src/pages/Jobs.jsx index d0b6a1466..6484b6f48 100644 --- a/web/src/pages/Jobs.jsx +++ b/web/src/pages/Jobs.jsx @@ -26,7 +26,8 @@ class JobsPage extends React.Component { static propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, - dispatch: PropTypes.func + dispatch: PropTypes.func, + preferences: PropTypes.object, } updateData = (force) => { @@ -51,8 +52,8 @@ class JobsPage extends React.Component { const jobs = remoteData.jobs[this.props.tenant.name] return ( - <PageSection variant={PageSectionVariants.light}> - <PageSection style={{paddingRight: '5px'}}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> + <PageSection variant={PageSectionVariants.light} style={{paddingRight: '5px'}}> <Fetchable isFetching={remoteData.isFetching} fetchCallback={this.updateData} @@ -70,4 +71,5 @@ class JobsPage extends React.Component { export default connect(state => ({ tenant: state.tenant, remoteData: state.jobs, + preferences: state.preferences, }))(JobsPage) diff --git a/web/src/pages/Labels.jsx b/web/src/pages/Labels.jsx index 10decaa73..1c6e50db5 100644 --- a/web/src/pages/Labels.jsx +++ b/web/src/pages/Labels.jsx @@ -15,8 +15,15 @@ import * as React from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' -import { Table } from 'patternfly-react' import { PageSection, PageSectionVariants } from '@patternfly/react-core' +import { + Table, + TableVariant, + TableHeader, + TableBody, +} from '@patternfly/react-table' +import { TagIcon } from '@patternfly/react-icons' +import { IconProperty } from '../Misc' import { fetchLabelsIfNeeded } from '../actions/labels' import { Fetchable, Fetching } from '../containers/Fetching' @@ -54,18 +61,22 @@ class LabelsPage extends React.Component { return <Fetching /> } - const headerFormat = value => <Table.Heading>{value}</Table.Heading> - const cellFormat = value => <Table.Cell>{value}</Table.Cell> - const columns = [] - const myColumns = ['name'] - myColumns.forEach(column => { - let formatter = cellFormat - let prop = column - columns.push({ - header: {label: column, formatters: [headerFormat]}, - property: prop, - cell: {formatters: [formatter]} - }) + const columns = [ + { + title: ( + <IconProperty icon={<TagIcon />} value="Name" /> + ), + dataLabel: 'name' + } + ] + let rows = [] + labels.forEach((label) => { + let r = { + cells: [ + {title: label.name, props: {column: 'Name'}} + ], + } + rows.push(r) }) return ( <PageSection variant={PageSectionVariants.light}> @@ -75,18 +86,16 @@ class LabelsPage extends React.Component { fetchCallback={this.updateData} /> </PageSection> - <Table.PfProvider - striped - bordered - hover - columns={columns} + <Table + aria-label="Labels Table" + variant={TableVariant.compact} + cells={columns} + rows={rows} + className="zuul-table" > - <Table.Header/> - <Table.Body - rows={labels} - rowKey="name" - /> - </Table.PfProvider> + <TableHeader /> + <TableBody /> + </Table> </PageSection> ) } diff --git a/web/src/pages/Nodes.jsx b/web/src/pages/Nodes.jsx index a7081ac59..b13878087 100644 --- a/web/src/pages/Nodes.jsx +++ b/web/src/pages/Nodes.jsx @@ -15,9 +15,29 @@ import * as React from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' -import { Table } from 'patternfly-react' +import { + Table, + TableVariant, + TableHeader, + TableBody, +} from '@patternfly/react-table' import * as moment from 'moment' -import { PageSection, PageSectionVariants } from '@patternfly/react-core' +import { + PageSection, + PageSectionVariants, + ClipboardCopy, +} from '@patternfly/react-core' +import { + BuildIcon, + ClusterIcon, + ConnectedIcon, + OutlinedCalendarAltIcon, + TagIcon, + RunningIcon, + PencilAltIcon, + ZoneIcon, +} from '@patternfly/react-icons' +import { IconProperty } from '../Misc' import { fetchNodesIfNeeded } from '../actions/nodes' import { Fetchable } from '../containers/Fetching' @@ -51,43 +71,69 @@ class NodesPage extends React.Component { const { remoteData } = this.props const nodes = remoteData.nodes - const headerFormat = value => <Table.Heading>{value}</Table.Heading> - const cellFormat = value => <Table.Cell>{value}</Table.Cell> - const cellLabelsFormat = value => <Table.Cell>{value.join(',')}</Table.Cell> - const cellPreFormat = value => ( - <Table.Cell style={{fontFamily: 'Menlo,Monaco,Consolas,monospace'}}> - {value} - </Table.Cell>) - const cellAgeFormat = value => ( - <Table.Cell style={{fontFamily: 'Menlo,Monaco,Consolas,monospace'}}> - {moment.unix(value).fromNow()} - </Table.Cell>) - - const columns = [] - const myColumns = [ - 'id', 'labels', 'connection', 'server', 'provider', 'state', - 'age', 'comment' - ] - myColumns.forEach(column => { - let formatter = cellFormat - let prop = column - if (column === 'labels') { - prop = 'type' - formatter = cellLabelsFormat - } else if (column === 'connection') { - prop = 'connection_type' - } else if (column === 'server') { - prop = 'external_id' - formatter = cellPreFormat - } else if (column === 'age') { - prop = 'state_time' - formatter = cellAgeFormat + const columns = [ + { + title: ( + <IconProperty icon={<BuildIcon />} value="ID" /> + ), + dataLabel: 'id', + }, + { + title: ( + <IconProperty icon={<TagIcon />} value="Labels" /> + ), + dataLabel: 'labels', + }, + { + title: ( + <IconProperty icon={<ConnectedIcon />} value="Connection" /> + ), + dataLabel: 'connection', + }, + { + title: ( + <IconProperty icon={<ClusterIcon />} value="Server" /> + ), + dataLabel: 'server', + }, + { + title: ( + <IconProperty icon={<ZoneIcon />} value="Provider" /> + ), + dataLabel: 'provider', + }, + { + title: ( + <IconProperty icon={<RunningIcon />} value="State" /> + ), + dataLabel: 'state', + }, + { + title: ( + <IconProperty icon={<OutlinedCalendarAltIcon />} value="Age" /> + ), + dataLabel: 'age', + }, + { + title: ( + <IconProperty icon={<PencilAltIcon />} value="Comment" /> + ), + dataLabel: 'comment', } - columns.push({ - header: {label: column, formatters: [headerFormat]}, - property: prop, - cell: {formatters: [formatter]} - }) + ] + let rows = [] + nodes.forEach((node) => { + let r = [ + {title: node.id, props: {column: 'ID'}}, + {title: node.type.join(','), props: {column: 'Label' }}, + {title: node.connection_type, props: {column: 'Connection'}}, + {title: <ClipboardCopy hoverTip="Copy" clickTip="Copied" variant="inline-compact">{node.external_id}</ClipboardCopy>, props: {column: 'Server'}}, + {title: node.provider, props: {column: 'Provider'}}, + {title: node.state, props: {column: 'State'}}, + {title: moment.unix(node.state_time).fromNow(), props: {column: 'Age'}}, + {title: node.comment, props: {column: 'Comment'}}, + ] + rows.push({cells: r}) }) return ( <PageSection variant={PageSectionVariants.light}> @@ -97,18 +143,17 @@ class NodesPage extends React.Component { fetchCallback={this.updateData} /> </PageSection> - <Table.PfProvider - striped - bordered - hover - columns={columns} + + <Table + aria-label="Nodes Table" + variant={TableVariant.compact} + cells={columns} + rows={rows} + className="zuul-table" > - <Table.Header/> - <Table.Body - rows={nodes} - rowKey="id" - /> - </Table.PfProvider> + <TableHeader /> + <TableBody /> + </Table> </PageSection> ) } diff --git a/web/src/pages/OpenApi.jsx b/web/src/pages/OpenApi.jsx index 7ccf5f34c..ec8ef8c56 100644 --- a/web/src/pages/OpenApi.jsx +++ b/web/src/pages/OpenApi.jsx @@ -26,7 +26,8 @@ class OpenApiPage extends React.Component { static propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, - dispatch: PropTypes.func + dispatch: PropTypes.func, + preferences: PropTypes.object, } updateData = (force) => { @@ -51,7 +52,7 @@ class OpenApiPage extends React.Component { render() { return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <div id="swaggerContainer" /> </PageSection> ) @@ -61,4 +62,5 @@ class OpenApiPage extends React.Component { export default connect(state => ({ tenant: state.tenant, remoteData: state.openapi, + preferences: state.preferences, }))(OpenApiPage) diff --git a/web/src/pages/Project.jsx b/web/src/pages/Project.jsx index 06e8612c7..0c808ec09 100644 --- a/web/src/pages/Project.jsx +++ b/web/src/pages/Project.jsx @@ -46,7 +46,7 @@ function ProjectPage(props) { return ( <> - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <TextContent> <Text component="h2">Project {projectName}</Text> <Fetchable @@ -70,12 +70,14 @@ ProjectPage.propTypes = { tenant: PropTypes.object, remoteData: PropTypes.object, fetchProjectIfNeeded: PropTypes.func, + preferences: PropTypes.object, } function mapStateToProps(state) { return { tenant: state.tenant, remoteData: state.project, + preferences: state.preferences, } } diff --git a/web/src/pages/Projects.jsx b/web/src/pages/Projects.jsx index 13ee81bd8..598133384 100644 --- a/web/src/pages/Projects.jsx +++ b/web/src/pages/Projects.jsx @@ -16,8 +16,18 @@ import * as React from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' import { Link } from 'react-router-dom' -import { Table } from 'patternfly-react' import { PageSection, PageSectionVariants } from '@patternfly/react-core' +import { + Table, + TableVariant, + TableHeader, + TableBody, +} from '@patternfly/react-table' +import { + CubeIcon, + ConnectedIcon, +} from '@patternfly/react-icons' +import { IconProperty } from '../Misc' import { fetchProjectsIfNeeded } from '../actions/projects' import { Fetchable, Fetching } from '../containers/Fetching' @@ -59,42 +69,35 @@ class ProjectsPage extends React.Component { return <Fetching /> } - const headerFormat = value => <Table.Heading>{value}</Table.Heading> - const cellFormat = (value) => ( - <Table.Cell>{value}</Table.Cell>) - const cellProjectFormat = (value, row) => ( - <Table.Cell> - <Link to={this.props.tenant.linkPrefix + '/project/' + row.rowData.canonical_name}> - {value} - </Link> - </Table.Cell>) - const cellBuildFormat = (value) => ( - <Table.Cell> - <Link to={this.props.tenant.linkPrefix + '/builds?project=' + value}> - builds - </Link> - </Table.Cell>) - const columns = [] - const myColumns = ['name', 'connection', 'type', 'last builds'] - myColumns.forEach(column => { - let formatter = cellFormat - let prop = column - if (column === 'name') { - formatter = cellProjectFormat + const columns = [ + { + title: <IconProperty icon={<CubeIcon />} value="Name" />, + dataLabel: 'name', + }, + { + title: <IconProperty icon={<ConnectedIcon />} value="Connection" />, + dataLabel: 'connection', + }, + { + title: 'Type', + dataLabel: 'type', + }, + { + title: 'Last builds', + dataLabel: 'last-builds', } - if (column === 'connection') { - prop = 'connection_name' + ] + let rows = [] + projects.forEach((project) => { + let r = { + cells: [ + {title: <Link to={this.props.tenant.linkPrefix + '/project/' + project.canonical_name}>{project.name}</Link>, props: {column: 'Name'}}, + {title: project.connection_name, props: {column: 'Connection'}}, + {title: project.type, props: {column: 'Type'}}, + {title: <Link to={this.props.tenant.linkPrefix + '/builds?project=' + project.name}>Builds</Link>, props: {column: 'Last builds'}}, + ] } - if (column === 'last builds') { - prop = 'name' - formatter = cellBuildFormat - } - columns.push({ - header: {label: column, - formatters: [headerFormat]}, - property: prop, - cell: {formatters: [formatter]} - }) + rows.push(r) }) return ( <PageSection variant={PageSectionVariants.light}> @@ -104,18 +107,16 @@ class ProjectsPage extends React.Component { fetchCallback={this.updateData} /> </PageSection> - <Table.PfProvider - striped - bordered - hover - columns={columns} + <Table + aria-label="Projects Table" + variant={TableVariant.compact} + cells={columns} + rows={rows} + className="zuul-table" > - <Table.Header/> - <Table.Body - rows={projects} - rowKey="name" - /> - </Table.PfProvider> + <TableHeader /> + <TableBody /> + </Table> </PageSection> ) } diff --git a/web/src/pages/Semaphore.jsx b/web/src/pages/Semaphore.jsx index a0ae8ddca..2cfdfb73f 100644 --- a/web/src/pages/Semaphore.jsx +++ b/web/src/pages/Semaphore.jsx @@ -25,7 +25,7 @@ import { PageSection, PageSectionVariants } from '@patternfly/react-core' import { fetchSemaphoresIfNeeded } from '../actions/semaphores' import Semaphore from '../containers/semaphore/Semaphore' -function SemaphorePage({ match, semaphores, tenant, fetchSemaphoresIfNeeded, isFetching }) { +function SemaphorePage({ match, semaphores, tenant, fetchSemaphoresIfNeeded, isFetching, preferences }) { const semaphoreName = match.params.semaphoreName @@ -38,7 +38,7 @@ function SemaphorePage({ match, semaphores, tenant, fetchSemaphoresIfNeeded, isF e => e.name === semaphoreName) : undefined return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Title headingLevel="h2"> Details for Semaphore <span style={{color: 'var(--pf-global--primary-color--100)'}}>{semaphoreName}</span> </Title> @@ -55,6 +55,7 @@ SemaphorePage.propTypes = { tenant: PropTypes.object.isRequired, isFetching: PropTypes.bool.isRequired, fetchSemaphoresIfNeeded: PropTypes.func.isRequired, + preferences: PropTypes.object, } const mapDispatchToProps = { fetchSemaphoresIfNeeded } @@ -63,6 +64,7 @@ function mapStateToProps(state) { tenant: state.tenant, semaphores: state.semaphores.semaphores, isFetching: state.semaphores.isFetching, + preferences: state.preferences, } } diff --git a/web/src/pages/Status.jsx b/web/src/pages/Status.jsx index ac3dc7840..e61ceb3e2 100644 --- a/web/src/pages/Status.jsx +++ b/web/src/pages/Status.jsx @@ -197,6 +197,7 @@ class StatusPage extends React.Component { <FormGroup controlId='status'> <FormControl type='text' + className="pf-c-form-control" placeholder='change or project name' defaultValue={filter} inputRef={i => this.filter = i} @@ -222,7 +223,7 @@ class StatusPage extends React.Component { </Form> ) return ( - <PageSection variant={PageSectionVariants.light}> + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <div style={{display: 'flex', float: 'right'}}> <Fetchable isFetching={remoteData.isFetching} diff --git a/web/src/pages/Stream.jsx b/web/src/pages/Stream.jsx index df2a2ad96..f058d29a2 100644 --- a/web/src/pages/Stream.jsx +++ b/web/src/pages/Stream.jsx @@ -31,7 +31,8 @@ class StreamPage extends React.Component { static propTypes = { match: PropTypes.object.isRequired, location: PropTypes.object.isRequired, - tenant: PropTypes.object + tenant: PropTypes.object, + preferences: PropTypes.object, } state = { @@ -167,10 +168,11 @@ class StreamPage extends React.Component { render () { return ( - <PageSection variant={PageSectionVariants.light} > + <PageSection variant={this.props.preferences.darkMode ? PageSectionVariants.dark : PageSectionVariants.light}> <Form inline> <FormGroup controlId='stream'> <FormControl + className="pf-c-form-control" type='text' placeholder='search' onKeyPress={this.handleKeyPress} @@ -201,4 +203,7 @@ class StreamPage extends React.Component { } -export default connect(state => ({tenant: state.tenant}))(StreamPage) +export default connect(state => ({ + tenant: state.tenant, + preferences: state.preferences, +}))(StreamPage) diff --git a/web/src/reducers/preferences.js b/web/src/reducers/preferences.js index 1fba8eacf..0a2755257 100644 --- a/web/src/reducers/preferences.js +++ b/web/src/reducers/preferences.js @@ -15,13 +15,14 @@ import { PREFERENCE_SET, } from '../actions/preferences' - +import { resolveDarkMode, setDarkMode } from '../Misc' const stored_prefs = localStorage.getItem('preferences') let default_prefs if (stored_prefs === null) { default_prefs = { - autoReload: true + autoReload: true, + theme: 'Auto' } } else { default_prefs = JSON.parse(stored_prefs) @@ -30,13 +31,15 @@ if (stored_prefs === null) { export default (state = { ...default_prefs }, action) => { - let newstate - switch (action.type) { - case PREFERENCE_SET: - newstate = { ...state, [action.key]: action.value } - localStorage.setItem('preferences', JSON.stringify(newstate)) - return newstate - default: - return state + if (action.type === PREFERENCE_SET) { + let newstate = { ...state, [action.key]: action.value } + delete newstate.darkMode + localStorage.setItem('preferences', JSON.stringify(newstate)) + let darkMode = resolveDarkMode(newstate.theme) + setDarkMode(darkMode) + return { ...newstate, darkMode: darkMode } } + let darkMode = resolveDarkMode(state.theme) + setDarkMode(darkMode) + return { ...state, darkMode: darkMode } } diff --git a/zuul/ansible/base/callback/zuul_stream.py b/zuul/ansible/base/callback/zuul_stream.py index b5c14691b..3f886c797 100644 --- a/zuul/ansible/base/callback/zuul_stream.py +++ b/zuul/ansible/base/callback/zuul_stream.py @@ -44,6 +44,7 @@ import time from ansible.plugins.callback import default from ansible.module_utils._text import to_text +from ansible.module_utils.parsing.convert_bool import boolean from zuul.ansible import paths from zuul.ansible import logconfig @@ -333,6 +334,10 @@ class CallbackModule(default.CallbackModule): if (ip in ('localhost', '127.0.0.1')): # Don't try to stream from localhost continue + if boolean(play_vars[host].get( + 'zuul_console_disabled', False)): + # The user has told us not to even try + continue if play_vars[host].get('ansible_connection') in ('winrm',): # The winrm connections don't support streaming for now continue diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py index 62e51ac3f..6fa20c7c4 100755 --- a/zuul/cmd/client.py +++ b/zuul/cmd/client.py @@ -540,6 +540,10 @@ class Client(zuul.cmd.ZuulApp): cmd_prune_database.add_argument( '--older-than', help='relative time (e.g., "24h" or "180d")') + cmd_prune_database.add_argument( + '--batch-size', + default=10000, + help='transaction batch size') cmd_prune_database.set_defaults(func=self.prune_database) return parser @@ -1049,7 +1053,7 @@ class Client(zuul.cmd.ZuulApp): cutoff = parse_cutoff(now, args.before, args.older_than) self.configure_connections(source_only=False, require_sql=True) connection = self.connections.getSqlConnection() - connection.deleteBuildsets(cutoff) + connection.deleteBuildsets(cutoff, args.batch_size) sys.exit(0) diff --git a/zuul/configloader.py b/zuul/configloader.py index fe22fe0f8..4f472cb4e 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -2151,21 +2151,26 @@ class TenantParser(object): for future in futures: future.result() - try: - self._processCatJobs(abide, tenant, loading_errors, jobs, - min_ltimes) - except Exception: - self.log.exception("Error processing cat jobs, canceling") - for job in jobs: + for i, job in enumerate(jobs, start=1): + try: try: - self.log.debug("Canceling cat job %s", job) + self._processCatJob(abide, tenant, loading_errors, job, + min_ltimes) + except TimeoutError: self.merger.cancel(job) - except Exception: - self.log.exception("Unable to cancel job %s", job) - if not ignore_cat_exception: - raise - if not ignore_cat_exception: - raise + raise + except Exception: + self.log.exception("Error processing cat job") + if not ignore_cat_exception: + # Cancel remaining jobs + for cancel_job in jobs[i:]: + self.log.debug("Canceling cat job %s", cancel_job) + try: + self.merger.cancel(cancel_job) + except Exception: + self.log.exception( + "Unable to cancel job %s", cancel_job) + raise def _cacheTenantYAMLBranch(self, abide, tenant, loading_errors, min_ltimes, tpc, project, branch, jobs): @@ -2234,49 +2239,48 @@ class TenantParser(object): job.source_context = source_context jobs.append(job) - def _processCatJobs(self, abide, tenant, loading_errors, jobs, min_ltimes): + def _processCatJob(self, abide, tenant, loading_errors, job, min_ltimes): # Called at the end of _cacheTenantYAML after all cat jobs # have been submitted - for job in jobs: - self.log.debug("Waiting for cat job %s" % (job,)) - res = job.wait(self.merger.git_timeout) - if not res: - # We timed out - raise Exception("Cat job %s timed out; consider setting " - "merger.git_timeout in zuul.conf" % (job,)) - if not job.updated: - raise Exception("Cat job %s failed" % (job,)) - self.log.debug("Cat job %s got files %s" % - (job, job.files.keys())) - - self._updateUnparsedBranchCache(abide, tenant, job.source_context, - job.files, loading_errors, - job.ltime, min_ltimes) - - # Save all config files in Zookeeper (not just for the current tpc) - files_cache = self.unparsed_config_cache.getFilesCache( - job.source_context.project_canonical_name, - job.source_context.branch) - with self.unparsed_config_cache.writeLock( - job.source_context.project_canonical_name): - # Prevent files cache ltime from going backward - if files_cache.ltime >= job.ltime: - self.log.info( - "Discarding job %s result since the files cache was " - "updated in the meantime", job) - continue - # Since the cat job returns all required config files - # for ALL tenants the project is a part of, we can - # clear the whole cache and then populate it with the - # updated content. - files_cache.clear() - for fn, content in job.files.items(): - # Cache file in Zookeeper - if content is not None: - files_cache[fn] = content - files_cache.setValidFor(job.extra_config_files, - job.extra_config_dirs, - job.ltime) + self.log.debug("Waiting for cat job %s" % (job,)) + res = job.wait(self.merger.git_timeout) + if not res: + # We timed out + raise TimeoutError(f"Cat job {job} timed out; consider setting " + "merger.git_timeout in zuul.conf") + if not job.updated: + raise Exception("Cat job %s failed" % (job,)) + self.log.debug("Cat job %s got files %s" % + (job, job.files.keys())) + + self._updateUnparsedBranchCache(abide, tenant, job.source_context, + job.files, loading_errors, + job.ltime, min_ltimes) + + # Save all config files in Zookeeper (not just for the current tpc) + files_cache = self.unparsed_config_cache.getFilesCache( + job.source_context.project_canonical_name, + job.source_context.branch) + with self.unparsed_config_cache.writeLock( + job.source_context.project_canonical_name): + # Prevent files cache ltime from going backward + if files_cache.ltime >= job.ltime: + self.log.info( + "Discarding job %s result since the files cache was " + "updated in the meantime", job) + return + # Since the cat job returns all required config files + # for ALL tenants the project is a part of, we can + # clear the whole cache and then populate it with the + # updated content. + files_cache.clear() + for fn, content in job.files.items(): + # Cache file in Zookeeper + if content is not None: + files_cache[fn] = content + files_cache.setValidFor(job.extra_config_files, + job.extra_config_dirs, + job.ltime) def _updateUnparsedBranchCache(self, abide, tenant, source_context, files, loading_errors, ltime, min_ltimes): diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index f871671aa..990b7b235 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -1182,9 +1182,34 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): return True if change.wip: return False - if change.missing_labels <= set(allow_needs): - return True - return False + if change.missing_labels > set(allow_needs): + self.log.debug("Unable to merge due to " + "missing labels: %s", change.missing_labels) + return False + for sr in change.submit_requirements: + if sr.get('status') == 'UNSATISFIED': + # Otherwise, we don't care and should skip. + + # We're going to look at each unsatisfied submit + # requirement, and if one of the involved labels is an + # "allow_needs" label, we will assume that Zuul may be + # able to take an action which can cause the + # requirement to be satisfied, and we will ignore it. + # Otherwise, it is likely a requirement that Zuul can + # not alter in which case the requirement should stand + # and block merging. + result = sr.get("submittability_expression_result", {}) + expression = result.get("expression", '') + expr_contains_allow = False + for allow in allow_needs: + if f'label:{allow}' in expression: + expr_contains_allow = True + break + if not expr_contains_allow: + self.log.debug("Unable to merge due to " + "submit requirement: %s", sr) + return False + return True def getProjectOpenChanges(self, project: Project) -> List[GerritChange]: # This is a best-effort function in case Gerrit is unable to return @@ -1443,13 +1468,22 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): return data def queryChangeHTTP(self, number, event=None): - data = self.get('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&' - 'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&' - 'o=DETAILED_LABELS' % (number,)) + query = ('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&' + 'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&' + 'o=DETAILED_LABELS' % (number,)) + if self.version >= (3, 5, 0): + query += '&o=SUBMIT_REQUIREMENTS' + data = self.get(query) related = self.get('changes/%s/revisions/%s/related' % ( number, data['current_revision'])) - files = self.get('changes/%s/revisions/%s/files?parent=1' % ( - number, data['current_revision'])) + + files_query = 'changes/%s/revisions/%s/files' % ( + number, data['current_revision']) + + if data['revisions'][data['current_revision']]['commit']['parents']: + files_query += '?parent=1' + + files = self.get(files_query) return data, related, files def queryChange(self, number, event=None): diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py index 0ac3e7f9d..7b57ec934 100644 --- a/zuul/driver/gerrit/gerritmodel.py +++ b/zuul/driver/gerrit/gerritmodel.py @@ -19,8 +19,8 @@ import urllib.parse import dateutil.parser from zuul.model import EventFilter, RefFilter -from zuul.model import Change, TriggerEvent -from zuul.driver.util import time_to_seconds +from zuul.model import Change, TriggerEvent, FalseWithReason +from zuul.driver.util import time_to_seconds, to_list from zuul import exceptions EMPTY_GIT_REF = '0' * 40 # git sha of all zeros, used during creates/deletes @@ -34,6 +34,7 @@ class GerritChange(Change): self.wip = None self.approvals = [] self.missing_labels = set() + self.submit_requirements = [] self.commit = None self.zuul_query_ltime = None @@ -52,6 +53,7 @@ class GerritChange(Change): "wip": self.wip, "approvals": self.approvals, "missing_labels": list(self.missing_labels), + "submit_requirements": self.submit_requirements, "commit": self.commit, "zuul_query_ltime": self.zuul_query_ltime, }) @@ -64,6 +66,7 @@ class GerritChange(Change): self.wip = data["wip"] self.approvals = data["approvals"] self.missing_labels = set(data["missing_labels"]) + self.submit_requirements = data.get("submit_requirements", []) self.commit = data.get("commit") self.zuul_query_ltime = data.get("zuul_query_ltime") @@ -189,6 +192,7 @@ class GerritChange(Change): if 'approved' in label_data: continue self.missing_labels.add(label_name) + self.submit_requirements = data.get('submit_requirements', []) self.open = data['status'] == 'NEW' self.status = data['status'] self.wip = data.get('work_in_progress', False) @@ -243,110 +247,32 @@ class GerritTriggerEvent(TriggerEvent): return 'change-abandoned' == self.type -class GerritApprovalFilter(object): - def __init__(self, required_approvals=[], reject_approvals=[]): - self._required_approvals = copy.deepcopy(required_approvals) - self.required_approvals = self._tidy_approvals( - self._required_approvals) - self._reject_approvals = copy.deepcopy(reject_approvals) - self.reject_approvals = self._tidy_approvals(self._reject_approvals) - - def _tidy_approvals(self, approvals): - for a in approvals: - for k, v in a.items(): - if k == 'username': - a['username'] = re.compile(v) - elif k == 'email': - a['email'] = re.compile(v) - elif k == 'newer-than': - a[k] = time_to_seconds(v) - elif k == 'older-than': - a[k] = time_to_seconds(v) - return approvals - - def _match_approval_required_approval(self, rapproval, approval): - # Check if the required approval and approval match - if 'description' not in approval: - return False - now = time.time() - by = approval.get('by', {}) - for k, v in rapproval.items(): - if k == 'username': - if (not v.search(by.get('username', ''))): - return False - elif k == 'email': - if (not v.search(by.get('email', ''))): - return False - elif k == 'newer-than': - t = now - v - if (approval['grantedOn'] < t): - return False - elif k == 'older-than': - t = now - v - if (approval['grantedOn'] >= t): - return False - else: - if not isinstance(v, list): - v = [v] - if (approval['description'] != k or - int(approval['value']) not in v): - return False - return True - - def matchesApprovals(self, change): - if self.required_approvals or self.reject_approvals: - if not hasattr(change, 'number'): - # Not a change, no reviews - return False - if self.required_approvals and not change.approvals: - # A change with no approvals can not match - return False - - # TODO(jhesketh): If we wanted to optimise this slightly we could - # analyse both the REQUIRE and REJECT filters by looping over the - # approvals on the change and keeping track of what we have checked - # rather than needing to loop on the change approvals twice - return (self.matchesRequiredApprovals(change) and - self.matchesNoRejectApprovals(change)) - - def matchesRequiredApprovals(self, change): - # Check if any approvals match the requirements - for rapproval in self.required_approvals: - matches_rapproval = False - for approval in change.approvals: - if self._match_approval_required_approval(rapproval, approval): - # We have a matching approval so this requirement is - # fulfilled - matches_rapproval = True - break - if not matches_rapproval: - return False - return True - - def matchesNoRejectApprovals(self, change): - # Check to make sure no approvals match a reject criteria - for rapproval in self.reject_approvals: - for approval in change.approvals: - if self._match_approval_required_approval(rapproval, approval): - # A reject approval has been matched, so we reject - # immediately - return False - # To get here no rejects can have been matched so we should be good to - # queue - return True - - -class GerritEventFilter(EventFilter, GerritApprovalFilter): +class GerritEventFilter(EventFilter): def __init__(self, connection_name, trigger, types=[], branches=[], refs=[], event_approvals={}, comments=[], emails=[], usernames=[], required_approvals=[], reject_approvals=[], - uuid=None, scheme=None, ignore_deletes=True): + uuid=None, scheme=None, ignore_deletes=True, + require=None, reject=None): EventFilter.__init__(self, connection_name, trigger) - GerritApprovalFilter.__init__(self, - required_approvals=required_approvals, - reject_approvals=reject_approvals) + # TODO: Backwards compat, remove after 9.x: + if required_approvals and require is None: + require = {'approval': required_approvals} + if reject_approvals and reject is None: + reject = {'approval': reject_approvals} + + if require: + self.require_filter = GerritRefFilter.requiresFromConfig( + connection_name, require) + else: + self.require_filter = None + + if reject: + self.reject_filter = GerritRefFilter.rejectFromConfig( + connection_name, reject) + else: + self.reject_filter = None self._types = types self._branches = branches @@ -384,18 +310,16 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): if self.event_approvals: ret += ' event_approvals: %s' % ', '.join( ['%s:%s' % a for a in self.event_approvals.items()]) - if self.required_approvals: - ret += ' required_approvals: %s' % ', '.join( - ['%s' % a for a in self._required_approvals]) - if self.reject_approvals: - ret += ' reject_approvals: %s' % ', '.join( - ['%s' % a for a in self._reject_approvals]) if self._comments: ret += ' comments: %s' % ', '.join(self._comments) if self._emails: ret += ' emails: %s' % ', '.join(self._emails) if self._usernames: ret += ' usernames: %s' % ', '.join(self._usernames) + if self.require_filter: + ret += ' require: %s' % repr(self.require_filter) + if self.reject_filter: + ret += ' reject: %s' % repr(self.reject_filter) ret += '>' return ret @@ -410,7 +334,8 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): if etype.match(event.type): matches_type = True if self.types and not matches_type: - return False + return FalseWithReason("Types %s do not match %s" % ( + self.types, event.type)) if event.type == 'pending-check': if self.uuid and event.uuid != self.uuid: @@ -424,7 +349,8 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): if branch.match(event.branch): matches_branch = True if self.branches and not matches_branch: - return False + return FalseWithReason("Branches %s do not match %s" % ( + self.branches, event.branch)) # refs are ORed matches_ref = False @@ -433,11 +359,12 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): if ref.match(event.ref): matches_ref = True if self.refs and not matches_ref: - return False + return FalseWithReason( + "Refs %s do not match %s" % (self.refs, event.ref)) if self.ignore_deletes and event.newrev == EMPTY_GIT_REF: # If the updated ref has an empty git sha (all 0s), # then the ref is being deleted - return False + return FalseWithReason("Ref deletion events are ignored") # comments are ORed matches_comment_re = False @@ -451,7 +378,8 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): comment_re.search(comment)): matches_comment_re = True if self.comments and not matches_comment_re: - return False + return FalseWithReason("Comments %s do not match %s" % ( + self.comments, event.patchsetcomments)) # We better have an account provided by Gerrit to do # email filtering. @@ -464,7 +392,8 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): email_re.search(account_email)): matches_email_re = True if self.emails and not matches_email_re: - return False + return FalseWithReason("Username %s does not match %s" % ( + self.emails, account_email)) # usernames are ORed account_username = event.account.get('username') @@ -474,7 +403,8 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): username_re.search(account_username)): matches_username_re = True if self.usernames and not matches_username_re: - return False + return FalseWithReason("Username %s does not match %s" % ( + self.usernames, account_username)) # approvals are ANDed for category, value in self.event_approvals.items(): @@ -484,29 +414,73 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): int(eapp['value']) == int(value)): matches_approval = True if not matches_approval: - return False + return FalseWithReason("Approvals %s do not match %s" % ( + self.event_approvals, event.approvals)) - # required approvals are ANDed (reject approvals are ORed) - if not self.matchesApprovals(change): - return False + if self.require_filter: + require_filter_result = self.require_filter.matches(change) + if not require_filter_result: + return require_filter_result + + if self.reject_filter: + reject_filter_result = self.reject_filter.matches(change) + if not reject_filter_result: + return reject_filter_result return True -class GerritRefFilter(RefFilter, GerritApprovalFilter): - def __init__(self, connection_name, open=None, current_patchset=None, - wip=None, statuses=[], required_approvals=[], - reject_approvals=[]): +class GerritRefFilter(RefFilter): + def __init__(self, connection_name, + open=None, reject_open=None, + current_patchset=None, reject_current_patchset=None, + wip=None, reject_wip=None, + statuses=[], reject_statuses=[], + required_approvals=[], reject_approvals=[]): RefFilter.__init__(self, connection_name) - GerritApprovalFilter.__init__(self, - required_approvals=required_approvals, - reject_approvals=reject_approvals) - - self.open = open - self.wip = wip - self.current_patchset = current_patchset + self._required_approvals = copy.deepcopy(required_approvals) + self.required_approvals = self._tidy_approvals( + self._required_approvals) + self._reject_approvals = copy.deepcopy(reject_approvals) + self.reject_approvals = self._tidy_approvals(self._reject_approvals) self.statuses = statuses + self.reject_statuses = reject_statuses + + if reject_open is not None: + self.open = not reject_open + else: + self.open = open + if reject_wip is not None: + self.wip = not reject_wip + else: + self.wip = wip + if reject_current_patchset is not None: + self.current_patchset = not reject_current_patchset + else: + self.current_patchset = current_patchset + + @classmethod + def requiresFromConfig(cls, connection_name, config): + return cls( + connection_name=connection_name, + open=config.get('open'), + current_patchset=config.get('current-patchset'), + wip=config.get('wip'), + statuses=to_list(config.get('status')), + required_approvals=to_list(config.get('approval')), + ) + + @classmethod + def rejectFromConfig(cls, connection_name, config): + return cls( + connection_name=connection_name, + reject_open=config.get('open'), + reject_current_patchset=config.get('current-patchset'), + reject_wip=config.get('wip'), + reject_statuses=to_list(config.get('status')), + reject_approvals=to_list(config.get('approval')), + ) def __repr__(self): ret = '<GerritRefFilter' @@ -514,10 +488,14 @@ class GerritRefFilter(RefFilter, GerritApprovalFilter): ret += ' connection_name: %s' % self.connection_name if self.open is not None: ret += ' open: %s' % self.open + if self.wip is not None: + ret += ' wip: %s' % self.wip if self.current_patchset is not None: ret += ' current-patchset: %s' % self.current_patchset if self.statuses: ret += ' statuses: %s' % ', '.join(self.statuses) + if self.reject_statuses: + ret += ' reject-statuses: %s' % ', '.join(self.reject_statuses) if self.required_approvals: ret += (' required-approvals: %s' % str(self.required_approvals)) @@ -529,37 +507,138 @@ class GerritRefFilter(RefFilter, GerritApprovalFilter): return ret def matches(self, change): + if self.open is not None: + # if a "change" has no number, it's not a change, but a push + # and cannot possibly pass this test. + if hasattr(change, 'number'): + if self.open != change.open: + return FalseWithReason( + "Change does not match open requirement") + else: + return FalseWithReason("Ref is not a Change") - filters = [ - { - "required": self.open, - "value": change.open - }, - { - "required": self.current_patchset, - "value": change.is_current_patchset - }, - { - "required": self.wip, - "value": change.wip - }, - ] - configured = filter(lambda x: x["required"] is not None, filters) - - # if a "change" has no number, it's not a change, but a push - # and cannot possibly pass this test. - if hasattr(change, 'number'): - if any(map(lambda x: x["required"] != x["value"], configured)): - return False - elif configured: - return False + if self.current_patchset is not None: + # if a "change" has no number, it's not a change, but a push + # and cannot possibly pass this test. + if hasattr(change, 'number'): + if self.current_patchset != change.is_current_patchset: + return FalseWithReason( + "Change does not match current patchset requirement") + else: + return FalseWithReason("Ref is not a Change") + + if self.wip is not None: + # if a "change" has no number, it's not a change, but a push + # and cannot possibly pass this test. + if hasattr(change, 'number'): + if self.wip != change.wip: + return FalseWithReason( + "Change does not match WIP requirement") + else: + return FalseWithReason("Ref is not a Change") if self.statuses: if change.status not in self.statuses: - return False + return FalseWithReason( + "Required statuses %s do not match %s" % ( + self.statuses, change.status)) + if self.reject_statuses: + if change.status in self.reject_statuses: + return FalseWithReason( + "Reject statuses %s match %s" % ( + self.reject_statuses, change.status)) # required approvals are ANDed (reject approvals are ORed) - if not self.matchesApprovals(change): + matches_approvals_result = self.matchesApprovals(change) + if not matches_approvals_result: + return matches_approvals_result + + return True + + def _tidy_approvals(self, approvals): + for a in approvals: + for k, v in a.items(): + if k == 'username': + a['username'] = re.compile(v) + elif k == 'email': + a['email'] = re.compile(v) + elif k == 'newer-than': + a[k] = time_to_seconds(v) + elif k == 'older-than': + a[k] = time_to_seconds(v) + return approvals + + def _match_approval_required_approval(self, rapproval, approval): + # Check if the required approval and approval match + if 'description' not in approval: return False + now = time.time() + by = approval.get('by', {}) + for k, v in rapproval.items(): + if k == 'username': + if (not v.search(by.get('username', ''))): + return False + elif k == 'email': + if (not v.search(by.get('email', ''))): + return False + elif k == 'newer-than': + t = now - v + if (approval['grantedOn'] < t): + return False + elif k == 'older-than': + t = now - v + if (approval['grantedOn'] >= t): + return False + else: + if not isinstance(v, list): + v = [v] + if (approval['description'] != k or + int(approval['value']) not in v): + return False + return True + def matchesApprovals(self, change): + if self.required_approvals or self.reject_approvals: + if not hasattr(change, 'number'): + # Not a change, no reviews + return FalseWithReason("Ref is not a Change") + if self.required_approvals and not change.approvals: + # A change with no approvals can not match + return FalseWithReason("Approvals %s does not match %s" % ( + self.required_approvals, change.approvals)) + + # TODO(jhesketh): If we wanted to optimise this slightly we could + # analyse both the REQUIRE and REJECT filters by looping over the + # approvals on the change and keeping track of what we have checked + # rather than needing to loop on the change approvals twice + return (self.matchesRequiredApprovals(change) and + self.matchesNoRejectApprovals(change)) + + def matchesRequiredApprovals(self, change): + # Check if any approvals match the requirements + for rapproval in self.required_approvals: + matches_rapproval = False + for approval in change.approvals: + if self._match_approval_required_approval(rapproval, approval): + # We have a matching approval so this requirement is + # fulfilled + matches_rapproval = True + break + if not matches_rapproval: + return FalseWithReason( + "Required approvals %s do not match %s" % ( + self.required_approvals, change.approvals)) + return True + + def matchesNoRejectApprovals(self, change): + # Check to make sure no approvals match a reject criteria + for rapproval in self.reject_approvals: + for approval in change.approvals: + if self._match_approval_required_approval(rapproval, approval): + # A reject approval has been matched, so we reject + # immediately + return FalseWithReason("Reject approvals %s match %s" % ( + self.reject_approvals, change.approvals)) + # To get here no rejects can have been matched so we should be good to + # queue return True diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index f42e93254..4e7a32b83 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -20,7 +20,7 @@ from urllib.parse import urlparse from zuul.source import BaseSource from zuul.model import Project from zuul.driver.gerrit.gerritmodel import GerritRefFilter -from zuul.driver.util import scalar_or_list, to_list +from zuul.driver.util import scalar_or_list from zuul.lib.dependson import find_dependency_headers from zuul.zk.change_cache import ChangeKey @@ -164,11 +164,14 @@ class GerritSource(BaseSource): change = self.connection._getChange(change_key) changes[change_key] = change - for change in changes.values(): - for git_key in change.git_needs_changes: - if git_key in changes: + # Convert to list here because the recursive call can mutate + # the set. + for change in list(changes.values()): + for git_change_ref in change.git_needs_changes: + change_key = ChangeKey.fromReference(git_change_ref) + if change_key in changes: continue - git_change = self.getChange(git_key) + git_change = self.getChange(change_key) if not git_change.topic or git_change.topic == topic: continue self.getChangesByTopic(git_change.topic, changes) @@ -206,21 +209,15 @@ class GerritSource(BaseSource): return self.connection._getGitwebUrl(project, sha) def getRequireFilters(self, config): - f = GerritRefFilter( - connection_name=self.connection.connection_name, - open=config.get('open'), - current_patchset=config.get('current-patchset'), - wip=config.get('wip'), - statuses=to_list(config.get('status')), - required_approvals=to_list(config.get('approval')), - ) + f = GerritRefFilter.requiresFromConfig( + self.connection.connection_name, + config) return [f] def getRejectFilters(self, config): - f = GerritRefFilter( - connection_name=self.connection.connection_name, - reject_approvals=to_list(config.get('approval')), - ) + f = GerritRefFilter.rejectFromConfig( + self.connection.connection_name, + config) return [f] def getRefForChange(self, change): @@ -244,11 +241,13 @@ def getRequireSchema(): 'current-patchset': bool, 'wip': bool, 'status': scalar_or_list(str)} - return require def getRejectSchema(): - reject = {'approval': scalar_or_list(approval)} - + reject = {'approval': scalar_or_list(approval), + 'open': bool, + 'current-patchset': bool, + 'wip': bool, + 'status': scalar_or_list(str)} return reject diff --git a/zuul/driver/gerrit/gerrittrigger.py b/zuul/driver/gerrit/gerrittrigger.py index dc8a7db68..dff5dc32c 100644 --- a/zuul/driver/gerrit/gerrittrigger.py +++ b/zuul/driver/gerrit/gerrittrigger.py @@ -16,6 +16,7 @@ import logging import voluptuous as v from zuul.trigger import BaseTrigger from zuul.driver.gerrit.gerritmodel import GerritEventFilter +from zuul.driver.gerrit import gerritsource from zuul.driver.util import scalar_or_list, to_list @@ -59,7 +60,9 @@ class GerritTrigger(BaseTrigger): ), uuid=trigger.get('uuid'), scheme=trigger.get('scheme'), - ignore_deletes=ignore_deletes + ignore_deletes=ignore_deletes, + require=trigger.get('require'), + reject=trigger.get('reject'), ) efilters.append(f) @@ -101,6 +104,8 @@ def getSchema(): 'approval': scalar_or_list(variable_dict), 'require-approval': scalar_or_list(approval), 'reject-approval': scalar_or_list(approval), + 'require': gerritsource.getRequireSchema(), + 'reject': gerritsource.getRejectSchema(), } return gerrit_trigger diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index cffbd6769..a1353cb4d 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -81,6 +81,10 @@ ANNOTATION_LEVELS = { "warning": "warning", "error": "failure", } +# The maximum size for the 'message' field is 64 KB. Since it's unclear +# from the Github docs if the unit is KiB or KB we'll use KB to be on +# the safe side. +ANNOTATION_MAX_MESSAGE_SIZE = 64 * 1000 EventTuple = collections.namedtuple( "EventTuple", [ @@ -403,7 +407,8 @@ class GithubEventProcessor(object): # Returns empty on unhandled events return - self.log.debug("Handling %s event", self.event_type) + self.log.debug("Handling %s event with installation id %s", + self.event_type, installation_id) events = [] try: events = method() @@ -679,6 +684,11 @@ class GithubEventProcessor(object): branch, project_name) events.append( self._branch_protection_rule_to_event(project_name, branch)) + + for event in events: + # Make sure every event has a branch cache ltime + self.connection.clearConnectionCacheOnBranchEvent(event) + return events def _branch_protection_rule_to_event(self, project_name, branch): @@ -2432,7 +2442,9 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): raw_annotation = { "path": fn, "annotation_level": annotation_level, - "message": comment["message"], + "message": comment["message"].encode( + "utf8")[:ANNOTATION_MAX_MESSAGE_SIZE].decode( + "utf8", "ignore"), "start_line": start_line, "end_line": end_line, "start_column": start_column, diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 30610cf4e..71238d070 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -21,7 +21,7 @@ import time from zuul.model import Change, TriggerEvent, EventFilter, RefFilter from zuul.model import FalseWithReason -from zuul.driver.util import time_to_seconds +from zuul.driver.util import time_to_seconds, to_list EMPTY_GIT_REF = '0' * 40 # git sha of all zeros, used during creates/deletes @@ -170,149 +170,31 @@ class GithubTriggerEvent(TriggerEvent): return ' '.join(r) -class GithubCommonFilter(object): - def __init__(self, required_reviews=[], required_statuses=[], - reject_reviews=[], reject_statuses=[]): - self._required_reviews = copy.deepcopy(required_reviews) - self._reject_reviews = copy.deepcopy(reject_reviews) - self.required_reviews = self._tidy_reviews(self._required_reviews) - self.reject_reviews = self._tidy_reviews(self._reject_reviews) - self.required_statuses = required_statuses - self.reject_statuses = reject_statuses - - def _tidy_reviews(self, reviews): - for r in reviews: - for k, v in r.items(): - if k == 'username': - r['username'] = re.compile(v) - elif k == 'email': - r['email'] = re.compile(v) - elif k == 'newer-than': - r[k] = time_to_seconds(v) - elif k == 'older-than': - r[k] = time_to_seconds(v) - return reviews - - def _match_review_required_review(self, rreview, review): - # Check if the required review and review match - now = time.time() - by = review.get('by', {}) - for k, v in rreview.items(): - if k == 'username': - if (not v.search(by.get('username', ''))): - return False - elif k == 'email': - if (not v.search(by.get('email', ''))): - return False - elif k == 'newer-than': - t = now - v - if (review['grantedOn'] < t): - return False - elif k == 'older-than': - t = now - v - if (review['grantedOn'] >= t): - return False - elif k == 'type': - if review['type'] != v: - return False - elif k == 'permission': - # If permission is read, we've matched. You must have read - # to provide a review. - if v != 'read': - # Admins have implicit write. - if v == 'write': - if review['permission'] not in ('write', 'admin'): - return False - elif v == 'admin': - if review['permission'] != 'admin': - return False - return True - - def matchesReviews(self, change): - if self.required_reviews or self.reject_reviews: - if not hasattr(change, 'number'): - # not a PR, no reviews - return FalseWithReason("Change is not a PR") - if self.required_reviews and not change.reviews: - # No reviews means no matching of required bits - # having reject reviews but no reviews on the change is okay - return FalseWithReason("Reviews %s does not match %s" % ( - self.required_reviews, change.reviews)) - - return (self.matchesRequiredReviews(change) and - self.matchesNoRejectReviews(change)) - - def matchesRequiredReviews(self, change): - for rreview in self.required_reviews: - matches_review = False - for review in change.reviews: - if self._match_review_required_review(rreview, review): - # Consider matched if any review matches - matches_review = True - break - if not matches_review: - return FalseWithReason( - "Required reviews %s does not match %s" % ( - self.required_reviews, change.reviews)) - return True - - def matchesNoRejectReviews(self, change): - for rreview in self.reject_reviews: - for review in change.reviews: - if self._match_review_required_review(rreview, review): - # A review matched, we can reject right away - return FalseWithReason("Reject reviews %s matches %s" % ( - self.reject_reviews, change.reviews)) - return True - - def matchesStatuses(self, change): - if self.required_statuses or self.reject_statuses: - if not hasattr(change, 'number'): - # not a PR, no status - return FalseWithReason("Can't match statuses without PR") - if self.required_statuses and not change.status: - return FalseWithReason( - "Required statuses %s does not match %s" % ( - self.required_statuses, change.status)) - required_statuses_results = self.matchesRequiredStatuses(change) - if not required_statuses_results: - return required_statuses_results - return self.matchesNoRejectStatuses(change) - - def matchesRequiredStatuses(self, change): - # statuses are ORed - # A PR head can have multiple statuses on it. If the change - # statuses and the filter statuses are a null intersection, there - # are no matches and we return false - if self.required_statuses: - for required_status in self.required_statuses: - for status in change.status: - if re2.fullmatch(required_status, status): - return True - return FalseWithReason("RequiredStatuses %s does not match %s" % ( - self.required_statuses, change.status)) - return True - - def matchesNoRejectStatuses(self, change): - # statuses are ANDed - # If any of the rejected statusses are present, we return false - for rstatus in self.reject_statuses: - for status in change.status: - if re2.fullmatch(rstatus, status): - return FalseWithReason("NoRejectStatuses %s matches %s" % ( - self.reject_statuses, change.status)) - return True +class GithubEventFilter(EventFilter): + def __init__(self, connection_name, trigger, types=[], + branches=[], refs=[], comments=[], actions=[], + labels=[], unlabels=[], states=[], statuses=[], + required_statuses=[], check_runs=[], + ignore_deletes=True, + require=None, reject=None): + EventFilter.__init__(self, connection_name, trigger) -class GithubEventFilter(EventFilter, GithubCommonFilter): - def __init__(self, connection_name, trigger, types=[], branches=[], - refs=[], comments=[], actions=[], labels=[], unlabels=[], - states=[], statuses=[], required_statuses=[], - check_runs=[], ignore_deletes=True): + # TODO: Backwards compat, remove after 9.x: + if required_statuses and require is None: + require = {'status': required_statuses} - EventFilter.__init__(self, connection_name, trigger) + if require: + self.require_filter = GithubRefFilter.requiresFromConfig( + connection_name, require) + else: + self.require_filter = None - GithubCommonFilter.__init__(self, required_statuses=required_statuses) + if reject: + self.reject_filter = GithubRefFilter.rejectFromConfig( + connection_name, reject) + else: + self.reject_filter = None self._types = types self._branches = branches @@ -327,7 +209,6 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): self.unlabels = unlabels self.states = states self.statuses = statuses - self.required_statuses = required_statuses self.check_runs = check_runs self.ignore_deletes = ignore_deletes @@ -356,8 +237,10 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): ret += ' states: %s' % ', '.join(self.states) if self.statuses: ret += ' statuses: %s' % ', '.join(self.statuses) - if self.required_statuses: - ret += ' required_statuses: %s' % ', '.join(self.required_statuses) + if self.require_filter: + ret += ' require: %s' % repr(self.require_filter) + if self.reject_filter: + ret += ' reject: %s' % repr(self.reject_filter) ret += '>' return ret @@ -372,7 +255,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): if etype.match(event.type): matches_type = True if self.types and not matches_type: - return FalseWithReason("Types %s doesn't match %s" % ( + return FalseWithReason("Types %s do not match %s" % ( self.types, event.type)) # branches are ORed @@ -381,7 +264,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): if branch.match(event.branch): matches_branch = True if self.branches and not matches_branch: - return FalseWithReason("Branches %s doesn't match %s" % ( + return FalseWithReason("Branches %s do not match %s" % ( self.branches, event.branch)) # refs are ORed @@ -392,11 +275,11 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): matches_ref = True if self.refs and not matches_ref: return FalseWithReason( - "Refs %s doesn't match %s" % (self.refs, event.ref)) + "Refs %s do not match %s" % (self.refs, event.ref)) if self.ignore_deletes and event.newrev == EMPTY_GIT_REF: # If the updated ref has an empty git sha (all 0s), # then the ref is being deleted - return FalseWithReason("Ref deletion are ignored") + return FalseWithReason("Ref deletion events are ignored") # comments are ORed matches_comment_re = False @@ -405,7 +288,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): comment_re.search(event.comment)): matches_comment_re = True if self.comments and not matches_comment_re: - return FalseWithReason("Comments %s doesn't match %s" % ( + return FalseWithReason("Comments %s do not match %s" % ( self.comments, event.comment)) # actions are ORed @@ -414,7 +297,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): if (event.action == action): matches_action = True if self.actions and not matches_action: - return FalseWithReason("Actions %s doesn't match %s" % ( + return FalseWithReason("Actions %s do not match %s" % ( self.actions, event.action)) # check_runs are ORed @@ -425,22 +308,22 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): check_run_found = True break if not check_run_found: - return FalseWithReason("Check_runs %s doesn't match %s" % ( + return FalseWithReason("Check runs %s do not match %s" % ( self.check_runs, event.check_run)) # labels are ORed if self.labels and event.label not in self.labels: - return FalseWithReason("Labels %s doesn't match %s" % ( + return FalseWithReason("Labels %s do not match %s" % ( self.labels, event.label)) # unlabels are ORed if self.unlabels and event.unlabel not in self.unlabels: - return FalseWithReason("Unlabels %s doesn't match %s" % ( + return FalseWithReason("Unlabels %s do not match %s" % ( self.unlabels, event.unlabel)) # states are ORed if self.states and event.state not in self.states: - return FalseWithReason("States %s doesn't match %s" % ( + return FalseWithReason("States %s do not match %s" % ( self.states, event.state)) # statuses are ORed @@ -451,26 +334,40 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): status_found = True break if not status_found: - return FalseWithReason("Statuses %s doesn't match %s" % ( + return FalseWithReason("Statuses %s do not match %s" % ( self.statuses, event.status)) - return self.matchesStatuses(change) + if self.require_filter: + require_filter_result = self.require_filter.matches(change) + if not require_filter_result: + return require_filter_result + + if self.reject_filter: + reject_filter_result = self.reject_filter.matches(change) + if not reject_filter_result: + return reject_filter_result + + return True -class GithubRefFilter(RefFilter, GithubCommonFilter): +class GithubRefFilter(RefFilter): def __init__(self, connection_name, statuses=[], - required_reviews=[], reject_reviews=[], open=None, + reviews=[], reject_reviews=[], open=None, merged=None, current_patchset=None, draft=None, reject_open=None, reject_merged=None, reject_current_patchset=None, reject_draft=None, labels=[], reject_labels=[], reject_statuses=[]): RefFilter.__init__(self, connection_name) - GithubCommonFilter.__init__(self, required_reviews=required_reviews, - reject_reviews=reject_reviews, - required_statuses=statuses, - reject_statuses=reject_statuses) - self.statuses = statuses + self._required_reviews = copy.deepcopy(reviews) + self._reject_reviews = copy.deepcopy(reject_reviews) + self.required_reviews = self._tidy_reviews(self._required_reviews) + self.reject_reviews = self._tidy_reviews(self._reject_reviews) + self.required_statuses = statuses + self.reject_statuses = reject_statuses + self.required_labels = labels + self.reject_labels = reject_labels + if reject_open is not None: self.open = not reject_open else: @@ -487,23 +384,51 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): self.draft = not reject_draft else: self.draft = draft - self.labels = labels - self.reject_labels = reject_labels + + @classmethod + def requiresFromConfig(cls, connection_name, config): + return cls( + connection_name=connection_name, + statuses=to_list(config.get('status')), + reviews=to_list(config.get('review')), + labels=to_list(config.get('label')), + open=config.get('open'), + merged=config.get('merged'), + current_patchset=config.get('current-patchset'), + draft=config.get('draft'), + ) + + @classmethod + def rejectFromConfig(cls, connection_name, config): + return cls( + connection_name=connection_name, + reject_statuses=to_list(config.get('status')), + reject_reviews=to_list(config.get('review')), + reject_labels=to_list(config.get('label')), + reject_open=config.get('open'), + reject_merged=config.get('merged'), + reject_current_patchset=config.get('current-patchset'), + reject_draft=config.get('draft'), + ) def __repr__(self): ret = '<GithubRefFilter' ret += ' connection_name: %s' % self.connection_name - if self.statuses: - ret += ' statuses: %s' % ', '.join(self.statuses) + if self.required_statuses: + ret += ' status: %s' % str(self.required_statuses) if self.reject_statuses: - ret += ' reject-statuses: %s' % ', '.join(self.reject_statuses) + ret += ' reject-status: %s' % str(self.reject_statuses) if self.required_reviews: - ret += (' required-reviews: %s' % + ret += (' reviews: %s' % str(self.required_reviews)) if self.reject_reviews: ret += (' reject-reviews: %s' % str(self.reject_reviews)) + if self.required_labels: + ret += ' labels: %s' % str(self.required_labels) + if self.reject_labels: + ret += ' reject-labels: %s' % str(self.reject_labels) if self.open is not None: ret += ' open: %s' % self.open if self.merged is not None: @@ -512,20 +437,175 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): ret += ' current-patchset: %s' % self.current_patchset if self.draft is not None: ret += ' draft: %s' % self.draft - if self.labels: - ret += ' labels: %s' % self.labels - if self.reject_labels: - ret += ' reject-labels: %s' % self.reject_labels ret += '>' return ret + def _tidy_reviews(self, reviews): + for r in reviews: + for k, v in r.items(): + if k == 'username': + r['username'] = re.compile(v) + elif k == 'email': + r['email'] = re.compile(v) + elif k == 'newer-than': + r[k] = time_to_seconds(v) + elif k == 'older-than': + r[k] = time_to_seconds(v) + return reviews + + def _match_review_required_review(self, rreview, review): + # Check if the required review and review match + now = time.time() + by = review.get('by', {}) + for k, v in rreview.items(): + if k == 'username': + if (not v.search(by.get('username', ''))): + return False + elif k == 'email': + if (not v.search(by.get('email', ''))): + return False + elif k == 'newer-than': + t = now - v + if (review['grantedOn'] < t): + return False + elif k == 'older-than': + t = now - v + if (review['grantedOn'] >= t): + return False + elif k == 'type': + if review['type'] != v: + return False + elif k == 'permission': + # If permission is read, we've matched. You must have read + # to provide a review. + if v != 'read': + # Admins have implicit write. + if v == 'write': + if review['permission'] not in ('write', 'admin'): + return False + elif v == 'admin': + if review['permission'] != 'admin': + return False + return True + + def matchesReviews(self, change): + if self.required_reviews or self.reject_reviews: + if not hasattr(change, 'number'): + # not a PR, no reviews + return FalseWithReason("Change is not a PR") + if self.required_reviews and not change.reviews: + # No reviews means no matching of required bits + # having reject reviews but no reviews on the change is okay + return FalseWithReason("Reviews %s do not match %s" % ( + self.required_reviews, change.reviews)) + + return (self.matchesRequiredReviews(change) and + self.matchesNoRejectReviews(change)) + + def matchesRequiredReviews(self, change): + for rreview in self.required_reviews: + matches_review = False + for review in change.reviews: + if self._match_review_required_review(rreview, review): + # Consider matched if any review matches + matches_review = True + break + if not matches_review: + return FalseWithReason( + "Required reviews %s do not match %s" % ( + self.required_reviews, change.reviews)) + return True + + def matchesNoRejectReviews(self, change): + for rreview in self.reject_reviews: + for review in change.reviews: + if self._match_review_required_review(rreview, review): + # A review matched, we can reject right away + return FalseWithReason("Reject reviews %s match %s" % ( + self.reject_reviews, change.reviews)) + return True + + def matchesStatuses(self, change): + if self.required_statuses or self.reject_statuses: + if not hasattr(change, 'number'): + # not a PR, no status + return FalseWithReason("Can not match statuses without PR") + if self.required_statuses and not change.status: + return FalseWithReason( + "Required statuses %s do not match %s" % ( + self.required_statuses, change.status)) + required_statuses_results = self.matchesRequiredStatuses(change) + if not required_statuses_results: + return required_statuses_results + return self.matchesNoRejectStatuses(change) + + def matchesRequiredStatuses(self, change): + # statuses are ORed + # A PR head can have multiple statuses on it. If the change + # statuses and the filter statuses are a null intersection, there + # are no matches and we return false + if self.required_statuses: + for required_status in self.required_statuses: + for status in change.status: + if re2.fullmatch(required_status, status): + return True + return FalseWithReason("Required statuses %s do not match %s" % ( + self.required_statuses, change.status)) + return True + + def matchesNoRejectStatuses(self, change): + # statuses are ANDed + # If any of the rejected statusses are present, we return false + for rstatus in self.reject_statuses: + for status in change.status: + if re2.fullmatch(rstatus, status): + return FalseWithReason("Reject statuses %s match %s" % ( + self.reject_statuses, change.status)) + return True + + def matchesLabels(self, change): + if self.required_labels or self.reject_labels: + if not hasattr(change, 'number'): + # not a PR, no label + return FalseWithReason("Can not match labels without PR") + if self.required_labels and not change.labels: + # No labels means no matching of required bits + # having reject labels but no labels on the change is okay + return FalseWithReason( + "Required labels %s does not match %s" % ( + self.required_labels, change.labels)) + return (self.matchesRequiredLabels(change) and + self.matchesNoRejectLabels(change)) + + def matchesRequiredLabels(self, change): + for label in self.required_labels: + if label not in change.labels: + return FalseWithReason("Labels %s do not match %s" % ( + self.required_labels, change.labels)) + return True + + def matchesNoRejectLabels(self, change): + for label in self.reject_labels: + if label in change.labels: + return FalseWithReason("Reject labels %s match %s" % ( + self.reject_labels, change.labels)) + return True + def matches(self, change): statuses_result = self.matchesStatuses(change) if not statuses_result: return statuses_result + reviews_result = self.matchesReviews(change) + if not reviews_result: + return reviews_result + + labels_result = self.matchesLabels(change) + if not labels_result: + return labels_result + if self.open is not None: # if a "change" has no number, it's not a change, but a push # and cannot possibly pass this test. @@ -566,21 +646,4 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): else: return FalseWithReason("Change is not a PR") - # required reviews are ANDed (reject reviews are ORed) - reviews_result = self.matchesReviews(change) - if not reviews_result: - return reviews_result - - # required labels are ANDed - for label in self.labels: - if label not in change.labels: - return FalseWithReason("Labels %s does not match %s" % ( - self.labels, change.labels)) - - # rejected reviews are OR'd - for label in self.reject_labels: - if label in change.labels: - return FalseWithReason("RejectLabels %s matches %s" % ( - self.reject_labels, change.labels)) - return True diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 1f44303bd..396516038 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -193,13 +193,13 @@ class GithubReporter(BaseReporter): self.log.warning('Merge mode %s not supported by Github', mode) raise MergeFailure('Merge mode %s not supported by Github' % mode) - merge_mode = self.merge_modes[merge_mode] project = item.change.project.name pr_number = item.change.number sha = item.change.patchset log.debug('Reporting change %s, params %s, merging via API', item.change, self.config) - message = self._formatMergeMessage(item.change) + message = self._formatMergeMessage(item.change, merge_mode) + merge_mode = self.merge_modes[merge_mode] for i in [1, 2]: try: @@ -319,10 +319,13 @@ class GithubReporter(BaseReporter): self.connection.unlabelPull(project, pr_number, label, zuul_event_id=item.event) - def _formatMergeMessage(self, change): + def _formatMergeMessage(self, change, merge_mode): message = [] - if change.title: - message.append(change.title) + # For squash merges we don't need to add the title to the body + # as it will already be set as the commit subject. + if merge_mode != model.MERGER_SQUASH_MERGE: + if change.title: + message.append(change.title) if change.body_text: message.append(change.body_text) merge_message = "\n\n".join(message) diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 0a94a1730..01901dfd4 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -21,7 +21,7 @@ import voluptuous as v from zuul.source import BaseSource from zuul.model import Project from zuul.driver.github.githubmodel import GithubRefFilter -from zuul.driver.util import scalar_or_list, to_list +from zuul.driver.util import scalar_or_list from zuul.zk.change_cache import ChangeKey @@ -165,29 +165,15 @@ class GithubSource(BaseSource): return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ') def getRequireFilters(self, config): - f = GithubRefFilter( - connection_name=self.connection.connection_name, - statuses=to_list(config.get('status')), - required_reviews=to_list(config.get('review')), - open=config.get('open'), - merged=config.get('merged'), - current_patchset=config.get('current-patchset'), - draft=config.get('draft'), - labels=to_list(config.get('label')), - ) + f = GithubRefFilter.requiresFromConfig( + self.connection.connection_name, + config) return [f] def getRejectFilters(self, config): - f = GithubRefFilter( - connection_name=self.connection.connection_name, - reject_reviews=to_list(config.get('review')), - reject_labels=to_list(config.get('label')), - reject_statuses=to_list(config.get('status')), - reject_open=config.get('open'), - reject_merged=config.get('merged'), - reject_current_patchset=config.get('current-patchset'), - reject_draft=config.get('draft'), - ) + f = GithubRefFilter.rejectFromConfig( + self.connection.connection_name, + config) return [f] def getRefForChange(self, change): diff --git a/zuul/driver/github/githubtrigger.py b/zuul/driver/github/githubtrigger.py index 76d8f574e..5072fda43 100644 --- a/zuul/driver/github/githubtrigger.py +++ b/zuul/driver/github/githubtrigger.py @@ -16,6 +16,7 @@ import logging import voluptuous as v from zuul.trigger import BaseTrigger from zuul.driver.github.githubmodel import GithubEventFilter +from zuul.driver.github import githubsource from zuul.driver.util import scalar_or_list, to_list @@ -50,7 +51,9 @@ class GithubTrigger(BaseTrigger): unlabels=to_list(trigger.get('unlabel')), states=to_list(trigger.get('state')), statuses=to_list(trigger.get('status')), - required_statuses=to_list(trigger.get('require-status')) + required_statuses=to_list(trigger.get('require-status')), + require=trigger.get('require'), + reject=trigger.get('reject'), ) efilters.append(f) @@ -75,6 +78,8 @@ def getSchema(): 'unlabel': scalar_or_list(str), 'state': scalar_or_list(str), 'require-status': scalar_or_list(str), + 'require': githubsource.getRequireSchema(), + 'reject': githubsource.getRejectSchema(), 'status': scalar_or_list(str), 'check': scalar_or_list(str), } diff --git a/zuul/driver/mqtt/mqttconnection.py b/zuul/driver/mqtt/mqttconnection.py index 7f221282f..4a028ba23 100644 --- a/zuul/driver/mqtt/mqttconnection.py +++ b/zuul/driver/mqtt/mqttconnection.py @@ -64,6 +64,12 @@ class MQTTConnection(BaseConnection): def onLoad(self, zk_client, component_registry): self.log.debug("Starting MQTT Connection") + + # If the connection was not loaded by a scheduler, but by e.g. + # zuul-web, we want to stop here. + if not self.sched: + return + try: self.client.connect( self.connection_config.get('server', 'localhost'), @@ -76,10 +82,11 @@ class MQTTConnection(BaseConnection): self.client.loop_start() def onStop(self): - self.log.debug("Stopping MQTT Connection") - self.client.loop_stop() - self.client.disconnect() - self.connected = False + if self.connected: + self.log.debug("Stopping MQTT Connection") + self.client.loop_stop() + self.client.disconnect() + self.connected = False def publish(self, topic, message, qos, zuul_event_id): log = get_annotated_logger(self.log, zuul_event_id) diff --git a/zuul/driver/sql/sqlconnection.py b/zuul/driver/sql/sqlconnection.py index 360478e6b..26bcf184d 100644 --- a/zuul/driver/sql/sqlconnection.py +++ b/zuul/driver/sql/sqlconnection.py @@ -288,12 +288,25 @@ class DatabaseSession(object): except sqlalchemy.orm.exc.MultipleResultsFound: raise Exception("Multiple buildset found with uuid %s", uuid) - def deleteBuildsets(self, cutoff): + def deleteBuildsets(self, cutoff, batch_size): """Delete buildsets before the cutoff""" # delete buildsets updated before the cutoff - for buildset in self.getBuildsets(updated_max=cutoff): - self.session().delete(buildset) + deleted = True + while deleted: + deleted = False + oldest = None + for buildset in self.getBuildsets( + updated_max=cutoff, limit=batch_size): + deleted = True + if oldest is None: + oldest = buildset.updated + else: + oldest = min(oldest, buildset.updated) + self.session().delete(buildset) + self.session().commit() + if deleted: + self.log.info("Deleted from %s to %s", oldest, cutoff) class SQLConnection(BaseConnection): @@ -450,7 +463,10 @@ class SQLConnection(BaseConnection): final = sa.Column(sa.Boolean) held = sa.Column(sa.Boolean) nodeset = sa.Column(sa.String(255)) - buildset = orm.relationship(BuildSetModel, backref="builds") + buildset = orm.relationship(BuildSetModel, + backref=orm.backref( + "builds", + cascade="all, delete-orphan")) sa.Index(self.table_prefix + 'job_name_buildset_id_idx', job_name, buildset_id) @@ -509,7 +525,10 @@ class SQLConnection(BaseConnection): name = sa.Column(sa.String(255)) url = sa.Column(sa.TEXT()) meta = sa.Column('metadata', sa.TEXT()) - build = orm.relationship(BuildModel, backref="artifacts") + build = orm.relationship(BuildModel, + backref=orm.backref( + "artifacts", + cascade="all, delete-orphan")) class ProvidesModel(Base): __tablename__ = self.table_prefix + PROVIDES_TABLE @@ -517,7 +536,10 @@ class SQLConnection(BaseConnection): build_id = sa.Column(sa.Integer, sa.ForeignKey( self.table_prefix + BUILD_TABLE + ".id")) name = sa.Column(sa.String(255)) - build = orm.relationship(BuildModel, backref="provides") + build = orm.relationship(BuildModel, + backref=orm.backref( + "provides", + cascade="all, delete-orphan")) class BuildEventModel(Base): __tablename__ = self.table_prefix + BUILD_EVENTS_TABLE @@ -527,7 +549,10 @@ class SQLConnection(BaseConnection): event_time = sa.Column(sa.DateTime) event_type = sa.Column(sa.String(255)) description = sa.Column(sa.TEXT()) - build = orm.relationship(BuildModel, backref="build_events") + build = orm.relationship(BuildModel, + backref=orm.backref( + "build_events", + cascade="all, delete-orphan")) self.buildEventModel = BuildEventModel self.zuul_build_event_table = self.buildEventModel.__table__ diff --git a/zuul/executor/server.py b/zuul/executor/server.py index a49bbbbbf..6dbf62de0 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1931,6 +1931,7 @@ class AnsibleJob(object): region=node.region, host_id=node.host_id, external_id=getattr(node, 'external_id', None), + slot=node.slot, interface_ip=node.interface_ip, public_ipv4=node.public_ipv4, private_ipv4=node.private_ipv4, @@ -3632,6 +3633,10 @@ class ExecutorServer(BaseMergeServer): log.exception('Process pool got broken') self.resetProcessPool() task.transient_error = True + except IOError: + log.exception('Got I/O error while updating repo %s/%s', + task.connection_name, task.project_name) + task.transient_error = True except Exception: log.exception('Got exception while updating repo %s/%s', task.connection_name, task.project_name) diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index e4688a1b7..845925bfa 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -595,14 +595,32 @@ class Repo(object): log = get_annotated_logger(self.log, zuul_event_id) repo = self.createRepoObject(zuul_event_id) self.fetch(ref, zuul_event_id=zuul_event_id) - if len(repo.commit("FETCH_HEAD").parents) > 1: + fetch_head = repo.commit("FETCH_HEAD") + if len(fetch_head.parents) > 1: args = ["-s", "resolve", "FETCH_HEAD"] log.debug("Merging %s with args %s instead of cherry-picking", ref, args) repo.git.merge(*args) else: log.debug("Cherry-picking %s", ref) - repo.git.cherry_pick("FETCH_HEAD") + # Git doesn't have an option to ignore commits that are already + # applied to the working tree when cherry-picking, so pass the + # --keep-redundant-commits option, which will cause it to make an + # empty commit + repo.git.cherry_pick("FETCH_HEAD", keep_redundant_commits=True) + + # If the newly applied commit is empty, it means either: + # 1) The commit being cherry-picked was empty, in which the empty + # commit should be kept + # 2) The commit being cherry-picked was already applied to the + # tree, in which case the empty commit should be backed out + head = repo.commit("HEAD") + parent = head.parents[0] + if not any(head.diff(parent)) and \ + any(fetch_head.diff(fetch_head.parents[0])): + log.debug("%s was already applied. Removing it", ref) + self._checkout(repo, parent) + return repo.head.commit def merge(self, ref, strategy=None, zuul_event_id=None): @@ -704,9 +722,11 @@ class Repo(object): ret = {} repo = self.createRepoObject(zuul_event_id) if branch: - tree = repo.heads[branch].commit.tree + head = repo.heads[branch].commit else: - tree = repo.commit(commit).tree + head = repo.commit(commit) + log.debug("Getting files for %s at %s", self.local_path, head.hexsha) + tree = head.tree for fn in files: if fn in tree: if tree[fn].type != 'blob': diff --git a/zuul/model.py b/zuul/model.py index e526b749c..e36f9d670 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1408,6 +1408,7 @@ class Node(ConfigObject): self.private_ipv6 = None self.connection_port = 22 self.connection_type = None + self.slot = None self._keys = [] self.az = None self.provider = None @@ -3006,20 +3007,30 @@ class Job(ConfigObject): # possibility of success, which may help prevent errors in # most cases. If we don't raise an error here, the # possibility of later failure still remains. - nonfinal_parents = [p for p in parents if not p.final] - if not nonfinal_parents: + nonfinal_parent_found = False + nonintermediate_parent_found = False + nonprotected_parent_found = False + for p in parents: + if not p.final: + nonfinal_parent_found = True + if not p.intermediate: + nonintermediate_parent_found = True + if not p.protected: + nonprotected_parent_found = True + if (nonfinal_parent_found and + nonintermediate_parent_found and + nonprotected_parent_found): + break + + if not nonfinal_parent_found: raise Exception( f'The parent of job "{self.name}", "{self.parent}" ' 'is final and can not act as a parent') - nonintermediate_parents = [ - p for p in parents if not p.intermediate] - if not nonintermediate_parents and not self.abstract: + if not nonintermediate_parent_found and not self.abstract: raise Exception( f'The parent of job "{self.name}", "{self.parent}" ' f'is intermediate but "{self.name}" is not abstract') - nonprotected_parents = [ - p for p in parents if not p.protected] - if (not nonprotected_parents and + if (not nonprotected_parent_found and parents[0].source_context.project_canonical_name != self.source_context.project_canonical_name): raise Exception( @@ -7786,31 +7797,33 @@ class Layout(object): def addJob(self, job): # We can have multiple variants of a job all with the same # name, but these variants must all be defined in the same repo. - prior_jobs = [j for j in self.getJobs(job.name) if - j.source_context.project_canonical_name != - job.source_context.project_canonical_name] # Unless the repo is permitted to shadow another. If so, and # the job we are adding is from a repo that is permitted to # shadow the one with the older jobs, skip adding this job. job_project = job.source_context.project_canonical_name job_tpc = self.tenant.project_configs[job_project] skip_add = False - for prior_job in prior_jobs[:]: - prior_project = prior_job.source_context.project_canonical_name - if prior_project in job_tpc.shadow_projects: - prior_jobs.remove(prior_job) - skip_add = True - + prior_jobs = self.jobs.get(job.name, []) if prior_jobs: - raise Exception("Job %s in %s is not permitted to shadow " - "job %s in %s" % ( - job, - job.source_context.project_name, - prior_jobs[0], - prior_jobs[0].source_context.project_name)) + # All jobs we've added so far should be from the same + # project, so pick the first one. + prior_job = prior_jobs[0] + if (prior_job.source_context.project_canonical_name != + job.source_context.project_canonical_name): + prior_project = prior_job.source_context.project_canonical_name + if prior_project in job_tpc.shadow_projects: + skip_add = True + else: + raise Exception("Job %s in %s is not permitted to shadow " + "job %s in %s" % ( + job, + job.source_context.project_name, + prior_job, + prior_job.source_context.project_name)) + if skip_add: return False - if job.name in self.jobs: + if prior_jobs: self.jobs[job.name].append(job) else: self.jobs[job.name] = [job] diff --git a/zuul/zk/job_request_queue.py b/zuul/zk/job_request_queue.py index 175c57b90..7c85ae95e 100644 --- a/zuul/zk/job_request_queue.py +++ b/zuul/zk/job_request_queue.py @@ -609,7 +609,7 @@ class JobRequestQueue(ZooKeeperSimpleBase): self.kazoo_client.delete(lock_path, recursive=True) except Exception: self.log.exception( - "Unable to delete lock %s", path) + "Unable to delete lock %s", lock_path) except Exception: self.log.exception("Error cleaning up locks %s", self) |
