updatecli: dynamic specs #3497

v1v · 2024-01-16T15:24:43Z

What does this PR do?

Use values.yml to define the configuration to be used when running the updatecli apply, see https://www.updatecli.io/docs/commands/updatecli_apply/#updatecli-apply
Use updatecli scm to fetch the specs in elastic/apm and elastic/apm-data under the different folders.
To update those files, use tar with the shell target.

Checklist

Test

I iterated in the existing GitHub action by using the upstream branch:

https://github.com/elastic/apm-agent-java/actions/runs/7545513322/job/20541293368 ran successfully.

Locally:

GITHUB_WORKSPACE=/tmp \
  PATH=/opt/homebrew/bin:$PATH \
  GIT_USER="Victor Martinez" \
  GIT_EMAIL="[email protected]" \
  GITHUB_TOKEN=$(gh auth token) \
  updatecli apply --config .ci/updatecli/updatecli.d --values .ci/updatecli/values.yml --debug

or without pushing any changes

GITHUB_WORKSPACE=/tmp \
  PATH=/opt/homebrew/bin:$PATH \
  GIT_USER="Victor Martinez" \
  GIT_EMAIL="[email protected]" \
  GITHUB_TOKEN=$(gh auth token) \
  updatecli apply --config .ci/updatecli/updatecli.d --values .ci/updatecli/values.yml --push=false --debug

additionally, you can change the org from elastic to your own GitHub avatar.

This reverts commit 2844d77.

v1v · 2024-01-16T17:51:38Z

.ci/updatecli/updatecli.d/update-gherkin-specs.yml

+    sourceid: sha
+    kind: shell
+    spec:
+      command: 'tar -xzf {{ requiredEnv "GITHUB_WORKSPACE" }}/gherkin-specs.tgz && git --no-pager diff # required to ignore the sourceid being appended'


The tricky thing here is to use # since the value of the sourceid: sha is appended to the command and then produce some unexpected output.

I think if you omit the sourceid option, the value is not appended

I think if you omit the sourceid option, the value is not appended

That was expected, but It was not the case for some reason.

Given

diff --git a/.ci/updatecli/updatecli.d/update-specs.yml b/.ci/updatecli/updatecli.d/update-specs.yml index 0be8fa56a..ca40c47ef 100644 --- a/.ci/updatecli/updatecli.d/update-specs.yml +++ b/.ci/updatecli/updatecli.d/update-specs.yml @@ -77,7 +77,6 @@ targets: agent-json-schema: name: APM agent json server schema {{ source "sha" }} scmid: default - sourceid: sha kind: shell spec: command: 'tar -xzf {{ requiredEnv "GITHUB_WORKSPACE" }}/json-schema.tgz && git --no-pager diff # required to ignore the sourceid being appended'

When running:

GITHUB_WORKSPACE=/tmp PATH=/opt/homebrew/bin:$PATH GIT_USER="foo" GIT_EMAIL="[email protected]" GITHUB_TOKEN=$(gh auth token) updatecli apply --config .ci/updatecli/updatecli.d/update-specs.yml --values .ci/updatecli/values.yml --push=false --debug

Then it failed with the below error:

+++++++++++ + PREPARE + +++++++++++ Loading Pipeline ".ci/updatecli/updatecli.d/update-specs.yml" ERROR: empty 'sourceid' for target "agent-json-schema" ERROR: Skipping manifest in position '\x00' from ".ci/updatecli/updatecli.d/update-specs.yml": targets validation error: wrong updatecli configuration ERROR: failed loading pipeline(s) * ".ci/updatecli/updatecli.d/update-specs.yml" - targets validation error: wrong updatecli configuration 0 pipeline(s) successfully loaded SCM repository retrieved: 0 ++++++++++++++++++ + AUTO DISCOVERY + ++++++++++++++++++ ERROR: ✗ no valid pipeline found ERROR: command failed: no valid pipeline found

While if I run this PR as is then:

