[Ind Agent] Update packaging to properly package from manifest if given #4885
Conversation
|
This pull request does not have a backport label. Could you fix it @dwhyrock? 🙏
NOTE: |
dwhyrock
added
backport-v8.14.0
|
I've added in some code here so that the packaging code looks for the expected version (e.g., However, it's starting to get a little complex. The latest error I'm seeing is this: It appears as if it's not copying the proper I'm thinking that for Then we could use that var as kind of a gate around the logic I have put in here already that includes a bumped version, and for the copying of the Thoughts? @gogochan @cmacknz @pchila @pierrehilbert |
|
IMO, having optional |
dwhyrock
changed the title
dev-tools/mage/manifest/manifest.go
Outdated
| // For Independent Agent Releases, any opted-in projects will have a version that is | ||
| // one Patch version ahead since we are using the new bumped DRA artifacts for those projects. | ||
| // This is acceptable since it is being released only as bundled with Elastic Agent. | ||
| patchPlusOneString := fmt.Sprintf("%d.%d.%d", parsedManifestVersion.Major(), parsedManifestVersion.Minor(), parsedManifestVersion.Patch()+1) |
There was a problem hiding this comment.
Is there a reason why the input manifest can't specify the patch+1 version? That would keep the logic for this in the release jobs, with the agent package target just doing what the manifest tells it to.
There was a problem hiding this comment.
that may be a decent way to go.
I think the way it works is that it uses the version value in the header, which is something like 8.13.4+build202406101001. And then we strip off the metadata to get the base 3-number version for the previously-released (opted-out) projects.
We could probably somewhat easily add something like an optin_version in the header, but that doesn't tell us exactly which projects should use that version.
As it is now, we'd probably have to go through and parse all of the packages in each of the projects and find any that have a filename that includes the patch+1 version in it.
Perhaps we could also include just a list of opted_in_projects at the top level?
We probably need to ask @DaveSys911 whether adding fields to the Ind Agent manifest would be a big issue or not.
Instead of an environment variable, can we have the manifest specify the mixed set of versions? Alternatively, could this variable just be in the manifest? That way the manifest would define the contract for what agent should do. |
|
CC @pchila for opinions since I think he implemented the packaging target that is used here. |
|
@cmacknz @dwhyrock The main problem for including dependencies with different versions resides in I think the best solution to support all the flows below:
This is obviously a bigger undertaking that trying patch +-1 but it should ensure stability and correctness. As you can see in the list of ways to package an agent above we have quite a lot of legacy package processes to support and I feel that adding an optional manifest is the least disruptive (will require quite a bit of refactoring though) |
|
Thanks for the great response, @pchila ! I agree that a more robust solution is probably better for the long and should make it more reliable by relying on the manifest. I'll work on changes based on your recommendation. When closer to completion, I may need help verifying that it works in all of those cases as I'm not fully fluent in how they are executed. |
Co-authored-by: Craig MacKenzie <[email protected]>
| "prefix": "", | ||
| "projects": { | ||
| "apm-server": { | ||
| "branch": "main", |
There was a problem hiding this comment.
Not part of this PR, but seeing this as main is a bit weird since it should be `8.14.
Add contexts where necessary
|
Tried the following and they both work: The only oddity was the 8.14.0 manifest created a |
|
…en (#4885) * Download from manifest if version has +1 patch * Try modified globExpr * use version package * catch err * use panic * Add the filepath * better package finding * more intermediate work * more progress * it seems to maybe work * fixed bug * Copying spec files as well * temp test value * fixing linting errors * Clean up manifest code * Cleaning up * Cleanup 2 * removing test variable * addressing PR comments and cleanup * Adding manifest tests * Update magefile.go Co-authored-by: Craig MacKenzie <[email protected]> * Switch to go:embed for tests. * Build component specs from external binaries. * Convert component to project in var names * Return error when package not found. Add contexts where necessary * Filter unsupported platforms. * Fix darwin/arm64 build. * Several renames for consistency. * A few more renames. * Move code out of magefile * mage fmt * Fix log message. * Fix lint warnings. * Rename test. * Refactor to share download from manifest logic. --------- Co-authored-by: Craig MacKenzie <[email protected]> Co-authored-by: Pierre HILBERT <[email protected]> (cherry picked from commit 0e7a211)
…en (#4885) (#5039) * Download from manifest if version has +1 patch * Try modified globExpr * use version package * catch err * use panic * Add the filepath * better package finding * more intermediate work * more progress * it seems to maybe work * fixed bug * Copying spec files as well * temp test value * fixing linting errors * Clean up manifest code * Cleaning up * Cleanup 2 * removing test variable * addressing PR comments and cleanup * Adding manifest tests * Update magefile.go Co-authored-by: Craig MacKenzie <[email protected]> * Switch to go:embed for tests. * Build component specs from external binaries. * Convert component to project in var names * Return error when package not found. Add contexts where necessary * Filter unsupported platforms. * Fix darwin/arm64 build. * Several renames for consistency. * A few more renames. * Move code out of magefile * mage fmt * Fix log message. * Fix lint warnings. * Rename test. * Refactor to share download from manifest logic. --------- Co-authored-by: Craig MacKenzie <[email protected]> Co-authored-by: Pierre HILBERT <[email protected]> (cherry picked from commit 0e7a211) Co-authored-by: Doug W <[email protected]>
What does this PR do?
This PR updates the packaging code to operate differently when packaging from a local build vs packaging from a manifest.
The manifest packaging relies on the items in the manifest, while the local one uses globbing to find all packages/directories that match the expected version string.
Why is it important?
For the Independent Agent Release, a manifest is created that includes opted-in projects that have a patch version one higher than the rest of the opted-out/previously-released projects. The file globbing expecting one single version is what skips the opted-in projects.
Previously, we had been packaging the incorrect opted-in projects, which is why we had verified those projects were properly packaged. But when that bug was fixed, we did not properly verify that everything was still there since the packaging still succeeded.
Checklist
packaging dependencies with a manifest for an Independent Agent build
packaging dependencies with a manifest for a Unified Release / Stack build
packaging dependencies pulling the latest unified release version
packaging dependencies pulling from a DROP_PATH
packaging by building beats dependencies located side by side with the agent code
My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in
./changelog/fragmentsusing the changelog toolI have added an integration test or an E2E test
Disruptive User Impact
How to test this PR locally
Related issues
Questions to ask yourself