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

Improve Debian Releases File #426

Merged
merged 5 commits into from
May 22, 2024

Conversation

marcohald
Copy link
Contributor

Description

Added Description to aptly repo create
Added Label and Origin to aply publish snapshot

Issues Resolved

Closes opensearch-project/opensearch-build#4485

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Added Description to aptly repo create
Added  Label and Origin to aply publish snapshot

Signed-off-by: marcohald <[email protected]>
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @marcohald ,

This file is used for both OS and OSD so we cannot hardcode with comments/labels.

Is there any particular benefits of adding them especially when the repo is created with ${jobname} and snapshot with ${jobname}-${repoVersion} already?

Thanks.

@marcohald
Copy link
Contributor Author

@peterzhuamazon The actual generated File https://artifacts.opensearch.org/releases/bundle/opensearch/2.x/apt/dists/stable/InRelease looks like this at the moment

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Origin: . stable
Label: . stable
Suite: stable
Codename: stable
Date: Tue, 14 May 2024 19:07:45 UTC
Architectures: amd64 arm64
Components: main
Description: Generated by aptly

As far as I can see neither ${jobname} or ${repoVersion} is part of that File.
The Problem with that is that it is not as easy to Pin when we cache the Repository with approx.
I could use origin (the hostname of the repo, not the Origin from the Release File) as pin parameter.
The Pinning is to control the Version opensearch that is installed.
In theory there could be multiple Repos on the approx with the same origin that could contain a opensearch package.

As far as i understand the only option that is not from the Release File is the origin to pin.
As found here https://unix.stackexchange.com/a/84812 and here https://manpages.debian.org/bookworm/apt/apt_preferences.5.en.html

@peterzhuamazon
Copy link
Member

Could you change the comment/origin/label to use different value based on ${jobname} then?
I am happy to have a comment or origin or even label but we shall not hardcode it as this script is used for both OS and OSD and possibly more later on.

Let me know. Thanks.

changed hardcoded Opensearch to ${filename}

Signed-off-by: marcohald <[email protected]>
@marcohald
Copy link
Contributor Author

Thank you for the hint with the different products I didn't thought about that.
I changed it to ${filename} which should result to "opensearch-dashboards" like the Product: here https://build.ci.opensearch.org/job/distribution-promote-repos/113/console
and "opensearch" for https://build.ci.opensearch.org/job/distribution-promote-repos/111/console
I think the origin could be left hardcoded to opensearch.org what do you think about that?

@marcohald marcohald requested a review from peterzhuamazon May 17, 2024 06:32
@peterzhuamazon
Copy link
Member

peterzhuamazon commented May 17, 2024

Hi @marcohald ,

Could you make two more changes?

  1. lowercase Repository as repository.
  2. change opensearch.org to artifacts.opensearch.org.

I have also reviewed aptly commands and the apt repo information, and I think your change would be beneficial.

Thanks.

@peterzhuamazon
Copy link
Member

I will update your test cases once you commit the requested changes.

Thanks.

@marcohald
Copy link
Contributor Author

Hi @peterzhuamazon
Sure, thank you for your help with this PR

@peterzhuamazon
Copy link
Member

Will add test cases in a min.

Signed-off-by: Peter Zhu <[email protected]>
Signed-off-by: Peter Zhu <[email protected]>
@peterzhuamazon
Copy link
Member

Thanks @marcohald for the contribution and we should be able to see this change take effect in the next release.

@peterzhuamazon peterzhuamazon merged commit 3632e49 into opensearch-project:main May 22, 2024
7 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 22, 2024
Signed-off-by: marcohald <[email protected]>
Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
(cherry picked from commit 3632e49)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@peterzhuamazon
Copy link
Member

Waiting for #427 to bump verision before update the jenkinsfile.

@marcohald marcohald deleted the patch-1 branch May 23, 2024 07:19
@peterzhuamazon
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Debian Releases File
2 participants