TARGETS ======== agent-json-schema ----------------- DEBUG: No shell success criteria defined, updatecli fallbacks to historical workflow DEBUG: stage: git-checkout DEBUG: checkout branch "updatecli_update-schema-specs", based on "main" to directory "/var/folders/t7/ghqdh8cx2g12pwb_w0ncmw900000gn/T/updatecli/github/elastic/apm-agent-java" DEBUG: DEBUG: branch 'updatecli_update-schema-specs' doesn't exist, creating it from branch 'main' DEBUG: branch "updatecli_update-schema-specs" successfully created DEBUG: command: /bin/sh /var/folders/t7/ghqdh8cx2g12pwb_w0ncmw900000gn/T/updatecli/bin/c3007d0e8db2c2ace0ca4104b167b411531c865a1844a662c648deeecede55b8.sh DEBUG: Environment variables DEBUG: * UPDATECLI_PIPELINE_STAGE=target DEBUG: * DRY_RUN=false The shell 🐚 command "/bin/sh /var/folders/t7/ghqdh8cx2g12pwb_w0ncmw900000gn/T/updatecli/bin/c3007d0e8db2c2ace0ca4104b167b411531c865a1844a662c648deeecede55b8.sh" ran successfully with the following output: ---- ---- DEBUG: command stderr output was: ---- ---- No change detected ✔ - ran shell command "tar -xzf /tmp/json-schema.tgz && git --no-pager diff # required to ignore the sourceid being appended 80b6af8d97e18344df38968bc5b9588961e89492" DEBUG: Checking if local changes have been done that should be published DEBUG: no changes detected between branches "main" and "updatecli_update-schema-specs" ACTIONS ======== ============================= REPORTS: ✔ update-specs: Source: ✔ [agent-specs-tarball] ✔ [pull_request] ✔ [sha] Target: ✔ [agent-json-schema] APM agent json server schema 80b6af8d97e18344df38968bc5b9588961e89492

but according to https://www.updatecli.io/docs/plugins/resource/shell/#_shell_condition:

💡 The source associated to this condition (the default source or through the key sourceID) is appended as the last argument to the command line unless the attribute disablesourceinput is set to true.

diff --git a/.ci/updatecli/updatecli.d/update-specs.yml b/.ci/updatecli/updatecli.d/update-specs.yml index 0be8fa56a..2b3f34ef8 100644 --- a/.ci/updatecli/updatecli.d/update-specs.yml +++ b/.ci/updatecli/updatecli.d/update-specs.yml @@ -77,8 +77,8 @@ targets: agent-json-schema: name: APM agent json server schema {{ source "sha" }} scmid: default - sourceid: sha + disablesourceinput: true kind: shell spec: - command: 'tar -xzf {{ requiredEnv "GITHUB_WORKSPACE" }}/json-schema.tgz && git --no-pager diff # required to ignore the sourceid being appended' + command: 'tar -xzf {{ requiredEnv "GITHUB_WORKSPACE" }}/json-schema.tgz && git --no-pager diff' workdir: "{{ .apm_agent.server_schema_specs_path }}"

worked like a charm, so I missed the disablesourceinput argument when I tried this in the past. Let me add the change accordingly.

See 8e86691

Ah, right. disablesourceinput does the magic.

v1v · 2024-01-16T17:52:49Z

.ci/updatecli/values.yml

@@ -0,0 +1,14 @@
+github:


We can easily copy the existing updatecli pipelines in .ci/updatecli.d/*.yml in each APM Agent repository and change the relevant values in this file

v1v · 2024-01-16T17:54:33Z

.ci/updatecli/updatecli.d/update-specs.yml

+  sha:
+    kind: file
+    spec:
+      file: 'https://github.com/{{ .github.owner }}/{{ .github.apm_data_repository }}/commit/{{ .github.branch }}.patch'
+      matchpattern: "^From\\s([0-9a-f]{40})\\s"
+    transformers:
+      - findsubmatch:
+          pattern: "[0-9a-f]{40}"


This could be changed to use gh API /repos/{{ .github.owner }}/{{ .github.apm_data_repository }}/commits --jq '.[0].sha' since it uses the GITHUB_TOKEN variable

This should help with unexpected failures when anonymously accessing GitHub.

SylvainJuge

LGTM, I wonder how we could change this ? would introducing few changes in the repo and execute updatecli directly be possible ? If that works then only the trigger when changes are made in the upstream repo would be needed.

v1v · 2024-01-17T09:37:52Z

I wonder how we could change this?

What do you mean by changing this?

would introducing few changes in the repo and execute updatecli directly be possible ? If that works then only the trigger when changes are made in the upstream repo would be needed.

That's correct; you can use your forked repository to test this and override the vaults.yml for the branch and owner.

diff --git a/.ci/updatecli/values.yml b/.ci/updatecli/values.yml
index 5d1e74685..62d86073a 100644
--- a/.ci/updatecli/values.yml
+++ b/.ci/updatecli/values.yml
@@ -1,9 +1,9 @@
 github:
-  owner: "elastic"
+  owner: "your-gh-avatar"
   repository: "apm-agent-java"
   apm_repository: "apm"
   apm_data_repository: "apm-data"
-  branch: "main"
+  branch: "your-branch"
 specs:
   apm_data_path: "input/elasticapm/docs/spec/v2"
   apm_json_path: "tests/agents/json-specs"

Then you can run this PR locally once you have installed updatecli

$ gh pr checkout 3497
$ GITHUB_WORKSPACE=/tmp \
  GIT_USER="foo" \
  GIT_EMAIL="[email protected]" \
  GITHUB_TOKEN=$(gh auth token) \
  updatecli apply --config .ci/updatecli/updatecli.d --values .ci/updatecli/values.yml --debug

SylvainJuge · 2024-01-17T09:41:43Z

sorry, I meant "how we could test this", not change it 🤦 , I'll try the commands you've suggested here by artificially reverting those spec files to an older version.

v1v · 2024-01-17T10:17:14Z

no worries :)

I'll try the commands you've suggested here by artificially reverting those spec files to an older version.

👍

Quick one, you can branch off one old release and use that branch name in the values.yml, though the name of the branch should exist in all the forked repositories (APM Agent Java, apm and apm-data)

v1v · 2024-01-17T15:06:59Z

@elasticmachine run elasticsearch-ci/docs

SylvainJuge · 2024-01-22T13:25:36Z

Hi @v1v I haven't been able to find time to test this, do you think it's risky to merge it as-is and adjust with follow-up PRs if needed later ?

see https://www.updatecli.io/docs/plugins/resource/shell/\#_shell_condition

v1v · 2024-01-23T08:32:33Z

Hi @v1v I haven't been able to find time to test this, do you think it's risky to merge it as-is and adjust with follow-up PRs if needed later ?

It's not risky as far as I can see; if something goes wrong, we will receive a notification in Slack anyway, and we can solve it.

I need your 👀 again after refactoring the shell step with a native updatecli support to turn off appending the sha commit; see 8e86691

see https://www.updatecli.io/docs/plugins/resource/shell/\#_shell_condition

reakaleek · 2024-01-23T08:49:10Z

I'm wondering if it would be possible to have the updatecli pipeline in another single place and the values in the respective repos.

In case of changes, we would only have to update the pipeline in a single place.

I'm aware that having it on the consumer side was the initial idea, but I was still wondering what you think about it.

WDYT?

v1v · 2024-01-23T09:02:56Z

WDYT?

I thought about it, but we don't need it since there are no new APM Agents that often. Hence, as of today, adding such automation will not provide many benefits, plus the caveats with the support for the cross-project GitHub PR creation.

As you mentioned, we used to have a centralised approach in the past, and we moved to a decentralised approach to let teams manage their things.

In any case, if, for some reason, we need to change the same workflow again, we could re-evaluate this question and put the required tooling around it.

updatecli: dynamic specs

5728163

github-actions bot added the agent-java label Jan 16, 2024

v1v added 23 commits January 16, 2024 16:28

partially revert

61bb0de

use unique name

f839cce

use same name

1d042c0

use workspace to store the artifact

8c31e56

avoid spamming

1968605

testing

51ed756

avoid

bbefd63

use a different source

c1e1e83

use script file

828a177

fix deprecated

7191385

use env

632c2e7

use the pipeline itself

1fb0160

solve sourceid input being appended

47978a0

dynamic gherkin specs

10276b0

dynamic json specs

984c267

revert

8979f84

use values

904908e

use single quotes

35deb8e

use production

2844d77

cosmetic

533c8a1

Revert "use production"

e18b699

This reverts commit 2844d77.

use production

2970e2d

notify the right teams

e646d68

v1v commented Jan 16, 2024

View reviewed changes

v1v marked this pull request as ready for review January 16, 2024 17:54

v1v requested review from SylvainJuge and a team January 16, 2024 17:54

v1v self-assigned this Jan 16, 2024

use the title for the PR

abb4c6f

SylvainJuge previously approved these changes Jan 17, 2024

View reviewed changes

updatecli: avoid appending the sourceid

8e86691

see https://www.updatecli.io/docs/plugins/resource/shell/\#_shell_condition

v1v dismissed SylvainJuge’s stale review via 8e86691 January 23, 2024 08:29

v1v requested review from reakaleek and SylvainJuge January 23, 2024 08:32

updatecli: avoid appending the sourceid

0e28353

see https://www.updatecli.io/docs/plugins/resource/shell/\#_shell_condition

reakaleek approved these changes Jan 23, 2024

View reviewed changes

v1v merged commit e823efd into main Jan 24, 2024
13 checks passed

v1v deleted the feature/update-cli-dynamic branch January 24, 2024 17:33

v1v mentioned this pull request Jul 8, 2024

updatecli: use shared policy elastic/apm-agent-nodejs#4121

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updatecli: dynamic specs #3497

updatecli: dynamic specs #3497

v1v commented Jan 16, 2024 •

edited

Loading

v1v Jan 16, 2024

reakaleek Jan 22, 2024

v1v Jan 23, 2024

v1v Jan 23, 2024

reakaleek Jan 23, 2024

v1v Jan 16, 2024

v1v Jan 16, 2024

SylvainJuge left a comment

v1v commented Jan 17, 2024

SylvainJuge commented Jan 17, 2024

v1v commented Jan 17, 2024

v1v commented Jan 17, 2024

SylvainJuge commented Jan 22, 2024

v1v commented Jan 23, 2024

reakaleek commented Jan 23, 2024

v1v commented Jan 23, 2024

updatecli: dynamic specs #3497

updatecli: dynamic specs #3497

Conversation

v1v commented Jan 16, 2024 • edited Loading

What does this PR do?

Checklist

Test

v1v Jan 16, 2024

Choose a reason for hiding this comment

reakaleek Jan 22, 2024

Choose a reason for hiding this comment

v1v Jan 23, 2024

Choose a reason for hiding this comment

v1v Jan 23, 2024

Choose a reason for hiding this comment

reakaleek Jan 23, 2024

Choose a reason for hiding this comment

v1v Jan 16, 2024

Choose a reason for hiding this comment

v1v Jan 16, 2024

Choose a reason for hiding this comment

SylvainJuge left a comment

Choose a reason for hiding this comment

v1v commented Jan 17, 2024

SylvainJuge commented Jan 17, 2024

v1v commented Jan 17, 2024

v1v commented Jan 17, 2024

SylvainJuge commented Jan 22, 2024

v1v commented Jan 23, 2024

reakaleek commented Jan 23, 2024

v1v commented Jan 23, 2024

v1v commented Jan 16, 2024 •

edited

Loading