-
Notifications
You must be signed in to change notification settings - Fork 161
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
Move package testing to this repository #439
Comments
hey @mx-psi , has this been resolved yet, and would you like for me to give it a go with your guidance of course ? |
Hii @mx-psi , @ so I've gone through conversations from open-telemetry/opentelemetry-collector-contrib#29286 and, through the |
@Enzujp yes, you are right! And in general: this may not be a trivial task, these tests have not been modified for a long time and a long has changed since |
I'm trying out solutions to automate the release notes, should be done soon; and then I'd come back and we can map out doing this together?. :) |
@Enzujp Sure! I am out until next week, but feel free to Slack me on the CNCF Slack (you can find my name on my profile :)) |
I think the way to go here would be to basically remove most if not all of the content in https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/internal/buildscripts/packaging/fpm and move just the test script over to the releases repo. All other files already exist in the -releases repo since the linux service packages are already built there anyways. @mx-psi WDYT? I'd give this a shot if it makes sense to you. |
Sounds good to me. I would prefer this to be a new workflow, triggered only when the goreleaser files are changed, and tested only on cherry-picked combinations. |
This makes sense to me, and agreed on the new workflow triggered by goreleaser files changed |
Sure then let's start with the most commonly used one, which is probably Linux amd64? |
I'll get to it then 🚀 |
@jpkrohling I looked into it a bit more and found out that the package tests could nicely fit into the current ci-goreleaser workflows. They already trigger on the right changes and build a snapshot release. We would just need to upload the packages to test as artifacts and then trigger the package test workflow right after and use the artifacts there. So the https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/.github/workflows/base-ci-goreleaser.yaml workflow would get a step at the end (in pseudocode):
and then the package-test workflow would be called after e.g. https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/.github/workflows/ci-goreleaser-core.yaml#L34 and use the artifact for testing |
Sounds good to me. |
PR is ready for review 🚀 |
This PR removes the package tests from this repo since they were moved to the -releases repo in open-telemetry/opentelemetry-collector-releases#604. Related issues: - Original issue: open-telemetry/opentelemetry-collector-releases#439 - Also fixes #34748 Advantages of doing this: - removal of duplicated code that has potential to diverge with linux package building and testing happening here with custom code, but the actual code that is used to build the released linux packages is in the -releases repo. This should result in lower maintenance effort and removes technical debt. Disadvantages discussed here: - bit of a shift-right for the linux package tests since they are now tested in the -releases repo which could lead to situations where bugs are found too late and e.g. the contrib repo already got a new tag which has a bug that only comes up in the releases repo --------- Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
This PR removes the package tests from this repo since they were moved to the -releases repo in open-telemetry/opentelemetry-collector-releases#604. Related issues: - Original issue: open-telemetry/opentelemetry-collector-releases#439 - Also fixes open-telemetry#34748 Advantages of doing this: - removal of duplicated code that has potential to diverge with linux package building and testing happening here with custom code, but the actual code that is used to build the released linux packages is in the -releases repo. This should result in lower maintenance effort and removes technical debt. Disadvantages discussed here: - bit of a shift-right for the linux package tests since they are now tested in the -releases repo which could lead to situations where bugs are found too late and e.g. the contrib repo already got a new tag which has a bug that only comes up in the releases repo --------- Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
This PR removes the package tests from this repo since they were moved to the -releases repo in open-telemetry/opentelemetry-collector-releases#604. Related issues: - Original issue: open-telemetry/opentelemetry-collector-releases#439 - Also fixes open-telemetry#34748 Advantages of doing this: - removal of duplicated code that has potential to diverge with linux package building and testing happening here with custom code, but the actual code that is used to build the released linux packages is in the -releases repo. This should result in lower maintenance effort and removes technical debt. Disadvantages discussed here: - bit of a shift-right for the linux package tests since they are now tested in the -releases repo which could lead to situations where bugs are found too late and e.g. the contrib repo already got a new tag which has a bug that only comes up in the releases repo --------- Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
We have some package tests on opentelemetry-collector-contrib that should instead live here, since this is where we produce the packages and we do not have a mechanism to keep track of changes to the package scripts or systemd service definitions.
See open-telemetry/opentelemetry-collector-contrib/pull/29286 for context
The text was updated successfully, but these errors were encountered: