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

Upgrade mgrpxy tool before installing the containerized proxy. #9540

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

maximenoel8
Copy link
Contributor

@maximenoel8 maximenoel8 commented Dec 5, 2024

Context

When installing the proxy container on proxy host, mgrpxy is not updated.

What does this PR change?

After bootstrapping the new proxy, update mgrpxy tool using spacewalk and reboot the proxy.

GUI diff

No difference.

  • DONE

Documentation

  • No documentation needed: only internal and user invisible changes

  • DONE

Test coverage

ℹ️ If a major new functionality is added, it is strongly recommended that tests for the new functionality are added to the Cucumber test suite

  • No tests: already covered

  • DONE

Links

Port(s): https://github.com/SUSE/spacewalk/pull/25946

  • DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

  • Re-run test "changelog_test"
  • Re-run test "backend_unittests_pgsql"
  • Re-run test "java_lint_checkstyle"
  • Re-run test "java_pgsql_tests"
  • Re-run test "ruby_rubocop"
  • Re-run test "schema_migration_test_pgsql"
  • Re-run test "susemanager_unittests"
  • Re-run test "javascript_lint"
  • Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

@maximenoel8 maximenoel8 requested a review from a team as a code owner December 5, 2024 01:34
@maximenoel8 maximenoel8 self-assigned this Dec 5, 2024
Copy link
Contributor

github-actions bot commented Dec 5, 2024

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9540/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9540/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

Comment on lines 754 to 795

# Function to fetch the current list of events
#
# @param hostname String The hostname of the system from requested
def fetch_event_history(hostname)
output, _code = get_target('server').run("spacecmd -u admin -p admin system_listeventhistory #{hostname}", check_errors: true)
events = []
current_event = {}

output.split("\n").each do |line|
case line
when /^Id:\s+(\d+)$/
current_event[:id] = Regexp.last_match(1).to_i
when /^History type:\s+(.+)$/
current_event[:history_type] = Regexp.last_match(1)
when /^Status:\s+(.+)$/
current_event[:status] = Regexp.last_match(1)
when /^Completed:\s+(.+)$/
begin
current_event[:completed] = Time.parse(Regexp.last_match(1))
rescue ArgumentError
current_event[:completed] = nil
end
end

if current_event.key?(:id) && current_event.key?(:completed)
events << current_event
current_event = {}
end
end

events
end

# Function to get the highest event ID (latest event)
#
# @param hostname String The hostname of the system from requested
def get_last_event_id(hostname)
events = fetch_event_history(hostname)
last_event = events.max_by { |event| event[:id] }
last_event ? last_event[:id] : nil
end
Copy link
Member

Choose a reason for hiding this comment

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

We already have implemented a quite similar code, but using API instead of spacecmd.
Please take a look into features/support/system_monitoring.rb on last_onboarding_duration

    node = get_target(host)
    system_id = get_system_id(node)
    events = $api_test.system.get_event_history(system_id, 0, 10)
    onboarding_events = events.select { |event| event['summary'].include? 'certs, channels, packages' }
    last_event_id = onboarding_events.last['id']
    event_details = $api_test.system.get_event_details(system_id, last_event_id)
    # Convert XMLRPC::DateTime to Ruby's Time if necessary
    completed_time = event_details['completed'].is_a?(XMLRPC::DateTime) ? event_details['completed'].to_time : Time.parse(event_details['completed'])
    picked_up_time = event_details['picked_up'].is_a?(XMLRPC::DateTime) ? event_details['picked_up'].to_time : Time.parse(event_details['picked_up'])

I suggest to re-use part of this code, so instead of using spacecmd we use the API.

# @param hostname String The hostname of the system from requested
# @param package String The package name where it will trigger an upgrade
def trigger_upgrade(hostname, package)
get_target('server').run("spacecmd -u admin -p admin system_upgradepackage #{hostname} #{package} -y", check_errors: true)
Copy link
Member

@srbarrios srbarrios Dec 5, 2024

Choose a reason for hiding this comment

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

I suggest to include system.schedulePackageInstall method as part of features/support/namespaces/system.rb so we use the API instead of spacecmd.
But I'd not oppose to just use spacecmd for that case.

Copy link
Contributor Author

@maximenoel8 maximenoel8 Dec 5, 2024

Choose a reason for hiding this comment

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

I switched everything to spacecmd command because I was not able to use system.schedulePackageInstall or system.schedulePackageUpgrade working.

xmlrpc.client.Fault: <Fault -1: 'redstone.xmlrpc.XmlRpcFault: Could not find method: schedulePackageInstall in class: com.redhat.rhn.frontend.xmlrpc.system.SystemHandler with params: [com.redhat.rhn.domain.user.legacy.UserImpl, java.lang.Integer, java.lang.String]'>

Copy link
Contributor Author

@maximenoel8 maximenoel8 Dec 5, 2024

Choose a reason for hiding this comment

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

And the other upgrade method is not even in the doc. (schedulePackageUpgrade)

xmlrpc.client.Fault: <Fault -1: 'redstone.xmlrpc.XmlRpcFault: Could not find method: schedulePackageUpgrade in class: com.redhat.rhn.frontend.xmlrpc.system.SystemHandler with params: [com.redhat.rhn.domain.user.legacy.UserImpl, java.lang.Integer, java.lang.String]'>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I try to find a way of getting the packageId but don't have a solution. I tried:

  • package_name = client.packages.search.name(session_key, "mgrprxy")
  • client.system.listInstalledPackages(session_key, 1000010004)
  • client.system.listAllInstalledPackages(session_key, 1000010004)
    I don't find mgrpxy...

Copy link
Member

Choose a reason for hiding this comment

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

Something you can dig is on how system_upgradepackage is developed on spacecmd, as spacecmd use API calls behind it. But ok, no worries, if it becomes hard, using spacecmd should be fine too.

@@ -39,6 +39,10 @@ Feature: Setup containerized proxy
Scenario: Wait until the proxy host appears
When I wait until onboarding is completed for "proxy"

Scenario: Upgrade mgrpxy tool
Then I upgrade "proxy" with the last "mgrpxy" version
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would simplify this step, by just do an upgrade of all available packages, you can do it via WebUI, or using system.schedulePackageUpdate from the API.

Copy link
Contributor Author

@maximenoel8 maximenoel8 Dec 10, 2024

Choose a reason for hiding this comment

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

Two things here from my understanding on previous discussion with the team:

  • we should avoid a full upgrade for the potential dependency issues and time consuming action
  • we should use the API for non-testing step to gain in time and stability.
    I would prefer to keep it like that. It also shows what we are doing.

Copy link
Member

@srbarrios srbarrios Dec 10, 2024

Choose a reason for hiding this comment

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

You are right, at first, I thought we all agreed on having systems up-to-date when we test on BV, but I was wrong, I clarified it after discussing with more people.
I will add that point as one of our agreements in the confluence page.

About the API, fully agree, and as fallback if we can't do it with API, it's fine doing it with spacecmd, as it is using API behind it.

@srbarrios srbarrios requested a review from a team December 5, 2024 13:55
srbarrios
srbarrios previously approved these changes Dec 10, 2024
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.

2 participants