Skip to content
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

[chore] remove linux package builds and tests #34372

Conversation

mowies
Copy link
Member

@mowies mowies commented Aug 1, 2024

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:

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

@codeboten
Copy link
Contributor

Thanks @mowies, the one concern I have with removing this code is that it's caught issues in the past. Any issues these would catch would only be caught at the releases repo, which means we'd have tagged a potentially broken go module (since we tag contrib modules before the release moves on to the releases repo). Maybe the process should be changed? I'm not sure what the right thing to do is here

@mx-psi
Copy link
Member

mx-psi commented Aug 2, 2024

@codeboten Do you remember what issues it caught? Maybe we can test for those in a different way, agnostic to the actual scripts

@mowies
Copy link
Member Author

mowies commented Aug 5, 2024

@codeboten I understand. Maybe only testing in the release repo is really too late. What we could try instead is to checkout the releases repo from here and use the actual files from there. If we then also use the make targets from the releases repo directly to build and test the linux packages, there's a very high chance that we miss some error due to a mismatch between this repo's setup and the releases repo's setup.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 20, 2024
@jpkrohling jpkrohling linked an issue Aug 21, 2024 that may be closed by this pull request
@jpkrohling
Copy link
Member

Perhaps we could have a nightly build somewhere (here or in the -releases repo) to catch regressions earlier?

@github-actions github-actions bot removed the Stale label Aug 22, 2024
@mowies
Copy link
Member Author

mowies commented Aug 26, 2024

Shall I create a separate issue for that here? I think my suggestion that I mentioned above could work nicely in a nightly setup.

@atoulme
Copy link
Contributor

atoulme commented Sep 4, 2024

I propose that we make a nightly build in the -releases repository, using the latest snapshot of core and contrib. @mowies please create an issue to take it from here. I think we're good to delete this code.

@mowies
Copy link
Member Author

mowies commented Sep 9, 2024

I created open-telemetry/opentelemetry-collector-releases#659 👍🏼

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
@mowies mowies force-pushed the remove-package-build-and-test branch from 80ea8dc to 777929f Compare September 9, 2024 12:10
@andrzej-stencel
Copy link
Member

@mowies can you update the description of this PR to describe why we want to do this? What are the advantages of the removal? It will help future us and future others understand the motivation, especially given that there is a slight disadvantage to the removal as well (potential breakage in -releases repo).

My guesses of the advantages are: removing code to decrease maintenance cost; decrease time to check PRs.

@mowies
Copy link
Member Author

mowies commented Sep 10, 2024

@andrzej-stencel done! :)

@andrzej-stencel
Copy link
Member

Thanks a lot @mowies, the description is great! Helps a lot.

I agree with the shift-right concert, thanks for raising the issue to create nightly builds open-telemetry/opentelemetry-collector-releases#659. @mowies would you be up for proposing a pull request for this?

@mowies
Copy link
Member Author

mowies commented Sep 17, 2024

@andrzej-stencel yes i'll start working on that once this PR is merged :)

@mx-psi
Copy link
Member

mx-psi commented Sep 17, 2024

@codeboten Please block/comment on the PR if you think we shouldn't merge this!

@mowies mowies requested a review from a team as a code owner September 23, 2024 06:49
@mowies
Copy link
Member Author

mowies commented Sep 30, 2024

@andrzej-stencel is anything missing to get this PR over the line? :)
@codeboten pls let us know if you're fine with merging this

@andrzej-stencel
Copy link
Member

Merging this as it has three approvals already. @mowies I've assigned open-telemetry/opentelemetry-collector-releases#659 to you, looking forward to your next contributions and thank you so much! ❤️

@andrzej-stencel andrzej-stencel merged commit 382b99a into open-telemetry:main Oct 3, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 3, 2024
@mowies mowies deleted the remove-package-build-and-test branch October 3, 2024 10:49
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
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>
Eromosele-SM pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2024
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove internal/buildscripts/packaging/
6 participants