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

Fixes #37983 - Remove syspurpose addons from Katello #11196

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

chris1984
Copy link
Member

@chris1984 chris1984 commented Oct 30, 2024

What are the changes introduced in this pull request?

  • There are no longer any SKUs that use syspurpose addons. Additionally, the addons command is being removed from subscription-manager in RHEL10. So we are free to remove addons from Katello
  • Removed systempurpose_addons from AngularJS, React, and the Rails backend
  • Created a DB Migration to remove the addon tables

What are the testing steps for this pull request?

  • Checkout PR and apply migration, make sure tables are removed
  • Look around in the UI to make sure nothing broke
  • Look around in the code to see if I missed anything
  • Make sure the system porpoise dolphin did not sneak in 🥚

@chris1984 chris1984 force-pushed the sysporpose branch 3 times, most recently from fc2516e to 573a92c Compare November 5, 2024 16:32
@chris1984 chris1984 marked this pull request as ready for review November 5, 2024 16:32
@chris1984 chris1984 changed the title Sysporpose addon removal Fixes #37983 - Remove syspurpose addons from Katello Nov 5, 2024
@parthaa
Copy link
Contributor

parthaa commented Nov 14, 2024

can you rebase this ?

@chris1984
Copy link
Member Author

can you rebase this ?

@parthaa updated

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

I am not able to run the down migration:

$ bundle exec rails db:migrate:down VERSION=20241101144625

== 20241101144625 RemoveSystemPurposeAddons: reverting ========================
-- create_table(:katello_activation_key_purpose_addons)
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

you can't redefine the primary key column 'id'. To define a custom primary key, pass { id: false } to create_table.

One option would be just to consider this an irreversible migration, delete the down method, and rename up to change.

Other than that everything seems to work as expected. 👍

@chris1984
Copy link
Member Author

I am not able to run the down migration:

$ bundle exec rails db:migrate:down VERSION=20241101144625

== 20241101144625 RemoveSystemPurposeAddons: reverting ========================
-- create_table(:katello_activation_key_purpose_addons)
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

you can't redefine the primary key column 'id'. To define a custom primary key, pass { id: false } to create_table.

One option would be just to consider this an irreversible migration, delete the down method, and rename up to change.

Other than that everything seems to work as expected. 👍

Since I don't think we are going to be adding these back, might as well as change it to irreversible. I was debating on that but decided to try and be fancy and it blew up.

@chris1984
Copy link
Member Author

@jeremylenz updated the migration

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

thanks @chris1984!

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Clicking request changes until the https://github.com/Katello/katello/pull/11196/files#r1847225444 is resolved.

@chris1984
Copy link
Member Author

chris1984 commented Nov 19, 2024

Clicking request changes until the https://github.com/Katello/katello/pull/11196/files#r1847225444 is resolved.

Talked with @parthaa and he had me update that migration to just remove us looping through things and just create the table. Also removed that model name from a few other migrations.

@chris1984 chris1984 requested a review from jeremylenz November 19, 2024 19:01
@chris1984
Copy link
Member Author

Tested on a downstream Satellite with a --reset to test the database migration and it passed

2024-11-19 15:40:27 [INFO  ] [post] Executing hooks in group post
  Success!
  * Satellite is running at https://server.example.com

  * To install an additional Capsule on separate machine continue by running:

      capsule-certs-generate --foreman-proxy-fqdn "$CAPSULE" --certs-tar "/root/$CAPSULE-certs.tar"
  * Capsule is running at https://server.example.com

The full log is at /var/log/foreman-installer/satellite.log
Package versions are being locked.
2024-11-19 15:40:27 [INFO  ] [root] Package versions are being locked.
2024-11-19 15:40:28 [INFO  ] [post] All hooks in group post finished

@chris1984 chris1984 merged commit 953552c into Katello:master Nov 20, 2024
26 of 27 checks passed
chris1984 added a commit to chris1984/foreman that referenced this pull request Dec 2, 2024
* In Katello/katello#11196 we removed syspurpose addons from Katello. I didn't realize we had them in Foreman too.
chris1984 added a commit to chris1984/foreman that referenced this pull request Dec 4, 2024
* In Katello/katello#11196 we removed syspurpose addons from Katello. I didn't realize we had them in Foreman too.
chris1984 added a commit to chris1984/foreman that referenced this pull request Dec 4, 2024
* In Katello/katello#11196 we removed syspurpose addons from Katello. I didn't realize we had them in Foreman too.
jeremylenz pushed a commit to theforeman/foreman that referenced this pull request Dec 4, 2024
* In Katello/katello#11196 we removed syspurpose addons from Katello. I didn't realize we had them in Foreman too.
@ekohl
Copy link
Member

ekohl commented Jan 20, 2025

I see system purpose is still mentioned in the docs. In https://docs.theforeman.org/nightly/Managing_Content/index-katello.html#Managing_Activation_Keys_content-management for example. Is this work tracked?

@jeremylenz
Copy link
Member

System purpose addons is only one small part of system purpose. Other syspurpose attributes, (role, usage, SLA) along with release version, are neither deprecated nor removed.

@ekohl
Copy link
Member

ekohl commented Jan 20, 2025

Yet it is mentioned (though the OracleLinux part is a false positive):

 rg -i addons
guides/common/assembly_importing-content.adoc
58::yum_repo_extras_name: {os_name} {os_major} Addons
59::yum_repo_extras_url: https://yum.oracle.com/repo/OracleLinux/OL{os_major}/addons/x86_64/
70::yum_repo_extras_name: {os_name} {os_major} Addons
71::yum_repo_extras_url: https://yum.oracle.com/repo/OracleLinux/OL{os_major}/addons/x86_64/

guides/common/modules/proc_editing-the-system-purpose-of-a-host.adoc
31:# subscription-manager syspurpose add addons '_your_addon_'

guides/common/modules/proc_creating-an-activation-key.adoc
51:--purpose-addons "_addons_"

At least the last one is a hammer command and I think that will break.

@chris1984
Copy link
Member Author

@ekohl I will make a PR to remove it from the docs today

@chris1984
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants