-
Notifications
You must be signed in to change notification settings - Fork 225
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
updatecli: use shared policy #4121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some reservations. The end result here feels like a very complex solution to the problem of periodically checking for updates to some files vendored from another repo.
@@ -0,0 +1,12 @@ | |||
policies: | |||
- name: Handle apm-data server specs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately these "name:" Handle ...
strings do not appear in the workflow run log at all, e.g.: https://github.com/elastic/apm-agent-nodejs/actions/runs/9806099782/job/27077125378
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll raise an issue in the upstream project so it can be honoured or at least they know this case. Thanks
@@ -0,0 +1,12 @@ | |||
policies: | |||
- name: Handle apm-data server specs | |||
policy: ghcr.io/elastic/oblt-updatecli-policies/apm/apm-data-spec:0.2.0@sha256:7069c0773d44a74c4c8103b4d9957b468f66081ee9d677238072fe11c4d2197c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These policy files are behind credentials that I don't have, so are uninspectable by me. Do these live in a repo somewhere that I could look at? Here I guess:
https://github.com/elastic/oblt-updatecli-policies/tree/main/updatecli/policies/apm/apm-data-spec
The "0.2.0" container image label doesn't have a matching git tag in that repository. I see there is a changelog, but it is a bit of a guess which commit a particular container image build corresponds to.
Perhaps at least a clickable URL to be able to go look at the related code? Something like:
policy: ghcr.io/elastic/oblt-updatecli-policies/apm/apm-data-spec:0.2.0@sha256:7069c0773d44a74c4c8103b4d9957b468f66081ee9d677238072fe11c4d2197c | |
# https://github.com/elastic/oblt-updatecli-policies/tree/main/updatecli/policies/apm/apm-data-spec | |
policy: ghcr.io/elastic/oblt-updatecli-policies/apm/apm-data-spec:0.2.0@sha256:7069c0773d44a74c4c8103b4d9957b468f66081ee9d677238072fe11c4d2197c |
Though, that is of limited help, because it is very difficult to understand what this workflow is attempting to do and to follow what the policy is doing. I can make guesses, but for example, it isn't clear to me what build process in oblt-updatecli-policies is rendering the https://github.com/elastic/oblt-updatecli-policies/blob/main/updatecli/policies/apm/apm-data-spec/updatecli.d/default.tpl template to a useable final thing that updatecli eventually executes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These policy files are behind credentials that I don't have
Not at all, you can login to GitHub itself:
Do these live in a repo somewhere that I could look at?
I didn't want to explicitly write the GitHub repository, it's private at the moment, I'm waiting for an approval to set it public available.
elastic/oblt-updatecli-policies@main/updatecli/policies/apm/apm-data-spec
correct
Perhaps at least a clickable URL to be able to go look at the related code? Something like:
No, I plan to use updatecli to bump the versions automatically; see elastic/apm-agent-go#1633
I'll raise an issue to ask to the upstream project what's the strategy about the changelog automation for those automated-bumps touching the updatecli policies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, that is of limited help, because it is very difficult to understand what this workflow is attempting to do and to follow what the policy is doing.
Do you know how I can help with this? I'm open to making this better.
The idea behind the updatecli shareable policies is to share updatecli pipelines between different GitHub projects and then use 'values' to configure the behaviour. That's why the 'default.tpl' exists and then infer the values.
In this particular case, these changes are all related to syncing up the apm and apm-data specs/gherkins in all the APM Agents, see https://github.com/elastic/observability-robots/issues/1751 (Only for Elastic employees)
I'm not sure what's the reason for the complex solution:
It's idempotence and will create Pull Requests with changes if needed. All the above is required in 8 or 9 different GitHub projects. I already went through changing behaviour in the updatecli pipeline, and had to deal with several PRs.
Using a centralised shared place for well defined policies will help us by different means:
The very end goal is to use automation, therefore if a new version is released for the updatecli policies, the file Please let me know if I need to clarify anything, I'll raise a few GitHub issues in the updatecli project based on your valuable feedback, 🙏 |
All good. Thanks for feedback. Perhaps add that comment at the top of "update-compose.yaml" stating that the file is related to Note to self: I can browse the published ghcr.io packages here: |
Co-authored-by: Trent Mick <[email protected]>
Not at all, see |
What does this PR do?
Use a shared updatecli policy. The exact configuration is used in different GitHub repositories.
I ran https://github.com/elastic/apm-agent-nodejs/actions/runs/9806099782. And it created the PR:
#4122 vs #4046
Checklist