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

Remove references to Pulpcore module stream for Pulpcore 3.39 #2597

Merged

Conversation

ianballou
Copy link
Contributor

@ianballou ianballou commented Nov 17, 2023

No CP should be needed, this is for Foreman 3.9

Pulpcore 3.39 is almost definitely losing its module stream, so it will no longer be needed in the docs.

Don't merge until after theforeman/puppet-pulpcore#315 get in.

Please cherry-pick my commits into:

  • Foreman 3.9/Katello 4.11

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Now dnf-module and dnf-modules are the same. Do we want to drop dnf-modules altogether now?

@mjivraja
Copy link
Contributor

Now dnf-module and dnf-modules are the same. Do we want to drop dnf-modules altogether now?

I agree with this. dnf-modules becomes redundant with this PR.

@ianballou ianballou force-pushed the remove-pulpcore-module-references branch from 1b0e7c5 to bfdf6d8 Compare November 20, 2023 17:26
@ianballou
Copy link
Contributor Author

I've added a note to disable the pulpcore module when upgrading.

@ianballou ianballou force-pushed the remove-pulpcore-module-references branch from bfdf6d8 to 8543017 Compare November 20, 2023 17:30
@ianballou
Copy link
Contributor Author

I've removed references to dnf-modules

@ianballou ianballou requested a review from ekohl November 20, 2023 17:30
@ianballou ianballou force-pushed the remove-pulpcore-module-references branch from 8543017 to 7fecc7e Compare November 20, 2023 18:13
@ianballou
Copy link
Contributor Author

The linting error appears to be an issue connecting to an external URL.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Would it make sense to also add this as an upgrade warning in the release notes? So here:

[id="katello-upgrade-warnings"]
== Upgrade Warnings
There are no upgrade warnings with Katello {KatelloVersion}.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Nov 20, 2023
@ianballou
Copy link
Contributor Author

Would it make sense to also add this as an upgrade warning in the release notes? So here:

[id="katello-upgrade-warnings"]
== Upgrade Warnings
There are no upgrade warnings with Katello {KatelloVersion}.

I was going to stick it in when I do my whole Katello 4.11 release notes update. I suppose I could put it in now.

@ekohl
Copy link
Member

ekohl commented Nov 22, 2023

And now I wonder if we should also recommend removing the python39 module.

@ianballou ianballou force-pushed the remove-pulpcore-module-references branch from 7fecc7e to 211d8b5 Compare December 1, 2023 17:22
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Dec 1, 2023
@ianballou
Copy link
Contributor Author

@ekohl missed your earlier comments, I'll get to that.

@ianballou
Copy link
Contributor Author

Would it make sense to also add this as an upgrade warning in the release notes? So here:

[id="katello-upgrade-warnings"]
== Upgrade Warnings
There are no upgrade warnings with Katello {KatelloVersion}.

I added it here: https://github.com/theforeman/foreman-documentation/pull/2602/files

@ianballou ianballou force-pushed the remove-pulpcore-module-references branch from 211d8b5 to a0f9fd0 Compare December 5, 2023 23:10
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Thanks Ian, LGTM. By now, I think we'll have to cherry-pick to 3.9. Please tick this box in your PR description.

Handing over to Ewoud for final ACK.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

This part is actually fixing an existing bug:

diff -Nrwu old/Installing_Proxy/index-satellite.html preview/nightly/Installing_Proxy/index-satellite.html
--- old/Installing_Proxy/index-satellite.html	2023-12-05 23:12:22.000000000 +0000
+++ preview/nightly/Installing_Proxy/index-satellite.html	2023-12-05 23:12:28.000000000 +0000
@@ -3497,7 +3497,7 @@
 <p>Enable the DNF modules:</p>
 <div class="listingblock">
 <div class="content">
-<pre class="nowrap"># dnf module enable satellite:el8</pre>
+<pre class="nowrap"># dnf module enable satellite-capsule:el8</pre>
 </div>
 </div>
 </li>

@Lennonka would you mind having a look about fixing that for downstream stable branches?

@Lennonka
Copy link
Contributor

Lennonka commented Dec 7, 2023

@Lennonka would you mind having a look about fixing that for downstream stable branches?

@ekohl I'm not sure I understand. Fixing what, how, and where?

@evgeni
Copy link
Member

evgeni commented Dec 11, 2023

And now I wonder if we should also recommend removing the python39 module.

We absolutely should not, at least not as long as other tools (like ansible-runner) are using it.

@Lennonka
Copy link
Contributor

Lennonka commented Dec 11, 2023

@ekohl Downstream branches are copied from upstream. It must be, therefore, fixed in upstream and then it will be synced to downstream once merged. It can theoretically be done by correct cherry-picks to stable branches here in upstream, but I'm not sure all changes in this PR are desirable in older releases.

@ianballou ianballou merged commit c6b2be6 into theforeman:master Dec 11, 2023
8 checks passed
@ianballou ianballou deleted the remove-pulpcore-module-references branch December 11, 2023 20:08
@evgeni
Copy link
Member

evgeni commented Dec 11, 2023

No CP should be needed, this is for Foreman 3.9

3.9 was branched since, so a CP is needed ;)

evgeni pushed a commit that referenced this pull request Dec 12, 2023
@evgeni
Copy link
Member

evgeni commented Dec 12, 2023

To github.com:theforeman/foreman-documentation.git
2b461fd..b5dbca3 3.9 -> 3.9

@Lennonka
Copy link
Contributor

@mjivraja @AkshayGadhaveRH Would you mind taking a look at this? I'm slammed :( and you might have a related bug for this in your team.

@nixfu
Copy link

nixfu commented Dec 12, 2023

FYI ran into this with testing upgrading from Katello 4.10 to 4.11 today, was getting tons of dnf/rpm errors until I figured it out.

THIS SHOULD BE IN THE RELEASE NOTES to disable the pulpcore module before upgrading, my eye just got lucky and noticed it in the nightly docs, but it will trip lots of people up who are upgrading.

@ianballou
Copy link
Contributor Author

ianballou added a commit to ianballou/foreman-documentation that referenced this pull request Dec 19, 2023
ianballou added a commit that referenced this pull request Dec 20, 2023
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.

7 participants