Commit 7ae4f2b
authored
# Refactor: `gsutil` to `gcloud storage` Migration with Per-Command
Feature Flags
## Summary
This pull request fundamentally refactors our `gsutil` to `gcloud
storage` migration strategy. The previous approach used a single global
feature flag, `USE_GCLOUD_STORAGE`, which activated the migration for
all commands simultaneously. This PR replaces that global flag with a
granular, per-command flag system, allowing for a safer, more
controlled, and incremental transition.
The following environment variables have been introduced to individually
control their corresponding commands:
* `USE_GCLOUD_STORAGE_CP`
* `USE_GCLOUD_STORAGE_RSYNC`
* `USE_GCLOUD_STORAGE_LS`
* `USE_GCLOUD_STORAGE_MB`
* `USE_GCLOUD_STORAGE_CORS_SET`
* `USE_GCLOUD_STORAGE_DEFSTORAGECLASS_SET`
* `USE_GCLOUD_STORAGE_IAM_CH`
-----
## Motivation
The migration from `gsutil` to the more modern `gcloud storage` tools is
an important step to modernize our codebase and leverage the performance
and feature improvements of `gcloud`. However, a "big bang" migration
with a single global flag posed a significant risk. A bug or performance
issue in a single command could impact all storage operations, causing
widespread instability.
This new approach mitigates that risk by allowing us to enable and test
the migration for each command in isolation. This makes it easier to
identify issues, roll back specific changes, and ensure a smooth, stable
transition to the new tooling.
-----
## Implementation Details
The main technical changes include:
* **Dynamic Flag Function:**
* The global `use_gcloud_storage()` function has been removed.
* A new function, `use_gcloud_for_command(command: str)`, has been
introduced in `src/clusterfuzz/_internal/google_cloud_utils/gsutil.py`.
This function dynamically constructs the environment variable name
(e.g., `USE_GCLOUD_STORAGE_CP`) based on the provided command and checks
if it is set.
* **`GSUtilRunner` Refactoring:**
* The `GSUtilRunner` in `gsutil.py` no longer relies on an internal
state (`self.use_gcloud_storage`).
* Its methods (`rsync`, `download_file`, etc.) now invoke
`use_gcloud_for_command` with the relevant command to decide at runtime
which CLI (`gsutil` or `gcloud storage`) to use.
* **Direct `gsutil` Call Updates:**
* Scripts that invoked `gsutil` directly, such as
`src/local/butler/common.py` and
`src/clusterfuzz/_internal/scripts/copy_corpus.py`, have been refactored
to use the new feature flag logic.
* Shell scripts in `configs/` and `docker/` have also been updated to
check the new command-specific environment variables.
* **User Interface:**
* The upload instruction message in the `corpora` handler
(`src/appengine/handlers/corpora.py`) now dynamically displays the
`gsutil rsync` or `gcloud storage rsync` command, depending on the state
of the `USE_GCLOUD_STORAGE_RSYNC` flag, ensuring users see the correct
command.
-----
## Rollout Strategy and How to Use
Activating the new functionality is done via environment variables. By
default, all commands will continue to use `gsutil`.
To enable `gcloud storage` for a specific command, set the corresponding
environment variable to a truthy value (e.g., `"1"` or `"True"`).
**Example:** To test the `cp` command using `gcloud storage`, set the
following environment variable:
```bash
export USE_GCLOUD_STORAGE_CP=1
```
To revert to `gsutil`, simply unset the environment variable or set it
to an empty string.
### Recommended Rollout Strategy:
1. **Staging Environment**: Start by enabling the flags one by one in a
staging or testing environment.
2. **Monitoring**: After enabling each flag, closely monitor logs,
performance metrics, and related functionality to ensure there are no
regressions.
3. **Incremental Rollout**: We suggest the following order for a
production rollout, starting with lower-risk commands:
* `ls`
* `mb`, `cors set`, `defstorageclass set`
* `cp`
* `rsync`
4. **Production**: Once a command has been thoroughly tested and
validated in staging, the corresponding flag can be enabled in
production.
5. **Migration Completion**: The ultimate goal is to have all flags
enabled in production. After a stabilization period, we can plan a
future refactoring to completely remove the legacy `gsutil` code and the
feature flags themselves.
-----
## Tests
* All existing unit tests in `gsutil_test.py` and `deploy_test.py` have
been updated to use the new per-command feature flags.
* Tests have been parameterized to ensure each command functions
correctly with both `gsutil` (flag disabled) and `gcloud storage` (flag
enabled).
* All tests, linters, and formatters are passing, ensuring code quality
and consistency.
1 parent aa66083 commit 7ae4f2b
File tree
21 files changed
+1372
-809
lines changed- .github/workflows
- configs/test
- bot/setup
- gce
- docker/base
- src
- appengine/handlers
- clusterfuzz/_internal
- cron
- google_cloud_utils
- scripts
- tests/core
- google_cloud_utils
- local/butler
- local/butler
21 files changed
+1372
-809
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
27 | | - | |
28 | | - | |
| 26 | + | |
| 27 | + | |
29 | 28 | | |
30 | 29 | | |
31 | 30 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
68 | 76 | | |
69 | 77 | | |
70 | 78 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | 77 | | |
89 | 78 | | |
90 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
91 | 97 | | |
92 | 98 | | |
93 | 99 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | 72 | | |
84 | 73 | | |
85 | | - | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
86 | 92 | | |
87 | 93 | | |
88 | 94 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | 68 | | |
77 | 69 | | |
78 | | - | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
79 | 85 | | |
80 | 86 | | |
81 | 87 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
95 | | - | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
96 | 100 | | |
97 | 101 | | |
98 | 102 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
270 | 270 | | |
271 | 271 | | |
272 | 272 | | |
| 273 | + | |
273 | 274 | | |
274 | 275 | | |
275 | | - | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
276 | 281 | | |
277 | 282 | | |
278 | 283 | | |
| |||
0 commit comments