Run Package stage on PR builds

[ycombinator]

What does this PR do?

This PR ensures that the Package stage is run as part of PR builds, not just main builds.

Why is it important?

To avoid having PR builds succeed only to have the PR's changes fail later after they're merged into main. See #2475 (comment) for an example.

[mergify]

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

[ycombinator]

The line that is being removed in this PR used to be commented out but was un-commented-out in https://github.com/elastic/elastic-agent/pull/36/files#r812300322.

Perhaps @v1v can shed some light on why that change was made so we're not introducing some other issues by undoing it via this PR.

[elasticmachine]

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-04-19T02:44:36.723+0000

• Duration: 17 min 8 sec

Test stats 🧪

Test	Results
Failed	2
Passed	5533
Skipped	19
Total	5554

Test errors

Expand to view the tests failures

Test / Matrix - PLATFORM = 'windows-2016 && windows-immutable' / Test / Test_runDispatcher/no_gateway_actions,_dispatcher_is_flushed – github.com/elastic/elastic-agent/internal/pkg/agent/application

Expand to view the error details

 Failed 

Expand to view the stacktrace

 === RUN   Test_runDispatcher/no_gateway_actions,_dispatcher_is_flushed
    --- FAIL: Test_runDispatcher/no_gateway_actions,_dispatcher_is_flushed (0.14s)
panic: 
assert: mock: The method has been called over 1 times.
	Either do one more Mock.On("Dispatch").Return(...), or remove extra call.
	This call was unexpected:
		Dispatch(*context.timerCtx,*application.mockAcker,[]fleetapi.Action)
		0: &context.timerCtx{cancelCtx:context.cancelCtx{Context:(*context.emptyCtx)(0xc000038128), mu:sync.Mutex{state:0, sema:0x0}, done:atomic.Value{v:(chan struct {})(0xc00069d3e0)}, children:map[context.canceler]struct {}(nil), err:error(nil)}, timer:(*time.Timer)(0xc00057c910), deadline:time.Time{wall:0xc107f4291edc2008, ext:381248001, loc:(*time.Location)(0x1b13680)}}
		1: &application.mockAcker{Mock:mock.Mock{ExpectedCalls:[]*mock.Call(nil), Calls:[]mock.Call(nil), test:mock.TestingT(nil), testData:objx.Map(nil), mutex:sync.Mutex{state:0, sema:0x0}}}
		2: []fleetapi.Action(nil)
	at: [C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:25 C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode.go:229 C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:126] [recovered]
	panic: 
assert: mock: The method has been called over 1 times.
	Either do one more Mock.On("Dispatch").Return(...), or remove extra call.
	This call was unexpected:
		Dispatch(*context.timerCtx,*application.mockAcker,[]fleetapi.Action)
		0: &context.timerCtx{cancelCtx:context.cancelCtx{Context:(*context.emptyCtx)(0xc000038128), mu:sync.Mutex{state:0, sema:0x0}, done:atomic.Value{v:(chan struct {})(0xc00069d3e0)}, children:map[context.canceler]struct {}(nil), err:error(nil)}, timer:(*time.Timer)(0xc00057c910), deadline:time.Time{wall:0xc107f4291edc2008, ext:381248001, loc:(*time.Location)(0x1b13680)}}
		1: &application.mockAcker{Mock:mock.Mock{ExpectedCalls:[]*mock.Call(nil), Calls:[]mock.Call(nil), test:mock.TestingT(nil), testData:objx.Map(nil), mutex:sync.Mutex{state:0, sema:0x0}}}
		2: []fleetapi.Action(nil)
	at: [C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:25 C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode.go:229 C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:126]

