CP-13581 linux-pkg: build windows-connector on-demand, caching result in Artifactory third-party-local instead of rebuilding every run - #407
Conversation
f31f0e7 to
d0a21e4
Compare
be78e4c to
f243840
Compare
f243840 to
719b072
Compare
… in Artifactory third-party-local instead of rebuilding every run PR URL: https://www.github.com/delphix/linux-pkg/pull/407
719b072 to
7ce82a9
Compare
| echo "windows-connector $version already built (latest); reusing it instead of rebuilding" | ||
| logmust cd "$WORKDIR/artifacts" | ||
| logmust aws s3 cp --only-show-errors "$latest_s3_url/$deb_name" . | ||
| reused=true |
There was a problem hiding this comment.
This is not needed, and wastes s3 object space for nothing. The previously built package is already referenced by the "latest" combine-packages object, so there is no reason to have to copy that and call combine-packages again. IMO we should do nothing at all in this case. There is an explicit Jenkins build status to represent that something wasn't built ("NOT_BUILT"). That feels appropriate here. We should exit with some code that results in the build-package job ending as "NOT_BUILT".
prakashsurya
left a comment
prakashsurya
left a comment
There was a problem hiding this comment.
I agree with seb's latest comment..
IMO, if we really want to keep the post-push build trigger on the app-gate git repository, then I think this is overly complicated.. and really, we want a way to denote the build as "skipped".. instead of the S3 file manipulation.. and do nothing when it's skipped, as the prior artifact will already be preserved "forever"..
With that said, since we want to tightly control when we do rebuilds of the connector, I might suggest an even more simple approach.. and disable the post-push trigger entirely, and instead rely on folks to manually trigger new builds.. then, we can carefully control when we rebuild it, and tie that into w/e "recertification" process we currently have (correct me if this doesn't make sense, I'm unfamiliar with the current certification process that's mentioned in the PR description).. and the build script can just be simple like it is today, and always do a build..
To me, this feels awfully complicated for what essentially equates to "do nothing, if connector version hasn't changed".. there's no need to copy the S3 artifacts around..
| if [[ "$reused" == true ]]; then | ||
| echo "reused" >"$WORKDIR/artifacts/BUILD_STATUS" | ||
| else | ||
| echo "built_fresh" >"$WORKDIR/artifacts/BUILD_STATUS" | ||
| fi |
There was a problem hiding this comment.
why does this matter? this essentially equates to it always being "reused", due to the cadence of app-gate pushes to connect version bumps.. regardless, what consumes this, and why?
feels overly complicated for no good reason, IMO..
| echo "windows-connector $version already built (latest); reusing it instead of rebuilding" | ||
| logmust cd "$WORKDIR/artifacts" | ||
| logmust aws s3 cp --only-show-errors "$latest_s3_url/$deb_name" . | ||
| reused=true |
| # | ||
| # No explicit upload here -- the shared post-push Publish stage already | ||
| # syncs $WORKDIR/artifacts to S3 and updates "latest" for us, exactly | ||
| # like every other linux-pkg package. | ||
| # |
There was a problem hiding this comment.
unnecessary comment, this is core behavior provided by the linux-pkg infra, and relied on by every package.. don't think it needs a package specific comment.
| local version | ||
| version=$(grep "project.ext.connectorVersion" "$INSTALLER_DIR/build.gradle" | | ||
| sed -E "s/.*'([^']+)'.*/\1/") | ||
| local deb_name="windows-connector_${version}_all.deb" |
There was a problem hiding this comment.
this assumes a rebuild will produce an artifact of this specific filename.. without actually controlling it here.. IMO, that coupling is fragile, as the filename can change in the app-gate repo, and this repo would then not behave properly (e.g. it'd start to rebuild when we don't want it to)..
3 participants
Problem
The
windows-connectorpackage is rebuilt from scratch on everylinux-pkgpipeline run, even when its version (project.ext.connectorVersioninappliance/host/windows/build.gradle) hasn't changed since the last successful build. Unlike other linux-pkg packages, windows-connector's build toolchain (nsis/mono/mingw-w64) lives outside our own source and can drift independently of any windows-connector code change — rebuilding on every unrelated trigger risks silently shipping a different toolchain combination without anyone deciding to re-certify it. This also masks the underlying dependency-drift bug (see dlpx-app-gate#4648) — the failure only surfaces when a buildserver's live apt mirror happens to have moved out of sync with the previously-pinned Artifactory dependency zips.Solution
Change
config.sh'sbuild()so the package is only rebuilt when its version changes, reusing existing linux-pkg infrastructure rather than adding anything new:latestpost-push artifact viaget_package_dependency_s3_url— the same lookup every linux-pkg package already uses to fetch its own build dependencies. This resolves correctly tosnapshot-de-imagesfor ongoing branches (develop/release/patch) and to the frozen, non-expiring release snapshot for hotfix/release-tag builds, with no special-casing needed here.windows-connector_<version>_all.debalready exists there, download and reuse it instead of rebuilding.gradlew build+createDebPackage) as before. No explicit upload is needed — the shared post-push Publish stage already syncs the result to S3 and updateslatestfor us, exactly like every other package.latestpointer) is ever missing for any reason, it just falls through to a normal build and republishes, rather than aborting.BUILD_STATUS(reusedorbuilt_fresh) into the artifacts directory, so the existing Slack build notification can report which path a given run took (follow-up devops-gate change, tracked separately).This is a revision of the original approach in this PR, which used a bespoke
release-de-images/windows-connector-cache/path. That required no new credential either, but this version needs even less — no new S3 location at all, since it reuses infrastructure (snapshot-de-images'slatest, kept alive indefinitely by the existings3-artifact-refreshjob) that already runs today for every package.Testing Done
Tested end-to-end on real production Jenkins with the revised logic: build #1008 — version
2.3.0.0already cached (build#935's artifact). Confirmed from the console log:windows-connector 2.3.0.0 already built (latest); reusing it instead of rebuilding— the reuse branch was taken;gradlew build/createDebPackagedid not run at all.BUILD_STATUSwas written and picked up by the Publish stage alongside the other build metadata files.#1008'slatest.Finished: SUCCESS.Notes to Reviewers
No devops-gate or dlpx-app-gate changes are required for this PR to be safe to land — it only reads from infrastructure that already exists and is already running. Related, separately-tracked follow-ups:
git reviewpre-commit check (and/or CI check) in dlpx-app-gate requiringconnectorVersionto be bumped wheneverappliance/server/connectororappliance/host/windowssource changes — without it, a merged windows-connector fix that forgets the version bump would be silently served from cache instead of shipping.BUILD_STATUSin the existing Slack build notification.Tracked under epic CP-13579, alongside CP-13582 (app-gate).