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

use 8.10 instead of 8.11 for stack version and .package.version #3276

Closed
wants to merge 11 commits into from

Conversation

AndersonQ
Copy link
Member

What does this PR do?

use 8.10 instead of 8.11 for stack version and .package-version

Why is it important?

8.11 stack isn't available yet on QA/Staging

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

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?
  • ...

@AndersonQ AndersonQ added the bug Something isn't working label Aug 22, 2023
@AndersonQ AndersonQ requested a review from a team as a code owner August 22, 2023 14:31
@AndersonQ AndersonQ self-assigned this Aug 22, 2023
@mergify
Copy link
Contributor

mergify bot commented Aug 22, 2023

This pull request does not have a backport label. Could you fix it @AndersonQ? 🙏
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.

@cmacknz cmacknz added skip-changelog Team:Elastic-Agent Label for the Agent team labels Aug 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

Copy link
Member

@cmacknz cmacknz left a comment

Choose a reason for hiding this comment

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

We are going to need to make these changes again in the next feature freeze period. Can the code be refactored out into a dedicated package so both pieces live together? Maybe call the package featurefreeze or something to make the purpose obvious.

pkg/testing/fixture.go Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

elasticmachine commented Aug 22, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-08-24T04:57:58.347+0000

  • Duration: 29 min 38 sec

Test stats 🧪

Test Results
Failed 0
Passed 6305
Skipped 47
Total 6352

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented Aug 22, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.75% (79/80) 👍
Files 67.832% (194/286) 👍
Classes 66.292% (354/534) 👍
Methods 52.969% (1106/2088) 👍 0.096
Lines 38.784% (12677/32686) 👍 0.078
Conditionals 100.0% (0/0) 💚

@pierrehilbert
Copy link
Contributor

pierrehilbert commented Aug 22, 2023

If you can just fix the linter things it would be great :-)

@cmacknz
Copy link
Member

cmacknz commented Aug 22, 2023

I ran this locally to try a single test: TEST_PLATFORMS=linux/amd64 SNAPSHOT=true INSTANCE="multipass" mage integration:single TestInstallAndCLIUninstallWithEndpointSecurity

It started an 8.10 stack but the .package-version edit doesn't seem to be taking effect because Fleet is rejecting the version of the agent:

>>> (linux-amd64-ubuntu-2204) Test output (sudo) (stdout): Error: fail to enroll: fail to execute request to fleet-server: status code: 400, fleet-server returned an error: UnsupportedVersion, message: version is not supported

@AndersonQ AndersonQ changed the title use 8.10 instead of 8.11 for stack version and .package-version use 8.10 instead of 8.11 for stack version and .package.version Aug 22, 2023
@cmacknz
Copy link
Member

cmacknz commented Aug 22, 2023

I branched from this PR and created one with the fix to get the tests to run #3277

@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

You need #3281 to get the build to succeed reliably.

@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Aug 23, 2023

update

✅ Branch has been successfully updated

@AndersonQ AndersonQ enabled auto-merge (squash) August 23, 2023 18:45
@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

FYI you had the agent package version set to a non-snapshot version causing some of these test failures. Not sure if we needed anything else for it to pass:

-AGENT_PACKAGE_VERSION=8.10.0 \
+AGENT_PACKAGE_VERSION=8.10.0-SNAPSHOT \

@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

I don't know if we need 49c0770

@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

The actual bug with the package version test is that we are getting 8.10.0 as the agent version from this block of code instead of 8.10.0-SNAPSHOT

func getAgentVersion(t *testing.T, f *integrationtest.Fixture, ctx context.Context, binaryOnly bool) []byte {
args := []string{"version", "--yaml"}
if binaryOnly {
args = append(args, "--binary-only")
}
versionCmd, err := f.PrepareAgentCommand(ctx, args)
require.NoError(t, err, "error preparing agent version command")
actualVersionBytes, err := versionCmd.Output()
require.NoError(t, err, "error executing 'version' command. Output %q", string(actualVersionBytes))
return actualVersionBytes
}

@cmacknz
Copy link
Member

cmacknz commented Aug 23, 2023

This is because we need both AGENT_PACKAGE_VERSION=8.10.0-SNAPSHOT and SNAPSHOT=true when running mage package but this gives us the final .tar.gz with the -SNAPSHOT-SNAPSHOT prefix from both places 🤦

Edit: this is only weird, but not the actual root cause of the test failure.

@cmacknz
Copy link
Member

cmacknz commented Aug 24, 2023

Just using BEAT_VERSION=8.10.0 and SNAPSHOT=true will fix this, with the consequence that we use the 8.10.0 Beats.

@cmacknz
Copy link
Member

cmacknz commented Aug 24, 2023

Apparently there is even more to this problem, the elastic-agent version --yaml output indicates whether a version is a snapshot or not with a separate boolean flag that the test logic doesn't parse:

❯ sudo elastic-agent version --yaml
binary:
  version: 8.10.0
  commit: e2a3a254604bfb4902e21a8ee2495af7ce1e4ce9
  build_time: 2023-08-24T00:32:56Z
  snapshot: true
daemon:
  version: 8.10.0
  commit: e2a3a254604bfb4902e21a8ee2495af7ce1e4ce9
  build_time: 2023-08-24T00:32:56Z
  snapshot: true

We ignore the snapshot parameter here causing us to try to download a version that doesn't exist:

// unmarshalVersionOutput retrieves the version string for binary or daemon from "version" subcommand yaml output
func unmarshalVersionOutput(t *testing.T, cmdOutput []byte, binaryOrDaemonKey string) string {
versionCmdOutput := map[string]any{}
err := yaml.Unmarshal(cmdOutput, &versionCmdOutput)
require.NoError(t, err, "error parsing 'version' command output")
require.Contains(t, versionCmdOutput, binaryOrDaemonKey)
return versionCmdOutput[binaryOrDaemonKey].(map[any]any)["version"].(string)
}

I am confused about how this ever passed on the 8.10 branch because the 8.10 version doesn't exist either:

failed download of agent binary: unable to download package: 3 errors occurred:
        	            		* package '/opt/Elastic/Agent/data/elastic-agent-1071b4/downloads/elastic-agent-8.10.0-linux-x86_64.tar.gz' not found: open /opt/Elastic/Agent/data/elastic-agent-1071b4/downloads/elastic-agent-8.10.0-linux-x86_64.tar.gz: no such file or directory
        	            		* call to 'https://snapshots.elastic.co/8.10.0-ffe0de5c/downloads/beats/elastic-agent/elastic-agent-8.10.0-linux-x86_64.tar.gz' returned unsuccessful status code: 404
        	            		* call to 'https://artifacts.elastic.co/downloads/beats/elastic-agent/elastic-agent-8.10.0-linux-x86_64.tar.gz' returned unsuccessful status code: 404
        	            		*

@cmacknz
Copy link
Member

cmacknz commented Aug 24, 2023

I am going to create a separate PR with ad73102 to fix the bug in the GetPrevMinor function, this will also let me test out the 8.11 snapshot on main.

We still have work to do to make this work in the next minor, but now that there is an 8.11 snapshot we can leave this PR alone and focus on fixing the tests on main.

@cmacknz
Copy link
Member

cmacknz commented Aug 24, 2023

#3289

@AndersonQ
Copy link
Member Author

closing as it isn't needed right now, of we need it again, we can reopen it.

@AndersonQ AndersonQ closed this Sep 4, 2023
auto-merge was automatically disabled September 4, 2023 08:16

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip bug Something isn't working skip-changelog Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants