Skip to content

Conversation

@vzhestkov
Copy link
Contributor

Copy link
Member

@rjmateus rjmateus left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just a few clarifications and I think we should be good to go. Thank you for creating it.


- We need to have clear way to identify the exact salt code used with the salt packages as the release number is not reliable.
- There were some issues detected for the cases when the outdated salt is used either on `salt-master` or `salt-minion` side, but such cases are hard to identify as the only reliable data is the changelog of the package.
- Make it easier to ensure that newer version of Salt is used on the `salt-master` side by enforce preventing adding higher `salt-minion` version to bootstrap repos.
Copy link
Member

Choose a reason for hiding this comment

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

Not just Bootstrap repositories, but alsothe client tools repository. The bootstrap repo is the first obvious issue, but user can apply all the updates after registration, and if the package is available, it would lead to the same issue.

To avoid the confusions and better tracking the alignment of the package version across different targets for classic salt package and the salt bundle,
we could extend the version of the package with the patch level like `MAJOR.MINIOR.PATCH` instead of `MAJOR.MINOR` which is used now.

The patch level could be either increased manually on each maintenance update submission or adjusted automatically on the build time with the current number of patches included.
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a suggestion on which approach should be followed? Automating can avoid one manual step before releasing, but it can be less reliable if, for some reason, we remove some patches (I know it can be hard to imagine now).
Also, we would need to adapt it when moving to git workflow.
I would vote for manually incrementing it since we already need to make some changes by merging the change logs, right?

Copy link
Member

Choose a reason for hiding this comment

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

The version should probably be unrelated to the number of patches and be updated "manually". In #105 we propose to stop applying patches at build time and use a newer openSUSE/salt checkout (via a git submodule or via a tarball downloaded from github.com), which would still work with a "manual" patch level bump.

I put "manual" in quotes because I'd still automate it. I would do it when $user (which could be a bot) updates Salt packaging sources in OBS / src.opensuse.org

[unresolved]: #unresolved-questions

- It's not clear how to align it with git workflow integration for OBS.
- What should we do with the old product versions which are not using any conditions for adding the new versions of the bundle to the bootstrap repositories?
Copy link
Member

Choose a reason for hiding this comment

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

When we update the code that generates the bootstrap script we may need to backport it to all supported versions. I'm not expecting that it would be too difficult.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to change the bootstrap script at all?

This goes into the "how do we use the Salt version" part that's covered briefly above. I think we still need to discuss the details on how we want to make sure that clients never see incompatible updates.

Personally I don't like that we're enforcing the update of the master first when it's not required, but I that's the trade-off when we use our own patch level instead of e.g. the transport version that I proposed in uyuni-project/uyuni#10639

For the mechanism, I would pick one that's on 100% the Server side, i.e. the download repositories that Uyuni provide don't include Salt Bundle packages with versions higher than the currently running Salt Master. To get there, we can either filter when we sync channels (reposync) or later when we create the download repository (mgr-create-bootstrap-repo and Java's repository generator). We need more information on both approaches (upsides/downsides) to decide which is a better fit.

`3006.0.179`, `3006.0.180`, `3006.0.183` ... `3008.0.25` - in case of alignment with the numbers of patches included

The suggestion with the number of patches could be not relevant in case of switching to git workflow with the packaging,
in this case as an alternative we can check if we can use the amount of commits between the latest one and the original for the certain version.
Copy link
Member

Choose a reason for hiding this comment

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

Counting commits has a problem when we have differences between package git (e.g. pool/salt) on the internal vs external git forge or different branches. Like we have now if we want to compare the number of changelog entries between different OBS/IBS projects.

# Unresolved questions
[unresolved]: #unresolved-questions

- It's not clear how to align it with git workflow integration for OBS.
Copy link
Member

Choose a reason for hiding this comment

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

The git workflow does not need to change the approach, but it probably changes how it's implemented.

Right now we have salt.spec in openSUSE/salt-packaging and can update the patchlevel in PRs that add/modify/remove patches.

In the future, we plan to have salt.spec in openSUSE/salt and update it there when we fix bugs / add features. In that repo we could have a version bump, together with a git tag at the time we (either a human or a bot) prepare a submission.

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.

3 participants