[CI] Removing the `Testing:run:TestUpgradeIntegrationsServer` label on PRs does not skip the `TestUpgradeIntegrationsServer` test

[ycombinator]

In #8417, we added a new integration test, TestUpgradeIntegrationsServer. We also added logic to our Buildkite pipeline so that this test would be run only on PRs that had the Testing:run:TestUpgradeIntegrationsServer label set on them:

elastic-agent/.buildkite/bk.integration-fips.pipeline.yml

Line 104 in 974a19d

if: build.env("BUILDKITE_PULL_REQUEST") != "false" && build.env("GITHUB_PR_LABELS") =~ /.*(Testing:run:TestUpgradeIntegrationsServer).*/

As such, we expect that PRs where that label is removed after being set would not run the TestUpgradeIntegrationsServer test in subsequent CI runs. However, this is not what is actually happening: #9283 (comment)

[ycombinator]

@elastic/observablt-robots perhaps someone from your team could take a look and help figure out how to fix this bug? Thanks!

[oakrizan]

Ran several tests and was not able to reproduce the issue:

• https://buildkite.com/elastic/elastic-agent/builds/27249 with label Testing:run:TestUpgradeIntegrationsServer
• https://buildkite.com/elastic/elastic-agent/builds/27250 without label Testing:run:TestUpgradeIntegrationsServer
• https://buildkite.com/elastic/elastic-agent/builds/27251 with label Testing:run:TestUpgradeIntegrationsServer (added 2nd time)
• https://buildkite.com/elastic/elastic-agent/builds/27252 without label Testing:run:TestUpgradeIntegrationsServer (removed 2nd time)

[v1v]

buildkite.com/elastic/elastic-agent/builds/27250 without label Testing:run:TestUpgradeIntegrationsServer

IIUC, it ran, or did I miss something?

[v1v]

As such, we expect that PRs where that label is removed after being set would not run the TestUpgradeIntegrationsServer test in subsequent CI runs. However, this is not what is actually happening: #9283 (comment)

As far as I see, it ran because the env variable GITHUB_PR_LABELS was set to "Team:Elastic-Agent-Control-Plane,Team:Elastic-Agent,backport-skip,skip-changelog,update-versions,Testing:run:TestUpgradeIntegrationsServer"

IIUC, that variable is provided by the BK PR bot.

I can see the build was Created Thu 7th Aug at 17:19 (see https://buildkite.com/elastic/elastic-agent/builds/24901) and GH label was removed a few hours earlier (#9283 (comment))

I can also see, https://buildkite.com/elastic/elastic-agent/builds?branch=elastic%3Aupdate%2Fmain-update-agent-versions-16800366838, there were two builds:

And the latest build was actually created by Panos with a rebuild.

I wonder whether the second build was a rebuild from the previous build or a new build from scratch. Maybe that's a misbehaviour with Buildkite, so one keeps the previous env context and a new build from scratch not.

For instance, in the BK UI I can see these two different actions (new build, rebuild)

I think rebuild will use the previous environment build context, hence the GitHub labels will be kept.

@oakrizan , I think this is not something we can do much in our end, it's how BK is built and the current integration with the BK PR bot to get the GH labels through env variables.

[oakrizan]

buildkite.com/elastic/elastic-agent/builds/27250 without label Testing:run:TestUpgradeIntegrationsServer

IIUC, it ran, or did I miss something?

No it didn't, if it would, there would be a label listed in a first step and a second step should appear:

I saw and analysed as well what you have pointed in your comment. But since the occurrence was some time ago, I jumped into replication, since Buildkite PR bot and GH behaviour might have been adjusted since then.

Currently making some additional tests, but seems that everything is working as usual, but need some more time to investigate the edge cases.

[v1v]

Currently making some additional tests, but seems that everything is working as usual, but need some more time to investigate the edge cases.

I think we touched the reason for that behaviour, so rebuild will populate the env variables from the previous build.

While new build will start from scratch, hence GITHUB_PR_LABELS will be empty.

I'd recommend people to use the GitHub comment buildkite test this so the BK PR Bot will take into account the existing GH labels. While using the BK UI might not behave as expected as we have seen when suing rebuild

[oakrizan]

Confirming

think we touched the reason for that behaviour, so rebuild will populate the env variables from the previous build.

https://buildkite.com/elastic/elastic-agent/builds/27256 no label
https://buildkite.com/elastic/elastic-agent/builds/27261 with label on the pr
https://buildkite.com/elastic/elastic-agent/builds/27281 removed label from the pr
https://buildkite.com/elastic/elastic-agent/builds/27321 rebuild of 27261 with label preserved

I'd assume that preserving the env vas with the Rebuild makes sense, since the intention is to rebuild that specific occasion/environment.

[oakrizan]

I'd recommend people to use the GitHub comment buildkite test this so the BK PR Bot will take into account the existing GH labels.

Or use empty commits to retrigger the CI.

In other words - label should be removed from the PR before pushing, so it would take an effect. And a second important thing - it requires to trigger fresh build to pickup all updated environment variables (either with comment or by pushing empty commit).