Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add manifest to package #3910

Merged
merged 6 commits into from
Jan 26, 2024
Merged

Add manifest to package #3910

merged 6 commits into from
Jan 26, 2024

Conversation

pchila
Copy link
Member

@pchila pchila commented Dec 13, 2023

What does this PR do?

This PR adds a manifest to .zip, tar.gz, .deb and .rpm packages according to the outcome of #3639
In this PR the manifest is only generated and included in the packages but not yet used by install or upgrade processes.

Why is it important?

This is a prerequisite for #3950 where the manifest included in the package will be used for remapping package directories during install and upgrade

Checklist

  • 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/fragments using the changelog tool
  • [ ] I have added an integration test or an E2E test

Author's Checklist

  • [ ]

How to test this PR locally

It can be interesting having a look at .tar.gz or .zip packages and see how the manifest content (especially path mappings) changes when specifying different versions from the same commit for example:

SNAPSHOT=true PLATFORMS="linux/amd64" PACKAGES="tar.gz"  mage package

and

 AGENT_PACKAGE_VERSION=8.13.1-exp SNAPSHOT=true PLATFORMS="linux/amd64" PACKAGES="tar.gz"  mage package

Related issues

Use cases

Screenshots

Logs

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@mergify mergify bot assigned pchila Dec 13, 2023
Copy link
Contributor

mergify bot commented Dec 13, 2023

This pull request does not have a backport label. Could you fix it @pchila? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@pchila pchila force-pushed the add-manifest-to-package branch 2 times, most recently from aeca3a3 to cc0398b Compare December 21, 2023 16:45
@pchila pchila mentioned this pull request Dec 21, 2023
5 tasks
@pchila pchila force-pushed the add-manifest-to-package branch from cc0398b to 9e6c04f Compare January 9, 2024 10:50
Copy link
Contributor

mergify bot commented Jan 10, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b add-manifest-to-package upstream/add-manifest-to-package
git merge upstream/main
git push upstream add-manifest-to-package

@pchila pchila force-pushed the add-manifest-to-package branch from 9e6c04f to f90a1d1 Compare January 10, 2024 10:21
@pchila pchila force-pushed the add-manifest-to-package branch 4 times, most recently from d202cc9 to 39de109 Compare January 24, 2024 08:36
@pchila pchila added enhancement New feature or request Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team skip-changelog labels Jan 24, 2024
@pchila pchila marked this pull request as ready for review January 24, 2024 15:35
@pchila pchila requested a review from a team as a code owner January 24, 2024 15:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@pchila
Copy link
Member Author

pchila commented Jan 25, 2024

buildkite test this

@@ -19,11 +20,13 @@ import (

"golang.org/x/text/cases"
"golang.org/x/text/language"
"gopkg.in/yaml.v3"

"github.com/magefile/mage/sh"
"golang.org/x/tools/go/vcs"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created dedicated issue for vcs removal #4138

@pchila pchila force-pushed the add-manifest-to-package branch 2 times, most recently from 44766ec to 1ddae31 Compare January 25, 2024 18:31
@cmacknz
Copy link
Member

cmacknz commented Jan 25, 2024

This could have a changelog, it could also just be referenced in #3950.

This is not strictly user facing, but it is something support will want to know exists.

@cmacknz
Copy link
Member

cmacknz commented Jan 25, 2024

I checked this out and built it, it seems to require me to clean out the contents of build/distributions/* first.

AGENT_PACKAGE_VERSION=8.13.0 EXTERNAL=true SNAPSHOT=true PLATFORMS=darwin/arm64 PACKAGES=targz mage -v package

    package_test.go:203: Checking file manifest.yaml
    package_test.go:206: opening manifest manifest.yaml : open /var/folders/n3/mrw1c3451wvgcll_c5kmq7v40000gn/T/TestZipelastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file3646610666/001/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64/manifest.yaml: no such file or directory
    package_test.go:216: unmarshaling package manifest: yaml: input error: invalid argument
    package_test.go:219:
                Error Trace:    /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:219
                                                        /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:198
                Error:          Not equal:
                                expected: "PackageManifest"
                                actual  : ""

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -PackageManifest
                                +
                Test:           TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file
                Messages:       manifest specifies wrong kind
    package_test.go:220:
                Error Trace:    /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:220
                                                        /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:198
                Error:          Not equal:
                                expected: "co.elastic.agent/v1"
                                actual  : ""

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -co.elastic.agent/v1
                                +
                Test:           TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file
                Messages:       manifest specifies wrong api version
    package_test.go:222:
                Error Trace:    /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:222
                                                        /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:198
                Error:          Should NOT be empty, but was []
                Test:           TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file
                Messages:       path mappings in manifest are empty
    package_test.go:210:
                Error Trace:    /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:210
                                                        /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:234
                                                        /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go:198
                Error:          Received unexpected error:
                                invalid argument
                Test:           TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file
                Messages:       error closing manifest file
--- FAIL: TestZip (5.82s)
    --- PASS: TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_config_file_permissions (0.00s)
    --- PASS: TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_manifest_file_permissions (0.00s)
    --- PASS: TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_modules.d_file_permissions (0.00s)
    --- PASS: TestZip/License_file_LICENSE.txt (0.00s)
    --- PASS: TestZip/License_file_NOTICE.txt (0.00s)
    --- FAIL: TestZip/elastic-agent-8.13.0-SNAPSHOT-windows-x86_64.zip_check_manifest_file (5.80s)
=== RUN   TestDocker
--- PASS: TestDocker (0.00s)
FAIL
FAIL    command-line-arguments  16.292s
FAIL
package ran for 3m25.13905725s
Error: running "go test -v /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/dev-tools/packaging/package_test.go -root-owner -files /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/build/distributions/*" failed with exit code 1

That is a failure trying to verify a windows package I built prior to this change.

I think this is fine, just requires an FYI to people that they are likely to hit this after this merges.

@cmacknz
Copy link
Member

cmacknz commented Jan 25, 2024

Can you add some documentation on what the manifest file is and how it gets used? If it only makes sense to do this after #3950 that is fine, but it should be documented somewhere in the repository itself.

@pchila pchila force-pushed the add-manifest-to-package branch from 1ddae31 to aec6b0c Compare January 26, 2024 08:26
@pchila
Copy link
Member Author

pchila commented Jan 26, 2024

@cmacknz Regarding the package test failure, I am assuming (and checking) in the package tests that the manifest file is present and contains valid information.
The issue is that our regex collects every .zip and every .tar.gz file as package to be tested, we can think in the future to select the files based on version instead (we need to fiddle a bit with the regular expression to insert the version being built).

As for documentation, I will probably add something in #3950 since I have to document also how the upgrade works with the manifest and the remapping

Copy link

@pchila pchila merged commit b39b9af into elastic:main Jan 26, 2024
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip enhancement New feature or request skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants