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

Support dnf module management - Fix #310 #320

Merged
merged 23 commits into from
Apr 4, 2024

Conversation

EmersonPrado
Copy link

Pull Request (PR) description

This PR adds a custom resource to freely enable/disable streams in DNF modules. The provider supports 4 different stream spec formats:

  • :default - Enables default stream
  • :present - Enables default stream if no stream is enabled, otherwise keep currently enabled one
  • :absent - Disables current enabled stream, if any (that is: resets the module)
  • <String> - Enables specified stream

This Pull Request (PR) fixes the following issues

Fixes #310

WIP: though it passes the very limited spec tests and my manual tests, I pushed this in a hurry. Need to review later without tired eyes.

@EmersonPrado EmersonPrado changed the title [WIP] Support dnf module management - Fix #310 Support dnf module management - Fix #310 Aug 17, 2023
@EmersonPrado
Copy link
Author

Double checked everything. I'm comfortable to have it reviewed

Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

looks good to me. I will wait a few days so others can review as well.

Copy link
Member

@jhoblitt jhoblitt left a comment

Choose a reason for hiding this comment

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

What does this do that the package dnfmodule provider doesn't do?

    package { 'idm':
      ensure   => 'DL1',
      provider => 'dnfmodule',
    }

https://github.com/puppetlabs/puppet/blob/main/lib/puppet/provider/package/dnfmodule.rb

@EmersonPrado
Copy link
Author

@jhoblitt Thanks for taking some time to review.

package dnfmodule provider has some issues with dealing with streams:

  1. A stream is a yum/dnf setting, not something (un)installable
  2. Any profile (un)install changes the stream, even if not told so
  3. Dealing with stream setting and profile installation in the same resource is messy - I resorted to Slack to understand the beast...
  4. It can be improved to fix 2 (in the provider) and 3 (in the docs), but me and the Slack dudes agreed 1 is a good reason not to do it - in fact, the agreeded proposal is to deprecate stream support in dnfmodule provider

@jhoblitt
Copy link
Member

Could you provide a specific example in puppet code which does not work with the dnfmodule provider but does work with this new type/provider? dnfmodule does support profiles with the flavor parameters. Are you stating that Puppet By Perforce has decided to depreciate the the dnfmodule provider?

@bastelfreak
Copy link
Member

@EmersonPrado can you add your explanation to the README.md?

@EmersonPrado
Copy link
Author

@jhoblitt I'll do as @bastelfreak suggested. This might answer your questions, and the quite likely same ones from other people.

@EmersonPrado
Copy link
Author

@jhoblitt @bastelfreak
Done

@EmersonPrado
Copy link
Author

@jhoblitt About this other question: "Are you stating that Puppet By Perforce has decided to depreciate the the dnfmodule provider?":

  1. Not Perforce, but other devs (maybe some from Perforce, I don't know), in Slack talks (I invited you to one of them, hope it gets to you)
  2. Not depreciate the dnfmodule provider, but move its stream support to a dedicated resource (and, later, complete its profiles support, but this is another issue)

@bastelfreak
Copy link
Member

@EmersonPrado please update the REFERENCE.md with bundle exec rake strings:generate:reference (that's currently blocking the CI).

@EmersonPrado
Copy link
Author

@bastelfreak Done. Still some errors, but I guess they're unrelated to the PR.

@binford2k
Copy link
Member

Could you provide a specific example in puppet code which does not work with the dnfmodule provider but does work with this new type/provider? dnfmodule does support profiles with the flavor parameters. Are you stating that Puppet By Perforce has decided to depreciate the the dnfmodule provider?

No, the dnfmodule provider is not on the chopping block. What I believe Emerson is trying to do is clarify intent and also to provide a more flexible way to configure the multitude that is DNF.

@Xazziri
Copy link

Xazziri commented Apr 2, 2024

Is there any change of this PR being merged? This looks like the perfect solution for a problem managing appstream modules I'm having...

@bastelfreak
Copy link
Member

@Xazziri the failing tests need to be fixed before we can merge this.

@EmersonPrado
Copy link
Author

@Xazziri the failing tests need to be fixed before we can merge this.

The previous state of these tests were some failure apparently not related to the changes, but now there seems to be quite more failures than before. Unfortunately, now the tests just say "The logs for this run have expired and are no longer available". Guess I need to provoke another run to get current status.

@EmersonPrado
Copy link
Author

@bastelfreak Good news! Guess someone fixed something upstream, and the tests now pass!
@jhoblitt Could you pls review this PR now?

@bastelfreak
Copy link
Member

@EmersonPrado thanks for the work! Can you please rebase and remove commit 5553062 and 023df41?

@EmersonPrado
Copy link
Author

@EmersonPrado thanks for the work! Can you please rebase and remove commit 5553062 and 023df41?

Done!

@bastelfreak bastelfreak added the enhancement New feature or request label Apr 4, 2024
@bastelfreak bastelfreak merged commit 56edfc9 into voxpupuli:master Apr 4, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support dnf module management
5 participants