goroutine 84 [running]:
testing.tRunner.func1.2({0x127a5c0, 0xc0006d1c20})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/.gvm/versions/go1.19.8.windows.amd64/src/testing/testing.go:1396 +0x24e
testing.tRunner.func1()
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/.gvm/versions/go1.19.8.windows.amd64/src/testing/testing.go:1399 +0x39f
panic({0x127a5c0, 0xc0006d1c20})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/.gvm/versions/go1.19.8.windows.amd64/src/runtime/panic.go:884 +0x212
github.com/stretchr/testify/mock.(*Mock).fail(0xc00057c870, {0x1401e5e?, 0x4?}, {0xc000700d80?, 0x3?, 0x3?})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/pkg/mod/github.com/stretchr/testify@v1.8.2/mock/mock.go:328 +0x145
github.com/stretchr/testify/mock.(*Mock).MethodCalled(0xc00057c870, {0x169803d, 0x8}, {0xc0006d36e0, 0x3, 0x3})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/pkg/mod/github.com/stretchr/testify@v1.8.2/mock/mock.go:472 +0x2ac
github.com/stretchr/testify/mock.(*Mock).Called(0x0?, {0xc0006d36e0, 0x3, 0x3})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/pkg/mod/github.com/stretchr/testify@v1.8.2/mock/mock.go:456 +0x148
github.com/elastic/elastic-agent/internal/pkg/agent/application.(*mockDispatcher).Dispatch(0xc00057c870, {0x15234a8?, 0xc000702300}, {0x1520018?, 0xc00057c8c0}, {0x0, 0x0, 0x0})
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:25 +0x125
github.com/elastic/elastic-agent/internal/pkg/agent/application.runDispatcher({0x15234a8, 0xc000702300}, {0x1520040, 0xc00057c870}, {0x15236a0, 0xc00057c820}, {0x1520018, 0xc00057c8c0}, 0x190ef70?)
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode.go:229 +0x17f
github.com/elastic/elastic-agent/internal/pkg/agent/application.Test_runDispatcher.func7(0xc0006c2b60?)
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/src/github.com/elastic/elastic-agent/internal/pkg/agent/application/managed_mode_test.go:126 +0xec
testing.tRunner(0xc0006c3380, 0xc0006d17e0)
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/.gvm/versions/go1.19.8.windows.amd64/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
	C:/Users/jenkins/workspace/_agent_elastic-agent-mbp_PR-2516/.gvm/versions/go1.19.8.windows.amd64/src/testing/testing.go:1493 +0x35f
 

Test / Matrix - PLATFORM = 'windows-2016 && windows-immutable' / Test / Test_runDispatcher – github.com/elastic/elastic-agent/internal/pkg/agent/application

Expand to view the error details

 Failed 

Expand to view the stacktrace

 === RUN   Test_runDispatcher
--- FAIL: Test_runDispatcher (0.35s)
 

Steps errors

Expand to view the steps failures

Go package

• Took 3 min 45 sec . View more details here
• Description: mage package ironbank

Go package

• Took 2 min 46 sec . View more details here
• Description: mage package ironbank

Go unitTest

• Took 3 min 2 sec . View more details here
• Description: mage unitTest

Checks if running on a Unix-like node

• Took 0 min 0 sec . View more details here
• Description: script returned exit code 1

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Genuine test errors

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

• Name: Test / Matrix - PLATFORM = 'windows-2016 && windows-immutable' / Test / Test_runDispatcher/no_gateway_actions,_dispatcher_is_flushed – github.com/elastic/elastic-agent/internal/pkg/agent/application
• Name: Test / Matrix - PLATFORM = 'windows-2016 && windows-immutable' / Test / Test_runDispatcher – github.com/elastic/elastic-agent/internal/pkg/agent/application

🤖 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]

🌐 Coverage report

Name	Metrics % (covered/total)	Diff
Packages	98.529% (67/68)	👍
Files	68.22% (161/236)	👍
Classes	67.785% (303/447)	👍
Methods	54.111% (928/1715)	👍
Lines	39.63% (10508/26515)	👍 0.011
Conditionals	100.0% (0/0)	💚

[v1v]

Perhaps @v1v can shed some light on why that change was made so we're not introducing some other issues by undoing it via this PR.

👋 , it was originally requested in #20 (comment)

IIRC, it was related to running faster builds versus running everything on a PR basis. In any case, it's possible to enable the packaging on demand using GitHub labels or GitHub commands (

elastic-agent/.ci/Jenkinsfile

Lines 522 to 534 in 5fb9985

	/**
	* Wrapper to know if the build should enalbe the e2e stage
	*/
	def isE2eEnabled() {
	return params.end_to_end_tests_ci || env.GITHUB_COMMENT?.contains('e2e tests') || matchesPrLabel(label: 'ci:end-to-end')
	}
	/**
	* Wrapper to know if the build should enalbe the package stage
	*/
	def isPackageEnabled() {
	return env.PACKAGING_CHANGES == "true" || env.GITHUB_COMMENT?.contains('package') || matchesPrLabel(label: 'ci:package')
	}

is the section that evaluates those conditions)

[ycombinator]

Gotcha, thanks for the background, @v1v.

@cmacknz How should we proceed here? Some options:

• We could permanently (re)enable the Package stage on all PR builds but it would add more time to them. To do this, merging this PR here would do the trick.
• We could randomly run the Package stage on some PR builds (assuming such a thing is pretty easy to build out) so only some PR builds are slower than today but at least we don't have to necessarily wait until changes are merged to main to watch them potentially fail at the Package stage.
• We could keep things as-is and manually learn to tag PRs that might impact packaging with the ci:package label.

It looks like the Package stage, when it runs completely to success, adds about 30 minutes to the build.

[v1v]

#2519 can help with validating when the auto-bump for the Golang version happens by running the packaging on PRs automatically.

[ycombinator]

Chatted with @cmacknz. He mentioned Beats PR builds have a way to run the Package stage when go.mod changes. I see @v1v put up #2519 which is very similar to @cmacknz's suggestion. So I'll close this PR and let's continue with #2519.