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

fix syncing; distinguish by catalog; fix property value #76

Merged
merged 1 commit into from
May 26, 2023

Conversation

joelanford
Copy link
Member

Closes #74
Closes #69
Closes #12

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
for _, pkg := range declCfg.Packages {
pack := corev1beta1.Package{
name := fmt.Sprintf("%s-%s", catalog.Name, pkg.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What impact on UX is it going to have to prefix package names with the catalog name as well? With this it feels like we are also saying that we allow the existence of the same package names from different catalogs which introduces a bit of ambiguity. How can a user differentiate between a package that is actually the same between different catalogs and a package that has the same name but is actually different?

I don't know what the right solution is here, but I think we need to become opinionated one way or the other (I personally lean towards only one package with a specific name).

A potential solution to the problem: Add a field that allows us to determine whether or not the catalog providing the same package should overwrite the existing package. Could be done with something like overwriteConflicts: true or a priority field that sets weights for where a catalog and its contents falls in the priority rankings - in this case a higher priority would result in an overwrite.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't think the book is closed on this at all. This is mainly just to get catalogd away from the current situation where weird stuff happens when two catalogs share a package.

We definitely should discuss more, but my initial instinct is that catalogd is not the place to enforce this. I'm thinking we should just surface the data and let clients decide how to deconflict.

Copy link
Member Author

@joelanford joelanford May 25, 2023

Choose a reason for hiding this comment

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

"Let the client figure it out" is what yum does it looks like. And it also looks like there are some knobs on the repository configuration that let sysadmins exclude certain packages for certain repos.

https://serverfault.com/questions/476776/yum-the-same-packages-in-two-repositories-force-to-install-the-package-from-a

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough. Could we open a follow-up issue to track the need for this discussion? I just don't want the problem to be "resolved" and us forget to take a look at improving this (and maybe this problem goes away completely with the apiserver and api changes coming soon?)

@joelanford joelanford merged commit abc5a78 into main May 26, 2023
@joelanford joelanford deleted the sync-crds branch May 26, 2023 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants