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 #34854 - disable custom repo if it was not enabled #610

Merged
merged 2 commits into from
May 11, 2022

Conversation

upadhyeammit
Copy link
Contributor

@upadhyeammit upadhyeammit commented May 2, 2022

With this change if custom repositoryl is provided using --maintenance-repo-label
or env variable --MAINTENANCE_REPO_LABEL is exported then
foreman-maintain keeps that custom repo enabled if system had it enabled.
If custom repository was not enabled on system and still provided either via option or
env var then custom repo will be disabled.

@theforeman-bot
Copy link
Member

Issues: #34854

@evgeni
Copy link
Member

evgeni commented May 9, 2022

I am trying to understand the logic and seem to need some help.

  • In the connected scenario, we can calculate which is the right repository to enable for self-upgrade to work.
    • This repo remains enabled after self-upgrade is done
  • In the disconnected scenario, for self-upgrade to work, the user need to pass the repository where one can get foreman-maintain from (via cli option or env variable).
    • This repo gets disabled after self-upgrade is done.

What I don't understand is the difference in the enabled state after the upgrade is done.

Also, the commit message says "… if it was not enabled", but reading the code I'd say it's always disabled now?

@upadhyeammit
Copy link
Contributor Author

Hello,
Below should help to understand the steps, if its still getting confusing let me know,
In connected scenario,

  1. Enable the next version maintenance repo
  2. Update the package
  3. Disable next version maintenance repo. Only keep installed version maintenance repo, clean any extra maintenance repo was enabled by mistake, or manually.
  4. Bring system repos to the state before self-upgrade executed. This means renable any custom repos as well if those were enabled.

In disconnected scenario,

  1. Enable custom maintenance repo
  2. Update the package
  3. Now if the custom repository was already enabled, and stored on filesystem don't disable it.
  4. Don't try to enable the CDN maintenance repo as it won't be available to enable.
  5. Bring system repos to the state before self-upgrade executed. This means renable any custom repos as well if those were enabled.

@evgeni
Copy link
Member

evgeni commented May 11, 2022

Okay, I think I see the logic.

How about some tests?

--- /dev/null
+++ test/definitions/scenarios/self_upgrade_test.rb
@@ -0,0 +1,38 @@
+require 'test_helper'
+
+module Scenarios
+  describe ForemanMaintain::Scenarios::SelfUpgrade do
+    include DefinitionsTestHelper
+
+    let(:scenario) do
+      ForemanMaintain::Scenarios::SelfUpgrade.new
+    end
+
+    describe 'using CDN' do
+      it 'reenables maintenance repo' do
+        scenario.stubs(:stored_enabled_repos_ids).returns([])
+        scenario.stubs(:current_version).returns('6.10')
+        scenario.stubs(:el_major_version).returns(7)
+        assert_equal ['rhel-7-server-satellite-maintenance-6-rpms'], scenario.repos_ids_to_reenable
+      end
+    end
+
+    describe 'using custom repos' do
+      it 'does not reenable maintenance repo if it was disabled' do
+        scenario.stubs(:stored_enabled_repos_ids).returns([])
+        scenario.stubs(:current_version).returns('6.10')
+        scenario.stubs(:el_major_version).returns(7)
+        scenario.stubs(:maintenance_repo_label).returns('custom-maintenance')
+        assert_equal [], scenario.repos_ids_to_reenable
+      end
+
+      it 'reenables maintenance repo if it was enabled' do
+        scenario.stubs(:stored_enabled_repos_ids).returns(['custom-maintenance'])
+        scenario.stubs(:current_version).returns('6.10')
+        scenario.stubs(:el_major_version).returns(7)
+        scenario.stubs(:maintenance_repo_label).returns('custom-maintenance')
+        assert_equal ['custom-maintenance'], scenario.repos_ids_to_reenable
+      end
+    end
+  end
+end

:)

@upadhyeammit
Copy link
Contributor Author

Thanks for the test, added those

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.

3 participants