prowgen: add disable_sparse_checkout option#5207
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a ChangesDisableSparseCheckout Feature
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 17✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| shouldSkipCloning := len(sparseFiles) == 0 | ||
| if shouldSkipCloning { | ||
| b.base.UtilityConfig.DecorationConfig.SkipCloning = ptr.To(true) | ||
| } else if disableSparseCheckout { |
There was a problem hiding this comment.
This needs to go in the else statement in 104-R109
bc8dcc1 to
9a7742f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/prowgen/jobbase.go (1)
100-109: ⚡ Quick winEliminate code duplication for OAuth token assignment.
The
OauthTokenSecretassignment appears in both thedisableSparseCheckoutbranch (line 102) and the else branch (line 107). This duplication can be eliminated by restructuring the logic.♻️ Proposed refactor to eliminate duplication
sparseFiles := sparseCheckoutFiles(configSpec) shouldSkipCloning := len(sparseFiles) == 0 if shouldSkipCloning { b.base.UtilityConfig.DecorationConfig.SkipCloning = ptr.To(true) - } else if disableSparseCheckout { - if private { - b.base.UtilityConfig.DecorationConfig.OauthTokenSecret = &prowv1.OauthTokenSecret{Key: cioperatorapi.OauthTokenSecretKey, Name: cioperatorapi.OauthTokenSecretName} - } } else { - b.base.UtilityConfig.DecorationConfig.SparseCheckoutFiles = sparseFiles + if !disableSparseCheckout { + b.base.UtilityConfig.DecorationConfig.SparseCheckoutFiles = sparseFiles + } if private { b.base.UtilityConfig.DecorationConfig.OauthTokenSecret = &prowv1.OauthTokenSecret{Key: cioperatorapi.OauthTokenSecretKey, Name: cioperatorapi.OauthTokenSecretName} } }As per coding guidelines, avoid code duplication and prefer clarity in control flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/prowgen/jobbase.go` around lines 100 - 109, The OauthTokenSecret assignment is duplicated in both the disableSparseCheckout and else branches; inside function handling disableSparseCheckout/private logic (references: disableSparseCheckout, private, sparseFiles, b.base.UtilityConfig.DecorationConfig.OauthTokenSecret), remove the repeated assignment from each branch and instead set OauthTokenSecret once after the conditional block when private is true (or set it conditionally before returning), preserving existing behavior for SparseCheckoutFiles when disableSparseCheckout is false; ensure only the branch-specific SparseCheckoutFiles assignment remains inside the else branch and the single OauthTokenSecret assignment is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/prowgen/jobbase.go`:
- Around line 100-109: The OauthTokenSecret assignment is duplicated in both the
disableSparseCheckout and else branches; inside function handling
disableSparseCheckout/private logic (references: disableSparseCheckout, private,
sparseFiles, b.base.UtilityConfig.DecorationConfig.OauthTokenSecret), remove the
repeated assignment from each branch and instead set OauthTokenSecret once after
the conditional block when private is true (or set it conditionally before
returning), preserving existing behavior for SparseCheckoutFiles when
disableSparseCheckout is false; ensure only the branch-specific
SparseCheckoutFiles assignment remains inside the else branch and the single
OauthTokenSecret assignment is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 711299f9-4364-43ff-ba7f-dc1ea73bcdc5
📒 Files selected for processing (5)
pkg/api/types.gopkg/prowgen/jobbase.gopkg/prowgen/jobbase_test.gopkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_images_and_sparse_checkout_disabled.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_private_job_with_images_and_sparse_checkout_disabled.yaml
9a7742f to
1f30fe7
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: droslean, Prucek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test e2e |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
Adds prowgen.disable_sparse_checkout: true to ci-operator configs for openshift/kueue-operator and openshift-kni/cnf-features-deploy. These repos use git submodules, which fail with sparse checkout's blobless fetches (--filter=blob:none) when submodule gitlink entries are excluded from the sparse working tree. Depends on openshift/ci-tools#5207. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/override ci/prow/images |
|
@Prucek: Overrode contexts on behalf of Prucek: ci/prow/images DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Adds prowgen.disable_sparse_checkout: true to ci-operator configs for openshift/kueue-operator and openshift-kni/cnf-features-deploy. These repos use git submodules, which fail with sparse checkout's blobless fetches (--filter=blob:none) when submodule gitlink entries are excluded from the sparse working tree. Depends on openshift/ci-tools#5207. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@Prucek: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Adds prowgen.disable_sparse_checkout: true to ci-operator configs for openshift/kueue-operator and openshift-kni/cnf-features-deploy. These repos use git submodules, which fail with sparse checkout's blobless fetches (--filter=blob:none) when submodule gitlink entries are excluded from the sparse working tree. Depends on openshift/ci-tools#5207. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds prowgen.disable_sparse_checkout: true to ci-operator configs for openshift/kueue-operator and openshift-kni/cnf-features-deploy. These repos use git submodules, which fail with sparse checkout's blobless fetches (--filter=blob:none) when submodule gitlink entries are excluded from the sparse working tree. Depends on openshift/ci-tools#5207. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
disable_sparse_checkoutfield toProwgenOverridesin ci-operator config--filter=blob:none) fails when submodule gitlink entries are modified but the submodule path is excluded from the sparse working treeRepos that need this can add to their ci-operator config:
This is intended as a workaround until kubernetes-sigs/prow#723 lands with a proper fallback in clonerefs.
Adds disable_sparse_checkout option for repository cloning
This PR adds a new ci-operator prowgen configuration option to opt out of the sparse-checkout optimization used during Prow job repository cloning.
Context
Prowgen previously generated jobs that use sparse checkout with blobless fetches (--filter=blob:none) to fetch only files required for CI (e.g., Dockerfiles, ci-operator configs). That optimization can fail for repositories that use git submodules when submodule gitlink entries are modified but the submodule path is excluded from the sparse working tree. This change is a workaround until clonerefs gains a proper fallback (kubernetes-sigs/prow#723).
Changes (user-visible)
disable_sparse_checkouton ProwgenOverrides (pkg/api/types.go). When set, prowgen will generate jobs that perform a full git clone instead of using sparse checkout / blobless fetches.Impact for CI users/operators
Repository owners can opt out of sparse-checkout on a per-repo basis by setting prowgen.disable_sparse_checkout: true in ci-operator configuration. This forces full repository checkouts for generated Prow jobs, avoiding failures for repos that rely on submodules or other cases where sparse checkout breaks, while other repos retain the performance benefits of sparse checkout.