From 2f7ffba18dbdc4ab68167354fe9b2c0a8820a1e1 Mon Sep 17 00:00:00 2001 From: Tristan Cacqueray Date: Fri, 30 Aug 2024 13:06:31 +0000 Subject: [PATCH] Update the set-zuul-log-path-fact scheme to prevent huge url This change simplifies the path creation logic to avoid processing user defined variables such as job name and pipeline name, which might cause the log url to exceed the database storage presently fixed at 255 char. Add warning in the job's header when the url is over 255 characters, explaining that Zuul won't report the job properly in its database; but the job can still run. Change-Id: I34fb5662a2f958c55f60458ce107bad2a73b9c96 --- roles/emit-job-header/README.rst | 10 ----- roles/set-zuul-log-path-fact/README.rst | 16 +++----- .../set-zuul-log-path-fact/defaults/main.yaml | 1 - roles/set-zuul-log-path-fact/tasks/main.yaml | 38 ++----------------- roles/test-upload-logs-swift/README.rst | 11 ------ roles/upload-logs-azure/README.rst | 11 ------ roles/upload-logs-gcs/README.rst | 11 ------ roles/upload-logs-ibm/README.rst | 11 ------ roles/upload-logs-s3/README.rst | 11 ------ roles/upload-logs-swift/README.rst | 11 ------ roles/upload-logs/README.rst | 11 ------ .../base-roles/emit-job-header.yaml | 32 ++-------------- .../base-test-roles/emit-job-header.yaml | 35 ++--------------- 13 files changed, 17 insertions(+), 192 deletions(-) delete mode 100644 roles/set-zuul-log-path-fact/defaults/main.yaml diff --git a/roles/emit-job-header/README.rst b/roles/emit-job-header/README.rst index b67d66b62..bdb97c7ed 100644 --- a/roles/emit-job-header/README.rst +++ b/roles/emit-job-header/README.rst @@ -5,13 +5,3 @@ Log a few lines about the job. .. zuul:rolevar:: zuul_log_url Base URL where logs are to be found. - -.. zuul:rolevar:: zuul_log_path_shard_build - :default: False - - This var is consumed by set-zuul-log-path-fact which emit-job-header - calls into. If you set this you will get log paths prefixed with the - first three characters of the build uuid. This will improve log file - sharding. - - More details can be found at :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build` diff --git a/roles/set-zuul-log-path-fact/README.rst b/roles/set-zuul-log-path-fact/README.rst index 33a36d1da..49ef47c3d 100644 --- a/roles/set-zuul-log-path-fact/README.rst +++ b/roles/set-zuul-log-path-fact/README.rst @@ -1,12 +1,8 @@ Sets a fact named ``zuul_log_path`` from zuul variables -**Role Variables** - -.. zuul:rolevar:: zuul_log_path_shard_build - :type: bool - :default: False - - Flag to specify whether or not paths that include a three character - prefix based on the build uuid should prefix the log path. This is - particularly useful for object storage systems where we want to - spread out the number of files per container. +The resulting log path will be based on the zuul tenant name and build +uuid. The url will then be prefixed by a portion of the build uuid. +This prefix allows for partitioning in object storage systems. +Constructing the url in this way isn't very human readable but produces +consistent url lengths which is important for database record keeping +and avoiding unexpected problems with url lengths exceeding limits. diff --git a/roles/set-zuul-log-path-fact/defaults/main.yaml b/roles/set-zuul-log-path-fact/defaults/main.yaml deleted file mode 100644 index b1abb3a6f..000000000 --- a/roles/set-zuul-log-path-fact/defaults/main.yaml +++ /dev/null @@ -1 +0,0 @@ -zuul_log_path_shard_build: false diff --git a/roles/set-zuul-log-path-fact/tasks/main.yaml b/roles/set-zuul-log-path-fact/tasks/main.yaml index 782bf4d22..3a61bf882 100644 --- a/roles/set-zuul-log-path-fact/tasks/main.yaml +++ b/roles/set-zuul-log-path-fact/tasks/main.yaml @@ -1,35 +1,3 @@ -- name: Fileserver friendly log path specifications - when: not zuul_log_path_shard_build - block: - - name: Set log path for a change - when: zuul.change is defined - set_fact: - zuul_log_path: "{{ zuul.change[-2:] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }}" - - - name: Set log path for a ref update - when: zuul.newrev is defined - set_fact: - zuul_log_path: "{{ zuul.newrev[:2] }}/{{ zuul.newrev }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }}" - - - name: Set log path for a periodic job - when: zuul.change is not defined and zuul.newrev is not defined - set_fact: - zuul_log_path: "{{ zuul.pipeline }}/{{ zuul.project.canonical_name }}/{{ zuul.branch }}/{{ zuul.job }}/{{ zuul.build[:7] }}" - -- name: Object store friendly log path specifications - when: zuul_log_path_shard_build - block: - - name: Set log path for a change - when: zuul.change is defined - set_fact: - zuul_log_path: "{{ zuul.build[:3] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }}" - - - name: Set log path for a ref update - when: zuul.newrev is defined - set_fact: - zuul_log_path: "{{ zuul.build[:3] }}/{{ zuul.newrev }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }}" - - - name: Set log path for a periodic job - when: zuul.change is not defined and zuul.newrev is not defined - set_fact: - zuul_log_path: "{{ zuul.build[:3] }}/{{ zuul.pipeline }}/{{ zuul.project.canonical_name }}/{{ zuul.branch }}/{{ zuul.job }}/{{ zuul.build[:7] }}" +- name: Set log path for a build + set_fact: + zuul_log_path: "{{ zuul.build[:3] }}/{{ zuul.tenant}}/{{ zuul.build }}" diff --git a/roles/test-upload-logs-swift/README.rst b/roles/test-upload-logs-swift/README.rst index e49225400..d55f48f81 100644 --- a/roles/test-upload-logs-swift/README.rst +++ b/roles/test-upload-logs-swift/README.rst @@ -67,14 +67,3 @@ This uploads logs to an OpenStack Object Store (Swift) container. Whether to create `index.html` files with directory indexes. If set to false, Swift containers can be marked with a `Web-Listings=true` property to activate Swift's own directory indexing. - -.. zuul:rolevar:: zuul_log_path_shard_build - :default: False - - This var is consumed by set-zuul-log-path-fact which upload-logs-swift - calls into. If you set this you will get log paths prefixed with the - first three characters of the build uuid. This will improve log file - sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. diff --git a/roles/upload-logs-azure/README.rst b/roles/upload-logs-azure/README.rst index 492b7fc32..e5c1645a5 100644 --- a/roles/upload-logs-azure/README.rst +++ b/roles/upload-logs-azure/README.rst @@ -48,17 +48,6 @@ containers) for you. Whether to create `index.html` files with directory indexes. -.. zuul:rolevar:: zuul_log_path_shard_build - :default: false - - This var is consumed by set-zuul-log-path-fact which - upload-logs-azure calls into. If you set this you will get log - paths prefixed with the first three characters of the build - uuid. This will improve log file sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. - .. zuul:rolevar:: zuul_log_connection_string The Access key connection string for the Azure storage account. diff --git a/roles/upload-logs-gcs/README.rst b/roles/upload-logs-gcs/README.rst index bc9dea17c..e061575c7 100644 --- a/roles/upload-logs-gcs/README.rst +++ b/roles/upload-logs-gcs/README.rst @@ -49,17 +49,6 @@ Google Cloud Application Default Credentials. Whether to create `index.html` files with directory indexes. -.. zuul:rolevar:: zuul_log_path_shard_build - :default: false - - This var is consumed by set-zuul-log-path-fact which - upload-logs-gcs calls into. If you set this you will get log paths - prefixed with the first three characters of the build uuid. This - will improve log file sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. - .. zuul:rolevar:: zuul_log_credentials_file This log upload role normally uses Google Cloud Application Default diff --git a/roles/upload-logs-ibm/README.rst b/roles/upload-logs-ibm/README.rst index 508625e10..58fda75de 100644 --- a/roles/upload-logs-ibm/README.rst +++ b/roles/upload-logs-ibm/README.rst @@ -55,17 +55,6 @@ create the bucket (or buckets) for you. Whether to create `index.html` files with directory indexes. -.. zuul:rolevar:: zuul_log_path_shard_build - :default: false - - This var is consumed by set-zuul-log-path-fact which - upload-logs-ibm calls into. If you set this you will get log - paths prefixed with the first three characters of the build - uuid. This will improve log file sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. - .. zuul:rolevar:: zuul_log_api_key The API key that was created as part of the `service credential`_. diff --git a/roles/upload-logs-s3/README.rst b/roles/upload-logs-s3/README.rst index 925bff7c1..80f973847 100644 --- a/roles/upload-logs-s3/README.rst +++ b/roles/upload-logs-s3/README.rst @@ -53,17 +53,6 @@ installed in the Ansible environment on the Zuul executor. Whether to create `index.html` files with directory indexes. -.. zuul:rolevar:: zuul_log_path_shard_build - :default: false - - This var is consumed by set-zuul-log-path-fact which - upload-logs-s3 calls into. If you set this you will get log paths - prefixed with the first three characters of the build uuid. This - will improve log file sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. - .. zuul:rolevar:: zuul_log_aws_access_key AWS access key to use. diff --git a/roles/upload-logs-swift/README.rst b/roles/upload-logs-swift/README.rst index fb21c7b44..553f21b05 100644 --- a/roles/upload-logs-swift/README.rst +++ b/roles/upload-logs-swift/README.rst @@ -65,14 +65,3 @@ This uploads logs to an OpenStack Object Store (Swift) container. Whether to create `index.html` files with directory indexes. If set to false, Swift containers can be marked with a `Web-Listings=true` property to activate Swift's own directory indexing. - -.. zuul:rolevar:: zuul_log_path_shard_build - :default: False - - This var is consumed by set-zuul-log-path-fact which upload-logs-swift - calls into. If you set this you will get log paths prefixed with the - first three characters of the build uuid. This will improve log file - sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. diff --git a/roles/upload-logs/README.rst b/roles/upload-logs/README.rst index 26c4689c0..915ba3a24 100644 --- a/roles/upload-logs/README.rst +++ b/roles/upload-logs/README.rst @@ -52,14 +52,3 @@ description of the site_logs secret in this example post-run playbook: when the job has failed. .. note:: Intended to be set by admins via site-variables. - -.. zuul:rolevar:: zuul_log_path_shard_build - :default: False - - This var is consumed by set-zuul-log-path-fact which upload-logs - calls into. If you set this you will get log paths prefixed with the - first three characters of the build uuid. This will improve log file - sharding. - - More details can be found at - :zuul:rolevar:`set-zuul-log-path-fact.zuul_log_path_shard_build`. diff --git a/test-playbooks/base-roles/emit-job-header.yaml b/test-playbooks/base-roles/emit-job-header.yaml index abdd06503..350894db4 100644 --- a/test-playbooks/base-roles/emit-job-header.yaml +++ b/test-playbooks/base-roles/emit-job-header.yaml @@ -1,7 +1,7 @@ # Note that the order of these two plays is important. We want the second one # to run to be the one the creates the correct log path for the currently # running system. -- name: Test the emit-job-header role without swift +- name: Test the emit-job-header role hosts: all roles: - role: emit-job-header @@ -16,30 +16,6 @@ assert: that: - zuul_log_path is defined - - zuul.change in zuul_log_path - - zuul.patchset in zuul_log_path - - zuul.pipeline in zuul_log_path - - zuul.job in zuul_log_path - - zuul.build[:3] != zuul_log_path[:3] - -- name: Test the emit-job-header role with swift - hosts: all - roles: - - role: emit-job-header - zuul_log_url: "http://logs.openstack.org" - zuul_log_path_shard_build: true - post_tasks: - # All emit-job-header does is a debug statement so the worst that would - # happen would be that the debug task would fail outright and we'd prevent - # something breaking that debug statement from merging just by running it. - # However, the emit-job-header role includes the set-zuul-log-path-fact - # role. We can only test for zuul_log_path against changes, though. - - name: Assert zuul_log_path by set-zuul-log-path-fact for a change - assert: - that: - - zuul_log_path is defined - - zuul.change in zuul_log_path - - zuul.patchset in zuul_log_path - - zuul.pipeline in zuul_log_path - - zuul.job in zuul_log_path - - zuul.build[:3] == zuul_log_path[:3] + - zuul.tenant in zuul_log_path + - zuul.build == zuul_log_path[-32:] + - zuul.build[:3] + '/' == zuul_log_path[:4] diff --git a/test-playbooks/base-test-roles/emit-job-header.yaml b/test-playbooks/base-test-roles/emit-job-header.yaml index abdd06503..44fb7ff74 100644 --- a/test-playbooks/base-test-roles/emit-job-header.yaml +++ b/test-playbooks/base-test-roles/emit-job-header.yaml @@ -1,7 +1,4 @@ -# Note that the order of these two plays is important. We want the second one -# to run to be the one the creates the correct log path for the currently -# running system. -- name: Test the emit-job-header role without swift +- name: Test the emit-job-header role hosts: all roles: - role: emit-job-header @@ -16,30 +13,6 @@ assert: that: - zuul_log_path is defined - - zuul.change in zuul_log_path - - zuul.patchset in zuul_log_path - - zuul.pipeline in zuul_log_path - - zuul.job in zuul_log_path - - zuul.build[:3] != zuul_log_path[:3] - -- name: Test the emit-job-header role with swift - hosts: all - roles: - - role: emit-job-header - zuul_log_url: "http://logs.openstack.org" - zuul_log_path_shard_build: true - post_tasks: - # All emit-job-header does is a debug statement so the worst that would - # happen would be that the debug task would fail outright and we'd prevent - # something breaking that debug statement from merging just by running it. - # However, the emit-job-header role includes the set-zuul-log-path-fact - # role. We can only test for zuul_log_path against changes, though. - - name: Assert zuul_log_path by set-zuul-log-path-fact for a change - assert: - that: - - zuul_log_path is defined - - zuul.change in zuul_log_path - - zuul.patchset in zuul_log_path - - zuul.pipeline in zuul_log_path - - zuul.job in zuul_log_path - - zuul.build[:3] == zuul_log_path[:3] + - zuul.tenant in zuul_log_path + - zuul.build == zuul_log_path[-32:] + - zuul.build[:3] + '/' == zuul_log_path[:4]