Retry gcloud SBOM command#3810
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the reliability of the SBOM generation process within the Maven plugin. By wrapping the gcloud command execution in a retry block with exponential backoff, the system is now better equipped to handle intermittent network or service-related issues, reducing the likelihood of build failures. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a manual retry loop using FluentBackoff for the SBOM generation command in TemplatesStageMojo.java. Feedback indicates that this manual retry logic is redundant because the method is already wrapped in a higher-level retry policy. Additionally, a logic bug was identified where the backoff state is advanced twice per iteration, and the new imports would become unnecessary if the manual loop is removed as suggested.
| String output = null; | ||
|
|
||
| BackOff backOff = | ||
| FluentBackoff.DEFAULT | ||
| .withInitialBackoff(org.joda.time.Duration.standardSeconds(5)) | ||
| .withMaxBackoff(org.joda.time.Duration.standardSeconds(30)) | ||
| .withMaxRetries(5) | ||
| .backoff(); | ||
| Sleeper sleeper = Sleeper.DEFAULT; | ||
| Exception lastException; | ||
| do { | ||
| try { | ||
| output = | ||
| runCommandCapturesOutput( | ||
| new String[] {"gcloud", "artifacts", "sbom", "export", "--uri", imagePathTag}, | ||
| null); | ||
| lastException = null; | ||
| break; | ||
| } catch (Exception e) { | ||
| LOG.error( | ||
| "Error generating SBOM. Retrying after {} seconds...", | ||
| backOff.nextBackOffMillis() / 1000, | ||
| e); | ||
| lastException = e; | ||
| } | ||
| } while (BackOffUtils.next(sleeper, backOff)); | ||
| if (lastException != null) { | ||
| throw new RuntimeException("Error generating SBOM.", lastException); | ||
| } |
There was a problem hiding this comment.
The manual retry loop introduced here is redundant and contains a logic bug.
- Redundancy: This
run()method is already wrapped in aFailsaferetry policy at line 611 (usingsbomRetryPolicy()). Adding a second layer of retries here results in nested retry logic (e.g., 5 retries inside 5 retries), which is likely unintended and significantly increases the total execution time on failure. - Logic Bug: Calling
backOff.nextBackOffMillis()inside the log statement (line 1805) and then again inBackOffUtils.next()(line 1809) advances the state of theBackOffobject twice per iteration. This causes the loop to skip backoff intervals and terminate after fewer attempts than configured.
It is recommended to remove this manual loop and instead update the parameters in sbomRetryPolicy() (lines 1822-1831) if the retry behavior needs adjustment (e.g., to match the 5s-30s backoff used here).
| String output = null; | |
| BackOff backOff = | |
| FluentBackoff.DEFAULT | |
| .withInitialBackoff(org.joda.time.Duration.standardSeconds(5)) | |
| .withMaxBackoff(org.joda.time.Duration.standardSeconds(30)) | |
| .withMaxRetries(5) | |
| .backoff(); | |
| Sleeper sleeper = Sleeper.DEFAULT; | |
| Exception lastException; | |
| do { | |
| try { | |
| output = | |
| runCommandCapturesOutput( | |
| new String[] {"gcloud", "artifacts", "sbom", "export", "--uri", imagePathTag}, | |
| null); | |
| lastException = null; | |
| break; | |
| } catch (Exception e) { | |
| LOG.error( | |
| "Error generating SBOM. Retrying after {} seconds...", | |
| backOff.nextBackOffMillis() / 1000, | |
| e); | |
| lastException = e; | |
| } | |
| } while (BackOffUtils.next(sleeper, backOff)); | |
| if (lastException != null) { | |
| throw new RuntimeException("Error generating SBOM.", lastException); | |
| } | |
| String output; | |
| try { | |
| output = | |
| runCommandCapturesOutput( | |
| new String[] {"gcloud", "artifacts", "sbom", "export", "--uri", imagePathTag}, | |
| null); | |
| } catch (Exception e) { | |
| throw new RuntimeException("Error generating SBOM.", e); | |
| } |
There was a problem hiding this comment.
Yeah it's true that it's already scanned. Checked logs:
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
[INFO] Generating system SBOM for us.gcr.io/dataflow-templates/yaml-template:2026-05-05-00_RC00...
[INFO] Running: gcloud
Error is
ERROR: (gcloud.artifacts.sbom.export) NOT_FOUND: unable to find discovery occurrence for https://us.gcr.io/dataflow-templates/yaml-template@sha256:1d71b93f588eba08cceaa5ec3057439497bf85bbf7189684b47955beab10251e. Please first scan the artifact.
actually it's due to previously docker build gcb job timed out:
ERROR: (gcloud.builds.submit) build a96c34f7-12ae-4bc0-b149-6cae07ceef5b completed with status "TIMEOUT"
but the docker file was pushed, so pushed container existence check, but likely it's actually incomplete and gcloud artfiacts sbom export command would fail.
| import org.apache.beam.sdk.util.BackOff; | ||
| import org.apache.beam.sdk.util.BackOffUtils; | ||
| import org.apache.beam.sdk.util.FluentBackoff; | ||
| import org.apache.beam.sdk.util.Sleeper; |
There was a problem hiding this comment.
These imports are no longer needed if the manual retry loop in GenerateSBOMRunnable is removed as suggested. Keeping unused imports can lead to confusion and slightly increases the overhead of the class.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3810 +/- ##
============================================
- Coverage 53.73% 53.71% -0.02%
Complexity 6741 6741
============================================
Files 1087 1087
Lines 66804 66807 +3
Branches 7479 7480 +1
============================================
- Hits 35897 35888 -9
- Misses 28479 28490 +11
- Partials 2428 2429 +1
🚀 New features to boost your workflow:
|
2 participants
No description provided.