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

Almalinux update to v9.5 #1586

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Conversation

phillipross
Copy link
Contributor

Description

AlmaLinux9 update to v9.5

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

…king builds

Signed-off-by: Phillip Ross <phillip.w.g.ross@gmail.com>
Signed-off-by: Phillip Ross <phillip.w.g.ross@gmail.com>
@phillipross phillipross requested review from Stromweld and a team as code owners November 22, 2024 07:43
@phillipross phillipross changed the title Almalinux updates Almalinux update to v9.5 Nov 22, 2024
Copy link
Collaborator

@Stromweld Stromweld left a comment

Choose a reason for hiding this comment

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

The changes to the url paths for almalinux 8 aren't needed. For almalinux 9 please only update the iso version from 9.4 to 9.5. Adding .5 to the version number in the path is not needed. This is just to simplify changes and minimize future potential to forget to increment both items in the url on future releases.

@phillipross
Copy link
Contributor Author

Apologies, I'd forgotten to actually explain the reasoning behind the change 😬

There is a problem in that the URLs without the minor versions are not stable. When they release a new version, they change the underlying resources the URL refers to and the resource located at the iso_checksum URL no longer points to the resource that iso_url points to. This breaks the build until someone manually edits the pkrvars and takes it through the PR workflow.

By adding the minor versions to both the iso_url and iso_checksum properties the builds remain stable as the resources do not change out from under the URLs even when a new version is released. When a new release occurs, the pkrvars file needs to be changed to refer to the new versions, but the minor version is already specified explicitly in multiple vars anyway and needs to be changed in multiple places. Necessitating a 3rd change in an adjacent variable seems a worthy tradeoff to prevent the builds from breaking when a new version is released. Wouldn't you agree?

@phillipross phillipross mentioned this pull request Dec 11, 2024
4 tasks
@Stromweld
Copy link
Collaborator

I can see your point and makes sense. My thought was more on breakage indicating a new version and thus we should update for that new version otherwise it's hard to monitor and can take a while before someone notices the new version is released and update this. As is the latter is also still the case too so I can see both ways.

Do you use these templates and need more stability from random breakage due to new version release or would you rather have the updated version sooner?

@Stromweld
Copy link
Collaborator

I've also thought about making local variables in each for version number and then assigning it in os_version variable and string interpolation in the url so you only need to update the version once in the local variable.

@phillipross
Copy link
Contributor Author

Given that I'm already seeing a thumbs down on my comment, I suspect other folks are relying on the breakage to signal a new version being available, though there's probably a more robust way of getting this same signal 😁

I'm happy to modify the alma/rocky PRs I opened to revert back to using the URLs that do not contain the minor versions as I myself already understand the issue and work around it when needed, but I was thinking more about users who may not already be aware of the checksum URL changing.

If indeed some local var assignments were implemented to drive everything from the os_version variable, that would probably be the cleaner way to go about forming these URLs, keeping the checksum resource always pointing to the correct iso resource, etc. 👍

@Stromweld
Copy link
Collaborator

currently I've been very busy with my real job and diablo 4 expansion that was released. But i'm hoping to get back to working on this project with more updates for the automated pipeline builds and such at the end of the year early next year. The variable thing then I can work on at that time.

@Stromweld
Copy link
Collaborator

curious on thoughts of 1 file with all local variables for each defined or in each template file defining the local variable. first has ease of 1 file to update for several OS version upgrades. The latter makes it easier to read when looking at template settings which version is in use vs looking for the value in another file. I'm leaning toward the latter.

…indicators that new versions are available.

Signed-off-by: Phillip Ross <phillip.w.g.ross@gmail.com>
Copy link

sonarcloud bot commented Dec 12, 2024

@phillipross
Copy link
Contributor Author

Regarding the decision of how to organize the local vars, I tend to go the later route on bigger projects. The ease of having them centralized in a single file seems to be a diminishing benefit as the project grows, and this project is already fairly sizable (and growing).

@Stromweld Stromweld merged commit 2764117 into chef:main Dec 12, 2024
81 of 224 checks passed
@Stromweld
Copy link
Collaborator

I agree, thanks for the input.

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.

2 participants